From 7f6334f7770ce824be03deca14b9d170ae89789b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Fri, 23 Aug 2024 22:54:37 +0200 Subject: [PATCH 1/3] tests: add unit tests to the code that loads config/flags At first, I had moved config.go and config_test.go to a separate folder to make things a bit cleaner, but it made reviewing super hard, so I ended up keeping them in the `agent` package. --- cmd/agent.go | 124 +------------- go.mod | 1 - go.sum | 2 - pkg/agent/config.go | 359 +++++++++++++++++++++++++++++++++++++++ pkg/agent/config_test.go | 237 +++++++++++++++++++++++++- pkg/agent/run.go | 303 +++++---------------------------- 6 files changed, 642 insertions(+), 384 deletions(-) diff --git a/cmd/agent.go b/cmd/agent.go index 7d30e328..5c1a5dc3 100644 --- a/cmd/agent.go +++ b/cmd/agent.go @@ -3,8 +3,6 @@ package cmd import ( "fmt" "io/ioutil" - "os" - "time" "github.com/jetstack/preflight/pkg/agent" "github.com/jetstack/preflight/pkg/logs" @@ -37,16 +35,16 @@ var agentRBACCmd = &cobra.Command{ Long: `Print RBAC string by reading GVRs`, Run: func(cmd *cobra.Command, args []string) { - b, err := ioutil.ReadFile(agent.ConfigFilePath) + b, err := ioutil.ReadFile(agent.Flags.ConfigFilePath) if err != nil { logs.Log.Fatalf("Failed to read config file: %s", err) } - config, err := agent.ParseConfig(b, false) + cfg, err := agent.ParseConfig(b, false) if err != nil { logs.Log.Fatalf("Failed to parse config file: %s", err) } - out := permissions.GenerateFullManifest(config.DataGatherers) + out := permissions.GenerateFullManifest(cfg.DataGatherers) fmt.Print(out) }, } @@ -55,119 +53,5 @@ func init() { rootCmd.AddCommand(agentCmd) agentCmd.AddCommand(agentInfoCmd) agentCmd.AddCommand(agentRBACCmd) - agentCmd.PersistentFlags().StringVarP( - &agent.ConfigFilePath, - "agent-config-file", - "c", - "./agent.yaml", - "Config file location, default is `agent.yaml` in the current working directory.", - ) - agentCmd.PersistentFlags().DurationVarP( - &agent.Period, - "period", - "p", - 0, - "Override time between scans in the configuration file (given as XhYmZs).", - ) - agentCmd.PersistentFlags().StringVarP( - &agent.CredentialsPath, - "credentials-file", - "k", - "", - "Location of the credentials file. For OAuth2 based authentication.", - ) - agentCmd.PersistentFlags().BoolVarP( - &agent.VenafiCloudMode, - "venafi-cloud", - "", - false, - "Runs agent with parsing config (and credentials file if provided) in Venafi Cloud format if true.", - ) - agentCmd.PersistentFlags().StringVarP( - &agent.ClientID, - "client-id", - "", - "", - "Venafi Cloud Service Account client ID. If you use this flag you don't need to use --venafi-cloud as it will assume you are authenticating against Venafi Cloud. Using this removes the need to use a credentials file with Venafi Cloud mode.", - ) - agentCmd.PersistentFlags().StringVarP( - &agent.PrivateKeyPath, - "private-key-path", - "", - "/etc/venafi/agent/key/privatekey.pem", - "Venafi Cloud Service Account private key path.", - ) - agentCmd.PersistentFlags().BoolVarP( - &agent.OneShot, - "one-shot", - "", - false, - "Runs agent a single time if true, or continously if false", - ) - agentCmd.PersistentFlags().StringVarP( - &agent.OutputPath, - "output-path", - "", - "", - "Output file path, if used, it will write data to a local file instead of uploading to the preflight server", - ) - agentCmd.PersistentFlags().StringVarP( - &agent.InputPath, - "input-path", - "", - "", - "Input file path, if used, it will read data from a local file instead of gathering data from clusters", - ) - agentCmd.PersistentFlags().DurationVarP( - &agent.BackoffMaxTime, - "backoff-max-time", - "", - 10*time.Minute, - "Max time for retrying failed data gatherers (given as XhYmZs).", - ) - agentCmd.PersistentFlags().BoolVarP( - &agent.StrictMode, - "strict", - "", - false, - "Runs agent in strict mode. No retry attempts will be made for a missing data gatherer's data.", - ) - agentCmd.PersistentFlags().StringVar( - &agent.APIToken, - "api-token", - os.Getenv("API_TOKEN"), - "Token used for authentication when API tokens are in use on the backend", - ) - agentCmd.PersistentFlags().StringVar( - &agent.VenConnName, - "venafi-connection", - "", - "Name of the VenafiConnection to be used. Using this flag will enable the VenafiConnection mode.", - ) - agentCmd.PersistentFlags().StringVar( - &agent.VenConnNS, - "venafi-connection-namespace", - "", - "Namespace of the VenafiConnection to be used. It is only useful when the VenafiConnection isn't in the same namespace as the agent. The field `allowReferencesFrom` must be present on the cross-namespace VenafiConnection for the agent to use it.", - ) - agentCmd.PersistentFlags().StringVar( - &agent.InstallNS, - "install-namespace", - "", - "Namespace in which the agent is running. Only needed when running the agent outside of Kubernetes. Used for testing purposes.", - ) - agentCmd.PersistentFlags().BoolVarP( - &agent.Profiling, - "enable-pprof", - "", - false, - "Enables the pprof profiling server on the agent (port: 6060).", - ) - agentCmd.PersistentFlags().BoolVarP( - &agent.Prometheus, - "enable-metrics", - "", - false, - "Enables Prometheus metrics server on the agent (port: 8081).", - ) + agent.InitAgentCmdFlags(agentCmd, &agent.Flags) } diff --git a/go.mod b/go.mod index 1504559f..39681769 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/spf13/cobra v1.8.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 - gopkg.in/d4l3k/messagediff.v1 v1.2.1 gopkg.in/yaml.v2 v2.4.0 k8s.io/api v0.30.3 k8s.io/apimachinery v0.30.3 diff --git a/go.sum b/go.sum index 776c9978..3f77ffc7 100644 --- a/go.sum +++ b/go.sum @@ -280,8 +280,6 @@ google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWn gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/d4l3k/messagediff.v1 v1.2.1 h1:70AthpjunwzUiarMHyED52mj9UwtAnE89l1Gmrt3EU0= -gopkg.in/d4l3k/messagediff.v1 v1.2.1/go.mod h1:EUzikiKadqXWcD1AzJLagx0j/BeeWGtn++04Xniyg44= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= diff --git a/pkg/agent/config.go b/pkg/agent/config.go index dd2d1b7c..8eb7b6fb 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -2,18 +2,29 @@ package agent import ( "fmt" + "io" + "log" "net/url" + "os" "time" "github.com/hashicorp/go-multierror" + "github.com/jetstack/preflight/api" "github.com/jetstack/preflight/pkg/client" "github.com/jetstack/preflight/pkg/datagatherer" "github.com/jetstack/preflight/pkg/datagatherer/k8s" "github.com/jetstack/preflight/pkg/datagatherer/local" + "github.com/jetstack/preflight/pkg/kubeconfig" + "github.com/jetstack/preflight/pkg/version" "github.com/pkg/errors" + "github.com/spf13/cobra" "gopkg.in/yaml.v3" ) +const ( + inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" +) + // Config wraps the options for a run of the agent. type Config struct { Schedule string `yaml:"schedule"` @@ -57,6 +68,354 @@ type VenafiCloudConfig struct { UploadPath string `yaml:"upload_path,omitempty"` } +type AgentCmdFlags struct { + // ConfigFilePath (--config-file, -c) is the path to the agent configuration + // YAML file. + ConfigFilePath string + + // Period (--period, -p) is the time waited between scans. + Period time.Duration + + // OneShot (--one-shot) flag causes agent to run once. + OneShot bool + + // VenafiCloudMode (--venafi-cloud) determines which format to load for + // config and credential type. + VenafiCloudMode bool + + // ClientID (--client-id) is the clientID in case of Venafi Cloud mode. + ClientID string + + // PrivateKeyPath (--private-key-path) is the path for the service account + // private key in case of Venafi Cloud mode. + PrivateKeyPath string + + // CredentialsPath (--credentials-file, -k) is the path to the credentials ) + // is where the agent will try to loads the credentials (Experimental). + CredentialsPath string + + // OutputPath (--output-path) is where the agent will write data to instead + // of uploading to server. + OutputPath string + + // InputPath (--input-path) is where the agent will read data from instead + // of gathering data from clusters. + InputPath string + + // BackoffMaxTime (--backoff-max-time) is the maximum time for which data + // gatherers will retry after a failure. + BackoffMaxTime time.Duration + + // StrictMode (--strict) causes the agent to fail at the first attempt. + StrictMode bool + + // APIToken (--api-token) is an authentication token used for the backend + // API as an alternative to OAuth flows. + APIToken string + + // VenConnName (--venafi-connection) is the name of the VenafiConnection + // resource to use. Using this flag will enable Venafi Connection mode. + VenConnName string + + // VenConnNS (--venafi-connection-namespace) is the namespace of the + // VenafiConnection resource to use. It is only useful when the + // VenafiConnection isn't in the same namespace as the agent. + // + // May be left empty to use the same namespace as the agent. + VenConnNS string + + // InstallNS (--install-namespace) is the namespace in which the agent is + // running in. Only needed when running the agent outside of Kubernetes. + // + // May be left empty when running in Kubernetes. In this case, the namespace + // is read from the file + // /var/run/secrets/kubernetes.io/serviceaccount/namespace. + InstallNS string + + // Profiling (--enable-pprof) enables the pprof server. + Profiling bool + + // Prometheus (--enable-metrics) enables the Prometheus metrics server. + Prometheus bool +} + +func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) { + c.PersistentFlags().StringVarP( + &cfg.ConfigFilePath, + "agent-config-file", + "c", + "./agent.yaml", + "Config file location, default is `agent.yaml` in the current working directory.", + ) + c.PersistentFlags().DurationVarP( + &cfg.Period, + "period", + "p", + 0, + "Override time between scans in the configuration file (given as XhYmZs).", + ) + c.PersistentFlags().StringVarP( + &cfg.CredentialsPath, + "credentials-file", + "k", + "", + "Location of the credentials file. For OAuth2 based authentication.", + ) + c.PersistentFlags().BoolVarP( + &cfg.VenafiCloudMode, + "venafi-cloud", + "", + false, + "Runs agent with parsing config (and credentials file if provided) in Venafi Cloud format if true.", + ) + c.PersistentFlags().StringVarP( + &cfg.ClientID, + "client-id", + "", + "", + "Venafi Cloud Service Account client ID. If you use this flag you don't need to use --venafi-cloud as it will assume you are authenticating against Venafi Cloud. Using this removes the need to use a credentials file with Venafi Cloud mode.", + ) + c.PersistentFlags().StringVarP( + &cfg.PrivateKeyPath, + "private-key-path", + "", + "/etc/venafi/agent/key/privatekey.pem", + "Venafi Cloud Service Account private key path.", + ) + c.PersistentFlags().BoolVarP( + &cfg.OneShot, + "one-shot", + "", + false, + "Runs agent a single time if true, or continously if false", + ) + c.PersistentFlags().StringVarP( + &cfg.OutputPath, + "output-path", + "", + "", + "Output file path, if used, it will write data to a local file instead of uploading to the preflight server", + ) + c.PersistentFlags().StringVarP( + &cfg.InputPath, + "input-path", + "", + "", + "Input file path, if used, it will read data from a local file instead of gathering data from clusters", + ) + c.PersistentFlags().DurationVarP( + &cfg.BackoffMaxTime, + "backoff-max-time", + "", + 10*time.Minute, + "Max time for retrying failed data gatherers (given as XhYmZs).", + ) + c.PersistentFlags().BoolVarP( + &cfg.StrictMode, + "strict", + "", + false, + "Runs agent in strict mode. No retry attempts will be made for a missing data gatherer's data.", + ) + c.PersistentFlags().StringVar( + &cfg.APIToken, + "api-token", + os.Getenv("API_TOKEN"), + "Token used for authentication when API tokens are in use on the backend", + ) + c.PersistentFlags().StringVar( + &cfg.VenConnName, + "venafi-connection", + "", + "Name of the VenafiConnection to be used. Using this flag will enable the VenafiConnection mode.", + ) + c.PersistentFlags().StringVar( + &cfg.VenConnNS, + "venafi-connection-namespace", + "", + "Namespace of the VenafiConnection to be used. It is only useful when the VenafiConnection isn't in the same namespace as the agent. The field `allowReferencesFrom` must be present on the cross-namespace VenafiConnection for the agent to use it.", + ) + c.PersistentFlags().StringVar( + &cfg.InstallNS, + "install-namespace", + "", + "Namespace in which the agent is running. Only needed when running the agent outside of Kubernetes. Used for testing purposes.", + ) + c.PersistentFlags().BoolVarP( + &cfg.Profiling, + "enable-pprof", + "", + false, + "Enables the pprof profiling server on the agent (port: 6060).", + ) + c.PersistentFlags().BoolVarP( + &cfg.Prometheus, + "enable-metrics", + "", + false, + "Enables Prometheus metrics server on the agent (port: 8081).", + ) +} + +// getConfiguration combines the input configuration with the flags passed to +// the agent and returns the final configuration as well as the Venafi client to +// be used to upload data. +func getConfiguration(log *log.Logger, cfg Config, flags AgentCmdFlags) (Config, client.Client, error) { + // If the ClientID of the service account is specified, then assume we are in Venafi Cloud mode. + if flags.ClientID != "" || flags.VenConnName != "" { + flags.VenafiCloudMode = true + } + + baseURL := cfg.Server + if baseURL == "" { + log.Printf("Using deprecated Endpoint configuration. User Server instead.") + baseURL = fmt.Sprintf("%s://%s", cfg.Endpoint.Protocol, cfg.Endpoint.Host) + _, err := url.Parse(baseURL) + if err != nil { + return Config{}, nil, fmt.Errorf("failed to parse server URL: %w", err) + } + } + + if flags.Period == 0 && cfg.Period == 0 && !flags.OneShot { + return Config{}, nil, fmt.Errorf("period must be set as a flag or in config") + } + + var credentials client.Credentials + var err error + if flags.ClientID != "" { + credentials = &client.VenafiSvcAccountCredentials{ + ClientID: flags.ClientID, + PrivateKeyFile: flags.PrivateKeyPath, + } + } else if flags.CredentialsPath != "" { + file, err := os.Open(flags.CredentialsPath) + if err != nil { + return Config{}, nil, fmt.Errorf("failed to load credentials from file %s: %w", flags.CredentialsPath, err) + } + defer file.Close() + + b, err := io.ReadAll(file) + if err != nil { + return Config{}, nil, fmt.Errorf("failed to read credentials file: %w", err) + } + if flags.VenafiCloudMode { + credentials, err = client.ParseVenafiCredentials(b) + } else { + credentials, err = client.ParseOAuthCredentials(b) + } + if err != nil { + return Config{}, nil, fmt.Errorf("failed to parse credentials file: %w", err) + } + } + + venConnMode := flags.VenConnName != "" + + if venConnMode && flags.InstallNS == "" { + flags.InstallNS, err = getInClusterNamespace() + if err != nil { + return Config{}, nil, fmt.Errorf("could not guess which namespace the agent is running in: %w", err) + } + } + if venConnMode && flags.VenConnNS == "" { + flags.VenConnNS = flags.InstallNS + } + + agentMetadata := &api.AgentMetadata{ + Version: version.PreflightVersion, + ClusterID: cfg.ClusterID, + } + + var preflightClient client.Client + switch { + case credentials != nil: + preflightClient, err = createCredentialClient(log, credentials, cfg, agentMetadata, baseURL) + case flags.VenConnName != "": + // Why wasn't this added to the createCredentialClient instead? Because + // the --venafi-connection mode of authentication doesn't need any + // secrets (or any other information for that matter) to be loaded from + // disk (using --credentials-path). Everything is passed as flags. + log.Println("Venafi Connection mode was specified, using Venafi Connection authentication.") + + // The venafi-cloud.upload_path was initially meant to let users + // configure HTTP proxies, but it has never been used since HTTP proxies + // don't rewrite paths. Thus, we've disabled the ability to change this + // value with the new --venafi-connection flag, and this field is simply + // ignored. + if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploadPath != "" { + log.Printf(`ignoring venafi-cloud.upload_path. In Venafi Connection mode, this field is not needed.`) + } + + // Regarding venafi-cloud.uploader_id, we found that it doesn't do + // anything in the backend. Since the backend requires it for historical + // reasons (but cannot be empty), we just ignore whatever the user has + // set in the config file, and set it to an arbitrary value in the + // client since it doesn't matter. + if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploaderID != "" { + log.Printf(`ignoring venafi-cloud.uploader_id. In Venafi Connection mode, this field is not needed.`) + } + + restCfg, err := kubeconfig.LoadRESTConfig("") + if err != nil { + return Config{}, nil, fmt.Errorf("failed to load kubeconfig: %w", err) + } + + preflightClient, err = client.NewVenConnClient(restCfg, agentMetadata, flags.InstallNS, flags.VenConnName, flags.VenConnNS, nil) + case flags.APIToken != "": + log.Println("An API token was specified, using API token authentication.") + preflightClient, err = client.NewAPITokenClient(agentMetadata, flags.APIToken, baseURL) + default: + log.Println("No credentials were specified, using with no authentication.") + preflightClient, err = client.NewUnauthenticatedClient(agentMetadata, baseURL) + } + + if err != nil { + return Config{}, nil, fmt.Errorf("failed to create client: %w", err) + } + + return cfg, preflightClient, nil +} + +func createCredentialClient(log *log.Logger, credentials client.Credentials, config Config, agentMetadata *api.AgentMetadata, baseURL string) (client.Client, error) { + switch creds := credentials.(type) { + case *client.VenafiSvcAccountCredentials: + log.Println("Venafi Cloud mode was specified, using Venafi Service Account authentication.") + // check if config has Venafi Cloud data, use config data if it's present + uploaderID := creds.ClientID + uploadPath := "" + if config.VenafiCloud != nil { + log.Println("Loading uploader_id and upload_path from \"venafi-cloud\" configuration.") + uploaderID = config.VenafiCloud.UploaderID + uploadPath = config.VenafiCloud.UploadPath + } + return client.NewVenafiCloudClient(agentMetadata, creds, baseURL, uploaderID, uploadPath) + + case *client.OAuthCredentials: + log.Println("A credentials file was specified, using oauth authentication.") + return client.NewOAuthClient(agentMetadata, creds, baseURL) + default: + return nil, errors.New("credentials file is in unknown format") + } +} + +// Inspired by the controller-runtime project. +func getInClusterNamespace() (string, error) { + // Check whether the namespace file exists. + // If not, we are not running in cluster so can't guess the namespace. + _, err := os.Stat(inClusterNamespacePath) + if os.IsNotExist(err) { + return "", fmt.Errorf("not running in cluster, please use --install-namespace to specify the namespace in which the agent is running") + } + if err != nil { + return "", fmt.Errorf("error checking namespace file: %w", err) + } + + namespace, err := os.ReadFile(inClusterNamespacePath) + if err != nil { + return "", fmt.Errorf("error reading namespace file: %w", err) + } + return string(namespace), nil +} + func reMarshal(rawConfig interface{}, config datagatherer.Config) error { bb, err := yaml.Marshal(rawConfig) if err != nil { diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 259eb24d..f34337e2 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1,15 +1,202 @@ package agent import ( + "bytes" "fmt" + "io" + "log" + "os" "strings" "testing" "time" + "github.com/d4l3k/messagediff" + "github.com/jetstack/preflight/pkg/client" "github.com/kylelemons/godebug/diff" - "gopkg.in/d4l3k/messagediff.v1" + "github.com/stretchr/testify/assert" ) +func TestGetConfiguration(t *testing.T) { + t.Run("minimal successful configuration", func(t *testing.T) { + got, cl, err := getConfiguration(discardLogs(t), + Config{Server: "http://localhost:8080", Period: 1 * time.Hour}, + AgentCmdFlags{}, + ) + assert.NoError(t, err) + assert.Equal(t, Config{ + Server: "http://localhost:8080", + Period: 1 * time.Hour, + }, got) + assert.IsType(t, &client.UnauthenticatedClient{}, cl) + }) + + t.Run("period must be given", func(t *testing.T) { + _, _, err := getConfiguration(discardLogs(t), + Config{Server: "http://localhost:8080"}, + AgentCmdFlags{}) + assert.EqualError(t, err, "period must be set as a flag or in config") + }) + + t.Run("server must be given", func(t *testing.T) { + got, _, err := getConfiguration(discardLogs(t), + Config{Period: 1 * time.Hour}, + AgentCmdFlags{}) + assert.EqualError(t, err, `failed to parse server URL: parse "://": missing protocol scheme`) + assert.Equal(t, Config{}, got) + }) + + t.Run("auth defaults to 'unauthenticated'", func(t *testing.T) { + got, cl, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{}) + assert.NoError(t, err) + assert.Equal(t, fillRequired(Config{}), got) + assert.IsType(t, &client.UnauthenticatedClient{}, cl) + }) + + t.Run("old jetstack-secure auth", func(t *testing.T) { + t.Run("--credential-path alone means jetstack-secure auth", func(t *testing.T) { + // `client_id`, `client_secret`, and `auth_server_domain` are + // usually injected at build time, but we can't do that in tests, so + // we need to provide them in the credentials file. + path := withFile(t, `{"user_id":"fpp2624799349@affectionate-hertz6.platform.jetstack.io","user_secret":"foo","client_id": "k3TrDbfLhCgnpAbOiiT2kIE1AbovKzjo","client_secret": "f39w_3KT9Vp0VhzcPzvh-uVbudzqCFmHER3Huj0dvHgJwVrjxsoOQPIw_1SDiCfa","auth_server_domain":"auth.jetstack.io"}`) + got, cl, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{CredentialsPath: path}) + assert.NoError(t, err) + assert.Equal(t, fillRequired(Config{}), got) + assert.IsType(t, &client.OAuthClient{}, cl) + }) + t.Run("--credential-path but file is missing", func(t *testing.T) { + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{CredentialsPath: "credentials.json"}) + assert.EqualError(t, err, "failed to load credentials from file credentials.json: open credentials.json: no such file or directory") + assert.Equal(t, Config{}, got) + }) + }) + + t.Run("vcp auth: private key jwt service account", func(t *testing.T) { + // When --client-id is used, --venafi-cloud is implied. + t.Run("--private-key-path is required when --client-id is used", func(t *testing.T) { + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{ + ClientID: "test-client-id", + PrivateKeyPath: "", + }) + assert.EqualError(t, err, "failed to create client: cannot create VenafiCloudClient: 1 error occurred:\n\t* private_key_file cannot be empty\n\n") + assert.Equal(t, Config{}, got) + }) + t.Run("valid --client-id and --private-key-path", func(t *testing.T) { + path := withFile(t, "-----BEGIN PRIVATE KEY-----\nMHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49\nAwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl\ndFjz6J/j6Gu4fLPqXmM61Hj6kiuRHx5eHQ==\n-----END PRIVATE KEY-----\n") + got, cl, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{ + ClientID: "5bc7d07c-45da-11ef-a878-523f1e1d7de1", + PrivateKeyPath: path, + }) + assert.NoError(t, err) + assert.Equal(t, fillRequired(Config{}), got) + assert.IsType(t, &client.VenafiCloudClient{}, cl) + }) + + // --credentials-path + --venafi-cloud can be used instead of + // --client-id and --private-key-path. Unfortunately, --credentials-path + // can't contain the private key material, just a path to it, so you + // still need to have the private key file somewhere one the filesystem. + t.Run("valid --venafi-cloud + --credential-path + private key stored to disk", func(t *testing.T) { + privKeyPath := withFile(t, "-----BEGIN PRIVATE KEY-----\nMHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49\nAwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl\ndFjz6J/j6Gu4fLPqXmM61Hj6kiuRHx5eHQ==\n-----END PRIVATE KEY-----\n") + credsPath := withFile(t, fmt.Sprintf(`{"client_id": "5bc7d07c-45da-11ef-a878-523f1e1d7de1","private_key_file": "%s"}`, privKeyPath)) + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{ + CredentialsPath: credsPath, + VenafiCloudMode: true, + }) + assert.NoError(t, err) + assert.Equal(t, fillRequired(Config{}), got) + }) + + t.Run("--private-key-file can be passed with --credential-path", func(t *testing.T) { + privKeyPath := withFile(t, "-----BEGIN PRIVATE KEY-----\nMHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49\nAwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl\ndFjz6J/j6Gu4fLPqXmM61Hj6kiuRHx5eHQ==\n-----END PRIVATE KEY-----\n") + credsPath := withFile(t, `{"client_id": "5bc7d07c-45da-11ef-a878-523f1e1d7de1"}`) + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{ + CredentialsPath: credsPath, + PrivateKeyPath: privKeyPath, + VenafiCloudMode: true, + }) + assert.EqualError(t, err, "failed to parse credentials file: 1 error occurred:\n\t* private_key_file cannot be empty\n\n") + assert.Equal(t, Config{}, got) + }) + + t.Run("config.venafi-cloud", func(t *testing.T) { + privKeyPath := withFile(t, "-----BEGIN PRIVATE KEY-----\nMHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49\nAwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl\ndFjz6J/j6Gu4fLPqXmM61Hj6kiuRHx5eHQ==\n-----END PRIVATE KEY-----\n") + credsPath := withFile(t, `{"client_id": "5bc7d07c-45da-11ef-a878-523f1e1d7de1"}`) + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{ + VenafiCloud: &VenafiCloudConfig{ + UploaderID: "test-agent", + UploadPath: "/testing/path", + }, + }), + AgentCmdFlags{ + CredentialsPath: credsPath, + PrivateKeyPath: privKeyPath, + VenafiCloudMode: true, + }) + assert.EqualError(t, err, "failed to parse credentials file: 1 error occurred:\n\t* private_key_file cannot be empty\n\n") + assert.Equal(t, Config{}, got) + }) + }) + + t.Run("vcp auth: workload identity federation", func(t *testing.T) { + os.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) + + t.Run("valid --venafi-connection", func(t *testing.T) { + got, cl, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{VenConnName: "venafi-components", InstallNS: "venafi"}) + assert.NoError(t, err) + assert.Equal(t, fillRequired(Config{}), got) + assert.IsType(t, &client.VenConnClient{}, cl) + }) + + t.Run("namespace can't be read from disk", func(t *testing.T) { + got, _, err := getConfiguration(discardLogs(t), + fillRequired(Config{}), + AgentCmdFlags{VenConnName: "venafi-components"}) + assert.EqualError(t, err, "could not guess which namespace the agent is running in: not running in cluster, please use --install-namespace to specify the namespace in which the agent is running") + assert.Equal(t, Config{}, got) + }) + + t.Run("warning about venafi-cloud.uploader_id and venafi-cloud.upload_path being skipped", func(t *testing.T) { + log, out := withLogs(t) + cfg := fillRequired(Config{VenafiCloud: &VenafiCloudConfig{ + UploaderID: "test-agent", + UploadPath: "/testing/path", + }}) + got, _, err := getConfiguration(log, + cfg, + AgentCmdFlags{VenConnName: "venafi-components", InstallNS: "venafi"}) + assert.NoError(t, err) + assert.Equal(t, cfg, got) + assert.Contains(t, out.String(), "ignoring venafi-cloud.uploader_id") + assert.Contains(t, out.String(), "ignoring venafi-cloud.upload_path") + }) + }) +} + +// Fills in the `server` and `period` as they appear in each and every test +// case. +func fillRequired(c Config) Config { + c.Server = "http://localhost:8080" + c.Period = 1 * time.Hour + return c +} + func TestValidConfigLoad(t *testing.T) { configFileContents := ` server: "http://localhost:8080" @@ -265,3 +452,51 @@ func TestInvalidDataGathered(t *testing.T) { t.Errorf("\ngot=\n%v\nwant=\n%s\ndiff=\n%s", got, want, diff.Diff(got, want)) } } + +func withFile(t testing.TB, content string) string { + t.Helper() + + f, err := os.CreateTemp(t.TempDir(), "file") + if err != nil { + t.Fatalf("failed to create temporary file: %v", err) + } + defer f.Close() + + _, err = f.WriteString(content) + if err != nil { + t.Fatalf("failed to write to temporary file: %v", err) + } + + return f.Name() +} + +func withLogs(t testing.TB) (*log.Logger, *bytes.Buffer) { + b := bytes.Buffer{} + return log.New(&b, "", 0), &b +} + +func discardLogs(t testing.TB) *log.Logger { + return log.New(io.Discard, "", 0) +} + +const fakeKubeconfig = ` +apiVersion: v1 +clusters: +- cluster: + certificate-authority-data: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURCVENDQWUyZ0F3SUJBZ0lJVGpXZTMvWXhJbXN3RFFZSktvWklodmNOQVFFTEJRQXdGVEVUTUJFR0ExVUUKQXhNS2EzVmlaWEp1WlhSbGN6QWVGdzB5TkRBM01UVXhOREUxTVRSYUZ3MHpOREEzTVRNeE5ESXdNVFJhTUJVeApFekFSQmdOVkJBTVRDbXQxWW1WeWJtVjBaWE13Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLCkFvSUJBUUMweVhZSmIyT0JRb0NrYXYySWw1NjNRM0t3RFpGSmluNFRFSkJJbWt6MnpJVU56cHIvV09MY01jdjYKVG9IaTl1c1oyL005dktMcnhYRE1FcFNJaTR4c1psZ3BDN2Erb3hqNW80MVdqRy9rdzhmcVc2MTRUV2ZEekRkWQppRkNKOC9PdmpKdFY2elREZ04vUGtWRytKQWJIOTdnVkc5NXRzRHBIazN3Nk12WkdYK3lqdnhXblV1enlpdFIzCkNLNkhYcE82Y0xBVzJva1FWZHYrZEFUSDFrZVpZZHpMOFp0U0txcUo2QWlRTUtEMG1FbXZPWDNBRk4vUUNQdXkKTVdDUXVkQ1RaQ0t1a1gwRzllakd3NGE1RC9CZnVmYmtWd1g3Vmo3OGJjQ0NId3JJMFZNOHVzYnJzcEs5eGtsVwpodjRXOGVaQ21KZWlMajFLVUhSbTdRVlFYVHNoQWdNQkFBR2pXVEJYTUE0R0ExVWREd0VCL3dRRUF3SUNwREFQCkJnTlZIUk1CQWY4RUJUQURBUUgvTUIwR0ExVWREZ1FXQkJTckNJaE44czZpMmRIMEpwQWU3dFdPL2p2clJqQVYKQmdOVkhSRUVEakFNZ2dwcmRXSmxjbTVsZEdWek1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQ0pQd2x1OFVhRgo5UnIvUG5QSDNtL0w2amhlcE5Kak5vNThFSWlEMWpjc1Y3R04zZUpha0h1b3g1MGRmR2gvMFFMZEwreUluamFtCkw0Y0R6RnVYeDhCL0ZXQlMwdnYvaG5WQ1JadER4bjB1OW92WC9iblNJdHpBOHNKMHA4cU1YeEFmbkxuZDI0TksKNFZXZmFXTThjbitQeUoybnJ3MHo2YmtYYnZZMGxEV2ZRakorOUJxU3IyeUZYZWM4eXljSzZ6aHlXeHJMV1p1OAoyQngrYjJML1JETDg2T3FXSkthRmljNGlWeDBoK2xDYlBIQmNwazhQOVFvSjZodThhdXdiWjZlMkwxbmZSdWFjCjB3Z1F5OEMzNVExMTdla0dOcjZKMUlrRlE5OGorYTNBTVQ2Z05KclZGZEJOOGlMcjlhMDZJQnRBb04wV2s0bysKL2F5akJBc3hONHo5Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + server: https://127.0.0.1:58453 + name: fake +contexts: +- context: + cluster: fake + user: fake + name: fake +current-context: fake +kind: Config +preferences: {} +users: +- name: fake + user: + client-certificate-data: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURLVENDQWhHZ0F3SUJBZ0lJV1JQVy9Nblo0VnN3RFFZSktvWklodmNOQVFFTEJRQXdGVEVUTUJFR0ExVUUKQXhNS2EzVmlaWEp1WlhSbGN6QWVGdzB5TkRBM01UVXhOREUxTVRSYUZ3MHlOVEEzTVRVeE5ESXdNVFZhTUR3eApIekFkQmdOVkJBb1RGbXQxWW1WaFpHMDZZMngxYzNSbGNpMWhaRzFwYm5NeEdUQVhCZ05WQkFNVEVHdDFZbVZ5CmJtVjBaWE10WVdSdGFXNHdnZ0VpTUEwR0NTcUdTSWIzRFFFQkFRVUFBNElCRHdBd2dnRUtBb0lCQVFDcGpIRW4KY2w3QlVURlJLdTVUeU54TmxEdWxHYittalNLcHdsd2FGa0ZyYUZPMXU0MVRVOE9FalZhNDlheHp1SHZYNTZpWgpLMEJCbkJ5aFdYeGVKNE1CTzRWdXk2K09zYVBHWUgxcDZIcGpmUTBwVW5QODFndTgzMloyWmRaazhmZkJVb0pjCjI4b25Mbjd0UERVdjhHVk9WbndZRzE4RGFDWFFjVGR3VjFNYVFKZCtsNGpveHQ5S0J6aDhZUUhZanJMdnl4RncKd2dPbTNITk5GQ3J3Zno2Wis2bi95bHliaTA3amNHVi9nMTVHaVl6azJNWW5EbFBYUHVQYzY0MVp0NWdBcGFwSgpUbUdsaW95Ym85bUVtZmRFbnd0aDJDSTZTdkx6eXlveTJidlhEVktNRzhZTzE5N25kRUd6TE95T1lYT1RMYUNkCnhaWVVCdlNadkxSK1pzMGpBZ01CQUFHalZqQlVNQTRHQTFVZER3RUIvd1FFQXdJRm9EQVRCZ05WSFNVRUREQUsKQmdnckJnRUZCUWNEQWpBTUJnTlZIUk1CQWY4RUFqQUFNQjhHQTFVZEl3UVlNQmFBRktzSWlFM3l6cUxaMGZRbQprQjd1MVk3K08rdEdNQTBHQ1NxR1NJYjNEUUVCQ3dVQUE0SUJBUUExeXpDdE55Rmp6SHlNZ0FFTVpXalR4OWxWClk2MHRpeTFvYjUvL0thR0MvWmhSbW94NmZ0Sy94dFJDRlptRVYxZ1ZzaXNLc0g2L0YwTEZHRys4V0lrNzVoZXkKVGtoRXUvRVpBdEpRMUNoSmFWMTg4QzNvMmtmSkZOOFlVRlRyS0k3K1NNb0RCTmJJU0VPV3FsZFRiVDdWdkVzNQpsWTRKcS9rU2xnNnNZcWNCRDYzY2pFOHpKU3Y4aDUra3J0d2JVRW90Y0ptN0IvNnpMZksxNWQ5WXBEb0F1anl0CjlVcTVROEhaSGRqWlZ1OWgvNmYvbVMvZkRyek9weDhNOTdPblU1T0MvY2dTNGtUNVhkdVo3SVB3TDJVMkZsTlIKVUdvZ0RndmxDQkFaMDV4WXh4Z2xjNlNYK3JrcURUK3VhWHNtR2dBU21oUjR4OXFkRzA1R2JIdXhoZkJhCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + client-key-data: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFcEFJQkFBS0NBUUVBcVl4eEozSmV3VkV4VVNydVU4amNUWlE3cFJtL3BvMGlxY0pjR2haQmEyaFR0YnVOClUxUERoSTFXdVBXc2M3aDcxK2VvbVN0QVFad2NvVmw4WGllREFUdUZic3V2anJHanhtQjlhZWg2WTMwTktWSnoKL05ZTHZOOW1kbVhXWlBIM3dWS0NYTnZLSnk1KzdUdzFML0JsVGxaOEdCdGZBMmdsMEhFM2NGZFRHa0NYZnBlSQo2TWJmU2djNGZHRUIySTZ5NzhzUmNNSURwdHh6VFJRcThIOCttZnVwLzhwY200dE80M0JsZjROZVJvbU01TmpHCkp3NVQxejdqM091TldiZVlBS1dxU1U1aHBZcU1tNlBaaEpuM1JKOExZZGdpT2tyeTg4c3FNdG03MXcxU2pCdkcKRHRmZTUzUkJzeXpzam1Gemt5MmduY1dXRkFiMG1ieTBmbWJOSXdJREFRQUJBb0lCQUY2dHkzNWdzcU0zYU5mUApwbmpwSUlTOTh6UzJGVHkzY1pUa3NUUHNHNm9UL3pMcndmYTNQdVpsV3ZrOFQ0bnJpbFM5eTN1RkdJUEszbjRICmo1aXdiY3FoWjFqQXE0OStpVnM5Qkt2QW81K3M5RTJQK3E5RkJCYjdsYWNtSlR3SGx2ZkEwSVYwUXdYd1EvYk0KZVZNRTVqMkJ0Qmh1S0hlcGovdy9UTnNTR0pqK2NlNmN2aXVVb2NXWGsxWDl2c1RDaUdtMVdnVkZGQVphVGpMTgpDcEU1dHFpdnpvbEZVbXZIbmVYNTZTOEdFWk01NFA5MFk1enJ3NHBGa0Vud1VMRlBLa1U0cUU0eWVPNVFsWUhCClQ0NklIOVNPcUU5T0pLL3JCSGVzQU45TWNrMTdKblF6Sy95bXh6eHhhcGdPMnk0bVBTcjJaaGk0SENMRHRQV2QKc0ZtRzc2RUNnWUVBeHhQTTJYVFV2bXV5ckZmUVgxblJTSW9jMGhxZFY0MnFaRFlkMzZWVWc1UUVMM0Y4S01aUwptSkNsWlJXYW9IY0NFVUdXakFTWEJaMW9hOHlOMVhSNURTV3ZJMmV5TjE1dnh3NFg1SjV5QzUvY0F4ZW00dUk3CnkzM0VWWktXZXpFQTVVeUFtNlF6ei9lR1R6QkZyNUlxYkJDUitTUldudHRXUHdJTUhkK0VoeEVDZ1lFQTJnY3QKT2h1U0xJeDZZbTFTRHVVT0pSdmtFZFlCazJPQWxRbk5kOVJoaWIxdVlVbjhPTkhYdHBsY2FHZEl3bFdkaEJlcwo4M1F4dXA4MEFydEFtM2FHMXZ6RlZ6Q05KeHA4ZGFxWlFsZk94YlJReUQ0cjdtT2Z5aENFY2VibHAxMkZKRTBQCmNhOFl2TkFuTTdkbnlTSFd0aUo2THFQWDVuMXlRSC9JY1NIaEdQTUNnWUVBa0ZDZFBzSy8rcTZ1SHR1bDFZbVIKK3FrTWpZNzNvdUd5dE9TNk1VZDBCZEtHV2pKRmxIVjRxTnFxMjZXV3ExNjZZL0lOQmNIS0RTcjM2TFduMkNhUQpIbVRFR3NGd1kwMFZjTktacFlUckhkd3NMUjIzUUdCS2dwRFFoRXc0eEdOWXgrRDJsbDJwcGNoRldDQ2hVODU4CjdFdnkxZzV1c01oR05IVHlmYkZzTEZFQ2dZRUF6QXJOVzhVenZuZFZqY25MY3Q4UXBzLzhXR2pVbnJBUFJPdWcKbTlWcDF2TXVXdVJYcElGV0JMQnYxOUZaT1czUWRTK0hEMndkb2c2ZUtUUS9HWDhLWUNhOU5JVGVoTXIzMFZMdwpEVE9KOG1KMiszK2JzNFVPcEpkaXJBb3Z3THI0QUdvUjJ3M0g4K1JGMjlOMzBMYlhieXJDOStVa0I3UTgrWG5kCkIydHljdHNDZ1lCZkxqUTNRUnpQN1Z5Y1VGNkFTYUNYVTJkcE5lckVUbGFpdldIb1FFWVo3NHEyMkFTeFcrMlEKWmtZTEM1RVNGMnZwUU5kZUZhZlRyRm9zR3pLQ1dwYXBUL2QwUC9qaG83TEF1TTJQZEcxSXFoNElRU3FUM3VqNwp4Sm9WUzhIbEg1Ri9sQzZzczZQSm1GWlpsanhFL1FVTDlucDNLYTVCRjFXdXZiZVp0Q2I5Mnc9PQotLS0tLUVORCBSU0EgUFJJVkFURSBLRVktLS0tLQo= +` diff --git a/pkg/agent/run.go b/pkg/agent/run.go index 4ade0038..39619f08 100644 --- a/pkg/agent/run.go +++ b/pkg/agent/run.go @@ -3,13 +3,12 @@ package agent import ( "bytes" "context" + "encoding/json" "errors" "fmt" - "io" "io/ioutil" "net/http" _ "net/http/pprof" - "net/url" "os" "strings" "sync" @@ -17,7 +16,6 @@ import ( "github.com/cenkalti/backoff" "github.com/hashicorp/go-multierror" - json "github.com/json-iterator/go" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/spf13/cobra" @@ -26,70 +24,11 @@ import ( "github.com/jetstack/preflight/api" "github.com/jetstack/preflight/pkg/client" "github.com/jetstack/preflight/pkg/datagatherer" - "github.com/jetstack/preflight/pkg/kubeconfig" "github.com/jetstack/preflight/pkg/logs" "github.com/jetstack/preflight/pkg/version" ) -// ConfigFilePath is where the agent will try to load the configuration from -var ConfigFilePath string - -// Period is the time waited between scans -var Period time.Duration - -// OneShot flag causes agent to run once -var OneShot bool - -// VenafiCloudMode flag determines which format to load for config and credential type -var VenafiCloudMode bool - -// ClientID is the clientID in case of Venafi Cloud mode -var ClientID string - -// PrivateKeyPath is the path for the service account private key in case of Venafi Cloud mode -var PrivateKeyPath string - -// CredentialsPath is where the agent will try to loads the credentials. (Experimental) -var CredentialsPath string - -// OutputPath is where the agent will write data to locally if specified -var OutputPath string - -// InputPath is where the agent will read data from instead of gathering from clusters if specified -var InputPath string - -// BackoffMaxTime is the maximum time for which data gatherers will be retried -var BackoffMaxTime time.Duration - -// StrictMode flag causes the agent to fail at the first attempt -var StrictMode bool - -// APIToken is an authentication token used for the backend API as an alternative to oauth flows. -var APIToken string - -// VenConnName is the name of the VenafiConnection resource to use. Using this -// flag will enable Venafi Connection mode. -var VenConnName string - -// VenConnNS is the namespace of the VenafiConnection resource to use. It is -// only useful when the VenafiConnection isn't in the same namespace as the -// agent. -// -// May be left empty to use the same namespace as the agent. -var VenConnNS string - -// InstallNS is the namespace in which the agent is running in. Only needed when -// running the agent outside of Kubernetes. -// -// May be left empty when running in Kubernetes. In this case, the namespace is -// read from the file /var/run/secrets/kubernetes.io/serviceaccount/namespace. -var InstallNS string - -// Profiling flag enabled pprof endpoints to run on the agent -var Profiling bool - -// Prometheus flag enabled Prometheus metrics endpoint to run on the agent -var Prometheus bool +var Flags AgentCmdFlags // schema version of the data sent by the agent. // The new default version is v2. @@ -99,17 +38,34 @@ var Prometheus bool // raw resource data of unstructuredList const schemaVersion string = "v2.0.0" -const ( - inClusterNamespacePath = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" -) - // Run starts the agent process func Run(cmd *cobra.Command, args []string) { + logs.Log.Printf("Preflight agent version: %s (%s)", version.PreflightVersion, version.Commit) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - config, preflightClient := getConfiguration() - if Profiling { + file, err := os.Open(Flags.ConfigFilePath) + if err != nil { + logs.Log.Fatalf("Failed to load config file for agent from: %s", Flags.ConfigFilePath) + } + defer file.Close() + + b, err := ioutil.ReadAll(file) + if err != nil { + logs.Log.Fatalf("Failed to read config file: %s", err) + } + + cfg, err := ParseConfig(b, Flags.StrictMode) + if err != nil { + logs.Log.Fatalf("Failed to parse config file: %s", err) + } + + config, preflightClient, err := getConfiguration(logs.Log, cfg, Flags) + if err != nil { + logs.Log.Fatalf("While evaluating configuration: %v", err) + } + + if Flags.Profiling { logs.Log.Printf("pprof profiling was enabled.\nRunning profiling on port :6060") go func() { err := http.ListenAndServe(":6060", nil) @@ -118,7 +74,7 @@ func Run(cmd *cobra.Command, args []string) { } }() } - if Prometheus { + if Flags.Prometheus { logs.Log.Printf("Prometheus was enabled.\nRunning prometheus server on port :8081") go func() { prometheus.MustRegister(metricPayloadSize) @@ -209,171 +165,18 @@ func Run(cmd *cobra.Command, args []string) { // APIs each cycle depending on datagatherer implementation for { // if period is set in the config, then use that if not already set - if Period == 0 && config.Period > 0 { + if Flags.Period == 0 && config.Period > 0 { logs.Log.Printf("Using period from config %s", config.Period) - Period = config.Period + Flags.Period = config.Period } gatherAndOutputData(config, preflightClient, dataGatherers) - if OneShot { + if Flags.OneShot { break } - time.Sleep(Period) - } -} - -func getConfiguration() (Config, client.Client) { - logs.Log.Printf("Preflight agent version: %s (%s)", version.PreflightVersion, version.Commit) - file, err := os.Open(ConfigFilePath) - if err != nil { - logs.Log.Fatalf("Failed to load config file for agent from: %s", ConfigFilePath) - } - defer file.Close() - - b, err := ioutil.ReadAll(file) - if err != nil { - logs.Log.Fatalf("Failed to read config file: %s", err) - } - - // If the ClientID of the service account is specified, then assume we are in Venafi Cloud mode. - if ClientID != "" || VenConnName != "" { - VenafiCloudMode = true - } - - config, err := ParseConfig(b, VenafiCloudMode) - if err != nil { - logs.Log.Fatalf("Failed to parse config file: %s", err) - } - - baseURL := config.Server - if baseURL == "" { - logs.Log.Printf("Using deprecated Endpoint configuration. User Server instead.") - baseURL = fmt.Sprintf("%s://%s", config.Endpoint.Protocol, config.Endpoint.Host) - _, err = url.Parse(baseURL) - if err != nil { - logs.Log.Fatalf("Failed to build URL: %s", err) - } - } - - if Period == 0 && config.Period == 0 && !OneShot { - logs.Log.Fatalf("Failed to load period, must be set as flag or in config") - } - - var credentials client.Credentials - if ClientID != "" { - credentials = &client.VenafiSvcAccountCredentials{ - ClientID: ClientID, - PrivateKeyFile: PrivateKeyPath, - } - } else if CredentialsPath != "" { - file, err = os.Open(CredentialsPath) - if err != nil { - logs.Log.Fatalf("Failed to load credentials from file %s", CredentialsPath) - } - defer file.Close() - - b, err = io.ReadAll(file) - if err != nil { - logs.Log.Fatalf("Failed to read credentials file: %v", err) - } - if VenafiCloudMode { - credentials, err = client.ParseVenafiCredentials(b) - } else { - credentials, err = client.ParseOAuthCredentials(b) - } - if err != nil { - logs.Log.Fatalf("Failed to parse credentials file: %s", err) - } - } - - venConnMode := VenConnName != "" - - if venConnMode && InstallNS == "" { - InstallNS, err = getInClusterNamespace() - if err != nil { - logs.Log.Fatalf("could not guess which namespace the agent is running in: %s", err) - } - } - if venConnMode && VenConnNS == "" { - VenConnNS = InstallNS - } - - agentMetadata := &api.AgentMetadata{ - Version: version.PreflightVersion, - ClusterID: config.ClusterID, - } - - var preflightClient client.Client - switch { - case credentials != nil: - preflightClient, err = createCredentialClient(credentials, config, agentMetadata, baseURL) - case VenConnName != "": - // Why wasn't this added to the createCredentialClient instead? Because - // the --venafi-connection mode of authentication doesn't need any - // secrets (or any other information for that matter) to be loaded from - // disk (using --credentials-path). Everything is passed as flags. - logs.Log.Println("Venafi Connection mode was specified, using Venafi Connection authentication.") - - // The venafi-cloud.upload_path was initially meant to let users - // configure HTTP proxies, but it has never been used since HTTP proxies - // don't rewrite paths. Thus, we've disabled the ability to change this - // value with the new --venafi-connection flag, and this field is simply - // ignored. - if config.VenafiCloud != nil && config.VenafiCloud.UploadPath != "" { - logs.Log.Printf(`ignoring venafi-cloud.upload_path. In Venafi Connection mode, this field is not needed.`) - } - - // Regarding venafi-cloud.uploader_id, we found that it doesn't do - // anything in the backend. Since the backend requires it for historical - // reasons (but cannot be empty), we just ignore whatever the user has - // set in the config file, and set it to an arbitrary value in the - // client since it doesn't matter. - if config.VenafiCloud.UploaderID != "" { - logs.Log.Printf(`ignoring venafi-cloud.uploader_id. In Venafi Connection mode, this field is not needed.`) - } - - cfg, err := kubeconfig.LoadRESTConfig("") - if err != nil { - logs.Log.Fatalf("failed to load kubeconfig: %v", err) - } - - preflightClient, err = client.NewVenConnClient(cfg, agentMetadata, InstallNS, VenConnName, VenConnNS, nil) - case APIToken != "": - logs.Log.Println("An API token was specified, using API token authentication.") - preflightClient, err = client.NewAPITokenClient(agentMetadata, APIToken, baseURL) - default: - logs.Log.Println("No credentials were specified, using with no authentication.") - preflightClient, err = client.NewUnauthenticatedClient(agentMetadata, baseURL) - } - - if err != nil { - logs.Log.Fatalf("failed to create client: %v", err) - } - - return config, preflightClient -} - -func createCredentialClient(credentials client.Credentials, config Config, agentMetadata *api.AgentMetadata, baseURL string) (client.Client, error) { - switch creds := credentials.(type) { - case *client.VenafiSvcAccountCredentials: - logs.Log.Println("Venafi Cloud mode was specified, using Venafi Service Account authentication.") - // check if config has Venafi Cloud data, use config data if it's present - uploaderID := creds.ClientID - uploadPath := "" - if config.VenafiCloud != nil { - logs.Log.Println("Loading uploader_id and upload_path from \"venafi-cloud\" configuration.") - uploaderID = config.VenafiCloud.UploaderID - uploadPath = config.VenafiCloud.UploadPath - } - return client.NewVenafiCloudClient(agentMetadata, creds, baseURL, uploaderID, uploadPath) - - case *client.OAuthCredentials: - logs.Log.Println("A credentials file was specified, using oauth authentication.") - return client.NewOAuthClient(agentMetadata, creds, baseURL) - default: - return nil, errors.New("credentials file is in unknown format") + time.Sleep(Flags.Period) } } @@ -381,16 +184,16 @@ func gatherAndOutputData(config Config, preflightClient client.Client, dataGathe var readings []*api.DataReading // Input/OutputPath flag overwrites agent.yaml configuration - if InputPath == "" { - InputPath = config.InputPath + if Flags.InputPath == "" { + Flags.InputPath = config.InputPath } - if OutputPath == "" { - OutputPath = config.OutputPath + if Flags.OutputPath == "" { + Flags.OutputPath = config.OutputPath } - if InputPath != "" { - logs.Log.Printf("Reading data from local file: %s", InputPath) - data, err := ioutil.ReadFile(InputPath) + if Flags.InputPath != "" { + logs.Log.Printf("Reading data from local file: %s", Flags.InputPath) + data, err := ioutil.ReadFile(Flags.InputPath) if err != nil { logs.Log.Fatalf("failed to read local data file: %s", err) } @@ -402,21 +205,21 @@ func gatherAndOutputData(config Config, preflightClient client.Client, dataGathe readings = gatherData(config, dataGatherers) } - if OutputPath != "" { + if Flags.OutputPath != "" { data, err := json.MarshalIndent(readings, "", " ") if err != nil { logs.Log.Fatal("failed to marshal JSON") } - err = ioutil.WriteFile(OutputPath, data, 0644) + err = ioutil.WriteFile(Flags.OutputPath, data, 0644) if err != nil { logs.Log.Fatalf("failed to output to local file: %s", err) } - logs.Log.Printf("Data saved to local file: %s", OutputPath) + logs.Log.Printf("Data saved to local file: %s", Flags.OutputPath) } else { backOff := backoff.NewExponentialBackOff() backOff.InitialInterval = 30 * time.Second backOff.MaxInterval = 3 * time.Minute - backOff.MaxElapsedTime = BackoffMaxTime + backOff.MaxElapsedTime = Flags.BackoffMaxTime post := func() error { return postData(config, preflightClient, readings) } @@ -426,7 +229,6 @@ func gatherAndOutputData(config Config, preflightClient client.Client, dataGathe if err != nil { logs.Log.Fatalf("Exiting due to fatal error uploading: %v", err) } - } } @@ -468,7 +270,7 @@ func gatherData(config Config, dataGatherers map[string]datagatherer.DataGathere } } - if StrictMode && dgError.ErrorOrNil() != nil { + if Flags.StrictMode && dgError.ErrorOrNil() != nil { logs.Log.Fatalf("halting datagathering in strict mode due to error: %s", dgError.ErrorOrNil()) } @@ -480,7 +282,7 @@ func postData(config Config, preflightClient client.Client, readings []*api.Data logs.Log.Println("Posting data to:", baseURL) - if VenafiCloudMode { + if Flags.VenafiCloudMode { // orgID and clusterID are not required for Venafi Cloud auth err := preflightClient.PostDataReadingsWithOptions(readings, client.Options{ ClusterName: config.ClusterID, @@ -541,22 +343,3 @@ func postData(config Config, preflightClient client.Client, readings []*api.Data return nil } - -// Inspired by the controller-runtime project. -func getInClusterNamespace() (string, error) { - // Check whether the namespace file exists. - // If not, we are not running in cluster so can't guess the namespace. - _, err := os.Stat(inClusterNamespacePath) - if os.IsNotExist(err) { - return "", fmt.Errorf("not running in cluster, please use --install-namespace to specify the namespace in which the agent is running") - } - if err != nil { - return "", fmt.Errorf("error checking namespace file: %w", err) - } - - namespace, err := os.ReadFile(inClusterNamespacePath) - if err != nil { - return "", fmt.Errorf("error reading namespace file: %w", err) - } - return string(namespace), nil -} From 0e8875d87dfbdab463a847578f332a50aea7b1a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Mon, 26 Aug 2024 14:33:07 +0200 Subject: [PATCH 2/3] tests: use a more expressive fake URL http://api.venafi.eu The URL http://localhost:8080 suggested that this test was using a fake server, and didn't indicate clearly which API (the old jetstack-secure API or the newer Venafi Cloud Plaform API). --- pkg/agent/config_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index f34337e2..2a0320e4 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -19,12 +19,12 @@ import ( func TestGetConfiguration(t *testing.T) { t.Run("minimal successful configuration", func(t *testing.T) { got, cl, err := getConfiguration(discardLogs(t), - Config{Server: "http://localhost:8080", Period: 1 * time.Hour}, + Config{Server: "http://api.venafi.eu", Period: 1 * time.Hour}, AgentCmdFlags{}, ) assert.NoError(t, err) assert.Equal(t, Config{ - Server: "http://localhost:8080", + Server: "http://api.venafi.eu", Period: 1 * time.Hour, }, got) assert.IsType(t, &client.UnauthenticatedClient{}, cl) @@ -32,7 +32,7 @@ func TestGetConfiguration(t *testing.T) { t.Run("period must be given", func(t *testing.T) { _, _, err := getConfiguration(discardLogs(t), - Config{Server: "http://localhost:8080"}, + Config{Server: "http://api.venafi.eu"}, AgentCmdFlags{}) assert.EqualError(t, err, "period must be set as a flag or in config") }) @@ -192,7 +192,7 @@ func TestGetConfiguration(t *testing.T) { // Fills in the `server` and `period` as they appear in each and every test // case. func fillRequired(c Config) Config { - c.Server = "http://localhost:8080" + c.Server = "http://api.venafi.eu" c.Period = 1 * time.Hour return c } From 21e52292fc0601cbc9a7af18ee500b0384d385b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Mon, 26 Aug 2024 16:02:49 +0200 Subject: [PATCH 3/3] tests: remove dependency on github.com/d4l3k/messagediff Testify, which we already import and that I trust, has the same functionality. github.com/d4l3k/messagediff is a relatively small Go project (250 stars) that hasn't been updated since Nov 11, 2020. --- pkg/agent/config_test.go | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 2a0320e4..1f8887f9 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/d4l3k/messagediff" "github.com/jetstack/preflight/pkg/client" "github.com/kylelemons/godebug/diff" "github.com/stretchr/testify/assert" @@ -213,9 +212,7 @@ func TestValidConfigLoad(t *testing.T) { ` loadedConfig, err := ParseConfig([]byte(configFileContents), false) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + assert.NoError(t, err) expected := Config{ Server: "http://localhost:8080", @@ -235,9 +232,7 @@ func TestValidConfigLoad(t *testing.T) { OutputPath: "/nothome", } - if diff, equal := messagediff.PrettyDiff(expected, loadedConfig); !equal { - t.Errorf("Diff %s", diff) - } + assert.Equal(t, expected, loadedConfig) } func TestValidConfigWithEndpointLoad(t *testing.T) { @@ -256,9 +251,7 @@ func TestValidConfigWithEndpointLoad(t *testing.T) { ` loadedConfig, err := ParseConfig([]byte(configFileContents), false) - if err != nil { - t.Errorf("unexpected error: %v", err) - } + assert.NoError(t, err) expected := Config{ Endpoint: Endpoint{ @@ -280,9 +273,7 @@ func TestValidConfigWithEndpointLoad(t *testing.T) { }, } - if diff, equal := messagediff.PrettyDiff(expected, loadedConfig); !equal { - t.Errorf("Diff %s", diff) - } + assert.Equal(t, expected, loadedConfig) } func TestValidVenafiCloudConfigLoad(t *testing.T) { @@ -302,9 +293,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) { ` loadedConfig, err := ParseConfig([]byte(configFileContents), false) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + assert.NoError(t, err) expected := Config{ Server: "http://localhost:8080", @@ -328,9 +317,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) { }, } - if diff, equal := messagediff.PrettyDiff(expected, loadedConfig); !equal { - t.Errorf("Diff %s", diff) - } + assert.Equal(t, expected, loadedConfig) } func TestInvalidConfigError(t *testing.T) {