peering: remove unnecessary expose servers service, add error cases, fix flaky test

CHANGELOG.md

@@ -12,7 +12,8 @@ BREAKING_CHANGES:
 * Peering:
   * Remove support for customizing the server addresses in peering token generation. Instead, mesh gateways should be used
     to establish peering connections if the server pods are not directly reachable. [[GH-1610](https://github.com/hashicorp/consul-k8s/pull/1610)]
-  * Enabling peering requires `tls.enabled`. [[GH-1610](https://github.com/hashicorp/consul-k8s/pull/1610)]
+  * Require `global.tls.enabled` when peering is enabled. [[GH-1610](https://github.com/hashicorp/consul-k8s/pull/1610)]
+  * Require `meshGateway.enabled` when peering is enabled. [[GH-1683](https://github.com/hashicorp/consul-k8s/pull/1683)]
 
 FEATURES:
 * Consul-dataplane:
@@ -34,6 +35,7 @@ IMPROVEMENTS:
   * API Gateway: Add `tolerations` to `apiGateway.managedGatewayClass` and `apiGateway.controller` [[GH-1650](https://github.com/hashicorp/consul-k8s/pull/1650)]
   * API Gateway: Create PodSecurityPolicy for controller when `global.enablePodSecurityPolicies=true`. [[GH-1656](https://github.com/hashicorp/consul-k8s/pull/1656)]
   * API Gateway: Create PodSecurityPolicy and allow controller to bind it to ServiceAccounts that it creates for Gateway Deployments when `global.enablePodSecurityPolicies=true`. [[GH-1672](https://github.com/hashicorp/consul-k8s/pull/1672)]
+  * Deploy `expose-servers` service only when Admin Partitions(ENT) is enabled. [[GH-1683](https://github.com/hashicorp/consul-k8s/pull/1683)]
 
 ## 1.0.0-beta4 (October 28, 2022)
 

acceptance/tests/peering/peering_connect_namespaces_test.go

@@ -179,14 +179,34 @@ func TestPeering_ConnectNamespaces(t *testing.T) {
 				k8s.KubectlDeleteK(t, staticClientPeerClusterContext.KubectlOptions(t), kustomizeMeshDir)
 			})
 
+			staticServerPeerClient, _ := staticServerPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
+			staticClientPeerClient, _ := staticClientPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
+
+			// Ensure mesh config entries are created in Consul.
+			timer := &retry.Timer{Timeout: 1 * time.Minute, Wait: 1 * time.Second}
+			retry.RunWith(timer, t, func(r *retry.R) {
+				ceServer, _, err := staticServerPeerClient.ConfigEntries().Get(api.MeshConfig, "mesh", &api.QueryOptions{})
+				require.NoError(r, err)
+				configEntryServer, ok := ceServer.(*api.MeshConfigEntry)
+				require.True(r, ok)
+				require.Equal(r, configEntryServer.GetName(), "mesh")
+				require.NoError(r, err)
+
+				ceClient, _, err := staticClientPeerClient.ConfigEntries().Get(api.MeshConfig, "mesh", &api.QueryOptions{})
+				require.NoError(r, err)
+				configEntryClient, ok := ceClient.(*api.MeshConfigEntry)
+				require.True(r, ok)
+				require.Equal(r, configEntryClient.GetName(), "mesh")
+				require.NoError(r, err)
+			})
+
 			// Create the peering acceptor on the client peer.
 			k8s.KubectlApply(t, staticClientPeerClusterContext.KubectlOptions(t), "../fixtures/bases/peering/peering-acceptor.yaml")
 			helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() {
 				k8s.KubectlDelete(t, staticClientPeerClusterContext.KubectlOptions(t), "../fixtures/bases/peering/peering-acceptor.yaml")
 			})
 
 			// Ensure the secret is created.
-			timer := &retry.Timer{Timeout: 1 * time.Minute, Wait: 1 * time.Second}
 			retry.RunWith(timer, t, func(r *retry.R) {
 				acceptorSecretName, err := k8s.RunKubectlAndGetOutputE(t, staticClientPeerClusterContext.KubectlOptions(t), "get", "peeringacceptor", "server", "-o", "jsonpath={.status.secret.name}")
 				require.NoError(r, err)
@@ -226,9 +246,6 @@ func TestPeering_ConnectNamespaces(t *testing.T) {
 				k8s.RunKubectl(t, staticClientPeerClusterContext.KubectlOptions(t), "delete", "ns", staticClientNamespace)
 			})
 
-			staticServerPeerClient, _ := staticServerPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
-			staticClientPeerClient, _ := staticClientPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
-
 			serverQueryOpts := &api.QueryOptions{Namespace: staticServerNamespace}
 			clientQueryOpts := &api.QueryOptions{Namespace: staticClientNamespace}
 

acceptance/tests/peering/peering_connect_test.go

@@ -69,7 +69,6 @@ func TestPeering_Connect(t *testing.T) {
 
 				"dns.enabled":           "true",
 				"dns.enableRedirection": strconv.FormatBool(cfg.EnableTransparentProxy),
-				"peering.tokenGeneration.serverAddresses.source": "consul",
 			}
 
 			staticServerPeerHelmValues := map[string]string{
@@ -131,14 +130,34 @@ func TestPeering_Connect(t *testing.T) {
 				k8s.KubectlDeleteK(t, staticClientPeerClusterContext.KubectlOptions(t), kustomizeMeshDir)
 			})
 
+			staticServerPeerClient, _ := staticServerPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
+			staticClientPeerClient, _ := staticClientPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
+
+			// Ensure mesh config entries are created in Consul.
+			timer := &retry.Timer{Timeout: 1 * time.Minute, Wait: 1 * time.Second}
+			retry.RunWith(timer, t, func(r *retry.R) {
+				ceServer, _, err := staticServerPeerClient.ConfigEntries().Get(api.MeshConfig, "mesh", &api.QueryOptions{})
+				require.NoError(r, err)
+				configEntryServer, ok := ceServer.(*api.MeshConfigEntry)
+				require.True(r, ok)
+				require.Equal(r, configEntryServer.GetName(), "mesh")
+				require.NoError(r, err)
+
+				ceClient, _, err := staticClientPeerClient.ConfigEntries().Get(api.MeshConfig, "mesh", &api.QueryOptions{})
+				require.NoError(r, err)
+				configEntryClient, ok := ceClient.(*api.MeshConfigEntry)
+				require.True(r, ok)
+				require.Equal(r, configEntryClient.GetName(), "mesh")
+				require.NoError(r, err)
+			})
+
 			// Create the peering acceptor on the client peer.
 			k8s.KubectlApply(t, staticClientPeerClusterContext.KubectlOptions(t), "../fixtures/bases/peering/peering-acceptor.yaml")
 			helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() {
 				k8s.KubectlDelete(t, staticClientPeerClusterContext.KubectlOptions(t), "../fixtures/bases/peering/peering-acceptor.yaml")
 			})
 
 			// Ensure the secret is created.
-			timer := &retry.Timer{Timeout: 1 * time.Minute, Wait: 1 * time.Second}
 			retry.RunWith(timer, t, func(r *retry.R) {
 				acceptorSecretName, err := k8s.RunKubectlAndGetOutputE(t, staticClientPeerClusterContext.KubectlOptions(t), "get", "peeringacceptor", "server", "-o", "jsonpath={.status.secret.name}")
 				require.NoError(r, err)
@@ -178,9 +197,6 @@ func TestPeering_Connect(t *testing.T) {
 				k8s.RunKubectl(t, staticClientPeerClusterContext.KubectlOptions(t), "delete", "ns", staticClientNamespace)
 			})
 
-			staticServerPeerClient, _ := staticServerPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
-			staticClientPeerClient, _ := staticClientPeerCluster.SetupConsulClient(t, c.ACLsEnabled)
-
 			// Create a ProxyDefaults resource to configure services to use the mesh
 			// gateways.
 			logger.Log(t, "creating proxy-defaults config")

charts/consul/templates/connect-inject-deployment.yaml

@@ -1,4 +1,6 @@
 {{- if and .Values.global.peering.enabled (not .Values.connectInject.enabled) }}{{ fail "setting global.peering.enabled to true requires connectInject.enabled to be true" }}{{ end }}
+{{- if and .Values.global.peering.enabled (not .Values.global.tls.enabled) }}{{ fail "setting global.peering.enabled to true requires global.tls.enabled to be true" }}{{ end }}
+{{- if and .Values.global.peering.enabled (not .Values.meshGateway.enabled) }}{{ fail "setting global.peering.enabled to true requires meshGateway.enabled to be true" }}{{ end }}
 {{- if (or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled)) }}
 {{- if and .Values.global.adminPartitions.enabled (not .Values.global.enableConsulNamespaces) }}{{ fail "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" }}{{ end }}
 {{ template "consul.validateVaultWebhookCertConfiguration" . }}

charts/consul/templates/expose-servers-service.yaml

@@ -1,5 +1,5 @@
 {{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}}
-{{- $serverExposeServiceEnabled := (or (and (ne (.Values.server.exposeService.enabled | toString) "-") .Values.server.exposeService.enabled) (and (eq (.Values.server.exposeService.enabled | toString) "-") (or .Values.global.peering.enabled .Values.global.adminPartitions.enabled))) -}}
+{{- $serverExposeServiceEnabled := (or (and (ne (.Values.server.exposeService.enabled | toString) "-") .Values.server.exposeService.enabled) (and (eq (.Values.server.exposeService.enabled | toString) "-") .Values.global.adminPartitions.enabled)) -}}
 {{- if (and $serverEnabled $serverExposeServiceEnabled) }}
 
 # Service with an external IP to reach Consul servers.

charts/consul/test/unit/client-daemonset.bats

@@ -992,6 +992,7 @@ load _helpers
       --set 'client.enabled=true' \
       --set 'global.tls.enabled=true' \
       --set 'global.peering.enabled=true' \
+      --set 'meshGateway.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
       yq '.spec.template.spec.containers[0].command | join(" ")' | tee /dev/stderr)
@@ -1014,6 +1015,7 @@ load _helpers
       --set 'client.enabled=true' \
       --set 'global.tls.enabled=true' \
       --set 'global.peering.enabled=true' \
+      --set 'meshGateway.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
       yq '.spec.template.spec.containers[0].command | join(" ")' | tee /dev/stderr)

charts/consul/test/unit/connect-inject-deployment.bats

@@ -1370,6 +1370,8 @@ load _helpers
       -s templates/connect-inject-deployment.yaml  \
       --set 'connectInject.enabled=true' \
       --set 'global.peering.enabled=true' \
+      --set 'meshGateway.enabled=true' \
+      --set 'global.tls.enabled=true' \
       . | tee /dev/stderr |
       yq '.spec.template.spec.containers[0].command | any(contains("-enable-peering=true"))' | tee /dev/stderr)
 
@@ -1386,6 +1388,28 @@ load _helpers
   [[ "$output" =~ "setting global.peering.enabled to true requires connectInject.enabled to be true" ]]
 }
 
+@test "connectInject/Deployment: fails if peering is enabled but tls is not" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'meshGateway.enabled=true' \
+      --set 'global.peering.enabled=true' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "setting global.peering.enabled to true requires global.tls.enabled to be true" ]]
+}
+
+@test "connectInject/Deployment: fails if peering is enabled but mesh gateways are not" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'global.peering.enabled=true' .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "setting global.peering.enabled to true requires meshGateway.enabled to be true" ]]
+}
+
 #--------------------------------------------------------------------
 # openshift
 

charts/consul/test/unit/connect-inject-mutatingwebhookconfiguration.bats

@@ -56,13 +56,17 @@ load _helpers
   local actual=$(helm template \
       -s templates/connect-inject-mutatingwebhookconfiguration.yaml  \
       --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'meshGateway.enabled=true' \
       --set 'global.peering.enabled=true' \
       . | tee /dev/stderr |
       yq '.webhooks[1].name | contains("peeringacceptors.consul.hashicorp.com")' | tee /dev/stderr)
   [ "${actual}" = "true" ]
   local actual=$(helm template \
       -s templates/connect-inject-mutatingwebhookconfiguration.yaml  \
       --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'meshGateway.enabled=true' \
       --set 'global.peering.enabled=true' \
       . | tee /dev/stderr |
       yq '.webhooks[2].name | contains("peeringdialers.consul.hashicorp.com")' | tee /dev/stderr)