Skip to content

Commit f42e98b

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. 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 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. Checks both ARK_USERNAME and ARK_SECRET, since selectAuthenticator's hasUP requires both — checking only ARK_USERNAME would pass validation and still fail at runtime for the one credential missing. MachineHub's cluster_name fallback order is cluster_name > cluster_id > ARK_USERNAME > empty. Explicit config always wins over the env var; the ARK_USERNAME fallback exists 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. This also keeps a migrating install's reported name unchanged: adding service_id to test Conjur alongside still-present ARK_USERNAME, before also setting cluster_id, doesn't blank out the name. 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.
1 parent 7e4e017 commit f42e98b

5 files changed

Lines changed: 400 additions & 47 deletions

File tree

pkg/agent/config.go

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"k8s.io/client-go/rest"
1818

1919
"github.com/jetstack/preflight/api"
20+
"github.com/jetstack/preflight/internal/cyberark"
2021
"github.com/jetstack/preflight/pkg/client"
2122
"github.com/jetstack/preflight/pkg/datagatherer"
2223
"github.com/jetstack/preflight/pkg/datagatherer/k8sdiscovery"
@@ -64,6 +65,9 @@ type Config struct {
6465
DataGatherers []DataGatherer `yaml:"data-gatherers"`
6566
VenafiCloud *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"`
6667

68+
// CyberArk holds configuration for MachineHub mode (POC).
69+
CyberArk *CyberArkConfig `yaml:"cyberark,omitempty"`
70+
6771
// For testing purposes.
6872
InputPath string `yaml:"input-path"`
6973
// For testing purposes.
@@ -101,6 +105,19 @@ type VenafiCloudConfig struct {
101105
UploadPath string `yaml:"upload_path,omitempty"`
102106
}
103107

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

479+
// MachineHub mode only.
480+
CyberArk CyberArkConfig
481+
462482
// Only used for testing purposes.
463483
OutputPath string
464484
InputPath string
@@ -753,17 +773,31 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
753773
clusterID = cfg.ClusterID
754774
case MachineHub:
755775
clusterName = cfg.ClusterName
776+
if clusterName == "" && cfg.ClusterID != "" {
777+
log.Info("Using cluster_id as cluster_name", "clusterID", cfg.ClusterID)
778+
clusterName = cfg.ClusterID
779+
}
756780
if clusterName == "" {
757-
if arkUsername, found := os.LookupEnv("ARK_USERNAME"); found {
758-
log.Info("Using ARK_USERNAME environment variable as cluster name", "clusterName", arkUsername)
781+
// Legacy fallback: pre-Conjur installs set neither
782+
// cluster_name nor cluster_id — the chart only ever emitted
783+
// cluster_name — and were named after ARK_USERNAME. Kept
784+
// below both config fields so explicit configuration always
785+
// wins; this never overrides a value the operator actually
786+
// set. Naming a cluster after a login identity is an
787+
// accident of the original design, worth retiring once
788+
// installs set cluster_name explicitly, but the env var
789+
// itself stays since it's still the username/password
790+
// credential.
791+
if arkUsername := os.Getenv("ARK_USERNAME"); arkUsername != "" {
792+
log.Info("Using ARK_USERNAME as cluster name because neither cluster_name nor cluster_id is set; prefer setting cluster_name explicitly", "clusterName", arkUsername)
759793
clusterName = arkUsername
760794
}
761795
}
762796
if cfg.OrganizationID != "" {
763797
log.Info(fmt.Sprintf(`Ignoring the organization_id field in the config file. This field is not needed in %s mode.`, res.OutputMode))
764798
}
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))
799+
if clusterName == "" {
800+
log.Info("cluster_name is not set in MachineHub mode; cluster name will be empty")
767801
}
768802
}
769803
res.OrganizationID = organizationID
@@ -773,6 +807,27 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
773807
res.ClaimableCerts = cfg.ClaimableCerts
774808
}
775809

810+
// Validation of `cyberark.*` (MachineHub mode only).
811+
if res.OutputMode == MachineHub {
812+
ark := CyberArkConfig{}
813+
if cfg.CyberArk != nil {
814+
ark = *cfg.CyberArk
815+
}
816+
// service_id selects the Conjur JWT exchange. It is no longer required:
817+
// the agent also supports the legacy username/password method via
818+
// ARK_USERNAME/ARK_SECRET. Checked here, at config-validation time,
819+
// rather than left to cyberark.selectAuthenticator alone — that only
820+
// runs at first upload, so a misconfigured agent would otherwise
821+
// report healthy for up to a full config.period before failing.
822+
if ark.ServiceID == "" && (os.Getenv("ARK_USERNAME") == "" || os.Getenv("ARK_SECRET") == "") {
823+
errs = multierror.Append(errs, fmt.Errorf("MachineHub mode requires either cyberark.service_id or ARK_USERNAME/ARK_SECRET"))
824+
}
825+
if err := cyberark.ValidateJWTSource(ark.JWTSource); err != nil {
826+
errs = multierror.Append(errs, fmt.Errorf("cyberark.jwt_source %w", err))
827+
}
828+
res.CyberArk = ark
829+
}
830+
776831
// Validation of `data-gatherers`.
777832
{
778833
if dgErr := ValidateDataGatherers(cfg.DataGatherers); dgErr != nil {
@@ -987,7 +1042,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
9871042
rootCAs *x509.CertPool
9881043
)
9891044
httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
990-
outputClient, err = client.NewCyberArk(httpClient)
1045+
outputClient, err = client.NewCyberArk(httpClient, cfg.CyberArk.ServiceID, cfg.CyberArk.Account, cfg.CyberArk.JWTSource, cfg.CyberArk.JWTFilePath)
9911046
if err != nil {
9921047
errs = multierror.Append(errs, err)
9931048
}

0 commit comments

Comments
 (0)