Skip to content

NGTS: Various further updates and fixes #790

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.

Sign up for GitHub

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

Open
SgtCoDFish wants to merge 9 commits into
base: master
Choose a base branch
from
Open

NGTS: Various further updates and fixes #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6cc3e85
remove image.registry for discovery agent, fix comments
SgtCoDFish Apr 9, 2026
83eff39
rework discovery-agent value comments
SgtCoDFish Apr 9, 2026
5d8045e
further chart changes / replace incorrect TSG acronym
SgtCoDFish Apr 10, 2026
5dcef20
make tsgID easier to set, improve comments
SgtCoDFish Apr 16, 2026
8826723
add NGTS release process to existing process
SgtCoDFish Apr 16, 2026
812abd5
fix up NGTS e2e, add github action
SgtCoDFish Apr 16, 2026
1bf9682
further helm values changes + reorderings for NGTS
SgtCoDFish Apr 17, 2026
ffaecaa
improve comments + flow for NGTS client
SgtCoDFish Apr 17, 2026
9e4e533
when running NGTS tests on macOS, include system trust store
SgtCoDFish Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
go-version: ${{ steps.go-version.outputs.result }}

- id: release
run: make release ark-release
run: make release ark-release ngts-release

outputs:
RELEASE_OCI_PREFLIGHT_IMAGE: ${{ steps.release.outputs.RELEASE_OCI_PREFLIGHT_IMAGE }}
Expand All @@ -50,6 +50,12 @@ jobs:
ARK_CHART: ${{ steps.release.outputs.ARK_CHART }}
ARK_CHART_TAG: ${{ steps.release.outputs.ARK_CHART_TAG }}
ARK_CHART_DIGEST: ${{ steps.release.outputs.ARK_CHART_DIGEST }}
NGTS_IMAGE: ${{ steps.release.outputs.NGTS_IMAGE }}
NGTS_IMAGE_TAG: ${{ steps.release.outputs.NGTS_IMAGE_TAG }}
NGTS_IMAGE_DIGEST: ${{ steps.release.outputs.NGTS_IMAGE_DIGEST }}
NGTS_CHART: ${{ steps.release.outputs.NGTS_CHART }}
NGTS_CHART_TAG: ${{ steps.release.outputs.NGTS_CHART_TAG }}
NGTS_CHART_DIGEST: ${{ steps.release.outputs.NGTS_CHART_DIGEST }}

github_release:
runs-on: ubuntu-latest
Expand All @@ -73,6 +79,12 @@ jobs:
echo "ARK_CHART: ${{ needs.build_and_push.outputs.ARK_CHART }}" >> .notes-file
echo "ARK_CHART_TAG: ${{ needs.build_and_push.outputs.ARK_CHART_TAG }}" >> .notes-file
echo "ARK_CHART_DIGEST: ${{ needs.build_and_push.outputs.ARK_CHART_DIGEST }}" >> .notes-file
echo "NGTS_IMAGE: ${{ needs.build_and_push.outputs.NGTS_IMAGE }}" >> .notes-file
echo "NGTS_IMAGE_TAG: ${{ needs.build_and_push.outputs.NGTS_IMAGE_TAG }}" >> .notes-file
echo "NGTS_IMAGE_DIGEST: ${{ needs.build_and_push.outputs.NGTS_IMAGE_DIGEST }}" >> .notes-file
echo "NGTS_CHART: ${{ needs.build_and_push.outputs.NGTS_CHART }}" >> .notes-file
echo "NGTS_CHART_TAG: ${{ needs.build_and_push.outputs.NGTS_CHART_TAG }}" >> .notes-file
echo "NGTS_CHART_DIGEST: ${{ needs.build_and_push.outputs.NGTS_CHART_DIGEST }}" >> .notes-file

- env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
37 changes: 36 additions & 1 deletion .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,41 @@ jobs:
ARK_USERNAME: ${{ secrets.ARK_USERNAME }}
ARK_SECRET: ${{ secrets.ARK_SECRET }}

ngts-test-e2e:
Copy link
Copy Markdown
Contributor Author

@SgtCoDFish SgtCoDFish Apr 16, 2026

Choose a reason for hiding this comment

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

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.

# TEMPORARY: require an explicit label to test NGTS until we have a stable test environment
if: contains(github.event.pull_request.labels.*.name, 'test-ngts')
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
# Adding `fetch-depth: 0` makes sure tags are also fetched. We need
# the tags so `git describe` returns a valid version.
# see https://github.com/actions/checkout/issues/701 for extra info about this option
with: { fetch-depth: 0 }

- uses: ./.github/actions/repo_access
with:
DEPLOY_KEY_READ_VENAFI_CONNECTION_LIB: ${{ secrets.DEPLOY_KEY_READ_VENAFI_CONNECTION_LIB }}

- id: go-version
run: |
make print-go-version >> "$GITHUB_OUTPUT"

- uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0
with:
go-version: ${{ steps.go-version.outputs.result }}

- uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
with:
path: _bin/downloaded
key: downloaded-${{ runner.os }}-${{ hashFiles('klone.yaml') }}-test-unit

