[DOM-47678] Secret querying fixes #110
Merged
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
ddl-ebrown
changed the title
ddl-ebrown
force-pushed
the
DOM-47678-add-secret-list-perms
branch
from
3b30988
to
26604e9
Compare
- Add secrets list perms The secret reading code had a late change to search for matching secrets with the correct hephaestus-accessible label instead of performing a simple get by namespace/name. While this prevents Hephaestus code from even accessing the secret, it also now requires the list permission to query secrets. - Correctly specify fields for secret name and namespace by specifying the metadata. prefix - Unit tests don't support the filter syntax and don't apply Kubernetes permissioning, and the repo functiona. tests which actually use Kubernetes are currently not operational, so this slipped past review
ddl-ebrown
force-pushed
the
DOM-47678-add-secret-list-perms
branch
from
26604e9
to
dcff29a
Compare
ddl-kfrench
approved these changes
Jul 7, 2023
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.
Add secrets list perms
The secret reading code had a late change to search for matching
secrets with the correct hephaestus-accessible label instead of
performing a simple get by namespace/name.
While this prevents Hephaestus code from even accessing the secret,
it also now requires the list permission to query secrets.
Correctly specify fields for secret name and namespace by specifying
the metadata. prefix
Unit tests don't support the filter syntax and don't apply Kubernetes
permissioning, and the repo functiona. tests which actually use
Kubernetes are currently not operational, so this slipped past review
Manual Test Results
As in #106, this requires manual testing given the functional tests in this repo are not working.
Using the follow resource definitions, I created k8s resources and then tailed the logs of Hephaestus to observe the behavior.
Resource definitions
This continues to use a
Dockerfilecontext available at Github at https://github.com/dominodatalab/hephaestus/raw/51585793939ab0de611bea74ab14e29d846b2070/test/functional/testdata/docker-context/secrets/archive.tgz with a simple Dockerfile like:Relevant logs
Secret retrieval
Note specifically:
Build logs
Note specifically
foo.baris the value present in the secret aboveImageBuild verification
The resource reaches the success status
The secret still exists
Deleting the
ImageBuildproperly cleans up the secret