diff --git a/.github/tests/it/client/helper.conf b/.github/tests/it/client/helper.conf index 95ae2a81..44cbddf3 100644 --- a/.github/tests/it/client/helper.conf +++ b/.github/tests/it/client/helper.conf @@ -1,8 +1,8 @@ -agentAddress = "/var/run/api.sock" +agent_address = "/var/run/api.sock" cmd = "/run/client/assert.sh" -cmdArgs = "" -certDir = "/run/client/certs/" -renewSignal = "SIGUSR1" -svidFileName = "svid.crt" -svidKeyFileName = "svid.key" -svidBundleFileName = "root.crt" +cmd_args = "" +cert_dir = "/run/client/certs/" +renew_signal = "SIGUSR1" +svid_file_name = "svid.crt" +svid_key_file_name = "svid.key" +svid_bundle_file_name = "root.crt" diff --git a/.github/tests/it/go-server/helper.conf b/.github/tests/it/go-server/helper.conf index e4224467..877aa288 100644 --- a/.github/tests/it/go-server/helper.conf +++ b/.github/tests/it/go-server/helper.conf @@ -1,8 +1,8 @@ -agentAddress = "/var/run/api.sock" +agent_address = "/var/run/api.sock" cmd = "" -cmdArgs = "" -certDir = "/run/go-server/certs/" -renewSignal = "SIGUSR1" -svidFileName = "svid.crt" -svidKeyFileName = "svid.key" -svidBundleFileName = "root.crt" +cmd_args = "" +cert_dir = "/run/go-server/certs/" +renew_signal = "SIGUSR1" +svid_file_name = "svid.crt" +svid_key_file_name = "svid.key" +svid_bundle_file_name = "root.crt" diff --git a/.github/tests/it/mysql/helper.conf b/.github/tests/it/mysql/helper.conf index 616fe35d..0376615d 100644 --- a/.github/tests/it/mysql/helper.conf +++ b/.github/tests/it/mysql/helper.conf @@ -1,9 +1,9 @@ -agentAddress = "/var/run/api.sock" +agent_address = "/var/run/api.sock" cmd = "mysql" -cmdArgs = "-e \"ALTER INSTANCE RELOAD TLS;\"" -certDir = "/var/lib/mysql/" -renewSignal = "SIGUSR1" -svidFileName = "server-cert.pem" -svidKeyFileName = "server-key.pem" -svidBundleFileName = "ca.pem" -addIntermediatesToBundle = true +cmd_args = "-e \"ALTER INSTANCE RELOAD TLS;\"" +cert_dir = "/var/lib/mysql/" +renew_signal = "SIGUSR1" +svid_file_name = "server-cert.pem" +svid_key_file_name = "server-key.pem" +svid_bundle_file_name = "ca.pem" +add_intermediates_to_bundle = true diff --git a/.github/tests/it/postgres/helper.conf b/.github/tests/it/postgres/helper.conf index 38ac18f2..4101a5f6 100644 --- a/.github/tests/it/postgres/helper.conf +++ b/.github/tests/it/postgres/helper.conf @@ -1,8 +1,8 @@ -agentAddress = "/var/run/api.sock" +agent_address = "/var/run/api.sock" cmd = "/run/postgresql/reload_certificates.sh" -cmdArgs = "" -certDir = "/run/postgresql/certs/" -renewSignal = "SIGUSR1" -svidFileName = "svid.crt" -svidKeyFileName = "svid.key" -svidBundleFileName = "root.crt" +cmd_args = "" +cert_dir = "/run/postgresql/certs/" +renew_signal = "SIGUSR1" +svid_file_name = "svid.crt" +svid_key_file_name = "svid.key" +svid_bundle_file_name = "root.crt" diff --git a/cmd/spiffe-helper/config/config.go b/cmd/spiffe-helper/config/config.go index 8c60f8b9..ce3a3455 100644 --- a/cmd/spiffe-helper/config/config.go +++ b/cmd/spiffe-helper/config/config.go @@ -24,32 +24,24 @@ const ( ) type Config struct { - AddIntermediatesToBundle bool `hcl:"add_intermediates_to_bundle"` - AddIntermediatesToBundleDeprecated bool `hcl:"addIntermediatesToBundle"` - AgentAddress string `hcl:"agent_address"` - AgentAddressDeprecated string `hcl:"agentAddress"` - Cmd string `hcl:"cmd"` - CmdArgs string `hcl:"cmd_args"` - PIDFileName string `hcl:"pid_file_name"` - CmdArgsDeprecated string `hcl:"cmdArgs"` - CertDir string `hcl:"cert_dir"` - CertDirDeprecated string `hcl:"certDir"` - CertFileMode int `hcl:"cert_file_mode"` - KeyFileMode int `hcl:"key_file_mode"` - JWTBundleFileMode int `hcl:"jwt_bundle_file_mode"` - JWTSVIDFileMode int `hcl:"jwt_svid_file_mode"` - IncludeFederatedDomains bool `hcl:"include_federated_domains"` - RenewSignal string `hcl:"renew_signal"` - RenewSignalDeprecated string `hcl:"renewSignal"` - DaemonMode *bool `hcl:"daemon_mode"` + AddIntermediatesToBundle bool `hcl:"add_intermediates_to_bundle"` + AgentAddress string `hcl:"agent_address"` + Cmd string `hcl:"cmd"` + CmdArgs string `hcl:"cmd_args"` + PIDFileName string `hcl:"pid_file_name"` + CertDir string `hcl:"cert_dir"` + CertFileMode int `hcl:"cert_file_mode"` + KeyFileMode int `hcl:"key_file_mode"` + JWTBundleFileMode int `hcl:"jwt_bundle_file_mode"` + JWTSVIDFileMode int `hcl:"jwt_svid_file_mode"` + IncludeFederatedDomains bool `hcl:"include_federated_domains"` + RenewSignal string `hcl:"renew_signal"` + DaemonMode *bool `hcl:"daemon_mode"` // x509 configuration - SVIDFileName string `hcl:"svid_file_name"` - SVIDFileNameDeprecated string `hcl:"svidFileName"` - SVIDKeyFileName string `hcl:"svid_key_file_name"` - SVIDKeyFileNameDeprecated string `hcl:"svidKeyFileName"` - SVIDBundleFileName string `hcl:"svid_bundle_file_name"` - SVIDBundleFileNameDeprecated string `hcl:"svidBundleFileName"` + SVIDFileName string `hcl:"svid_file_name"` + SVIDKeyFileName string `hcl:"svid_key_file_name"` + SVIDBundleFileName string `hcl:"svid_bundle_file_name"` // JWT configuration JWTSVIDs []JWTConfig `hcl:"jwt_svids"` @@ -94,7 +86,7 @@ func (c *Config) ParseConfigFlagOverrides(daemonModeFlag bool, daemonModeFlagNam } } -func (c *Config) ValidateConfig(log logrus.FieldLogger) error { +func (c *Config) ValidateConfig() error { if err := c.checkForUnknownConfig(); err != nil { return err } @@ -103,62 +95,6 @@ func (c *Config) ValidateConfig(log logrus.FieldLogger) error { return err } - if c.AgentAddressDeprecated != "" { - if c.AgentAddress != "" { - return errors.New("use of agent_address and agentAddress found, use only agent_address") - } - log.Warn(getWarning("agentAddress", "agent_address")) - c.AgentAddress = c.AgentAddressDeprecated - } - - if c.CmdArgsDeprecated != "" { - if c.CmdArgs != "" { - return errors.New("use of cmd_args and cmdArgs found, use only cmd_args") - } - log.Warn(getWarning("cmdArgs", "cmd_args")) - c.CmdArgs = c.CmdArgsDeprecated - } - - if c.CertDirDeprecated != "" { - if c.CertDir != "" { - return errors.New("use of cert_dir and certDir found, use only cert_dir") - } - log.Warn(getWarning("certDir", "cert_dir")) - c.CertDir = c.CertDirDeprecated - } - - if c.SVIDFileNameDeprecated != "" { - if c.SVIDFileName != "" { - return errors.New("use of svid_file_name and svidFileName found, use only svid_file_name") - } - log.Warn(getWarning("svidFileName", "svid_file_name")) - c.SVIDFileName = c.SVIDFileNameDeprecated - } - - if c.SVIDKeyFileNameDeprecated != "" { - if c.SVIDKeyFileName != "" { - return errors.New("use of svid_key_file_name and svidKeyFileName found, use only svid_key_file_name") - } - log.Warn(getWarning("svidKeyFileName", "svid_key_file_name")) - c.SVIDKeyFileName = c.SVIDKeyFileNameDeprecated - } - - if c.SVIDBundleFileNameDeprecated != "" { - if c.SVIDBundleFileName != "" { - return errors.New("use of svid_bundle_file_name and svidBundleFileName found, use only svid_bundle_file_name") - } - log.Warn(getWarning("svidBundleFileName", "svid_bundle_file_name")) - c.SVIDBundleFileName = c.SVIDBundleFileNameDeprecated - } - - if c.RenewSignalDeprecated != "" { - if c.RenewSignal != "" { - return errors.New("use of renew_signal and renewSignal found, use only renew_signal") - } - log.Warn(getWarning("renewSignal", "renew_signal")) - c.RenewSignal = c.RenewSignalDeprecated - } - for _, jwtConfig := range c.JWTSVIDs { if jwtConfig.JWTSVIDFilename == "" { return errors.New("'jwt_file_name' is required in 'jwt_svids'") @@ -275,10 +211,6 @@ func validateJWTConfig(c *Config) (bool, bool) { return jwtBundleEmptyCount == 0, len(c.JWTSVIDs) > 0 } -func getWarning(s1 string, s2 string) string { - return s1 + " will be deprecated, should be used as " + s2 -} - func countEmpty(configs ...string) int { cnt := 0 for _, config := range configs { diff --git a/cmd/spiffe-helper/config/config_test.go b/cmd/spiffe-helper/config/config_test.go index 6ac71f5b..43aadec3 100644 --- a/cmd/spiffe-helper/config/config_test.go +++ b/cmd/spiffe-helper/config/config_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "github.com/sirupsen/logrus" - "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -55,7 +53,6 @@ func TestValidateConfig(t *testing.T) { name string config *Config expectError string - expectLogs []shortEntry }{ { name: "no error", @@ -112,200 +109,9 @@ func TestValidateConfig(t *testing.T) { }, expectError: "'jwt_file_name' is required in 'jwt_svids'", }, - // Duplicated field error: - { - name: "Both agent_address & agentAddress in use", - config: &Config{ - AgentAddress: "path", - AgentAddressDeprecated: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectError: "use of agent_address and agentAddress found, use only agent_address", - }, - { - name: "Both cmd_args & cmdArgs in use", - config: &Config{ - AgentAddress: "path", - CmdArgs: "start_envoy.sh", - CmdArgsDeprecated: "start_envoy.sh", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectError: "use of cmd_args and cmdArgs found, use only cmd_args", - }, - { - name: "Both cert_dir & certDir in use", - config: &Config{ - AgentAddress: "path", - CertDir: "certs", - CertDirDeprecated: "certs", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectError: "use of cert_dir and certDir found, use only cert_dir", - }, - { - name: "Both svid_file_name & svidFileName in use", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDFileNameDeprecated: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectError: "use of svid_file_name and svidFileName found, use only svid_file_name", - }, - { - name: "Both svid_key_file_name & svidKeyFileName in use", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDKeyFileNameDeprecated: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectError: "use of svid_key_file_name and svidKeyFileName found, use only svid_key_file_name", - }, - { - name: "Both svid_bundle_file_name & svidBundleFileName in use", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - SVIDBundleFileNameDeprecated: "bundle.pem", - }, - expectError: "use of svid_bundle_file_name and svidBundleFileName found, use only svid_bundle_file_name", - }, - { - name: "Both renew_signal & renewSignal in use", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - RenewSignal: "SIGHUP", - RenewSignalDeprecated: "SIGHUP", - }, - expectError: "use of renew_signal and renewSignal found, use only renew_signal", - }, - // Deprecated field warning: - { - name: "Using AgentAddressDeprecated", - config: &Config{ - AgentAddressDeprecated: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "agentAddress will be deprecated, should be used as agent_address", - }, - }, - }, - { - name: "Using CmdArgsDeprecated", - config: &Config{ - AgentAddress: "path", - CmdArgsDeprecated: "start_envoy.sh", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "cmdArgs will be deprecated, should be used as cmd_args", - }, - }, - }, - { - name: "Using CertDirDeprecated", - config: &Config{ - AgentAddress: "path", - CertDirDeprecated: "certs", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "certDir will be deprecated, should be used as cert_dir", - }, - }, - }, - { - name: "Using SVIDFileNameDeprecated", - config: &Config{ - AgentAddress: "path", - SVIDFileNameDeprecated: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "svidFileName will be deprecated, should be used as svid_file_name", - }, - }, - }, - { - name: "Using SVIDKeyFileNameDeprecated", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileNameDeprecated: "key.pem", - SVIDBundleFileName: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "svidKeyFileName will be deprecated, should be used as svid_key_file_name", - }, - }, - }, - { - name: "Using SVIDBundleFileNameDeprecated", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileNameDeprecated: "bundle.pem", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "svidBundleFileName will be deprecated, should be used as svid_bundle_file_name", - }, - }, - }, - { - name: "Using RenewSignalDeprecated", - config: &Config{ - AgentAddress: "path", - SVIDFileName: "cert.pem", - SVIDKeyFileName: "key.pem", - SVIDBundleFileName: "bundle.pem", - RenewSignalDeprecated: "SIGHUP", - }, - expectLogs: []shortEntry{{ - Level: logrus.WarnLevel, - Message: "renewSignal will be deprecated, should be used as renew_signal", - }}, - }, } { t.Run(tt.name, func(t *testing.T) { - log, hook := test.NewNullLogger() - err := tt.config.ValidateConfig(log) - - require.ElementsMatch(t, tt.expectLogs, getShortEntries(hook.AllEntries())) + err := tt.config.ValidateConfig() if tt.expectError != "" { require.EqualError(t, err, tt.expectError) @@ -378,8 +184,7 @@ func TestDetectsUnknownConfig(t *testing.T) { c, err := ParseConfig(configFile.Name()) require.NoError(t, err) - log, _ := test.NewNullLogger() - err = c.ValidateConfig(log) + err = c.ValidateConfig() require.EqualError(t, err, tt.expectError) }) } @@ -421,8 +226,7 @@ func TestDefaultAgentAddress(t *testing.T) { SVIDKeyFileName: "key.pem", SVIDBundleFileName: "bundle.pem", } - log, _ := test.NewNullLogger() - err := config.ValidateConfig(log) + err := config.ValidateConfig() require.NoError(t, err) assert.Equal(t, config.AgentAddress, tt.expectedAgentAddress) }) @@ -482,19 +286,3 @@ func TestDaemonModeFlag(t *testing.T) { require.NotNil(t, config.DaemonMode) assert.Equal(t, false, *config.DaemonMode) } - -type shortEntry struct { - Level logrus.Level - Message string -} - -func getShortEntries(entries []*logrus.Entry) []shortEntry { - result := make([]shortEntry, 0, len(entries)) - for _, entry := range entries { - result = append(result, shortEntry{ - Level: entry.Level, - Message: entry.Message, - }) - } - return result -} diff --git a/cmd/spiffe-helper/main.go b/cmd/spiffe-helper/main.go index 295b67b8..14911764 100644 --- a/cmd/spiffe-helper/main.go +++ b/cmd/spiffe-helper/main.go @@ -43,7 +43,7 @@ func startSidecar(configFile string, daemonModeFlag bool, log logrus.FieldLogger } hclConfig.ParseConfigFlagOverrides(daemonModeFlag, daemonModeFlagName) - if err := hclConfig.ValidateConfig(log); err != nil { + if err := hclConfig.ValidateConfig(); err != nil { return fmt.Errorf("invalid configuration: %w", err) }