Conversation
The comments for the helper function were incorrect, so fix them so it doesn't seem like this repo is part of the cert-manager org. Also removes image.registry - no point supporting a deprecated field on a brand new chart Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
also moves the config block to the top Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
SgtCoDFish
force-pushed
the
helm-chart-updates
branch
from
281a1e1 to
9e21ec6
Compare
Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
SgtCoDFish
force-pushed
the
helm-chart-updates
branch
from
9e21ec6 to
692fdb8
Compare
This fails in GHA currently, likely due to IP restrictions. As such, it's opt-in Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
SgtCoDFish
force-pushed
the
helm-chart-updates
branch
from
692fdb8 to
901098d
Compare
Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
SgtCoDFish
force-pushed
the
helm-chart-updates
branch
from
901098d to
ffaecaa
Compare
SgtCoDFish
left a comment
SgtCoDFish
left a comment
There was a problem hiding this comment.
self-review comments!
| # This is a YAML-formatted file. | ||
| # Declare variables to be passed into your templates. | ||
| # Configuration for the Discovery Agent | ||
| config: |
There was a problem hiding this comment.
note: this config section is just moved from below; it makes sense to have it at the top of the file since these are the most important options. The only other changes are to comments and to the type of tsgID
| data-gatherers: | ||
| - kind: k8s-discovery | ||
| name: ngts/discovery | ||
| name: k8s/discovery |
There was a problem hiding this comment.
note: this fixes something claude introduced. The disco-agent uses the ark prefix because it collects fundamentally different data to the venafi-kubernetes-agent.
Claude copied the idea of a different prefix, but it doesn't really gain us anything so I reverted here. This file should be very similar to the venafi-kubernetes-agent configmap
| Util function for generating an image reference based on the provided options. | ||
| This function is derived from similar functions used in the cert-manager GitHub organization |
There was a problem hiding this comment.
note: replacing a copy-paste comment with one for this repo specifically!
| {{- /* | ||
| Backwards compatibility: if image.registry is set, additionally prefix the repository with this registry. | ||
| */ -}} | ||
| {{- if $image.registry -}} | ||
| {{- $repository = printf "%s/%s" $image.registry $repository -}} | ||
| {{- end -}} |
There was a problem hiding this comment.
note: I see no reason to have this backwards compat shim in a brand new chart, so I removed it for simplicity.
| ARK_USERNAME: ${{ secrets.ARK_USERNAME }} | ||
| ARK_SECRET: ${{ secrets.ARK_SECRET }} | ||
|
|
||
| ngts-test-e2e: |
There was a problem hiding this comment.
note: it makes sense to me to add this here gated behind a label.
I added the required variables to this repo already (NGTS_CLIENT_ID, etc) - but this test doesn't pass. I think it's because the relevant dev environment has IP address restrictions which github actions runners are falling foul of.
I don't think we should block this PR on the test passing - I've confirmed locally that the test can pass, I just don't think for now it will pass.
Ideally in the future when we have a stable prod environment for testing, we'll be able to point this test there.
| run: | | ||
| # Generate a timestamp in the format YYMMDD-HHMMSS. | ||
| # Extracting from PR name would require sanitization due to GKE cluster naming constraints | ||
| # Extracting from PR name would require sanitization due to GKE cluster naming constraints |
There was a problem hiding this comment.
note: simply removing trailing whitespace
There was a problem hiding this comment.
note: this script was never run before it originally merged, so the state it was in before this PR was filled with guesses
This gets around corporate MitM Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
1 participant
This PR mostly fixes up comments, errors and test setup. Each individual commit should be self-descriptive.
This also adds the discovery-agent to the release process.
IMPORTANT: This fixes the ngts-e2e test and adds the possibility to run it in CI but restricts it via a label - it seems the dev API endpoints are blocked from github actions runners currently and the test just hangs and dies for that reason.
I have run the e2e test locally and had success - but I don't think we can enable the test on every PR until we have a stable test tenant which is publicly accessible.