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.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..05909879 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,6 +1446,24 @@ func TestConfig_CyberArk_Validation(t *testing.T) { `)), withCmdLineFlags("--period", "1m", "--machine-hub")) require.NoError(t, err) + assert.Equal(t, "dev-cluster", got.ClusterName) + }) + + // 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) })