Extract temporary service account token by name directly #373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jose R. Gonzalez <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This was referenced Jul 25, 2024
acornett21
reviewed
Jul 29, 2024
| secrets.append({"name": token_name}) | ||
| secret_found = True | ||
| break | ||
| time.sleep(10) |
There was a problem hiding this comment.
Do you still need/wait to sleep some? Or is this no longer important/needed because the secrete is being proactively created?
There was a problem hiding this comment.
The sleep was moved to the top of the function, and is only executed on retries.
acornett21
approved these changes
Jul 29, 2024
mgoerens
approved these changes
Jul 30, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #366, I modified the code that creates namespaces, RBAC resources, etc. used in certification workflows because OpenShift 4.16 (which is our current runtime platform) did not automatically populate tokens for the service accounts. In effect, I just manually created a secret so that the cluster would populate it with a token we could use.
Since then, I've noticed we had intermittent failures in nightly runs.
https://github.com/openshift-helm-charts/sandbox/actions/runs/10060735219/job/27809079797#step:17:18
When we search for the service account token, we parse the service account in the temporary namespace to see if it has secrets populated. In certain (or all modern) cases, the token secret may not be listed in the service account's list of secrets, so the code would then use
oc describeto list a token secret, then parse that using regular expressions to find secret name, then extract the value, and so on.The failure we were seeing was because we would sometimes get a service account manifest back (in JSON format) with no
secretskey. I don't know why, but that's neither here nor there. I had originally just wrapped this bit of code in a try/except KeyError block, but after reading it, I think decided to simplify it a bit.This PR refactors this code to extract the token from the token secret directly because we now know it's name (because we create it in this same code).
In the previous iteration of this code (described above), the token secret would have been generated by Kubernetes with a randomized name. Given we didn't know that name we would use regular expressions to match any possible names, and then read data from it.
Now that we know the token secret we care about will have a fixed name, we can query it directly and extract the temporary token from that.
Aside: this script and library needs some refactor, but I didn't do that here/yet. I'm just addressing the intermittence to reduce invalid E2E failures we've been seeing in nightly runs. Refactoring this is a future task.