verify that SA secrets refs are not deleted #767
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
| // Skipping the TestDoNotOverrideServiceAccount test instead of deleting it because we will need to create SAs as part | ||
| // of the environment sub-workspaces so the test & logic will be useful to keep. |
There was a problem hiding this comment.
we could drop those now
| // Skipping the TestDoNotOverrideServiceAccount test instead of deleting it because we will need to create SAs as part | |
| // of the environment sub-workspaces so the test & logic will be useful to keep. |
There was a problem hiding this comment.
removed in #768
| secrets: | ||
| for j := 0; j < i; j++ { | ||
| expName := fmt.Sprintf("dummy-secret-%d", j) | ||
| for _, secretRef := range sa.Secrets { | ||
| if secretRef.Name == fmt.Sprintf("dummy-secret-%d", j) { | ||
| continue secrets | ||
| } | ||
| } | ||
| assert.Fail(t, fmt.Sprintf("secret '%s' not found", expName)) | ||
| } |
There was a problem hiding this comment.
just trying to understand - when i is 0 this check will be skipped (ok it will be checked once i>0).
But why do we need to start from secrets again every time we find a secret?
Could we check that all the expected number of secrets (j) are there , and fail the the first one that is missing ?
something like:
for j := 0; j <= i; j++ {
expName := fmt.Sprintf("dummy-secret-%d", j)
found := false
for _, secretRef := range sa.Secrets {
if secretRef.Name == fmt.Sprintf("dummy-secret-%d", j) {
found = true
break
}
}
if !found {
assert.Fail(t, fmt.Sprintf("secret '%s' not found", expName))
}
}
and remove the secrets tag and continue statement ?
There was a problem hiding this comment.
just trying to understand - when i is 0 this check will be skipped (ok it will be checked once i>0).
right - bad copy-paste from the for i line :-)
as for your version - isn't it the same?
the only benefit would be that I could combine secrets & pullSecrets together
There was a problem hiding this comment.
If I'm not mistaken is not the same because:
- you can combine them right
- you are not starting from j=0 every time you find a secret, but you validate them in one single loop, because the current
continue secretsexists the outer loop as well right ?
There was a problem hiding this comment.
addressed in #768
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MatousJobanek, mfrancisc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
paired PR: codeready-toolchain/member-operator#469