Skip to content

Cleanup: config.go #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
inteon merged 1 commit into from
Jun 10, 2025
Merged

Cleanup: config.go #661

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 24 additions & 44 deletions pkg/agent/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,11 +426,9 @@ type CombinedConfig struct {
// error.
func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
res := CombinedConfig{}
var errs error

if flags.MachineHubMode {
err := cfg.MachineHub.Validate()
if err != nil {
if err := cfg.MachineHub.Validate(); err != nil {
return CombinedConfig{}, nil, fmt.Errorf("invalid MachineHub config provided: %w", err)
}

Expand All @@ -453,14 +451,17 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
mode = VenafiCloudKeypair
reason = "--venafi-cloud and --credentials-path were specified"
keysAndValues = []any{"credentialsPath", flags.CredentialsPath}
case flags.ClientID != "" && flags.PrivateKeyPath != "":
case flags.ClientID != "" || flags.PrivateKeyPath != "":
if flags.PrivateKeyPath == "" {
return CombinedConfig{}, nil, fmt.Errorf("if --client-id is specified, --private-key-path must also be specified")
}
if flags.ClientID == "" {
return CombinedConfig{}, nil, fmt.Errorf("--private-key-path is specified, --client-id must also be specified")
}

mode = VenafiCloudKeypair
reason = "--client-id and --private-key-path were specified"
keysAndValues = []any{"clientID", flags.ClientID, "privateKeyPath", flags.PrivateKeyPath}
case flags.ClientID != "":
return CombinedConfig{}, nil, fmt.Errorf("if --client-id is specified, --private-key-path must also be specified")
case flags.PrivateKeyPath != "":
return CombinedConfig{}, nil, fmt.Errorf("--private-key-path is specified, --client-id must also be specified")
case flags.VenConnName != "":
mode = VenafiCloudVenafiConnection
reason = "--venafi-connection was specified"
Expand Down Expand Up @@ -493,6 +494,8 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
res.TLSPKMode = mode
}

var errs error

// Validation and defaulting of `server` and the deprecated `endpoint.path`.
if res.TLSPKMode != Off {
// Only relevant if using TLSPK backends
Expand Down Expand Up @@ -584,12 +587,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
var clusterID string
var organizationID string // Only used by the old jetstack-secure mode.
switch res.TLSPKMode { // nolint:exhaustive
case VenafiCloudKeypair:
if cfg.ClusterID == "" {
errs = multierror.Append(errs, fmt.Errorf("cluster_id is required in %s mode", res.TLSPKMode))
}
clusterID = cfg.ClusterID
case VenafiCloudVenafiConnection:
case VenafiCloudKeypair, VenafiCloudVenafiConnection:
if cfg.ClusterID == "" {
errs = multierror.Append(errs, fmt.Errorf("cluster_id is required in %s mode", res.TLSPKMode))
}
Expand All @@ -609,8 +607,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
res.ClusterDescription = cfg.ClusterDescription

// Validation of `data-gatherers`.
dgErr := ValidateDataGatherers(cfg.DataGatherers)
if dgErr != nil {
if dgErr := ValidateDataGatherers(cfg.DataGatherers); dgErr != nil {
errs = multierror.Append(errs, dgErr)
}
res.DataGatherers = cfg.DataGatherers
Expand Down Expand Up @@ -736,12 +733,12 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
break // Don't continue with the client if credentials file invalid.
}

preflightClient, err = createCredentialClient(log, creds, cfg, metadata)
preflightClient, err = client.NewOAuthClient(metadata, creds, cfg.Server)
if err != nil {
errs = multierror.Append(errs, err)
}
case VenafiCloudKeypair:
var creds client.Credentials
var creds *client.VenafiSvcAccountCredentials

if flagClientID != "" && flagCredentialsPath != "" {
errs = multierror.Append(errs, fmt.Errorf("--client-id and --credentials-file cannot be used simultaneously"))
Expand Down Expand Up @@ -779,8 +776,16 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
return nil, fmt.Errorf("programmer mistake: --client-id and --private-key-path or --credentials-file must have been provided")
}

// The uploader ID isn't actually used in the backend, let's use an
// arbitrary value.
uploaderID := "no"

// We don't do this for the VenafiCloudVenafiConnection mode because
// the upload_path field is ignored in that mode.
log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
Copy link
Copy Markdown
Member

@maelvls maelvls Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this unconditionally logged? It used to be only logged in "key pair" mode.

Copy link
Copy Markdown
Contributor Author

@inteon inteon Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still is only logged in "key pair" mode: https://github.com/jetstack/jetstack-secure/pull/661/files/84570e79542e103e7997c509f9d198052da5f97c#diff-4c826b1b395b31baef6313ac64f7d6e2283739d884418144b0ff908302af6eebR740


var err error
preflightClient, err = createCredentialClient(log, creds, cfg, metadata)
preflightClient, err = client.NewVenafiCloudClient(metadata, creds, cfg.Server, uploaderID, cfg.UploadPath)
if err != nil {
errs = multierror.Append(errs, err)
}
Expand Down Expand Up @@ -836,31 +841,6 @@ func ValidateDataGatherers(dataGatherers []DataGatherer) error {
return err
}

// The error returned may be a multierror.Error. Instead of adding context to
// the error with fmt.Errorf("%w", err), use multierror.Prefix(err, "context").
func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
switch creds := credentials.(type) {
case *client.VenafiSvcAccountCredentials:
// The uploader ID isn't actually used in the backend, let's use an
// arbitrary value.
uploaderID := "no"

var uploadPath string
if cfg.TLSPKMode == VenafiCloudKeypair {
// We don't do this for the VenafiCloudVenafiConnection mode because
// the upload_path field is ignored in that mode.
log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
uploadPath = cfg.UploadPath
}
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath)

case *client.OAuthCredentials:
return client.NewOAuthClient(agentMetadata, creds, cfg.Server)
default:
return nil, errors.New("credentials file is in unknown format")
}
}

// Inspired by the controller-runtime project.
func getInClusterNamespace() (string, error) {
ns := os.Getenv("POD_NAMESPACE")
Expand Down