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

Allow to select docker registry credentials #1679

Closed
wants to merge 1 commit into from

Conversation

andresmgot
Copy link
Contributor

Description of the change

Allow to add docker registry credentials in the spec of an AppRepository. This is still not ready to be reviewed but I'm sending a draft to validate the UX first:

Peek 2020-04-20 18-33

Applicable issues

Additional information

still needs to be properly tested

@andresmgot andresmgot requested a review from absoludity April 20, 2020 16:44
@absoludity
Copy link
Contributor

Thanks Andres! Yes, the UX looks exactly as I'd expect (ie. can select existing kubernetes.io/dockerconfigjson secrets as well as create new ones).

I'd move the "Server" field up to be the first piece of info after the secret name in the form.

Separate to this PR, we may want to use some validation on the server too (a few ways that could go).

@absoludity
Copy link
Contributor

Separate to this PR, we may want to use some validation on the server too (a few ways that could go).

We will need to ensure that this is disabled (but probably good to be visible) when not using helm3 (already in the dashboard config). Also need to ensure it's disabled or explicitly not available for global app repos (in fact, if it's not visible for global repos, you won't need a feature flag, as it'll only display on per-app repos).

@andresmgot
Copy link
Contributor Author

👍 thanks for the comments! I will try to split these changes so they are easier to review

@andresmgot
Copy link
Contributor Author

closing this in favor of #1684 and #1685

@andresmgot andresmgot closed this Apr 21, 2020
@andresmgot andresmgot deleted the dockerRegistryCreds branch April 21, 2020 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants