Docker registry secrets #1662
Docker registry secrets #1662
Conversation
| SyncJobPodTemplate corev1.PodTemplateSpec `json:"syncJobPodTemplate"` | ||
| ResyncRequests uint `json:"resyncRequests"` | ||
| } | ||
|
|
||
| var ErrGlobalRepositoryWithSecrets = fmt.Errorf("docker registry secrets cannot be set for app repositories available in all namespaces") |
There was a problem hiding this comment.
you need to add a comment for exported var or names (the linter complains for me)
| @@ -80,7 +80,7 @@ func CreateAppRepository(handler kube.AuthHandler) func(w http.ResponseWriter, r | |||
| func ValidateAppRepository(handler kube.AuthHandler) func(w http.ResponseWriter, req *http.Request) { | |||
| return func(w http.ResponseWriter, req *http.Request) { | |||
| token := auth.ExtractToken(req.Header.Get("Authorization")) | |||
| res, err := handler.AsUser(token).ValidateAppRepository(req.Body) | |||
| res, err := handler.AsUser(token).ValidateAppRepository(req.Body, mux.Vars(req)["namespace"]) | |||
There was a problem hiding this comment.
this is unrelated to the docker registries, right? (it's to run the validation in the proper namespace)
[EDIT] A well, I see that it's used to check if the namespace is the global one and have secrets
| @@ -139,10 +139,13 @@ type appRepositoryRequestDetails struct { | |||
| RepoURL string `json:"repoURL"` | |||
| AuthHeader string `json:"authHeader"` | |||
| CustomCA string `json:"customCA"` | |||
| RegistrySecrets []string `json:"registrySecrets"` | |||
There was a problem hiding this comment.
So this is a different approach than what we do with repositories credentials. There we ask for credentials rather than secrets and we create those secrets on behalf of the user. That has the advantage that we can use garbage collection to clean up stuff and it's easier for people to use it. Is your plan to move to that approach once we support setting multiple namespaces for an AppRepository maybe?
There was a problem hiding this comment.
So this is a different approach than what we do with repositories credentials. There we ask for credentials rather than secrets and we create those secrets on behalf of the user.
The change here doesn't necessarily mean a different approach, rather it's just the way we link any secrets to the app repository (via the name, with the necessary assumption of the same namespace). The UI can still require credentials from the user which we use to create the secrets, or we could allow selecting from the available dockerconfigjson secrets already present.
That has the advantage that we can use garbage collection to clean up stuff and it's easier for people to use it.
We chatted about this, but just ftr, I don't think these dockerconfigjson secrets should be garbage collected when the app repository is deleted. It should be no problem to delete the app repository and still leave an existing deployment which was deployed from that app repository. Furthermore, if a user increases the replica sets and has a pod spawned on a different node, that node should be able to pull the image from the private docker registry. So the docker registry secrets are a little different from the app repository secrets (ie. they're not for accessing things to install, but rather are run-time requirements)
Is your plan to move to that approach once we support setting multiple namespaces for an AppRepository maybe?
Not necessarily. The important thing there is that it should be easy and intuitive to link the secrets to an app repo, and we'd take care of the rest (ensuring those secrets are available in the other namespaces using the users creds).
| // DockerRegistrySecrets is a list of dockerconfigjson secrets which exist | ||
| // in the same namespace as the AppRepository and should be included | ||
| // automatically for matching images. | ||
| DockerRegistrySecrets []string `json:"dockerRegistrySecrets,omitempty"` |
There was a problem hiding this comment.
nit: I would move this param to the last (at first sight I thought you were adding 3 new params to the spec)
Ref: #1522
Enables AppRepositories to be created with a list of docker registry secrets which should be included for matching registries.
Also ensures that a global AppRepository is not able to be created (or validated) with docker registry secrets.