Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ask pull secret during start #21

Merged
merged 5 commits into from
Mar 22, 2023
Merged

Conversation

evidolob
Copy link
Contributor

@evidolob evidolob commented Mar 21, 2023

Fixes #18

This PR add check if pullsecret is missing during vm start:

Screenshot 2023-03-21 at 11 37 12

And it add basic validation, it just check that provided value is valid JSON:

Screenshot 2023-03-21 at 11 37 39

Also this PR provided various refactoring.

@evidolob evidolob requested a review from gbraad March 21, 2023 10:24
@deboer-tim
Copy link
Contributor

This PR says it must be valid JSON but neither the input prompt or error message do, so the user could be pasting something else thinking it's the pull secret (or e.g. have an extra character on it) and not understanding what's wrong. Could we change the input message, or error to something like "Pull secret is not valid JSON"?

If I paste some random JSON, do I get the same error message? It would be good if the error was different in that case or the error told me something about the format (e.g. main key) so that it's obvious what JSON snippet I'm looking for.

nit: "to start" is redundant in the error message, I'd remove it and just have "please try again".

@evidolob
Copy link
Contributor Author

@deboer-tim I add pullsecret validation(copy from crc codebase), and show more descriptive error:
Screenshot 2023-03-21 at 15 16 37

@gbraad
Copy link
Contributor

gbraad commented Mar 21, 2023

@evidolob Not sure if we should mention "Escape" to cancel... but I am sure people will; As I believe the information how to get the pull-secret is more important. The issue is that the current UI and API can't cater to this. The UI I and Mairin came up with shows the pull-secret dialog as the tray-electron did. People will be in a situation they might not have the right answer to.

I think we have to make a follow-up issue to make sure we revise this when possible.

@gbraad
Copy link
Contributor

gbraad commented Mar 21, 2023

and show more descriptive error

The ideal is that the pull-secret is automagically retrieved and does not see a copy-paste action. And instead, you will just provide the RH credentials, and see the pull-secret identity in a form as:


Pull-secret belongs to:

   g****d@r****t.com

@evidolob
Copy link
Contributor Author

@gbraad "Escape" to cancel... is from podman desktop, I cannot change with current API

@gbraad gbraad linked an issue Mar 21, 2023 that may be closed by this pull request
@gbraad
Copy link
Contributor

gbraad commented Mar 22, 2023

I cannot change with current API

Weird text. As an example, vscode does not state this.

@evidolob
Copy link
Contributor Author

vscode shows exactly same text:
Screenshot 2023-03-22 at 09 38 35

Signed-off-by: Yevhen Vydolob <[email protected]>
return got.get(url, {
enableUnixSockets: true,
throwHttpErrors: false,
retry: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@gbraad
Copy link
Contributor

gbraad commented Mar 22, 2023

OK, since this is a prompt and not a selection.

I am fine with this as the ideal is that this prompt is not shown at all.

@evidolob evidolob merged commit 41cc20a into crc-org:main Mar 22, 2023
@evidolob evidolob deleted the ask-pull-secret branch March 22, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Handle pullsecret
3 participants