From 64cd172f30a8aa3099c0b3492d93ce30b8c35049 Mon Sep 17 00:00:00 2001 From: Melissa Kam <3768460+mkam@users.noreply.github.com> Date: Tue, 13 Feb 2024 14:06:18 -0600 Subject: [PATCH] [CC-7411] Fix environment variable precedence when linking to HCP (#20527) Fix so that link API values are used over env vars When a link is created via the API, those values should take precedence over the values set by environment variables. This change loads all the env vars initially as part of the config builder rather than on demand. --- agent/config/builder.go | 56 ++++++++++++++---- agent/config/builder_test.go | 106 +++++++++++++++++++++++++++++++++++ agent/config/runtime_test.go | 4 +- agent/hcp/config/config.go | 4 +- 4 files changed, 156 insertions(+), 14 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index 0ed6f0304f..883be2b748 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -4,6 +4,7 @@ package config import ( + "crypto/tls" "encoding/base64" "encoding/json" "errors" @@ -2581,11 +2582,37 @@ func validateAutoConfigAuthorizer(rt RuntimeConfig) error { } func (b *builder) cloudConfigVal(v Config) hcpconfig.CloudConfig { + // Load the same environment variables expected by hcp-sdk-go + envHostname, ok := os.LookupEnv("HCP_API_ADDRESS") + if !ok { + if legacyEnvHostname, ok := os.LookupEnv("HCP_API_HOST"); ok { + // Remove only https scheme prefixes from the deprecated environment + // variable for specifying the API host. Mirrors the same behavior as + // hcp-sdk-go. + if strings.HasPrefix(strings.ToLower(legacyEnvHostname), "https://") { + legacyEnvHostname = legacyEnvHostname[8:] + } + envHostname = legacyEnvHostname + } + } + + var envTLSConfig *tls.Config + if os.Getenv("HCP_AUTH_TLS") == "insecure" || + os.Getenv("HCP_SCADA_TLS") == "insecure" || + os.Getenv("HCP_API_TLS") == "insecure" { + envTLSConfig = &tls.Config{InsecureSkipVerify: true} + } + val := hcpconfig.CloudConfig{ ResourceID: os.Getenv("HCP_RESOURCE_ID"), ClientID: os.Getenv("HCP_CLIENT_ID"), ClientSecret: os.Getenv("HCP_CLIENT_SECRET"), + AuthURL: os.Getenv("HCP_AUTH_URL"), + Hostname: envHostname, + ScadaAddress: os.Getenv("HCP_SCADA_ADDRESS"), + TLSConfig: envTLSConfig, } + // Node id might get overridden in setup.go:142 nodeID := stringVal(v.NodeID) val.NodeID = types.NodeID(nodeID) @@ -2595,20 +2622,29 @@ func (b *builder) cloudConfigVal(v Config) hcpconfig.CloudConfig { return val } - val.AuthURL = stringVal(v.Cloud.AuthURL) - val.Hostname = stringVal(v.Cloud.Hostname) - val.ScadaAddress = stringVal(v.Cloud.ScadaAddress) - - if resourceID := stringVal(v.Cloud.ResourceID); resourceID != "" { - val.ResourceID = resourceID + // Load configuration file variables for anything not set by environment variables + if val.AuthURL == "" { + val.AuthURL = stringVal(v.Cloud.AuthURL) } - if clientID := stringVal(v.Cloud.ClientID); clientID != "" { - val.ClientID = clientID + if val.Hostname == "" { + val.Hostname = stringVal(v.Cloud.Hostname) } - if clientSecret := stringVal(v.Cloud.ClientSecret); clientSecret != "" { - val.ClientSecret = clientSecret + if val.ScadaAddress == "" { + val.ScadaAddress = stringVal(v.Cloud.ScadaAddress) + } + + if val.ResourceID == "" { + val.ResourceID = stringVal(v.Cloud.ResourceID) + } + + if val.ClientID == "" { + val.ClientID = stringVal(v.Cloud.ClientID) + } + + if val.ClientSecret == "" { + val.ClientSecret = stringVal(v.Cloud.ClientSecret) } return val diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 9a3ed53743..b1a8ad8c5a 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + hcpconfig "github.com/hashicorp/consul/agent/hcp/config" "github.com/hashicorp/consul/types" ) @@ -707,3 +708,108 @@ func TestBuilder_WarnCloudConfigWithResourceApis(t *testing.T) { } } } + +func TestBuilder_CloudConfigWithEnvironmentVars(t *testing.T) { + tests := map[string]struct { + hcl string + env map[string]string + expected hcpconfig.CloudConfig + }{ + "ConfigurationOnly": { + hcl: `cloud{ resource_id = "config-resource-id" client_id = "config-client-id" + client_secret = "config-client-secret" auth_url = "auth.config.com" + hostname = "api.config.com" scada_address = "scada.config.com"}`, + expected: hcpconfig.CloudConfig{ + ResourceID: "config-resource-id", + ClientID: "config-client-id", + ClientSecret: "config-client-secret", + AuthURL: "auth.config.com", + Hostname: "api.config.com", + ScadaAddress: "scada.config.com", + }, + }, + "EnvVarsOnly": { + env: map[string]string{ + "HCP_RESOURCE_ID": "env-resource-id", + "HCP_CLIENT_ID": "env-client-id", + "HCP_CLIENT_SECRET": "env-client-secret", + "HCP_AUTH_URL": "auth.env.com", + "HCP_API_ADDRESS": "api.env.com", + "HCP_SCADA_ADDRESS": "scada.env.com", + }, + expected: hcpconfig.CloudConfig{ + ResourceID: "env-resource-id", + ClientID: "env-client-id", + ClientSecret: "env-client-secret", + AuthURL: "auth.env.com", + Hostname: "api.env.com", + ScadaAddress: "scada.env.com", + }, + }, + "EnvVarsOverrideConfig": { + hcl: `cloud{ resource_id = "config-resource-id" client_id = "config-client-id" + client_secret = "config-client-secret" auth_url = "auth.config.com" + hostname = "api.config.com" scada_address = "scada.config.com"}`, + env: map[string]string{ + "HCP_RESOURCE_ID": "env-resource-id", + "HCP_CLIENT_ID": "env-client-id", + "HCP_CLIENT_SECRET": "env-client-secret", + "HCP_AUTH_URL": "auth.env.com", + "HCP_API_ADDRESS": "api.env.com", + "HCP_SCADA_ADDRESS": "scada.env.com", + }, + expected: hcpconfig.CloudConfig{ + ResourceID: "env-resource-id", + ClientID: "env-client-id", + ClientSecret: "env-client-secret", + AuthURL: "auth.env.com", + Hostname: "api.env.com", + ScadaAddress: "scada.env.com", + }, + }, + "Combination": { + hcl: `cloud{ resource_id = "config-resource-id" client_id = "config-client-id" + client_secret = "config-client-secret"}`, + env: map[string]string{ + "HCP_AUTH_URL": "auth.env.com", + "HCP_API_ADDRESS": "api.env.com", + "HCP_SCADA_ADDRESS": "scada.env.com", + }, + expected: hcpconfig.CloudConfig{ + ResourceID: "config-resource-id", + ClientID: "config-client-id", + ClientSecret: "config-client-secret", + AuthURL: "auth.env.com", + Hostname: "api.env.com", + ScadaAddress: "scada.env.com", + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + for k, v := range tc.env { + t.Setenv(k, v) + } + devMode := true + builderOpts := LoadOpts{ + DevMode: &devMode, + Overrides: []Source{ + FileSource{ + Name: "overrides", + Format: "hcl", + Data: tc.hcl, + }, + }, + } + loaded, err := Load(builderOpts) + require.NoError(t, err) + + nodeName, err := os.Hostname() + require.NoError(t, err) + tc.expected.NodeName = nodeName + + actual := loaded.RuntimeConfig.Cloud + require.Equal(t, tc.expected, actual) + }) + } +} diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 4de84cb658..bc2222739c 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2329,7 +2329,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { expected: func(rt *RuntimeConfig) { rt.DataDir = dataDir rt.Cloud = hcpconfig.CloudConfig{ - // ID is only populated from env if not populated from other sources. ResourceID: "env-id", NodeName: "thehostname", NodeID: "", @@ -2371,8 +2370,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { expected: func(rt *RuntimeConfig) { rt.DataDir = dataDir rt.Cloud = hcpconfig.CloudConfig{ - // ID is only populated from env if not populated from other sources. - ResourceID: "file-id", + ResourceID: "env-id", NodeName: "thehostname", } diff --git a/agent/hcp/config/config.go b/agent/hcp/config/config.go index e91bfe079d..420af129ad 100644 --- a/agent/hcp/config/config.go +++ b/agent/hcp/config/config.go @@ -46,6 +46,8 @@ func (c *CloudConfig) Resource() (resource.Resource, error) { return resource.FromString(c.ResourceID) } +// HCPConfig returns a configuration to use with the HCP SDK. It assumes that the environment +// variables for the HCP configuration have already been loaded and set in the CloudConfig. func (c *CloudConfig) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error) { if c.TLSConfig == nil { c.TLSConfig = &tls.Config{} @@ -62,7 +64,7 @@ func (c *CloudConfig) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfi if c.ScadaAddress != "" { opts = append(opts, hcpcfg.WithSCADA(c.ScadaAddress, c.TLSConfig)) } - opts = append(opts, hcpcfg.FromEnv(), hcpcfg.WithoutBrowserLogin()) + opts = append(opts, hcpcfg.WithoutBrowserLogin()) return hcpcfg.NewHCPConfig(opts...) }