Merge branch 'main' into jm/3372

hashicorp · Mar 24, 2023 · 0b4359a · 0b4359a
2 parents 9bcd03d + 2236975
commit 0b4359a
Show file tree

Hide file tree

Showing 54 changed files with 163 additions and 290 deletions.
diff --git a/.changelog/_4832.txt b/.changelog/_4832.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+peering: **(Consul Enterprise only)** Fix issue where resolvers, routers, and splitters referencing peer targets may not work correctly for non-default partitions and namespaces. Enterprise customers leveraging peering are encouraged to upgrade both servers and agents to avoid this problem.
+```
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -9,10 +9,6 @@ parameters:
     type: string
     default: ""
     description: "Commit to run load tests against"
-  trigger-load-test:
-    type: boolean
-    default: false
-    description: "Boolean whether to run the load test workflow"
 
 references:
   paths:
@@ -1076,78 +1072,6 @@ jobs:
           path: *TEST_RESULTS_DIR
       - run: *notify-slack-failure
 
-  # Run load tests against a commit
-  load-test:
-    docker:
-      - image: hashicorp/terraform:latest
-    environment:
-      AWS_DEFAULT_REGION: us-east-2
-      BUCKET: consul-ci-load-tests
-      BASH_ENV: /etc/profile
-    shell: /bin/sh -leo pipefail
-    steps:
-      - checkout
-      - run: apk add jq curl bash
-      - run:
-          name: export load-test credentials
-          command: |
-            echo "export AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID_LOAD_TEST" >> $BASH_ENV
-            echo "export AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY_LOAD_TEST" >> $BASH_ENV
-      - run:
-          name: export role arn
-          command: |
-            echo "export TF_VAR_role_arn=$ROLE_ARN_LOAD_TEST" >> $BASH_ENV
-      - run:
-          name: setup TF_VARs
-          command: |
-            # if pipeline.parameters.commit="" it was not triggered/set through the API
-            # so we use the latest commit from _this_ branch. This is the case for nightly tests.
-            if [ "<< pipeline.parameters.commit >>" = "" ]; then
-              LOCAL_COMMIT_SHA=$(git rev-parse HEAD)
-            else
-              LOCAL_COMMIT_SHA="<< pipeline.parameters.commit >>"
-            fi
-            echo "export LOCAL_COMMIT_SHA=${LOCAL_COMMIT_SHA}" >> $BASH_ENV
-            git checkout ${LOCAL_COMMIT_SHA}
-
-            short_ref=$(git rev-parse --short ${LOCAL_COMMIT_SHA})
-            echo "export TF_VAR_ami_owners=$LOAD_TEST_AMI_OWNERS" >> $BASH_ENV
-            echo "export TF_VAR_vpc_name=$short_ref" >> $BASH_ENV
-            echo "export TF_VAR_cluster_name=$short_ref" >> $BASH_ENV
-            echo "export TF_VAR_consul_download_url=https://${S3_ARTIFACT_BUCKET}.s3.${AWS_DEFAULT_REGION}.amazonaws.com/${S3_ARTIFACT_PATH}/${LOCAL_COMMIT_SHA}.zip" >> $BASH_ENV
-      - run:
-          name: wait for dev build from test-integrations workflow
-          command: |
-            echo "curl-ing https://${S3_ARTIFACT_BUCKET}.s3.${AWS_DEFAULT_REGION}.amazonaws.com/${S3_ARTIFACT_PATH}/${LOCAL_COMMIT_SHA}.zip"
-            until [ $SECONDS -ge 300 ] && exit 1; do
-              curl -o /dev/null --fail --silent "https://${S3_ARTIFACT_BUCKET}.s3.${AWS_DEFAULT_REGION}.amazonaws.com/${S3_ARTIFACT_PATH}/${LOCAL_COMMIT_SHA}.zip" && exit
-              echo -n "."
-              sleep 2
-            done
-      - run:
-          working_directory: .circleci/terraform/load-test
-          name: terraform init
-          command: |
-            short_ref=$(git rev-parse --short HEAD)
-            echo "Testing commit id: $short_ref"
-            terraform init \
-            -backend-config="bucket=${BUCKET}" \
-            -backend-config="key=${LOCAL_COMMIT_SHA}" \
-            -backend-config="region=${AWS_DEFAULT_REGION}" \
-            -backend-config="role_arn=${ROLE_ARN_LOAD_TEST}"
-      - run:
-          working_directory: .circleci/terraform/load-test
-          name: run terraform apply
-          command: |
-            terraform apply -auto-approve
-      - run:
-          working_directory: .circleci/terraform/load-test
-          when: always
-          name: terraform destroy
-          command: |
-            for i in $(seq 1 5); do terraform destroy -auto-approve && s=0 && break || s=$? && sleep 20; done; (exit $s)
-      - run: *notify-slack-failure
-
   # The noop job is a used as a very fast job in the verify-ci workflow because every workflow
   # requires at least one job. It does nothing.
   noop:
@@ -1164,7 +1088,6 @@ workflows:
     jobs: [noop]
 
   go-tests:
-    unless: << pipeline.parameters.trigger-load-test >>
     jobs:
       - check-go-mod: &filter-ignore-non-go-branches
           filters:
@@ -1227,7 +1150,6 @@ workflows:
       - go-test-32bit: *filter-ignore-non-go-branches
       - noop
   build-distros:
-    unless: << pipeline.parameters.trigger-load-test >>
     jobs:
       - check-go-mod: *filter-ignore-non-go-branches
       - build-386: &require-check-go-mod
@@ -1259,7 +1181,6 @@ workflows:
           context: consul-ci
       - noop
   test-integrations:
-    unless: << pipeline.parameters.trigger-load-test >>
     jobs:
       - dev-build: *filter-ignore-non-go-branches
       - dev-upload-s3: &dev-upload
@@ -1303,7 +1224,6 @@ workflows:
 
       - noop
   frontend:
-    unless: << pipeline.parameters.trigger-load-test >>
     jobs:
       - frontend-cache:
           filters:
@@ -1336,19 +1256,3 @@ workflows:
           requires:
             - ember-build-ent
       - noop
-
-  load-test:
-    when: << pipeline.parameters.trigger-load-test >>
-    jobs:
-      - load-test
-
-  nightly-jobs:
-    triggers:
-      - schedule:
-          cron: "0 4 * * *" # 4AM UTC <> 12AM EST <> 9PM PST should have no impact
-          filters:
-            branches:
-              only:
-                - main
-    jobs:
-      - load-test
diff --git a/.github/workflows/load-test.yml b/.github/workflows/load-test.yml
diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go
@@ -824,6 +824,7 @@ func TestClient_ReloadConfig(t *testing.T) {
 	deps := newDefaultDeps(t, &Config{NodeName: "node1", Datacenter: "dc1"})
 	c, err := NewClient(cfg, deps)
 	require.NoError(t, err)
+	defer c.Shutdown()
 
 	limiter := c.rpcLimiter.Load().(*rate.Limiter)
 	require.Equal(t, rate.Limit(500), limiter.Limit())
@@ -869,7 +870,6 @@ func TestClient_ShortReconnectTimeout(t *testing.T) {
 		func() bool {
 			return len(cluster.Servers[0].LANMembersInAgentPartition()) == 2 &&
 				len(cluster.Clients[0].LANMembersInAgentPartition()) == 2
-
 		},
 		time.Second,
 		50*time.Millisecond,
@@ -901,6 +901,7 @@ func TestClient_RPC_Timeout(t *testing.T) {
 		c.MaxQueryTime = 200 * time.Millisecond
 		c.RPCHoldTimeout = 50 * time.Millisecond
 	})
+	defer c1.Shutdown()
 	joinLAN(t, c1, s1)
 
 	retry.Run(t, func(r *retry.R) {

diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go
@@ -720,10 +720,21 @@ func (c *compiler) newTarget(opts structs.DiscoveryTargetOpts) *structs.Discover
 	} else {
 		// Don't allow Peer and Datacenter.
 		opts.Datacenter = ""
-		// Peer and Partition cannot both be set.
-		opts.Partition = acl.PartitionOrDefault("")
+		// Since discovery targets (for peering) are ONLY used to query the catalog, and
+		// not to generate the SNI it is more correct to switch this to the calling-side
+		// of the peering's partition as that matches where the replicated data is stored
+		// in the catalog. This is done to simplify the usage of peer targets in both
+		// the xds and proxycfg packages.
+		//
+		// The peer info data attached to service instances will have the embedded opaque
+		// SNI/SAN information generated by the remote side and that will have the
+		// OTHER partition properly specified.
+		opts.Partition = acl.PartitionOrDefault(c.evaluateInPartition)
 		// Default to "default" rather than c.evaluateInNamespace.
-		opts.Namespace = acl.PartitionOrDefault(opts.Namespace)
+		// Note that the namespace is not swapped out, because it should
+		// always match the value in the remote cluster (and shouldn't
+		// have been changed anywhere).
+		opts.Namespace = acl.NamespaceOrDefault(opts.Namespace)
 	}
 
 	t := structs.NewDiscoveryTarget(opts)

diff --git a/agent/proxycfg/testing_api_gateway.go b/agent/proxycfg/testing_api_gateway.go
@@ -108,7 +108,7 @@ func TestConfigSnapshotAPIGateway(
 	upstreams := structs.TestUpstreams(t, false)
 
 	baseEvents = testSpliceEvents(baseEvents, setupTestVariationConfigEntriesAndSnapshot(
-		t, variation, upstreams, additionalEntries...,
+		t, variation, false, upstreams, additionalEntries...,
 	))
 
 	return testConfigSnapshotFixture(t, &structs.NodeService{

diff --git a/agent/proxycfg/testing_connect_proxy.go b/agent/proxycfg/testing_connect_proxy.go
@@ -145,7 +145,7 @@ func TestConfigSnapshotDiscoveryChain(
 			},
 		},
 	}, setupTestVariationConfigEntriesAndSnapshot(
-		t, variation, upstreams, additionalEntries...,
+		t, variation, enterprise, upstreams, additionalEntries...,
 	))
 
 	return testConfigSnapshotFixture(t, &structs.NodeService{

diff --git a/agent/proxycfg/testing_ingress_gateway.go b/agent/proxycfg/testing_ingress_gateway.go
@@ -88,7 +88,7 @@ func TestConfigSnapshotIngressGateway(
 		upstreams = structs.Upstreams{upstreams[0]} // just keep 'db'
 
 		baseEvents = testSpliceEvents(baseEvents, setupTestVariationConfigEntriesAndSnapshot(
-			t, variation, upstreams, additionalEntries...,
+			t, variation, false, upstreams, additionalEntries...,
 		))
 	}
 

diff --git a/agent/proxycfg/testing_mesh_gateway.go b/agent/proxycfg/testing_mesh_gateway.go
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/discoverychain"
 	"github.com/hashicorp/consul/agent/structs"
@@ -474,6 +475,8 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 		discoChains = make(map[structs.ServiceName]*structs.CompiledDiscoveryChain)
 		endpoints   = make(map[structs.ServiceName]structs.CheckServiceNodes)
 		entries     []structs.ConfigEntry
+		// This portion of the test is not currently enterprise-aware, but we need this to satisfy a function call.
+		entMeta = *acl.DefaultEnterpriseMeta()
 	)
 
 	switch variant {
@@ -660,17 +663,17 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				CorrelationID: "peering-connect-service:peer-a:db",
 				Result: &structs.IndexedCheckServiceNodes{
 					Nodes: structs.CheckServiceNodes{
-						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.1", false),
-						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.2", false),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.1", false, entMeta),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "db", "dc1", "peer-a", "10.40.1.2", false, entMeta),
 					},
 				},
 			},
 			UpdateEvent{
 				CorrelationID: "peering-connect-service:peer-b:alt",
 				Result: &structs.IndexedCheckServiceNodes{
 					Nodes: structs.CheckServiceNodes{
-						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.1", false),
-						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.2", true),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.1", false, entMeta),
+						structs.TestCheckNodeServiceWithNameInPeer(t, "alt", "remote-dc", "peer-b", "10.40.2.2", true, entMeta),
 					},
 				},
 			},