From 3135c84f05b3b537f5e69626733fee80f19626c9 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Wed, 2 Sep 2026 17:54:03 +0300 Subject: [PATCH 1/4] CP-25864: fall back cluster_name to cyberark.service_id under Conjur JWT The MachineHub cluster_name fallback chain (cluster_name -> cluster_id -> ARK_USERNAME -> empty) always resolves empty for wizard-deployed agents: the disco-agent chart never sets cluster_id, and ARK_USERNAME doesn't exist under Conjur JWT auth. Add cyberark.service_id (the Conjur authn-jwt service ID, always set by the onboarding wizard) as a final fallback before empty. --- pkg/agent/config.go | 8 ++++++++ pkg/agent/config_test.go | 27 +++++++++++++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 3b13139d..2cb74652 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -793,6 +793,14 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) clusterName = arkUsername } } + if clusterName == "" && cfg.CyberArk != nil && cfg.CyberArk.ServiceID != "" { + // Conjur JWT installs have no ARK_USERNAME to fall back to. + // cyberark.service_id (the Conjur authn-jwt service ID, + // typically the cluster UUID) is always set for these + // installs, so it's a better last resort than an empty name. + log.Info("Using cyberark.service_id as cluster name because cluster_name, cluster_id, and ARK_USERNAME are all unset; prefer setting cluster_name explicitly", "clusterName", cfg.CyberArk.ServiceID) + clusterName = cfg.CyberArk.ServiceID + } if cfg.OrganizationID != "" { log.Info(fmt.Sprintf(`Ignoring the organization_id field in the config file. This field is not needed in %s mode.`, res.OutputMode)) } diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index a8f8e324..6db10fb5 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1388,14 +1388,17 @@ func TestConfig_CyberArk_Validation(t *testing.T) { }) // cluster_name fallback order: cluster_name > cluster_id > ARK_USERNAME > - // empty. Explicit config (cluster_name, cluster_id) always wins over the - // env var; ARK_USERNAME is a legacy fallback kept only for pre-Conjur - // installs that set neither config field — the chart never emits - // cluster_id, so a chart-installed agent can't have set it, and - // ARK_USERNAME still applies. This also covers a migrating install + // cyberark.service_id > empty. Explicit config (cluster_name, cluster_id) + // always wins over the env var; ARK_USERNAME is a legacy fallback kept + // only for pre-Conjur installs that set neither config field — the chart + // never emits cluster_id, so a chart-installed agent can't have set it, + // and ARK_USERNAME still applies. This also covers a migrating install // (service_id set to test Conjur alongside still-present ARK_USERNAME, // no cluster_id yet): its reported name doesn't change just because - // service_id was added. + // service_id was added. service_id is checked last, below ARK_USERNAME, + // so it only kicks in for pure Conjur-JWT installs that have nothing + // else set — the onboarding wizard always sets it, so it's a real + // fallback even if the generated Helm command omits cluster_name. t.Run("cluster_name falls back to cluster_id when set, even if ARK_USERNAME is also set", func(t *testing.T) { setEnv(t) t.Setenv("ARK_USERNAME", "svc-agent@tenant") @@ -1434,7 +1437,7 @@ func TestConfig_CyberArk_Validation(t *testing.T) { assert.Equal(t, "svc-agent@tenant", got.ClusterName) }) - t.Run("cluster_name is empty when cluster_name, cluster_id, and ARK_USERNAME are all unset", func(t *testing.T) { + t.Run("cluster_name falls back to cyberark.service_id when cluster_name, cluster_id, and ARK_USERNAME are all unset", func(t *testing.T) { setEnv(t) got, _, err := ValidateAndCombineConfig(discardLogs(), withConfig(testutil.Undent(` @@ -1443,9 +1446,17 @@ func TestConfig_CyberArk_Validation(t *testing.T) { `)), withCmdLineFlags("--period", "1m", "--machine-hub")) require.NoError(t, err) - assert.Equal(t, "", got.ClusterName) + assert.Equal(t, "dev-cluster", got.ClusterName) }) + // The config-validation gate (either service_id or ARK_USERNAME+ARK_SECRET + // must be set) combined with the fallback chain means a valid MachineHub + // config can no longer produce an empty ClusterName: whichever of the two + // auth methods satisfies validation also satisfies a cluster_name + // fallback. There is deliberately no test asserting an empty ClusterName + // here — every combination that reaches this point has a non-empty + // fallback available. + t.Run("jwt_source spiffe is rejected", func(t *testing.T) { setEnv(t) _, _, err := ValidateAndCombineConfig(discardLogs(), From 777a67cbe35b78f46e6ababc0a7ff4dc585bce1f Mon Sep 17 00:00:00 2001 From: rzisholz Date: Wed, 2 Sep 2026 22:32:04 +0300 Subject: [PATCH 2/4] =?UTF-8?q?CP-25864:=20address=20review=20=E2=80=94=20?= =?UTF-8?q?assertive=20test,=20docs=20correction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the "empty ClusterName is unreachable" prose comment with a test that asserts the config-validation gate directly, so a future auth-method change that reopens the empty-cluster_name path fails this test instead of silently regressing. Fix values.yaml's stale clusterName fallback description (claimed a service-account-name fallback that was never implemented in Go) and regenerate README.md/values.schema.json via `make ark-generate`. --- deploy/charts/disco-agent/README.md | 2 +- deploy/charts/disco-agent/values.schema.json | 2 +- deploy/charts/disco-agent/values.yaml | 7 ++++-- pkg/agent/config_test.go | 24 ++++++++++++++------ 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index 8bbc4144..7f69e285 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -401,7 +401,7 @@ Example: excludeAnnotationKeysRegex: ['^kapp\.k14s\.io/original.*'] A human readable name for the cluster where the agent is deployed (optional). -This cluster name will be associated with the data that the agent uploads to the Discovery and Context service. If empty (the default), the service account name will be used instead. +This cluster name will be associated with the data that the agent uploads to the Discovery and Context service. If empty (the default) and using the legacy username/password authentication, ARK_USERNAME is used instead. If empty and using Conjur JWT authentication (no ARK_USERNAME), the cyberark.serviceId below is used instead. There is no service-account-name fallback — set this explicitly for a meaningful cluster name. #### **config.clusterDescription** ~ `string` > Default value: > ```yaml diff --git a/deploy/charts/disco-agent/values.schema.json b/deploy/charts/disco-agent/values.schema.json index a6f17f32..5e2c4e11 100644 --- a/deploy/charts/disco-agent/values.schema.json +++ b/deploy/charts/disco-agent/values.schema.json @@ -149,7 +149,7 @@ }, "helm-values.config.clusterName": { "default": "", - "description": "A human readable name for the cluster where the agent is deployed (optional).\n\nThis cluster name will be associated with the data that the agent uploads to the Discovery and Context service. If empty (the default), the service account name will be used instead.", + "description": "A human readable name for the cluster where the agent is deployed (optional).\n\nThis cluster name will be associated with the data that the agent uploads to the Discovery and Context service. If empty (the default) and using the legacy username/password authentication, ARK_USERNAME is used instead. If empty and using Conjur JWT authentication (no ARK_USERNAME), the cyberark.serviceId below is used instead. There is no service-account-name fallback — set this explicitly for a meaningful cluster name.", "type": "string" }, "helm-values.config.cyberark": { diff --git a/deploy/charts/disco-agent/values.yaml b/deploy/charts/disco-agent/values.yaml index 38c6ee22..0bedb48f 100644 --- a/deploy/charts/disco-agent/values.yaml +++ b/deploy/charts/disco-agent/values.yaml @@ -183,8 +183,11 @@ config: # A human readable name for the cluster where the agent is deployed (optional). # # This cluster name will be associated with the data that the agent uploads to - # the Discovery and Context service. If empty (the default), the service - # account name will be used instead. + # the Discovery and Context service. If empty (the default) and using the + # legacy username/password authentication, ARK_USERNAME is used instead. If + # empty and using Conjur JWT authentication (no ARK_USERNAME), the + # cyberark.serviceId below is used instead. There is no service-account-name + # fallback — set this explicitly for a meaningful cluster name. clusterName: "" # A short description of the cluster where the agent is deployed (optional). diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 6db10fb5..05909879 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1449,13 +1449,23 @@ func TestConfig_CyberArk_Validation(t *testing.T) { assert.Equal(t, "dev-cluster", got.ClusterName) }) - // The config-validation gate (either service_id or ARK_USERNAME+ARK_SECRET - // must be set) combined with the fallback chain means a valid MachineHub - // config can no longer produce an empty ClusterName: whichever of the two - // auth methods satisfies validation also satisfies a cluster_name - // fallback. There is deliberately no test asserting an empty ClusterName - // here — every combination that reaches this point has a non-empty - // fallback available. + // The only way ClusterName ends up empty is if config-validation already + // failed: whichever of the two auth methods (service_id, or + // ARK_USERNAME+ARK_SECRET) satisfies the gate at config.go's cyberark + // validation block also satisfies a cluster_name fallback above. Assert + // the gate directly, rather than only in prose, so a future change that + // makes either auth method optional without updating the fallback chain + // fails this test instead of silently reintroducing an empty + // cluster_name for a config that validates successfully. + t.Run("empty ClusterName only occurs when config validation also fails", func(t *testing.T) { + setEnv(t) + got, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(""), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.Error(t, err) + assert.Contains(t, err.Error(), "MachineHub mode requires either cyberark.service_id or ARK_USERNAME/ARK_SECRET") + assert.Equal(t, "", got.ClusterName) + }) t.Run("jwt_source spiffe is rejected", func(t *testing.T) { setEnv(t) From 62a26ad40c5bbc2a23fee3add259314f63aca195 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Thu, 3 Sep 2026 12:34:50 +0300 Subject: [PATCH 3/4] CP-25864: fix serviceId doc contradiction, note fallback source in log values.yaml's serviceId comment still said "configured for this tenant" with a "disco-agent" worked example two lines above where this PR's clusterName fallback comment now says it's used as the cluster name - the same generated file asserted both "per-tenant" and "becomes the cluster's display name" for the same field. Correct it to per-cluster, matching README.md's own hand-written prose, and drop the non-unique example. Also log which fallback fired for cluster_name so an operator who sees a surprising name (e.g. from a reused service_id) can tell where it came from. --- deploy/charts/disco-agent/README.md | 3 +-- deploy/charts/disco-agent/values.schema.json | 2 +- deploy/charts/disco-agent/values.yaml | 9 ++++++--- pkg/agent/config.go | 8 +++++--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index 7f69e285..336a8e30 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -424,8 +424,7 @@ Enable sending of Secret values to CyberArk in addition to metadata. Metadata is > "" > ``` -The Conjur authn-jwt authenticator service ID configured for this tenant. Set this to use the Conjur JWT exchange (preferred). Leave empty to use the legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET in the credentials Secret) for backward compatibility. If both are set, the serviceId (Conjur) wins. NOTE: bare service-id segment (e.g. "disco-agent"), NOT the policy path -"conjur/authn-jwt/disco-agent" — the agent builds the URL as +The Conjur authn-jwt authenticator service ID configured for this cluster (one authenticator per cluster, not shared across a tenant's clusters — see config.clusterName above, which falls back to this value and depends on it being cluster-unique). Set this to use the Conjur JWT exchange (preferred). Leave empty to use the legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET in the credentials Secret) for backward compatibility. If both are set, the serviceId (Conjur) wins. NOTE: bare service-id segment (e.g. a UUID chosen at onboarding), NOT the policy path "conjur/authn-jwt/" — the agent builds the URL as /authn-jwt///authenticate. #### **config.cyberark.account** ~ `string` > Default value: diff --git a/deploy/charts/disco-agent/values.schema.json b/deploy/charts/disco-agent/values.schema.json index 5e2c4e11..1087a483 100644 --- a/deploy/charts/disco-agent/values.schema.json +++ b/deploy/charts/disco-agent/values.schema.json @@ -179,7 +179,7 @@ }, "helm-values.config.cyberark.serviceId": { "default": "", - "description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Set this to use the Conjur JWT exchange (preferred). Leave empty to use the legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET in the credentials Secret) for backward compatibility. If both are set, the serviceId (Conjur) wins. NOTE: bare service-id segment (e.g. \"disco-agent\"), NOT the policy path\n\"conjur/authn-jwt/disco-agent\" — the agent builds the URL as\n/authn-jwt///authenticate.", + "description": "The Conjur authn-jwt authenticator service ID configured for this cluster (one authenticator per cluster, not shared across a tenant's clusters — see config.clusterName above, which falls back to this value and depends on it being cluster-unique). Set this to use the Conjur JWT exchange (preferred). Leave empty to use the legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET in the credentials Secret) for backward compatibility. If both are set, the serviceId (Conjur) wins. NOTE: bare service-id segment (e.g. a UUID chosen at onboarding), NOT the policy path \"conjur/authn-jwt/\" — the agent builds the URL as\n/authn-jwt///authenticate.", "type": "string" }, "helm-values.config.excludeAnnotationKeysRegex": { diff --git a/deploy/charts/disco-agent/values.yaml b/deploy/charts/disco-agent/values.yaml index 0bedb48f..805d6c83 100644 --- a/deploy/charts/disco-agent/values.yaml +++ b/deploy/charts/disco-agent/values.yaml @@ -210,13 +210,16 @@ config: # short-lived Conjur access token, then uses that token to authenticate to the # Discovery & Context upload API. cyberark: - # The Conjur authn-jwt authenticator service ID configured for this tenant. + # The Conjur authn-jwt authenticator service ID configured for this + # cluster (one authenticator per cluster, not shared across a tenant's + # clusters — see config.clusterName above, which falls back to this + # value and depends on it being cluster-unique). # Set this to use the Conjur JWT exchange (preferred). Leave empty to use the # legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET # in the credentials Secret) for backward compatibility. If both are set, the # serviceId (Conjur) wins. - # NOTE: bare service-id segment (e.g. "disco-agent"), NOT the policy path - # "conjur/authn-jwt/disco-agent" — the agent builds the URL as + # NOTE: bare service-id segment (e.g. a UUID chosen at onboarding), NOT the + # policy path "conjur/authn-jwt/" — the agent builds the URL as # /authn-jwt///authenticate. serviceId: "" diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 2cb74652..dcd92dd0 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -796,9 +796,11 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) if clusterName == "" && cfg.CyberArk != nil && cfg.CyberArk.ServiceID != "" { // Conjur JWT installs have no ARK_USERNAME to fall back to. // cyberark.service_id (the Conjur authn-jwt service ID, - // typically the cluster UUID) is always set for these - // installs, so it's a better last resort than an empty name. - log.Info("Using cyberark.service_id as cluster name because cluster_name, cluster_id, and ARK_USERNAME are all unset; prefer setting cluster_name explicitly", "clusterName", cfg.CyberArk.ServiceID) + // expected to be cluster-unique, but that uniqueness is an + // onboarding convention, not something this agent verifies) + // is always set for these installs, so it's a better last + // resort than an empty name. + log.Info("cluster_name and cluster_id are unset and ARK_USERNAME is not set; falling back to cyberark.service_id as the cluster name — set cluster_name explicitly for a human-readable name, and ensure service_id is unique per cluster if you rely on this fallback", "clusterName", cfg.CyberArk.ServiceID) clusterName = cfg.CyberArk.ServiceID } if cfg.OrganizationID != "" { From c876edfcdd4fb0058d2ea07b2f7bc15c244f57f5 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Thu, 3 Sep 2026 13:29:57 +0300 Subject: [PATCH 4/4] CP-25864: fix README's remaining per-tenant wording and shared literal "Per-tenant Conjur onboarding" and the worked example's literal --set config.cyberark.serviceId=disco-agent contradicted the per-cluster-unique framing this PR already established in the generated values comment. Retitle the section, correct the onboarding sentence, and replace the shared literal with a placeholder variable. --- deploy/charts/disco-agent/README.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index 336a8e30..3d4dce59 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -97,14 +97,15 @@ agent expects (`/var/run/secrets/tokens/jwt`) — this path isn't configurable, so there's one fewer way to misconfigure it. No manual volume configuration is required. -### Per-tenant Conjur onboarding +### Per-cluster Conjur onboarding -Before deploying the agent, the target tenant must be onboarded in Conjur +Before deploying the agent, the target cluster must be onboarded in Conjur Cloud: an `authn-jwt` authenticator scoped to the cluster's OIDC issuer and JWKS, a registered workload for the agent's ServiceAccount, and the grants that -let that workload authenticate and upload. Onboarding is performed through the -CyberArk web console — see the product documentation for the current -walkthrough. +let that workload authenticate and upload. Each cluster gets its own +authenticator and service ID — the value is not shared across a tenant's +clusters. Onboarding is performed through the CyberArk web console — see the +product documentation for the current walkthrough. Onboarding needs only the tenant administrator's own Conjur Cloud credentials. The agent itself holds no Conjur identity beyond its projected ServiceAccount @@ -117,12 +118,13 @@ value for `config.cyberark.serviceId` below. ### Deploy the agent ```sh +# $SERVICE_ID is this cluster's own authn-jwt service ID from onboarding above — do not reuse it across clusters. helm upgrade agent "oci://${OCI_BASE}/charts/disco-agent" \ --install \ --create-namespace \ --namespace "$NAMESPACE" \ --set fullnameOverride=disco-agent \ - --set config.cyberark.serviceId=disco-agent \ + --set config.cyberark.serviceId="$SERVICE_ID" \ --set acceptTerms=true ```