-
Notifications
You must be signed in to change notification settings - Fork 25
[VC-45025] cyberark-disco-agent: Change default config.period to 12h in values.yaml #720
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
Conversation
5370eed to
2f3454f
Compare
2f3454f to
b5a0ab5
Compare
b5a0ab5 to
532d009
Compare
532d009 to
498c1fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This snapshot was generated by make test-helm-snapshot.
| apiVersion: v1 | ||
| data: | ||
| config.yaml: |- | ||
| period: "1m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the output of helm template when a custom config.period value is supplied.
| apiVersion: v1 | ||
| data: | ||
| config.yaml: |- | ||
| period: "12h0m0s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the output of helm template without any supplied helm values (the defaults)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
helm unittest was already in use for the venafi-kubernetes-agent chart, but there it used slightly differently....there the tests contain various specific assertions about the fields and values in the resulting yaml.
Here, I've copied what @maelvls did in Firefly, which is to simply record some snapshots (golden files) with various values overridden. This is easier to maintain, but we do have to sanity check the snapshot files when they are first created and if they ever change.
| # Push data every hour unless changed. | ||
| period: "1h0m0s" | ||
| # Push data every 12 hours unless changed. | ||
| period: "12h0m0s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot suggested that I add a comment explaining the rationale for this rather long interval between uploads.
But I don't know why this interval has been chosen and I expect that it might be reduced in future when this agent is tested in the real world.
| ark-generate-helm-schema: helm_chart_source_dir := deploy/charts/cyberark-disco-agent | ||
| ark-generate-helm-schema: generate-helm-schema | ||
|
|
||
| shared_generate_targets += ark-generate-helm-schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was not being run by make generate because generate-helm-docs was already a dependency of generate:
- generate > ark-generate-helm-docs > generate-helm-docs
- generate > generate-helm-docs
So the first dependency (with different variables) was being dropped.
So I've changed it to invoke a make subprocess below, and tested that it works by altering values in the README.md files of both charts and observing that make generate reverts both those changes.
| ## @category Testing | ||
| test-helm: | $(NEEDS_HELM-UNITTEST) | ||
| $(HELM-UNITTEST) ./deploy/charts/venafi-kubernetes-agent/ | ||
| $(HELM-UNITTEST) ./deploy/charts/{venafi-kubernetes-agent,cyberark-disco-agent} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ make test-helm
/home/richard/projects/jetstack/jetstack-secure/_bin/tools/helm-unittest ./deploy/charts/{venafi-kubernetes-agent,cyberark-disco-agent}
### Chart [ venafi-kubernetes-agent ] ./deploy/charts/venafi-kubernetes-agent
PASS test deployment deploy/charts/venafi-kubernetes-agent/tests/deployment_test.yaml
### Chart [ cyberark-disco-agent ] ./deploy/charts/cyberark-disco-agent
PASS test the contents of the config.yaml deploy/charts/cyberark-disco-agent/tests/configmap_test.yaml
Charts: 2 passed, 2 total
Test Suites: 2 passed, 2 total
Tests: 10 passed, 10 total
Snapshot: 2 passed, 2 total
Time: 94.109718ms
…es.yaml The project design requires the agent to upload data every 12 hours. - Add helm unittest test suite and snapshots for configmap.yaml - Update default config.period to 12h0m0s in values.yaml - Extend make targets to run helm unittest for cyberark-disco-agent - Document helm unittest usage and snapshot update steps - Fix `make generate` so that it also invokes the ark-generate- targets Signed-off-by: Richard Wall <richard.wall@cyberark.com>
498c1fb to
5ebf20c
Compare
The original project design calls for the agent to upload data every 12 hours.
There is some background information about the 12h interval in the JIRA ticket:
make generateso that it also invokes the ark-generate- targets