From 3135c84f05b3b537f5e69626733fee80f19626c9 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Wed, 2 Sep 2026 17:54:03 +0300 Subject: [PATCH 1/2] 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/2] =?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)