From 9163afe0cd6d0ebaeae486977a0753f26e18bf5d Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Fri, 2 Feb 2024 13:12:12 -0800 Subject: [PATCH] Backport of missing prefix / into release/1.18.x (#20459) * backport of commit b76447fb80bffc4b09a33772799def978cbe0860 * backport of commit 395984c444eb8e23b47e9965f83bd1f4972aebde * backport of commit cc1246d8bacca82b5c30de0649662d0d1c60abc4 * backport of commit 43170a5a701df2fad8b28253e810562f720f0e27 --------- Co-authored-by: Xinyi Wang --- agent/grpc-external/server.go | 2 +- command/resource/client/client.go | 2 +- command/resource/client/grpc-flags.go | 2 +- .../consul-container/libs/cluster/agent.go | 1 + .../libs/cluster/container.go | 4 + .../test/resource/grpc_forwarding_test.go | 136 ++++++++++++++++++ 6 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 test/integration/consul-container/test/resource/grpc_forwarding_test.go diff --git a/agent/grpc-external/server.go b/agent/grpc-external/server.go index ea26301af3..8e3928eb7d 100644 --- a/agent/grpc-external/server.go +++ b/agent/grpc-external/server.go @@ -26,7 +26,7 @@ import ( "github.com/hashicorp/consul/tlsutil" ) -const FORWARD_SERVICE_NAME_PREFIX = "hashicorp.consul." +const FORWARD_SERVICE_NAME_PREFIX = "/hashicorp.consul." var ( metricsLabels = []metrics.Label{{ diff --git a/command/resource/client/client.go b/command/resource/client/client.go index 909e0a0e2c..c841e7b0f4 100644 --- a/command/resource/client/client.go +++ b/command/resource/client/client.go @@ -160,7 +160,7 @@ func dial(c *GRPCConfig) (*grpc.ClientConn, error) { func checkCertificates(c *GRPCConfig) error { if c.GRPCTLS { certFileEmpty := c.CertFile == "" - keyFileEmpty := c.CertFile == "" + keyFileEmpty := c.KeyFile == "" // both files need to be empty or both files need to be provided if certFileEmpty != keyFileEmpty { diff --git a/command/resource/client/grpc-flags.go b/command/resource/client/grpc-flags.go index 65720ae293..252b3dd516 100644 --- a/command/resource/client/grpc-flags.go +++ b/command/resource/client/grpc-flags.go @@ -52,7 +52,7 @@ func (f *GRPCFlags) ClientFlags() *flag.FlagSet { "127.0.0.1:8502. If you intend to communicate in TLS mode, you have to either "+ "include https:// schema in the address, use grpc-tls flag or set environment variable "+ "CONSUL_GRPC_TLS = true, otherwise it uses plaintext mode") - fs.Var(&f.caFile, "grpc-tls", + fs.Var(&f.grpcTLS, "grpc-tls", "Set to true if you aim to communicate in TLS mode in the GRPC call.") fs.Var(&f.certFile, "client-cert", "Path to a client cert file to use for TLS when 'verify_incoming' is enabled. This "+ diff --git a/test/integration/consul-container/libs/cluster/agent.go b/test/integration/consul-container/libs/cluster/agent.go index a6dcb54674..e44f588c46 100644 --- a/test/integration/consul-container/libs/cluster/agent.go +++ b/test/integration/consul-container/libs/cluster/agent.go @@ -29,6 +29,7 @@ type Agent interface { GetAgentName() string GetPartition() string GetPod() testcontainers.Container + GetConsulContainer() testcontainers.Container Logs(context.Context) (io.ReadCloser, error) ClaimAdminPort() (int, error) GetConfig() Config diff --git a/test/integration/consul-container/libs/cluster/container.go b/test/integration/consul-container/libs/cluster/container.go index eeddd1fe56..9c485a989a 100644 --- a/test/integration/consul-container/libs/cluster/container.go +++ b/test/integration/consul-container/libs/cluster/container.go @@ -77,6 +77,10 @@ func (c *consulContainerNode) GetPod() testcontainers.Container { return c.pod } +func (c *consulContainerNode) GetConsulContainer() testcontainers.Container { + return c.container +} + func (c *consulContainerNode) Logs(context context.Context) (io.ReadCloser, error) { return c.container.Logs(context) } diff --git a/test/integration/consul-container/test/resource/grpc_forwarding_test.go b/test/integration/consul-container/test/resource/grpc_forwarding_test.go new file mode 100644 index 0000000000..a348c976b8 --- /dev/null +++ b/test/integration/consul-container/test/resource/grpc_forwarding_test.go @@ -0,0 +1,136 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package resource + +import ( + "context" + "fmt" + "io" + "testing" + + "github.com/stretchr/testify/require" + + libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster" + libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology" +) + +const ( + RESOURCE_FILE_PATH_ON_HOST = "../../../../../command/resource/testdata/demo.hcl" + RESOURCE_FILE_PATH_ON_CONTAINER = "/consul/data/demo.hcl" +) + +func TestClientForwardToServer(t *testing.T) { + type operation struct { + action func(*testing.T, libcluster.Agent, string) (int, string) + includeToken bool + expectedCode int + expectedMsg string + } + type testCase struct { + description string + operations []operation + aclEnabled bool + } + + testCases := []testCase{ + { + description: "The apply request should be forwarded to consul server agent", + operations: []operation{ + { + action: applyResource, + includeToken: false, + expectedCode: 0, + expectedMsg: "demo.v2.Artist 'korn' created.", + }, + }, + aclEnabled: false, + }, + { + description: "The apply request should be denied if missing token when ACL is enabled", + operations: []operation{ + { + action: applyResource, + includeToken: false, + expectedCode: 1, + expectedMsg: "failed getting authorizer: ACL not found", + }, + }, + aclEnabled: true, + }, + { + description: "The apply request should be allowed if providing token when ACL is enabled", + operations: []operation{ + { + action: applyResource, + includeToken: true, + expectedCode: 0, + expectedMsg: "demo.v2.Artist 'korn' created.", + }, + }, + aclEnabled: true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.description, func(t *testing.T) { + t.Parallel() + + var clientAgent libcluster.Agent + cluster, clientAgent := setupClusterAndClient(t, tc.aclEnabled) + defer terminate(t, cluster) + + // perform actions and validate returned messages + for _, op := range tc.operations { + token := "" + if op.includeToken { + token = cluster.TokenBootstrap + } + code, res := op.action(t, clientAgent, token) + require.Equal(t, op.expectedCode, code) + require.Contains(t, res, op.expectedMsg) + } + }) + } +} + +func applyResource(t *testing.T, clientAgent libcluster.Agent, token string) (int, string) { + ctx := context.Background() + c := clientAgent.GetConsulContainer() + err := c.CopyFileToContainer(ctx, RESOURCE_FILE_PATH_ON_HOST, RESOURCE_FILE_PATH_ON_CONTAINER, 700) + require.NoError(t, err) + args := []string{"/bin/consul", "resource", "apply", fmt.Sprintf("-f=%s", RESOURCE_FILE_PATH_ON_CONTAINER)} + if token != "" { + args = append(args, fmt.Sprintf("-token=%s", token)) + } + code, reader, err := c.Exec(ctx, args) + require.NoError(t, err) + buf, err := io.ReadAll(reader) + require.NoError(t, err) + return code, string(buf) +} + +// passing two cmd args to set up the cluster +func setupClusterAndClient(t *testing.T, aclEnabled bool) (*libcluster.Cluster, libcluster.Agent) { + clusterConfig := &libtopology.ClusterConfig{ + NumServers: 1, + NumClients: 1, + LogConsumer: &libtopology.TestLogConsumer{}, + BuildOpts: &libcluster.BuildOptions{ + Datacenter: "dc1", + InjectAutoEncryption: true, + InjectGossipEncryption: true, + ACLEnabled: aclEnabled, + }, + ApplyDefaultProxySettings: false, + } + cluster, _, _ := libtopology.NewCluster(t, clusterConfig) + + return cluster, cluster.Clients()[0] +} + +func terminate(t *testing.T, cluster *libcluster.Cluster) { + err := cluster.Terminate() + require.NoError(t, err) +}