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

Add template for ServiceAccount annotations #239

Merged

Conversation

crdueck
Copy link
Contributor

@crdueck crdueck commented Nov 1, 2023

ServiceAccount annotations are needed to support web identity credentials: https://www.openpolicyagent.org/docs/latest/management-bundles/#web-identity-credentials

Copy link
Contributor

@eshepelyuk eshepelyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello

  1. values schema should be adjusted and corresponding lint test cases added into test/lint.
  2. helm unit tests should be added for affected template in test/unit.

@crdueck
Copy link
Contributor Author

crdueck commented Nov 2, 2023

I added a simple unit test for the serviceaccount annotations, but I'm less sure what to do about the values schema and linting you asked for.

The only example of schema & linting I found was for the image field, which makes sense because it has required sub-fields image.{repository,tag}. Whereas annotations aren't required and can have arbitrary sub-fields. My motivating use case just happens to be the IRSA role annotation.

So, could you help me better understand what you would want to see for annotations schema and linting?

@eshepelyuk
Copy link
Contributor

eshepelyuk commented Nov 2, 2023

I added a simple unit test for the serviceaccount annotations, but I'm less sure what to do about the values schema and linting you asked for.

The only example of schema & linting I found was for the image field, which makes sense because it has required sub-fields image.{repository,tag}. Whereas annotations aren't required and can have arbitrary sub-fields. My motivating use case just happens to be the IRSA role annotation.

So, could you help me better understand what you would want to see for annotations schema and linting?

Hello

  1. schema must reflect\describe entire structure of values.yaml. It was recently introduced, so it's kinda empty, but it's assumed it will be improved with time.
  2. annotations are not arbitrary values, it is dictionary of string to string and this can be described via json schema. Also in schema one can point that service account annotation by default is an empty dictionary.

You can use this as an example

https://github.com/eshepelyuk/cmak-operator/blob/master/values.schema.json

Copy link
Contributor

@eshepelyuk eshepelyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to rebase and squash a PR into a single commit.

test/unit/sa_annotations_test.yaml Outdated Show resolved Hide resolved
Signed-off-by: Chris Dueck <chris.dueck@wattpad.com>
@crdueck
Copy link
Contributor Author

crdueck commented Nov 3, 2023

I added a schema for the serviceAccount values and a lint check for the annotations. Rebased and squashed as well

@eshepelyuk eshepelyuk merged commit 358f1cc into open-policy-agent:master Nov 3, 2023
2 checks passed
@eshepelyuk
Copy link
Contributor

Thanks for the contribution
Will be abailable in 8.5.3

https://github.com/open-policy-agent/kube-mgmt/actions/runs/6748036699

@crdueck crdueck deleted the serviceaccount-annotations branch November 3, 2023 17:49
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.

None yet

2 participants