From a1f771afcc59392229903adea339defe9323f6a7 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 26 Sep 2022 18:28:27 -0400 Subject: [PATCH] improve: add backwards compat for connector access key This commit adds support for the deprecate env: and file: prefix for secrets in the connector. It should allow users to upgrade without changing their config, and print a warning when they are using a deprecated setting. --- .../infra/templates/connector/configmap.yaml | 2 +- internal/cmd/cmd.go | 18 +- internal/cmd/cmd_test.go | 168 +++++++++++++++--- 3 files changed, 158 insertions(+), 30 deletions(-) diff --git a/helm/charts/infra/templates/connector/configmap.yaml b/helm/charts/infra/templates/connector/configmap.yaml index 8154ae326b..60c0376413 100644 --- a/helm/charts/infra/templates/connector/configmap.yaml +++ b/helm/charts/infra/templates/connector/configmap.yaml @@ -22,7 +22,7 @@ data: {{- if and $accessKey (or (hasPrefix "file:" $accessKey) (hasPrefix "env:" $accessKey)) }} accessKey: {{ $accessKey }} {{- else }} - accessKey: file:/var/run/secrets/infrahq.com/access-key/access-key + accessKey: /var/run/secrets/infrahq.com/access-key/access-key {{- end }} {{- with .Values.connector.config.serverTrustedCertificate }} diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index a56a567637..3a047cfcff 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -295,9 +295,25 @@ func newConnectorCmd() *cobra.Command { return err } + // backwards compat for old access key values with a prefix + switch { + case strings.HasPrefix(options.Server.AccessKey.String(), "file:"): + filename := strings.TrimPrefix(options.Server.AccessKey.String(), "file:") + if err := options.Server.AccessKey.Set(filename); err != nil { + return err + } + logging.L.Warn().Msg("accessKey with 'file:' prefix is deprecated. Use the filename without the file: prefix instead.") + case strings.HasPrefix(options.Server.AccessKey.String(), "env:"): + key := strings.TrimPrefix(options.Server.AccessKey.String(), "env:") + options.Server.AccessKey = types.StringOrFile(os.Getenv(key)) + logging.L.Warn().Msg("accessKey with 'env:' prefix is deprecated. Use the INFRA_ACCESS_KEY env var instead.") + } + // Also accept the same env var as the CLI for setting the access key if accessKey, ok := os.LookupEnv("INFRA_ACCESS_KEY"); ok { - options.Server.AccessKey = types.StringOrFile(accessKey) + if err := options.Server.AccessKey.Set(accessKey); err != nil { + return err + } } return runConnector(cmd.Context(), options) }, diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 7138b3120a..53fcffda55 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -359,14 +359,40 @@ func TestInvalidSessions(t *testing.T) { }) } -func TestConnectorCmd(t *testing.T) { - var actual connector.Options - patchRunConnector(t, func(ctx context.Context, options connector.Options) error { - actual = options - return nil - }) +func TestConnectorCmd_LoadConfig(t *testing.T) { + type testCase struct { + name string + config string + setup func(t *testing.T) + expected func() connector.Options + } + + run := func(t *testing.T, tc testCase) { + var actual connector.Options + patchRunConnector(t, func(ctx context.Context, options connector.Options) error { + actual = options + return nil + }) + + if tc.setup != nil { + tc.setup(t) + } + + dir := fs.NewDir(t, t.Name(), fs.WithFile("config.yaml", tc.config)) + + ctx := context.Background() + err := Run(ctx, "connector", "-f", dir.Join("config.yaml")) + assert.NilError(t, err) + assert.DeepEqual(t, actual, tc.expected()) + } - content := ` + keyDir := fs.NewDir(t, t.Name(), fs.WithFile("accesskeyfile", "the-access-key")) + filename := keyDir.Join("accesskeyfile") + + testCases := []testCase{ + { + name: "full config", + config: ` server: url: the-server accessKey: /var/run/secrets/key @@ -375,30 +401,116 @@ server: name: the-name caCert: /path/to/cert caKey: /path/to/key -` - - dir := fs.NewDir(t, t.Name(), fs.WithFile("config.yaml", content)) - - ctx := context.Background() - err := Run(ctx, "connector", "-f", dir.Join("config.yaml")) - assert.NilError(t, err) - - expected := connector.Options{ - Name: "the-name", - Addr: connector.ListenerOptions{ - HTTPS: ":443", - Metrics: ":9090", +addr: + https: localhost:414 + metrics: 127.0.0.1:8000 +`, + expected: func() connector.Options { + return connector.Options{ + Name: "the-name", + Addr: connector.ListenerOptions{ + HTTPS: "localhost:414", + Metrics: "127.0.0.1:8000", + }, + Server: connector.ServerOptions{ + URL: "the-server", + AccessKey: "/var/run/secrets/key", + SkipTLSVerify: true, + TrustedCertificate: "ca.pem", + }, + CACert: "/path/to/cert", + CAKey: "/path/to/key", + } + }, + }, + { + name: "access key with file: prefix (deprecated)", + config: fmt.Sprintf("server:\n accessKey: file:%v\n", filename), + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-access-key" + return expected + }, + }, + { + name: "access key from file", + config: fmt.Sprintf("server:\n accessKey: %v\n", filename), + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-access-key" + return expected + }, + }, + { + name: "access key with env: prefix (deprecated)", + setup: func(t *testing.T) { + t.Setenv("CUSTOM_ENV_VAR", "the-key-from-env") + }, + config: ` +server: + accessKey: env:CUSTOM_ENV_VAR +`, + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-key-from-env" + return expected + }, + }, + { + name: "access key from INFRA_ACCESS_KEY", + setup: func(t *testing.T) { + t.Setenv("INFRA_ACCESS_KEY", "the-key-from-env") + }, + config: `{}`, + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-key-from-env" + return expected + }, }, - Server: connector.ServerOptions{ - URL: "the-server", - AccessKey: "/var/run/secrets/key", - SkipTLSVerify: true, - TrustedCertificate: "ca.pem", + { + name: "access key from INFRA_ACCESS_KEY points at a file", + setup: func(t *testing.T) { + t.Setenv("INFRA_ACCESS_KEY", filename) + }, + config: `{}`, + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-access-key" + return expected + }, }, - CACert: "/path/to/cert", - CAKey: "/path/to/key", + { + name: "access key from INFRA_CONNECTOR_SERVER_ACCESS_KEY", + config: `{}`, + setup: func(t *testing.T) { + t.Setenv("INFRA_CONNECTOR_SERVER_ACCESS_KEY", "the-key-from-env") + }, + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-key-from-env" + return expected + }, + }, + { + name: "access key literal from file", + config: ` +server: + accessKey: the-literal-key +`, + expected: func() connector.Options { + expected := defaultConnectorOptions() + expected.Server.AccessKey = "the-literal-key" + return expected + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + run(t, tc) + }) } - assert.DeepEqual(t, actual, expected) } func patchRunConnector(t *testing.T, fn func(context.Context, connector.Options) error) {