Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValidatingAdmissionPolicy for C-0012 #26

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

suhasgumma
Copy link
Contributor

@slashben slashben marked this pull request as ready for review January 25, 2023 14:53
Signed-off-by: Suhas Gumma <[email protected]>
Signed-off-by: Suhas Gumma <[email protected]>
@slashben
Copy link
Collaborator

@suhasgumma , I think there might be an issue with the way you handle environment variables. (correct me if I am wrong)

An environment variable can have a value defined in the pod spec, or it can get its value from a secret or a configmap.

For example, I have an environment variable "AWS_SECRET_KEY" defined in the pod spec. If it has an immediate value specified in the Pod spec it should fail the control/policy. If it is mounted from a Kubernetes secret it is good practice!
See here: https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-environment-variables

@suhasgumma
Copy link
Contributor Author

suhasgumma commented Jan 26, 2023

An environment variable can have a value defined in the pod spec, or it can get its value from a secret or a configmap.

  • Yes @slashben, you are right. But, the policy will not fail if the value is mounted by a secret or configMap. We are checking for them indirectly.

  • If the value is not set or set to a non-empty string, only then secretKeyRef or configMapKeyRef can be defined.

  • If the value is not set or set to a non-empty string, the control will pass regardless. So, we don't need to check if there is a secretKeyRef or configMapKeyRef set.

  • I think it is unclear for anyone, shall we add logic checking for secretKeyRef or configMapKeyRef just for the purpose of documentation?

@slashben
Copy link
Collaborator

You can add comment to the policy itself, no?

@suhasgumma
Copy link
Contributor Author

suhasgumma commented Jan 26, 2023

You can add comment to the policy itself, no?

Yeah, I will do that and clearly specify it in the documentation too.

@matthyx
Copy link
Collaborator

matthyx commented Aug 11, 2024

@suhasgumma can you rebase and fix conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants