CP-21164: wire Conjur JWT config into the top-level CyberArk client

NewCyberArk and the agent config schema were still legacy-username/password-only
at the top level, even though the underlying authenticator selection (previous
PR) already supports Conjur JWT. Threads service_id/account/jwt_source/jwt_file_path
through cyberark.service_id in the agent config down to NewCyberArk, so the
config file is the single place an operator chooses which authentication method
to use.

config.go's call site and client_cyberark.go's signature change together —
splitting them across two PRs would leave one non-building at every commit
in between.

jwt_source validation now delegates to the shared cyberark.ValidateJWTSource
(previous PR) instead of re-implementing the same rule with different wording.

Requiring an auth method (service_id or ARK_USERNAME) is now checked at
config-validation time rather than left to cyberark.selectAuthenticator alone
— that only runs at first upload, so a misconfigured agent could otherwise
report healthy for up to a full config.period before failing.

MachineHub's cluster_name fallback now checks cluster_id before ARK_USERNAME
(cluster_id > ARK_USERNAME > empty) instead of dropping ARK_USERNAME as a
fallback entirely — cluster_id still takes priority when both are set (needed
since ARK_USERNAME doesn't exist on the Conjur JWT path), but an existing
username/password install with neither cluster_name nor cluster_id configured
keeps reporting under its ARK_USERNAME-derived name instead of an empty one.

Adds FakeCyberArkUsernamePassword and matching integration tests: the only
existing test exercising NewCyberArk's username/password path set
ARK_USERNAME/ARK_SECRET but also passed a non-empty serviceID, which
selectAuthenticator prioritises — so it silently tested Conjur regardless of
those env vars, leaving the legacy path with no coverage through this seam.
Also adds a two-upload regression test for the previous PR's cfg.Secret
zeroing fix, which a single-upload test can't catch.

Author: rzisholz

pkg/agent/config.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/client-go/rest"
 
 	"github.com/jetstack/preflight/api"
+	"github.com/jetstack/preflight/internal/cyberark"
 	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/datagatherer"
 	"github.com/jetstack/preflight/pkg/datagatherer/k8sdiscovery"
@@ -64,6 +65,9 @@ type Config struct {
 	DataGatherers  []DataGatherer     `yaml:"data-gatherers"`
 	VenafiCloud    *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"`
 
+	// CyberArk holds configuration for MachineHub mode (POC).
+	CyberArk *CyberArkConfig `yaml:"cyberark,omitempty"`
+
 	// For testing purposes.
 	InputPath string `yaml:"input-path"`
 	// For testing purposes.
@@ -101,6 +105,19 @@ type VenafiCloudConfig struct {
 	UploadPath string `yaml:"upload_path,omitempty"`
 }
 
+// CyberArkConfig holds YAML configuration for MachineHub (CyberArk) mode (POC).
+type CyberArkConfig struct {
+	// ServiceID is the authn-jwt service ID configured in Conjur (e.g. "dev-cluster").
+	ServiceID string `yaml:"service_id"`
+	// Account is the Conjur account name. Defaults to "conjur" when empty.
+	Account string `yaml:"account"`
+	// JWTSource selects how the agent obtains its JWT. Must be "" or "file" in the POC.
+	JWTSource string `yaml:"jwt_source"`
+	// JWTFilePath is the path to the JWT file when jwt_source is "file".
+	// Defaults to the standard projected service-account token path when empty.
+	JWTFilePath string `yaml:"jwt_file_path"`
+}
+
 type AgentCmdFlags struct {
 	// ConfigFilePath (--config-file, -c) is the path to the agent configuration
 	// YAML file.
@@ -459,6 +476,9 @@ type CombinedConfig struct {
 	TSGID         string
 	NGTSServerURL string
 
+	// MachineHub mode only.
+	CyberArk CyberArkConfig
+
 	// Only used for testing purposes.
 	OutputPath string
 	InputPath  string
@@ -753,6 +773,10 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 			clusterID = cfg.ClusterID
 		case MachineHub:
 			clusterName = cfg.ClusterName
+			if clusterName == "" && cfg.ClusterID != "" {
+				log.Info("Using cluster_id as cluster_name", "clusterID", cfg.ClusterID)
+				clusterName = cfg.ClusterID
+			}
 			if clusterName == "" {
 				if arkUsername, found := os.LookupEnv("ARK_USERNAME"); found {
 					log.Info("Using ARK_USERNAME environment variable as cluster name", "clusterName", arkUsername)
@@ -762,8 +786,8 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 			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))
 			}
-			if cfg.ClusterID != "" {
-				log.Info(fmt.Sprintf(`Ignoring the cluster_id field in the config file. This field is not needed in %s mode.`, res.OutputMode))
+			if clusterName == "" {
+				log.Info("cluster_name is not set in MachineHub mode; cluster name will be empty")
 			}
 		}
 		res.OrganizationID = organizationID
@@ -773,6 +797,27 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 		res.ClaimableCerts = cfg.ClaimableCerts
 	}
 
+	// Validation of `cyberark.*` (MachineHub mode only).
+	if res.OutputMode == MachineHub {
+		ark := CyberArkConfig{}
+		if cfg.CyberArk != nil {
+			ark = *cfg.CyberArk
+		}
+		// service_id selects the Conjur JWT exchange. It is no longer required:
+		// the agent also supports the legacy username/password method via
+		// ARK_USERNAME/ARK_SECRET. Checked here, at config-validation time,
+		// rather than left to cyberark.selectAuthenticator alone — that only
+		// runs at first upload, so a misconfigured agent would otherwise
+		// report healthy for up to a full config.period before failing.
+		if ark.ServiceID == "" && os.Getenv("ARK_USERNAME") == "" {
+			errs = multierror.Append(errs, fmt.Errorf("MachineHub mode requires either cyberark.service_id or ARK_USERNAME/ARK_SECRET"))
+		}
+		if err := cyberark.ValidateJWTSource(ark.JWTSource); err != nil {
+			errs = multierror.Append(errs, fmt.Errorf("cyberark.jwt_source %w", err))
+		}
+		res.CyberArk = ark
+	}
+
 	// Validation of `data-gatherers`.
 	{
 		if dgErr := ValidateDataGatherers(cfg.DataGatherers); dgErr != nil {
@@ -987,7 +1032,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 			rootCAs *x509.CertPool
 		)
 		httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
-		outputClient, err = client.NewCyberArk(httpClient)
+		outputClient, err = client.NewCyberArk(httpClient, cfg.CyberArk.ServiceID, cfg.CyberArk.Account, cfg.CyberArk.JWTSource, cfg.CyberArk.JWTFilePath)
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}

pkg/agent/config_test.go

@@ -658,56 +658,74 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 		assert.Equal(t, VenafiConnection, got.OutputMode)
 	})
 
