Skip to content

CP-25962: Correct the sendSecretValues chart comment #836

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

Merged
wallrj-cyberark merged 2 commits into from
Sep 10, 2026
Merged

CP-25962: Correct the sendSecretValues chart comment #836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e3fc01
Correct the sendSecretValues chart comment
Sep 9, 2026
f368c94
Address review: template fallback and datagatherer doc also had the rot
Sep 10, 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
4 changes: 3 additions & 1 deletion deploy/charts/disco-agent/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,9 @@ This description will be associated with the data that the agent uploads to the
> true
> ```

Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.
Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, and Secret values are sent by default too.
Set this to false to send metadata only.

@wallrj-cyberark wallrj-cyberark Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is one more copy of the same stale claim, and it is stronger than the one you fixed. docs/datagatherers/k8s-dynamic.md:80 still says:

Before Secrets are sent to the Preflight backend, they are redacted so no secret data is transmitted.

With sendSecretValues defaulting to true, pkg/agent/run.go:170 sets encryptSecrets, redactList keeps the data field and adds _encryptedData, so secret data is transmitted (encrypted). A reader who checks the docs to answer "does the agent send my Secret data?" gets opposite answers depending on which file they open.

That line also points at pkg/datagatherer/k8s/fieldfilter.go, which no longer exists — the file moved to pkg/datagatherer/k8sdynamic/fieldfilter.go.

Out of scope if you want to keep this PR to the chart, but it is the same comment rot and worth sweeping in the same pass.

@roeezis roeezis Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed both — the file did move, and the claim contradicts run.go's encryptSecrets/_encryptedData path once sendSecretValues defaults true. Fixed in f368c94: corrected the redact-by-default / encrypt-when-enabled description, fixed the fieldfilter.go link, and pointed at the chart's sendSecretValues doc instead of restating it.

When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.
#### **config.cyberark.serviceId** ~ `string`
> Default value:
> ```yaml
Expand Down
2 changes: 1 addition & 1 deletion deploy/charts/disco-agent/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ spec:
key: ARK_SECRET
optional: true
- name: ARK_SEND_SECRET_VALUES
value: {{ .Values.config.sendSecretValues | default "false" | quote }}
value: {{ .Values.config.sendSecretValues | quote }}
{{- with .Values.http_proxy }}
- name: HTTP_PROXY
value: {{ . }}
Expand Down
2 changes: 1 addition & 1 deletion deploy/charts/disco-agent/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
},
"helm-values.config.sendSecretValues": {
"default": true,
"description": "Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.",
"description": "Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, and Secret values are sent by default too.\nSet this to false to send metadata only.\nWhen enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.",
"type": "boolean"
},
"helm-values.extraArgs": {
Expand Down
3 changes: 2 additions & 1 deletion deploy/charts/disco-agent/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ config:
clusterDescription: ""

# Enable sending of Secret values to CyberArk in addition to metadata.
# Metadata is always sent, but the actual values of Secrets are not sent by default.
# Metadata is always sent, and Secret values are sent by default too.

@wallrj-cyberark wallrj-cyberark Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The chart's own template does not agree with the default this comment now documents. templates/deployment.yaml:89 renders {{ .Values.config.sendSecretValues | default "false" | quote }}, so the template-level fallback is false, not true.

Today the two only disagree when the key is absent: helm template --set config.sendSecretValues=null renders ARK_SEND_SECRET_VALUES="false" while the README says values are sent by default. More to the point, this is the same rot trap you are fixing here. If anyone later drops the sendSecretValues: true line from values.yaml on the assumption that the template fallback covers it, the behaviour silently flips and the docs stay wrong.

Helm's default also treats a genuine false as empty, so the pipeline buys nothing even in the normal case. Changing line 89 to value: {{ .Values.config.sendSecretValues | quote }} would make the template say what the docs now say. Optional for this PR if you would rather keep the diff docs-only, but worth a follow-up.

@roeezis roeezis Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed via helm template: unset renders "false" today while the docs now say true — same rot class. Dropped default "false" (f368c94); unset now renders "true" (values.yaml's default), explicit false still renders "false".

# Set this to false to send metadata only.
# When enabled, Secret data is encrypted using envelope encryption using
# a key managed by CyberArk, fetched from the Discovery and Context service.
sendSecretValues: true
Expand Down
2 changes: 1 addition & 1 deletion docs/datagatherers/k8s-dynamic.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Secrets can be gathered using the following config:
resource: secrets
```

Before Secrets are sent to the Preflight backend, they are redacted so no secret data is transmitted. See [`fieldfilter.go`](./../../pkg/datagatherer/k8s/fieldfilter.go) to see the details of which fields are filtered and which ones are redacted.
By default, Secret `data` is redacted before being sent to the backend, keeping only non-sensitive fields (`tls.crt`, `ca.crt`, `conjur-map`). If `sendSecretValues` is enabled, the full `data` is kept and encrypted before being sent instead. See [`fieldfilter.go`](./../../pkg/datagatherer/k8sdynamic/fieldfilter.go) for the field list, and `sendSecretValues` in the [disco-agent chart](./../../deploy/charts/disco-agent/README.md) for the encryption behaviour.

> **All resource other than Kubernetes Secrets are sent in full, so make sure that you don't store secret information on arbitrary resources.**

Expand Down
Loading