Skip to content

Commit 4e2daeb

Browse files
author
rzisholz
committed
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. Thread 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.
1 parent 7b9a6fb commit 4e2daeb

5 files changed

Lines changed: 236 additions & 52 deletions

File tree

pkg/agent/config.go

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ type Config struct {
6464
DataGatherers []DataGatherer `yaml:"data-gatherers"`
6565
VenafiCloud *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"`
6666

67+
// CyberArk holds configuration for MachineHub mode (POC).
68+
CyberArk *CyberArkConfig `yaml:"cyberark,omitempty"`
69+
6770
// For testing purposes.
6871
InputPath string `yaml:"input-path"`
6972
// For testing purposes.
@@ -101,6 +104,19 @@ type VenafiCloudConfig struct {
101104
UploadPath string `yaml:"upload_path,omitempty"`
102105
}
103106

107+
// CyberArkConfig holds YAML configuration for MachineHub (CyberArk) mode (POC).
108+
type CyberArkConfig struct {
109+
// ServiceID is the authn-jwt service ID configured in Conjur (e.g. "dev-cluster").
110+
ServiceID string `yaml:"service_id"`
111+
// Account is the Conjur account name. Defaults to "conjur" when empty.
112+
Account string `yaml:"account"`
113+
// JWTSource selects how the agent obtains its JWT. Must be "" or "file" in the POC.
114+
JWTSource string `yaml:"jwt_source"`
115+
// JWTFilePath is the path to the JWT file when jwt_source is "file".
116+
// Defaults to the standard projected service-account token path when empty.
117+
JWTFilePath string `yaml:"jwt_file_path"`
118+
}
119+
104120
type AgentCmdFlags struct {
105121
// ConfigFilePath (--config-file, -c) is the path to the agent configuration
106122
// YAML file.
@@ -459,6 +475,9 @@ type CombinedConfig struct {
459475
TSGID string
460476
NGTSServerURL string
461477

478+
// MachineHub mode only.
479+
CyberArk CyberArkConfig
480+
462481
// Only used for testing purposes.
463482
OutputPath string
464483
InputPath string
@@ -753,17 +772,15 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
753772
clusterID = cfg.ClusterID
754773
case MachineHub:
755774
clusterName = cfg.ClusterName
756-
if clusterName == "" {
757-
if arkUsername, found := os.LookupEnv("ARK_USERNAME"); found {
758-
log.Info("Using ARK_USERNAME environment variable as cluster name", "clusterName", arkUsername)
759-
clusterName = arkUsername
760-
}
775+
if clusterName == "" && cfg.ClusterID != "" {
776+
log.Info("Using cluster_id as cluster_name for backwards compatibility", "clusterID", cfg.ClusterID)
777+
clusterName = cfg.ClusterID
761778
}
762779
if cfg.OrganizationID != "" {
763780
log.Info(fmt.Sprintf(`Ignoring the organization_id field in the config file. This field is not needed in %s mode.`, res.OutputMode))
764781
}
765-
if cfg.ClusterID != "" {
766-
log.Info(fmt.Sprintf(`Ignoring the cluster_id field in the config file. This field is not needed in %s mode.`, res.OutputMode))
782+
if clusterName == "" && cfg.ClusterID == "" {
783+
log.Info("cluster_name is not set in MachineHub mode; cluster name will be empty")
767784
}
768785
}
769786
res.OrganizationID = organizationID
@@ -773,6 +790,24 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
773790
res.ClaimableCerts = cfg.ClaimableCerts
774791
}
775792

793+
// Validation of `cyberark.*` (MachineHub mode only).
794+
if res.OutputMode == MachineHub {
795+
ark := CyberArkConfig{}
796+
if cfg.CyberArk != nil {
797+
ark = *cfg.CyberArk
798+
}
799+
// service_id selects the Conjur JWT exchange. It is no longer required:
800+
// the agent also supports the legacy username/password method via
801+
// ARK_USERNAME/ARK_SECRET (env, not visible here) for backward
802+
// compatibility. The auth method is chosen at runtime by
803+
// cyberark.selectAuthenticator, which fails closed if neither a
804+
// service_id nor username/password credentials are configured.
805+
if ark.JWTSource != "" && ark.JWTSource != "file" {
806+
errs = multierror.Append(errs, fmt.Errorf("cyberark.jwt_source %q is not supported (POC only supports \"\" or \"file\")", ark.JWTSource))
807+
}
808+
res.CyberArk = ark
809+
}
810+
776811
// Validation of `data-gatherers`.
777812
{
778813
if dgErr := ValidateDataGatherers(cfg.DataGatherers); dgErr != nil {
@@ -987,7 +1022,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
9871022
rootCAs *x509.CertPool
9881023
)
9891024
httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
990-
outputClient, err = client.NewCyberArk(httpClient)
1025+
outputClient, err = client.NewCyberArk(httpClient, cfg.CyberArk.ServiceID, cfg.CyberArk.Account, cfg.CyberArk.JWTSource, cfg.CyberArk.JWTFilePath)
9911026
if err != nil {
9921027
errs = multierror.Append(errs, err)
9931028
}

pkg/agent/config_test.go

Lines changed: 148 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -658,56 +658,74 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
658658
assert.Equal(t, VenafiConnection, got.OutputMode)
659659
})
660660

661-
const arkUsername = "cluster-1-region-1-cloud-1@cyberark.cloud.123456"
662-
663661
t.Run("--machine-hub selects MachineHub mode", func(t *testing.T) {
664662
t.Setenv("POD_NAMESPACE", "venafi")
665663
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
666664
t.Setenv("ARK_SUBDOMAIN", "tlspk")
667-
t.Setenv("ARK_USERNAME", arkUsername)
668-
t.Setenv("ARK_SECRET", "test-secret")
669665
got, cl, err := ValidateAndCombineConfig(discardLogs(),
670-
withConfig(""),
666+
withConfig(testutil.Undent(`
667+
cluster_name: my-cluster
668+
cyberark:
669+
service_id: dev-cluster
670+
`)),
671+
withCmdLineFlags("--period", "1m", "--machine-hub"))
672+
require.NoError(t, err)
673+
assert.Equal(t, MachineHub, got.OutputMode)
674+
assert.Equal(t, "my-cluster", got.ClusterName)
675+
assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
676+
assert.IsType(t, &client.CyberArkClient{}, cl)
677+
})
678+
679+
t.Run("--machine-hub with cluster_id fallback when cluster_name is empty", func(t *testing.T) {
680+
t.Setenv("POD_NAMESPACE", "venafi")
681+
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
682+
t.Setenv("ARK_SUBDOMAIN", "tlspk")
683+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
684+
withConfig(testutil.Undent(`
685+
cluster_id: my-cluster-id
686+
cyberark:
687+
service_id: dev-cluster
688+
`)),
671689
withCmdLineFlags("--period", "1m", "--machine-hub"))
672690
require.NoError(t, err)
673691
assert.Equal(t, MachineHub, got.OutputMode)
674-
assert.Equal(t, arkUsername, got.ClusterName,
675-
"the ClusterName should default to the ARK_USERNAME value if the cluster_name in the config file is empty")
692+
assert.Equal(t, "my-cluster-id", got.ClusterName,
693+
"cluster_id should be used as cluster_name when cluster_name is empty")
676694
assert.IsType(t, &client.CyberArkClient{}, cl)
677695
})
678696

679697
t.Run("--machine-hub with cluster_name override", func(t *testing.T) {
680698
t.Setenv("POD_NAMESPACE", "venafi")
681699
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
682700
t.Setenv("ARK_SUBDOMAIN", "tlspk")
683-
t.Setenv("ARK_USERNAME", arkUsername)
684-
t.Setenv("ARK_SECRET", "test-secret")
685701
got, cl, err := ValidateAndCombineConfig(discardLogs(),
686702
withConfig(testutil.Undent(`
687703
cluster_name: override-cluster-name
688-
`)),
704+
cyberark:
705+
service_id: dev-cluster
706+
`)),
689707
withCmdLineFlags("--period", "1m", "--machine-hub"))
690708
require.NoError(t, err)
691709
assert.Equal(t, MachineHub, got.OutputMode)
692-
assert.Equal(t, "override-cluster-name", got.ClusterName,
693-
"the cluster_name in the config file should be used if not empty, even if ARK_USERNAME is set")
710+
assert.Equal(t, "override-cluster-name", got.ClusterName)
694711
assert.IsType(t, &client.CyberArkClient{}, cl)
695712
})
696713

697-
t.Run("--machine-hub without required environment variables", func(t *testing.T) {
714+
t.Run("--machine-hub without ARK_SUBDOMAIN environment variable", func(t *testing.T) {
698715
t.Setenv("POD_NAMESPACE", "venafi")
699716
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
700717
t.Setenv("ARK_SUBDOMAIN", "")
701-
t.Setenv("ARK_USERNAME", "")
702-
t.Setenv("ARK_SECRET", "")
703718
got, cl, err := ValidateAndCombineConfig(discardLogs(),
704-
withConfig(""),
719+
withConfig(testutil.Undent(`
720+
cyberark:
721+
service_id: dev-cluster
722+
`)),
705723
withCmdLineFlags("--period", "1m", "--machine-hub"))
706724
assert.Equal(t, CombinedConfig{}, got)
707725
assert.Nil(t, cl)
708726
assert.EqualError(t, err, testutil.Undent(`
709727
validating creds: failed loading config using the MachineHub mode: 1 error occurred:
710-
* missing environment variables: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET
728+
* missing environment variables: ARK_SUBDOMAIN
711729
712730
`))
713731
})
@@ -1303,6 +1321,119 @@ func Test_ValidateAndCombineConfig_NGTS(t *testing.T) {
13031321
})
13041322
}
13051323

1324+
func TestConfig_CyberArk_Validation(t *testing.T) {
1325+
// Common env setup: ARK_SUBDOMAIN is the only required env var for MachineHub mode.
1326+
setEnv := func(t *testing.T) {
1327+
t.Helper()
1328+
t.Setenv("POD_NAMESPACE", "venafi")
1329+
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
1330+
t.Setenv("ARK_SUBDOMAIN", "tlspk")
1331+
}
1332+
1333+
// service_id is no longer required at config-validation time: the agent
1334+
// also supports the legacy username/password method (ARK_USERNAME/ARK_SECRET,
1335+
// set via env, not config), and cyberark.selectAuthenticator fails closed at
1336+
// runtime (ErrNoAuthMethod) if neither method ends up configured. See the
1337+
// comment on this validation block in config.go.
1338+
t.Run("empty service_id is valid at config time", func(t *testing.T) {
1339+
setEnv(t)
1340+
combined, _, err := ValidateAndCombineConfig(discardLogs(),
1341+
withConfig(testutil.Undent(`
1342+
cyberark:
1343+
service_id: ""
1344+
`)),
1345+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1346+
require.NoError(t, err)
1347+
assert.Equal(t, "", combined.CyberArk.ServiceID)
1348+
})
1349+
1350+
t.Run("missing cyberark block is valid at config time", func(t *testing.T) {
1351+
setEnv(t)
1352+
combined, _, err := ValidateAndCombineConfig(discardLogs(),
1353+
withConfig(""),
1354+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1355+
require.NoError(t, err)
1356+
assert.Equal(t, "", combined.CyberArk.ServiceID)
1357+
})
1358+
1359+
t.Run("jwt_source spiffe is rejected", func(t *testing.T) {
1360+
setEnv(t)
1361+
_, _, err := ValidateAndCombineConfig(discardLogs(),
1362+
withConfig(testutil.Undent(`
1363+
cyberark:
1364+
service_id: dev-cluster
1365+
jwt_source: spiffe
1366+
`)),
1367+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1368+
require.Error(t, err)
1369+
assert.Contains(t, err.Error(), `cyberark.jwt_source "spiffe" is not supported`)
1370+
})
1371+
1372+
t.Run("jwt_source file is accepted", func(t *testing.T) {
1373+
setEnv(t)
1374+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
1375+
withConfig(testutil.Undent(`
1376+
cyberark:
1377+
service_id: dev-cluster
1378+
jwt_source: file
1379+
jwt_file_path: /var/run/secrets/tokens/agent-token
1380+
`)),
1381+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1382+
require.NoError(t, err)
1383+
assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
1384+
assert.Equal(t, "file", got.CyberArk.JWTSource)
1385+
assert.Equal(t, "/var/run/secrets/tokens/agent-token", got.CyberArk.JWTFilePath)
1386+
assert.IsType(t, &client.CyberArkClient{}, cl)
1387+
})
1388+
1389+
t.Run("jwt_source empty string is accepted", func(t *testing.T) {
1390+
setEnv(t)
1391+
got, cl, err := ValidateAndCombineConfig(discardLogs(),
1392+
withConfig(testutil.Undent(`
1393+
cyberark:
1394+
service_id: dev-cluster
1395+
`)),
1396+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1397+
require.NoError(t, err)
1398+
assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID)
1399+
assert.Equal(t, "", got.CyberArk.JWTSource)
1400+
assert.IsType(t, &client.CyberArkClient{}, cl)
1401+
})
1402+
1403+
t.Run("account and jwt_file_path are optional", func(t *testing.T) {
1404+
setEnv(t)
1405+
got, _, err := ValidateAndCombineConfig(discardLogs(),
1406+
withConfig(testutil.Undent(`
1407+
cyberark:
1408+
service_id: dev-cluster
1409+
account: myaccount
1410+
jwt_file_path: /tmp/token
1411+
`)),
1412+
withCmdLineFlags("--period", "1m", "--machine-hub"))
1413+
require.NoError(t, err)
1414+
assert.Equal(t, "myaccount", got.CyberArk.Account)
1415+
assert.Equal(t, "/tmp/token", got.CyberArk.JWTFilePath)
1416+
})
1417+
1418+
t.Run("cyberark block is ignored in non-MachineHub modes", func(t *testing.T) {
1419+
t.Setenv("POD_NAMESPACE", "venafi")
1420+
fakeCredsPath := withFile(t, `{"user_id":"foo","user_secret":"bar","client_id": "baz","client_secret": "foobar","auth_server_domain":"bazbar"}`)
1421+
got, _, err := ValidateAndCombineConfig(discardLogs(),
1422+
withConfig(testutil.Undent(`
1423+
server: https://preflight.jetstack.io
1424+
organization_id: my-org
1425+
cluster_id: my-cluster
1426+
period: 1h
1427+
cyberark:
1428+
service_id: should-be-ignored
1429+
`)),
1430+
withCmdLineFlags("--credentials-file", fakeCredsPath))
1431+
require.NoError(t, err)
1432+
// CyberArk config is not copied into CombinedConfig for non-MachineHub modes.
1433+
assert.Equal(t, CyberArkConfig{}, got.CyberArk)
1434+
})
1435+
}
1436+
13061437
const fakePrivKeyPEM = `-----BEGIN PRIVATE KEY-----
13071438
MHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49
13081439
AwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl

pkg/client/client_cyberark.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,25 @@ type CyberArkClient struct {
3434

3535
var _ Client = &CyberArkClient{}
3636

37-
// NewCyberArk initializes a CyberArk client using configuration from environment variables.
38-
// It requires an HTTP client to be provided, which will be used for making requests.
39-
// The environment variables ARK_SUBDOMAIN, ARK_USERNAME, and ARK_SECRET must be set for authentication.
40-
// Sending secrets is controlled by the ARK_SEND_SECRETS environment variable (defaults to "false").
41-
// If sending secrets is enabled, the hardcoded public key will be loaded and an encryptor will be created.
42-
// If the configuration is invalid or missing, an error is returned.
43-
func NewCyberArk(httpClient *http.Client) (*CyberArkClient, error) {
44-
configLoader := cyberark.LoadClientConfigFromEnvironment
45-
46-
cfg, err := configLoader()
37+
// NewCyberArk initializes a CyberArk client.
38+
// Subdomain, and the legacy username/password credentials, are loaded from the
39+
// environment (ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET). The remaining fields
40+
// (serviceID, account, jwtSource, jwtFilePath) come from the agent YAML config
41+
// (config.cyberark.*) and select the Conjur JWT exchange when serviceID is set.
42+
// Sending secrets is controlled by the ARK_SEND_SECRETS environment variable
43+
// (defaults to "false"). If the configuration is invalid or missing, an error
44+
// is returned.
45+
func NewCyberArk(httpClient *http.Client, serviceID, account, jwtSource, jwtFilePath string) (*CyberArkClient, error) {
46+
cfg, err := cyberark.LoadClientConfigFromEnvironment()
4747
if err != nil {
4848
return nil, err
4949
}
50+
cfg.ServiceID = serviceID
51+
cfg.Account = account
52+
cfg.JWTSource = jwtSource
53+
cfg.JWTFilePath = jwtFilePath
54+
55+
configLoader := func() (cyberark.ClientConfig, error) { return cfg, nil }
5056

5157
return &CyberArkClient{
5258
configLoader: configLoader,

pkg/client/client_cyberark_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) {
3535
logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
3636
ctx := klog.NewContext(t.Context(), logger)
3737

38-
httpClient := testutil.FakeCyberArk(t)
38+
httpClient, jwtFilePath := testutil.FakeCyberArk(t)
3939

40-
c, err := client.NewCyberArk(httpClient)
40+
c, err := client.NewCyberArk(httpClient, "test-service", "", "file", jwtFilePath)
4141
require.NoError(t, err)
4242

4343
readings := fakeReadings()
@@ -66,7 +66,8 @@ func TestCyberArkClient_PostDataReadingsWithOptions_RealAPI(t *testing.T) {
6666
var rootCAs *x509.CertPool
6767
httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
6868

69-
c, err := client.NewCyberArk(httpClient)
69+
serviceID := os.Getenv("ARK_SERVICE_ID")
70+
c, err := client.NewCyberArk(httpClient, serviceID, "", "", "")
7071
if err != nil {
7172
if errors.Is(err, cyberark.ErrMissingEnvironmentVariables) {
7273
t.Skipf("Skipping: %s", err)

0 commit comments

Comments
 (0)