select default_secret for service account if there is more than one #281
Conversation
ghost
added
the
pablo-ruth
changed the title
Co-Authored-By: pablo-ruth <contact@pablo-ruth.fr>
There was a problem hiding this comment.
Maybe the matching string token could be replaced by something more specific like -token- and/or be configurable: https://github.com/terraform-providers/terraform-provider-kubernetes/pull/281/files#diff-375ac94c5bd4e304ddeb13226700ec77R111
Other than this, LGTM.
|
I changed matching string too, it's cleaner. |
| if len(diff) > 1 { | ||
| return fmt.Errorf("Expected 1 generated default secret, %d found: %s", len(diff), diff) | ||
| for _, secret := range diff { | ||
| if strings.Contains(secret.Name, "-token-") { |
There was a problem hiding this comment.
This approach is problematic because there is no API contract to guarantee that token secrets must have the -token- string will be part of the name for token secrets.
There is however a Type attribute on Secret objects (see here). You can filter the list of Secrets on this Type attribute for all objects with a value of kubernetes.io/service-account-token and that should give you a list of just the token type secrets. You'll most likely just get one during create, but I guess you can just pick the first one if you get more (very non-deterministic unfortunately).
There was a problem hiding this comment.
Filtering on the secret type is indeed much cleaner!
There was a problem hiding this comment.
I added the filtering on secret type. I think we can keep the previous logic and return an error if we have more than one service account token. The filtering applies even if there is only one secret, I'm not sure if it's what you want... And we possibly can move this part in the "resource.Retry" block if we want to wait for a service account token to appear ?
There was a problem hiding this comment.
@pablo-ruth I think adding the resource.Retry around it is a good idea.
However, I'm a bit concerned about doing .Get(..) calls for individual objects in a loop (the for...range around it) AND wrapping that in retries. It has the theoretical potential to cause a lot of requests in worst case conditions (and stress the API server unnecessarily).
The alternative would be to use a .Secrets(namespace).List(..) type of call OUTSIDE the loop and then filter it's results for the relevant secret name. We can that wrap that in a retry and not have it quadratically grow out of control.
What do you think? If you'd rather not add the retry, than I think we can merge like this.
There was a problem hiding this comment.
Ok done, I moved it to the retry func and used List with filtering instead of Get calls.
Proposal to fix #94
If there is more than one secret created by default with a service account (ex: openshift), default to the first and override by a secret with "token" in the name if there is any.