-	const arkUsername = "cluster-1-region-1-cloud-1@cyberark.cloud.123456"
-
 	t.Run("--machine-hub selects MachineHub mode", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
 		t.Setenv("ARK_SUBDOMAIN", "tlspk")
-		t.Setenv("ARK_USERNAME", arkUsername)
-		t.Setenv("ARK_SECRET", "test-secret")
 		got, cl, err := ValidateAndCombineConfig(discardLogs(),
-			withConfig(""),
+			withConfig(testutil.Undent(`
+				cluster_name: my-cluster
+				cyberark:
+				  service_id: dev-cluster
+			`)),
 			withCmdLineFlags("--period", "1m", "--machine-hub"))
 		require.NoError(t, err)
 		assert.Equal(t, MachineHub, got.OutputMode)
-		assert.Equal(t, arkUsername, got.ClusterName,
-			"the ClusterName should default to the ARK_USERNAME value if the cluster_name in the config file is empty")
+		assert.Equal(t, "my-cluster", got.ClusterName)
+		assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
+		assert.IsType(t, &client.CyberArkClient{}, cl)
+	})
+
+	t.Run("--machine-hub with cluster_id fallback when cluster_name is empty", func(t *testing.T) {
+		t.Setenv("POD_NAMESPACE", "venafi")
+		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
+		t.Setenv("ARK_SUBDOMAIN", "tlspk")
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cluster_id: my-cluster-id
+				cyberark:
+				  service_id: dev-cluster
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, MachineHub, got.OutputMode)
+		assert.Equal(t, "my-cluster-id", got.ClusterName,
+			"cluster_id should be used as cluster_name when cluster_name is empty")
 		assert.IsType(t, &client.CyberArkClient{}, cl)
 	})
 
 	t.Run("--machine-hub with cluster_name override", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
 		t.Setenv("ARK_SUBDOMAIN", "tlspk")
-		t.Setenv("ARK_USERNAME", arkUsername)
-		t.Setenv("ARK_SECRET", "test-secret")
 		got, cl, err := ValidateAndCombineConfig(discardLogs(),
 			withConfig(testutil.Undent(`
 				cluster_name: override-cluster-name
-            `)),
+				cyberark:
+				  service_id: dev-cluster
+			`)),
 			withCmdLineFlags("--period", "1m", "--machine-hub"))
 		require.NoError(t, err)
 		assert.Equal(t, MachineHub, got.OutputMode)
-		assert.Equal(t, "override-cluster-name", got.ClusterName,
-			"the cluster_name in the config file should be used if not empty, even if ARK_USERNAME is set")
+		assert.Equal(t, "override-cluster-name", got.ClusterName)
 		assert.IsType(t, &client.CyberArkClient{}, cl)
 	})
 
-	t.Run("--machine-hub without required environment variables", func(t *testing.T) {
+	t.Run("--machine-hub without ARK_SUBDOMAIN environment variable", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
 		t.Setenv("ARK_SUBDOMAIN", "")
-		t.Setenv("ARK_USERNAME", "")
-		t.Setenv("ARK_SECRET", "")
 		got, cl, err := ValidateAndCombineConfig(discardLogs(),
-			withConfig(""),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+			`)),
 			withCmdLineFlags("--period", "1m", "--machine-hub"))
 		assert.Equal(t, CombinedConfig{}, got)
 		assert.Nil(t, cl)
 		assert.EqualError(t, err, testutil.Undent(`
 			validating creds: failed loading config using the MachineHub mode: 1 error occurred:
-				* missing environment variables: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET
+				* missing environment variables: ARK_SUBDOMAIN
 
 	   `))
 	})