- run: make -j ngts-test-e2e
env:
OCI_BASE: ${{ secrets.NGTS_OCI_BASE }}
NGTS_CLIENT_ID: ${{ secrets.NGTS_CLIENT_ID }}
NGTS_PRIVATE_KEY: ${{ secrets.NGTS_PRIVATE_KEY }}
NGTS_TSG_ID: ${{ secrets.NGTS_TSG_ID }}

test-e2e:
if: contains(github.event.pull_request.labels.*.name, 'test-e2e')
runs-on: ubuntu-latest
Expand Down Expand Up @@ -149,7 +184,7 @@ jobs:
id: timestamp # Give the step an ID to reference its output
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
Copy link
Copy Markdown
Contributor Author

@SgtCoDFish SgtCoDFish Apr 16, 2026

Choose a reason for hiding this comment

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

note: simply removing trailing whitespace

TIMESTAMP=$(date +'%y%m%d-%H%M%S')
CLUSTER_NAME="test-secretless-${TIMESTAMP}"
echo "Generated cluster name: ${CLUSTER_NAME}"
Expand Down
6 changes: 2 additions & 4 deletions deploy/charts/disco-agent/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ Create the name of the service account to use
{{- end }}

{{/*
Util function for generating the image URL based on the provided options.
IMPORTANT: This function is standardized across all charts in the cert-manager GH organization.
Any changes to this function should also be made in cert-manager, trust-manager, approver-policy, ...
See https://github.com/cert-manager/cert-manager/issues/6329 for a list of linked PRs.
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
*/}}
{{- define "image" -}}
{{- /*
Expand Down
154 changes: 73 additions & 81 deletions deploy/charts/discovery-agent/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,79 @@ The Discovery Agent connects your Kubernetes or OpenShift cluster to Palo Alto N

<!-- AUTO-GENERATED -->

#### **config.tsgID** ~ `string,number`
> Default value:
> ```yaml
> ""
> ```

Required: The TSG (Tenant Service Group) ID to use when connecting to SCM. NB: TSG IDs are numeric, but should be treated as strings. If being set with the Helm CLI prefer `--set-string`.


#### **config.clusterName** ~ `string`
> Default value:
> ```yaml
> ""
> ```

Required: A human readable name for the cluster into which the agent is being deployed.

This cluster name will be associated with the data that the agent uploads to the backend.

#### **config.clusterDescription** ~ `string`
> Default value:
> ```yaml
> ""
> ```

A short description of the cluster where the agent is deployed (optional).

This description will be associated with the data that the agent uploads to the backend.

#### **config.period** ~ `string`
> Default value:
> ```yaml
> 0h1m0s
> ```

How often to push data to the remote server

#### **config.excludeAnnotationKeysRegex** ~ `array`
> Default value:
> ```yaml
> []
> ```

You can configure the agent to exclude some annotations or labels from being pushed. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being pushed.

Dots is the only character that needs to be escaped in the regex. Use either double quotes with escaped single quotes or unquoted strings for the regex to avoid YAML parsing issues with `\.`.

Example: excludeAnnotationKeysRegex: ['^kapp\.k14s\.io/original.*']
#### **config.excludeLabelKeysRegex** ~ `array`
> Default value:
> ```yaml
> []
> ```
#### **config.clientID** ~ `string`
> Default value:
> ```yaml
> ""
> ```

Deprecated: Client ID for the configured service account. The client ID should be provided in the "clientID" field of the authentication secret (see config.secretName). This field is provided for compatibility for users migrating from the "venafi-kubernetes-agent" chart.

#### **config.secretName** ~ `string`
> Default value:
> ```yaml
> discovery-agent-credentials
> ```

The name of the Secret containing the NGTS built-in service account credentials.
The Secret must contain the following key:
- privatekey.pem: PEM-encoded private key for the service account
The Secret should also contain the following key:
- clientID: Service account client ID (config.clientID must be set if not present)

#### **replicaCount** ~ `number`
> Default value:
> ```yaml
Expand All @@ -32,17 +105,6 @@ Examples:
- jetstack
- custom-namespace

#### **image.registry** ~ `string`

Deprecated: per-component registry prefix.

If set, this value is *prepended* to the image repository that the chart would otherwise render. This applies both when `image.repository` is set and when the repository is computed from
`imageRegistry` + `imageNamespace` + `image.name`.

This can produce "double registry" style references such as
`legacy.example.io/quay.io/jetstack/...`. Prefer using the global
`imageRegistry`/`imageNamespace` values.

#### **image.repository** ~ `string`
> Default value:
> ```yaml
Expand Down Expand Up @@ -219,76 +281,6 @@ Configures the NO_PROXY environment variable where a HTTP proxy is required, but

Configure a PodDisruptionBudget for the agent's Deployment. If running with multiple replicas, consider setting podDisruptionBudget.enabled to true.

#### **config.period** ~ `string`
> Default value:
> ```yaml
> 0h1m0s
> ```

Push data every 12 hours unless changed.
#### **config.excludeAnnotationKeysRegex** ~ `array`
> Default value:
> ```yaml
> []
> ```

You can configure the agent to exclude some annotations or labels from being pushed. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being pushed.

Dots is the only character that needs to be escaped in the regex. Use either double quotes with escaped single quotes or unquoted strings for the regex to avoid YAML parsing issues with `\.`.

Example: excludeAnnotationKeysRegex: ['^kapp\.k14s\.io/original.*']
#### **config.excludeLabelKeysRegex** ~ `array`
> Default value:
> ```yaml
> []
> ```
#### **config.clusterName** ~ `string`
> Default value:
> ```yaml
> ""
> ```

A human readable name for the cluster where the agent is deployed (required).

This cluster name will be associated with the data that the agent uploads to the NGTS service.
#### **config.clusterDescription** ~ `string`
> Default value:
> ```yaml
> ""
> ```

A short description of the cluster where the agent is deployed (optional).

This description will be associated with the data that the agent uploads to the NGTS service. The description may include contact information such as the email address of the cluster administrator.
#### **config.clientID** ~ `string`
> Default value:
> ```yaml
> ""
> ```

Deprecated: Client ID for the configured service account. The client ID should be provided in the "clientID" field of the authentication secret (see config.secretName).

#### **config.tsgID** ~ `string`
> Default value:
> ```yaml
> ""
> ```

The TSG (Tenant Security Group) ID for your NGTS account (required). NB: TSG IDs are numeric, but must be provided as strings (in double quotes).


#### **config.secretName** ~ `string`
> Default value:
> ```yaml
> discovery-agent-credentials
> ```

The name of the Secret containing the NGTS service account credentials.
The Secret must contain the following key:
- privatekey.pem: PEM-encoded private key for the service account
The Secret may contain the following key:
- clientID: Service account client ID (config.clientID must be set if not present)

#### **extraArgs** ~ `array`
> Default value:
> ```yaml
Expand Down
18 changes: 2 additions & 16 deletions deploy/charts/discovery-agent/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ Create the name of the service account to use
{{- end }}

{{/*
Util function for generating the image URL based on the provided options.
IMPORTANT: This function is standardized across all charts in the cert-manager GH organization.
Any changes to this function should also be made in cert-manager, trust-manager, approver-policy, ...
See https://github.com/cert-manager/cert-manager/issues/6329 for a list of linked PRs.
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
Comment on lines +65 to +66
Copy link
Copy Markdown
Contributor Author

@SgtCoDFish SgtCoDFish Apr 16, 2026

Choose a reason for hiding this comment

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

note: replacing a copy-paste comment with one for this repo specifically!

*/}}
{{- define "image" -}}
{{- /*
Expand All @@ -85,12 +83,6 @@ usage through tuple/variable indirection.
{{- $repository := "" -}}
{{- if $image.repository -}}
{{- $repository = $image.repository -}}
{{- /*
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 -}}
Comment on lines -88 to -93
Copy link
Copy Markdown
Contributor Author

@SgtCoDFish SgtCoDFish Apr 16, 2026

Choose a reason for hiding this comment

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

note: I see no reason to have this backwards compat shim in a brand new chart, so I removed it for simplicity.

{{- else -}}
{{- $name := required "ERROR: image.name must be set when image.repository is empty" $image.name -}}
{{- $repository = $name -}}
Expand All @@ -100,12 +92,6 @@ usage through tuple/variable indirection.
{{- if $imageRegistry -}}
{{- $repository = printf "%s/%s" $imageRegistry $repository -}}
{{- end -}}
{{- /*
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 -}}
{{- end -}}
{{- $repository -}}
{{- if and $image.tag $image.digest -}}
Expand Down
16 changes: 8 additions & 8 deletions deploy/charts/discovery-agent/templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ data:
{{- end }}
data-gatherers:
- kind: k8s-discovery
name: ngts/discovery
name: k8s/discovery
Copy link
Copy Markdown
Contributor Author

@SgtCoDFish SgtCoDFish Apr 16, 2026

Choose a reason for hiding this comment

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

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

- kind: k8s-dynamic
name: ngts/secrets
name: k8s/secrets
config:
resource-type:
version: v1
Expand All @@ -33,42 +33,42 @@ data:
- type!=bootstrap.kubernetes.io/token
- type!=helm.sh/release.v1
- kind: k8s-dynamic
name: ngts/jobs
name: k8s/jobs
config:
resource-type:
version: v1
group: batch
resource: jobs
- kind: k8s-dynamic
name: ngts/cronjobs
name: k8s/cronjobs
config:
resource-type:
version: v1
group: batch
resource: cronjobs
- kind: k8s-dynamic
name: ngts/deployments
name: k8s/deployments
config:
resource-type:
version: v1
group: apps
resource: deployments
- kind: k8s-dynamic
name: ngts/statefulsets
name: k8s/statefulsets
config:
resource-type:
version: v1
group: apps
resource: statefulsets
- kind: k8s-dynamic
name: ngts/daemonsets
name: k8s/daemonsets
config:
resource-type:
version: v1
group: apps
resource: daemonsets
- kind: k8s-dynamic
name: ngts/pods
name: k8s/pods
config:
resource-type:
version: v1
Expand Down
Loading
Loading