From 52199d52fbf5988a823f03d5f7e664706fbc47ef Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Wed, 16 Nov 2022 19:59:34 -0500 Subject: [PATCH 1/4] Add fallback to no gRPC --- internal/commands/exec/command.go | 2 + internal/commands/exec/exec_test.go | 1 + internal/commands/server/command.go | 8 +++- internal/commands/server/server.go | 13 +++-- internal/consul/connection.go | 73 +++++++++++++++++++++++++++++ internal/k8s/builder/gateway.go | 4 ++ 6 files changed, 97 insertions(+), 4 deletions(-) diff --git a/internal/commands/exec/command.go b/internal/commands/exec/command.go index 95f3cf996..5a09e69b4 100644 --- a/internal/commands/exec/command.go +++ b/internal/commands/exec/command.go @@ -174,7 +174,9 @@ func (c *Command) Run(args []string) (ret int) { } consulClientConfig := consul.ClientConfig{ + Name: c.flagGatewayName, ApiClientConfig: cfg, + UseDynamic: os.Getenv("CONSUL_DYNAMIC_SERVER_DISCOVERY") == "true", Addresses: c.flagConsulHTTPAddress, HTTPPort: c.flagConsulHTTPPort, GRPCPort: c.flagConsulXDSPort, diff --git a/internal/commands/exec/exec_test.go b/internal/commands/exec/exec_test.go index 1d9efc315..c5ace2122 100644 --- a/internal/commands/exec/exec_test.go +++ b/internal/commands/exec/exec_test.go @@ -468,6 +468,7 @@ func runMockConsulServer(t *testing.T, opts mockConsulOptions) *mockConsulServer }) server.clientConfig = consul.ClientConfig{ + UseDynamic: true, Addresses: serverURL.Hostname(), ApiClientConfig: clientConfig, GRPCPort: grpcPort, diff --git a/internal/commands/server/command.go b/internal/commands/server/command.go index d9d4b0114..2c68f59fd 100644 --- a/internal/commands/server/command.go +++ b/internal/commands/server/command.go @@ -172,11 +172,15 @@ func (c *Command) Run(args []string) int { MirrorKubernetesNamespacePrefix: c.flagMirrorK8SNamespacePrefix, } - consulHTTPAddressOrCommand, port, err := parseConsulHTTPAddress() + consulScheme, consulHTTPAddressOrCommand, port, err := parseConsulHTTPAddress() if err != nil { logger.Error("error reading "+consulHTTPAddressEnvName, "error", err) return 1 } + if consulCfg.Scheme != "https" { + // override only if it needs to be explicitly marked + consulCfg.Scheme = consulScheme + } tlsCfg, err := api.SetupTLSConfig(&consulCfg.TLSConfig) if err != nil { @@ -201,8 +205,10 @@ func (c *Command) Run(args []string) int { } consulClientConfig := consul.ClientConfig{ + Name: "api-gateway-controller", ApiClientConfig: consulCfg, Addresses: consulHTTPAddressOrCommand, + UseDynamic: os.Getenv("CONSUL_DYNAMIC_SERVER_DISCOVERY") == "true", HTTPPort: port, GRPCPort: grpcPort(), PlainText: consulCfg.Scheme == "http", diff --git a/internal/commands/server/server.go b/internal/commands/server/server.go index 78a8a55f1..3c464f38d 100644 --- a/internal/commands/server/server.go +++ b/internal/commands/server/server.go @@ -167,14 +167,21 @@ func registerSecretClients(config ServerConfig) (*envoy.MultiSecretClient, error return secretClient, nil } -func parseConsulHTTPAddress() (cmd string, port int, err error) { +func parseConsulHTTPAddress() (scheme string, cmd string, port int, err error) { consulhttpAddress := os.Getenv(consulHTTPAddressEnvName) + scheme = "http" + if os.Getenv("CONSUL_HTTP_SSL") == "true" || strings.HasPrefix(consulhttpAddress, "https://") { + scheme = "https" + } + // (?:http|https|ftp)://) + consulhttpAddress = strings.TrimPrefix(consulhttpAddress, "http://") + consulhttpAddress = strings.TrimPrefix(consulhttpAddress, "https://") index := strings.LastIndex(consulhttpAddress, ":") cmd, portString := consulhttpAddress[:index], consulhttpAddress[index+1:] port, err = strconv.Atoi(portString) if err != nil { - return "", 0, err + return "", "", 0, err } - return cmd, port, nil + return scheme, cmd, port, nil } diff --git a/internal/consul/connection.go b/internal/consul/connection.go index 60d6d35bc..a1abb1eca 100644 --- a/internal/consul/connection.go +++ b/internal/consul/connection.go @@ -43,7 +43,9 @@ type Client interface { } type ClientConfig struct { + Name string ApiClientConfig *api.Config + UseDynamic bool PlainText bool Addresses string HTTPPort int @@ -81,6 +83,46 @@ func (c *client) Wait(until time.Duration) error { } func (c *client) WatchServers(ctx context.Context) error { + if !c.config.UseDynamic { + cfg := c.config.ApiClientConfig + cfg.Address = fmt.Sprintf("%s:%d", c.config.Addresses, c.config.HTTPPort) + + var err error + var client *api.Client + var token string + if c.config.Credentials.Type == discovery.CredentialsTypeLogin { + client, err = api.NewClient(cfg) + if err != nil { + c.initialized <- err + return err + } + client, token, err = login(ctx, client, cfg, c.config) + if err != nil { + c.initialized <- err + return err + } + defer logout(client, token, c.config) + } else { + // this might be empty + cfg.Token = c.config.Credentials.Static.Token + client, err = api.NewClient(cfg) + if err != nil { + c.initialized <- err + return err + } + } + + c.mutex.Lock() + c.client = client + c.token = cfg.Token + c.mutex.Unlock() + + close(c.initialized) + + <-ctx.Done() + return nil + } + ctx, cancel := context.WithCancel(ctx) c.stop = cancel @@ -220,3 +262,34 @@ func (c *client) Internal() *api.Client { return c.client } + +func login(ctx context.Context, client *api.Client, cfg *api.Config, config ClientConfig) (*api.Client, string, error) { + authenticator := NewAuthenticator( + config.Logger.Named("authenticator"), + client, + config.Credentials.Login.AuthMethod, + config.Credentials.Login.Namespace, + ) + + token, err := authenticator.Authenticate(ctx, config.Name, config.Credentials.Login.BearerToken) + if err != nil { + return nil, "", fmt.Errorf("error logging in to consul: %w", err) + } + + // Now update the client so that it will read the ACL token we just fetched. + cfg.Token = token + newClient, err := api.NewClient(cfg) + if err != nil { + return nil, "", fmt.Errorf("error updating client connection with token: %w", err) + } + return newClient, token, nil +} + +func logout(client *api.Client, token string, config ClientConfig) error { + config.Logger.Info("deleting acl token") + _, err := client.ACL().Logout(&api.WriteOptions{Token: token}) + if err != nil { + return fmt.Errorf("error deleting acl token: %w", err) + } + return nil +} diff --git a/internal/k8s/builder/gateway.go b/internal/k8s/builder/gateway.go index 1b370f5fd..f2ce100f5 100644 --- a/internal/k8s/builder/gateway.go +++ b/internal/k8s/builder/gateway.go @@ -296,6 +296,10 @@ func (b *GatewayDeploymentBuilder) envVars() []corev1.EnvVar { Name: "CONSUL_LOGIN_DATACENTER", Value: os.Getenv("CONSUL_LOGIN_DATACENTER"), }, + { + Name: "CONSUL_DYNAMIC_SERVER_DISCOVERY", + Value: os.Getenv("CONSUL_DYNAMIC_SERVER_DISCOVERY"), + }, { Name: "CONSUL_PARTITION", Value: orDefault(b.gwConfig.Spec.ConsulSpec.Partition, defaultPartition), From 904eb246fc60b83c7facc8ca8387396811f0ede2 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Wed, 16 Nov 2022 20:23:39 -0500 Subject: [PATCH 2/4] fix tests --- internal/commands/exec/command_test.go | 2 ++ internal/k8s/builder/testdata/clusterip.deployment.golden.yaml | 1 + .../k8s/builder/testdata/loadbalancer.deployment.golden.yaml | 1 + .../k8s/builder/testdata/max-instances.deployment.golden.yaml | 1 + .../k8s/builder/testdata/min-instances.deployment.golden.yaml | 1 + .../builder/testdata/multiple-instances.deployment.golden.yaml | 1 + .../k8s/builder/testdata/static-mapping.deployment.golden.yaml | 1 + internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml | 1 + 8 files changed, 9 insertions(+) diff --git a/internal/commands/exec/command_test.go b/internal/commands/exec/command_test.go index e3ba2751c..5db56f6d4 100644 --- a/internal/commands/exec/command_test.go +++ b/internal/commands/exec/command_test.go @@ -2,6 +2,7 @@ package exec import ( "context" + "os" "testing" "github.com/mitchellh/cli" @@ -124,6 +125,7 @@ func TestExec(t *testing.T) { output: "did not get state within time limit", }} { t.Run(test.name, func(t *testing.T) { + os.Setenv("CONSUL_DYNAMIC_SERVER_DISCOVERY", "true") ctx := context.Background() ui := cli.NewMockUi() var buffer gwTesting.Buffer diff --git a/internal/k8s/builder/testdata/clusterip.deployment.golden.yaml b/internal/k8s/builder/testdata/clusterip.deployment.golden.yaml index 1d266066e..f64229150 100644 --- a/internal/k8s/builder/testdata/clusterip.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/clusterip.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/loadbalancer.deployment.golden.yaml b/internal/k8s/builder/testdata/loadbalancer.deployment.golden.yaml index ddf698f88..00b207eb9 100644 --- a/internal/k8s/builder/testdata/loadbalancer.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/loadbalancer.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/max-instances.deployment.golden.yaml b/internal/k8s/builder/testdata/max-instances.deployment.golden.yaml index 7f9fd8a94..13bb01c35 100644 --- a/internal/k8s/builder/testdata/max-instances.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/max-instances.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/min-instances.deployment.golden.yaml b/internal/k8s/builder/testdata/min-instances.deployment.golden.yaml index 619bd0dac..19548b1e9 100644 --- a/internal/k8s/builder/testdata/min-instances.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/min-instances.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/multiple-instances.deployment.golden.yaml b/internal/k8s/builder/testdata/multiple-instances.deployment.golden.yaml index 48af32142..745758ca4 100644 --- a/internal/k8s/builder/testdata/multiple-instances.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/multiple-instances.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/static-mapping.deployment.golden.yaml b/internal/k8s/builder/testdata/static-mapping.deployment.golden.yaml index 63e69d766..fb1007875 100644 --- a/internal/k8s/builder/testdata/static-mapping.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/static-mapping.deployment.golden.yaml @@ -77,6 +77,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH diff --git a/internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml b/internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml index 42a0b43e4..38464884c 100644 --- a/internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml +++ b/internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml @@ -75,6 +75,7 @@ spec: fieldPath: status.hostIP - name: CONSUL_LOGIN_PARTITION - name: CONSUL_LOGIN_DATACENTER + - name: CONSUL_DYNAMIC_SERVER_DISCOVERY - name: CONSUL_PARTITION - name: CONSUL_TLS_SERVER_NAME - name: PATH From 448607dc8cfe29ff3dee03e80c9c67ef17f78f9b Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Wed, 16 Nov 2022 20:17:26 -0600 Subject: [PATCH 3/4] update to use main branch for 1.14 --- .github/workflows/conformance_with_build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/conformance_with_build.yml b/.github/workflows/conformance_with_build.yml index 2b32897b0..0b134bd13 100644 --- a/.github/workflows/conformance_with_build.yml +++ b/.github/workflows/conformance_with_build.yml @@ -50,7 +50,7 @@ jobs: api-gateway-image: "hashicorppreview/consul-api-gateway:0.4-dev" consul-image: "hashicorppreview/consul:1.14-dev" envoy-image: "envoyproxy/envoy:v1.22-latest" - consul-k8s-version: "v0.49.0" + consul-k8s-version: "main" fail-fast: true name: "${{ matrix.config.name }}" From 95747e051843185a12f4fce39c7462d81518b01c Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Wed, 16 Nov 2022 22:15:42 -0500 Subject: [PATCH 4/4] Fix up namespae setting in exec mode --- internal/commands/exec/command.go | 5 +---- internal/consul/connection.go | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/internal/commands/exec/command.go b/internal/commands/exec/command.go index 5a09e69b4..abb1dd01c 100644 --- a/internal/commands/exec/command.go +++ b/internal/commands/exec/command.go @@ -169,12 +169,9 @@ func (c *Command) Run(args []string) (ret int) { bearerToken = strings.TrimSpace(string(data)) } - if c.flagGatewayNamespace != "" { - cfg.Namespace = c.flagGatewayNamespace - } - consulClientConfig := consul.ClientConfig{ Name: c.flagGatewayName, + Namespace: c.flagGatewayNamespace, ApiClientConfig: cfg, UseDynamic: os.Getenv("CONSUL_DYNAMIC_SERVER_DISCOVERY") == "true", Addresses: c.flagConsulHTTPAddress, diff --git a/internal/consul/connection.go b/internal/consul/connection.go index a1abb1eca..02864dca3 100644 --- a/internal/consul/connection.go +++ b/internal/consul/connection.go @@ -44,6 +44,7 @@ type Client interface { type ClientConfig struct { Name string + Namespace string ApiClientConfig *api.Config UseDynamic bool PlainText bool @@ -91,20 +92,27 @@ func (c *client) WatchServers(ctx context.Context) error { var client *api.Client var token string if c.config.Credentials.Type == discovery.CredentialsTypeLogin { - client, err = api.NewClient(cfg) + baseClient, err := api.NewClient(cfg) if err != nil { c.initialized <- err return err } - client, token, err = login(ctx, client, cfg, c.config) + if c.config.Namespace != "" { + cfg.Namespace = c.config.Namespace + } + client, token, err = login(ctx, baseClient, cfg, c.config) if err != nil { c.initialized <- err return err } - defer logout(client, token, c.config) + defer logout(baseClient, token, c.config) + } else { // this might be empty cfg.Token = c.config.Credentials.Static.Token + if c.config.Namespace != "" { + cfg.Namespace = c.config.Namespace + } client, err = api.NewClient(cfg) if err != nil { c.initialized <- err @@ -166,6 +174,9 @@ func (c *client) WatchServers(ctx context.Context) error { } updateClient := func(s discovery.State) error { cfg := c.config.ApiClientConfig + if c.config.Namespace != "" { + cfg.Namespace = c.config.Namespace + } cfg.Address = fmt.Sprintf("%s:%d", s.Address.IP.String(), c.config.HTTPPort) if static { // This is to fix the fact that s.Address always resolves to an IP, if