@@ -1303,6 +1321,167 @@ func Test_ValidateAndCombineConfig_NGTS(t *testing.T) {
 	})
 }
 
+func TestConfig_CyberArk_Validation(t *testing.T) {
+	// Common env setup: ARK_SUBDOMAIN is the only required env var for MachineHub mode.
+	setEnv := func(t *testing.T) {
+		t.Helper()
+		t.Setenv("POD_NAMESPACE", "venafi")
+		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
+		t.Setenv("ARK_SUBDOMAIN", "tlspk")
+	}
+
+	// service_id is not required at config-validation time as long as the
+	// legacy username/password method (ARK_USERNAME/ARK_SECRET, set via env,
+	// not config) is configured instead — one or the other is required,
+	// checked here rather than deferred to cyberark.selectAuthenticator at
+	// first upload. See the comment on this validation block in config.go.
+	t.Run("empty service_id is valid at config time when ARK_USERNAME is set", func(t *testing.T) {
+		setEnv(t)
+		t.Setenv("ARK_USERNAME", "test@example.com")
+		combined, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: ""
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "", combined.CyberArk.ServiceID)
+	})
+
+	t.Run("missing cyberark block is valid at config time when ARK_USERNAME is set", func(t *testing.T) {
+		setEnv(t)
+		t.Setenv("ARK_USERNAME", "test@example.com")
+		combined, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(""),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "", combined.CyberArk.ServiceID)
+	})
+
+	t.Run("neither service_id nor ARK_USERNAME is an error at config time", func(t *testing.T) {
+		setEnv(t)
+		_, _, 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")
+	})
+
+	// cluster_name fallback order: cluster_name > cluster_id > ARK_USERNAME > empty.
+	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")
+		got, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cluster_id: my-cluster-id
+				cyberark:
+				  service_id: dev-cluster
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "my-cluster-id", got.ClusterName)
+	})
+
+	t.Run("cluster_name falls back to ARK_USERNAME when cluster_id is unset", func(t *testing.T) {
+		setEnv(t)
+		t.Setenv("ARK_USERNAME", "svc-agent@tenant")
+		got, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(""),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		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) {
+		setEnv(t)
+		got, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "", got.ClusterName)
+	})
+
+	t.Run("jwt_source spiffe is rejected", func(t *testing.T) {
+		setEnv(t)
+		_, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+				  jwt_source: spiffe
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `cyberark.jwt_source "spiffe" is not supported`)
+	})
+
+	t.Run("jwt_source file is accepted", func(t *testing.T) {
+		setEnv(t)
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+				  jwt_source: file
+				  jwt_file_path: /var/run/secrets/tokens/agent-token
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
+		assert.Equal(t, "file", got.CyberArk.JWTSource)
+		assert.Equal(t, "/var/run/secrets/tokens/agent-token", got.CyberArk.JWTFilePath)
+		assert.IsType(t, &client.CyberArkClient{}, cl)
+	})
+
+	t.Run("jwt_source empty string is accepted", func(t *testing.T) {
+		setEnv(t)
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
+		assert.Equal(t, "", got.CyberArk.JWTSource)
+		assert.IsType(t, &client.CyberArkClient{}, cl)
+	})
+
+	t.Run("account and jwt_file_path are optional", func(t *testing.T) {
+		setEnv(t)
+		got, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				cyberark:
+				  service_id: dev-cluster
+				  account: myaccount
+				  jwt_file_path: /tmp/token
+			`)),
+			withCmdLineFlags("--period", "1m", "--machine-hub"))
+		require.NoError(t, err)
+		assert.Equal(t, "myaccount", got.CyberArk.Account)
+		assert.Equal(t, "/tmp/token", got.CyberArk.JWTFilePath)
+	})
+
+	t.Run("cyberark block is ignored in non-MachineHub modes", func(t *testing.T) {
+		t.Setenv("POD_NAMESPACE", "venafi")
+		fakeCredsPath := withFile(t, `{"user_id":"foo","user_secret":"bar","client_id": "baz","client_secret": "foobar","auth_server_domain":"bazbar"}`)
+		got, _, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: https://preflight.jetstack.io
+				organization_id: my-org
+				cluster_id: my-cluster
+				period: 1h
+				cyberark:
+				  service_id: should-be-ignored
+			`)),
+			withCmdLineFlags("--credentials-file", fakeCredsPath))
+		require.NoError(t, err)
+		// CyberArk config is not copied into CombinedConfig for non-MachineHub modes.
+		assert.Equal(t, CyberArkConfig{}, got.CyberArk)
+	})
+}
+
 const fakePrivKeyPEM = `-----BEGIN PRIVATE KEY-----
 MHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49
 AwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl