Refactor secrets linking #130
Conversation
rcerven
force-pushed
the
fixlinking
branch
2 times, most recently
from
145bbd1
to
afc28c9
Compare
|
/test image-controller-e2e |
|
1 similar comment
|
/test images |
| if errors.IsNotFound(err) { | ||
| return nil | ||
| } | ||
| log.Error(err, "Failed to read pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionView) |
There was a problem hiding this comment.
You can add common log entries to the log above
There was a problem hiding this comment.
not sure what you mean.
you mean in log := ctrllog.FromContext(ctx) ???
There was a problem hiding this comment.
I mean something like
log := ctrllog.FromContext(ctx).WithValues("ServiceAccountName", buildPipelineServiceAccountName)
so all the log entries will have the value. But up to you.
There was a problem hiding this comment.
ok didn't know that
will change it
|
|
||
| secretExists = false | ||
| log.Info("Secret doesn't exist, will unlink secret from service account", "SecretName", secretName) | ||
| log.Info("To recreate secret use regenerate-token") |
There was a problem hiding this comment.
This log is not visible to the user, so makes no sense.
There was a problem hiding this comment.
I know it isn't visible to the user, but it is good for debugging
keeping it
There was a problem hiding this comment.
For debugging we can relay on Secret doesn't exist, will unlink secret from service account, cannot we?
controllers/suite_util_test.go
Outdated
| @@ -296,6 +296,27 @@ func waitImageRepositoryFinalizerOnImageRepository(imageRepositoryKey types.Name | |||
| }, timeout, interval).Should(BeTrue()) | |||
| } | |||
|
|
|||
| // func waitImageRepositoryCredentialGone(imageRepositoryKey types.NamespacedName, credentialName string) { | |||
| func waitImageRepositoryCredentialGone(imageRepositoryKey types.NamespacedName, operationName string) { | |||
There was a problem hiding this comment.
Name is confusing. It seems that it waits until the secrets with credentials gone...
There was a problem hiding this comment.
it waits until imageRepository.Spec.Credentials sections are gone
hence the name : ImageRepositoryCredential
suggest better one if you have a idea
There was a problem hiding this comment.
waitImageRepositoryCredentialSectionGone at least? Or maybe waitImageRepositoryCredentialSectionRequestGone?
rcerven
force-pushed
the
fixlinking
branch
3 times, most recently
from
70274be
to
4d0be12
Compare
1. when linking secret to SA, don't add it if already present 2. unlink secret from SA upon imageRepository deletion 3. don't link secret anymore to imagePullSecrets (used for the image pod is using, task/pipeline bundle image) 4. new option to clean up secret links via spec.credentials.verify-linking - It will link secret to service account if link is missing. - It will remove duplicate links of secret in service account. - It will remove secret from imagePullSecrets in service account. - It will unlink secret from service account, if secret doesn't exist (can recreated by using 'regenerate-token'). STONEBLD-2540 Signed-off-by: Robert Cerven <[email protected]>
|
New changes are detected. LGTM label has been removed. |
STONEBLD-2540