Repository: kumahq/kuma
Pull request #11474 (2024-09-19 13:07:58)

fix(kuma-cp): allow specifying namespace when targeting MeshExternalService in policies
We are not always configuring policies in a system namespace when MES is created. We want users to be able to select MES in policies on their namespaces, but in order to do it they need to be able to specify namespace in `to.targetRef` section


### Checklist prior to review

<!--
Each of these sections need to be filled by the author when opening the PR.

If something doesn't apply please check the box and add a justification after the `--`
-->

- [x] [Link to relevant issue][1] as well as docs and UI issues --
- [x] This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like `syscall.Mkfifo` have equivalent implementation on the other OS --
- [x] Tests (Unit test, E2E tests, manual test on universal and k8s) --
  - Don't forget `ci/` labels to run additional/fewer tests
- [x] Do you need to update [`UPGRADE.md`](../blob/master/UPGRADE.md)? --
- [x] Does it need to be backported according to the [backporting policy](../blob/master/CONTRIBUTING.md#backporting)? ([this](https://github.com/kumahq/kuma/actions/workflows/auto-backport.yaml) GH action will add "backport" label based on these [file globs](https://github.com/kumahq/kuma/blob/master/.github/workflows/auto-backport.yaml#L6), if you want to prevent it from adding the "backport" label use [no-backport-autolabel](https://github.com/kumahq/kuma/blob/master/.github/workflows/auto-backport.yaml#L8) label) --

<!--
> Changelog: skip
-->
<!--
Uncomment the above section to explicitly set a [`> Changelog:` entry here](https://github.com/kumahq/kuma/blob/master/CONTRIBUTING.md#submitting-a-patch)?
-->

[1]: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword


Automaat: > Aren't we missing tests? Also can we get some description of this?

@slonka added description and test

Patch:
diff --git a/deployments/charts/kuma/crds/kuma.io_meshtlses.yaml b/deployments/charts/kuma/crds/kuma.io_meshtlses.yaml
index 82c2d1e92..4ddbfffcb 100644
--- a/deployments/charts/kuma/crds/kuma.io_meshtlses.yaml
+++ b/deployments/charts/kuma/crds/kuma.io_meshtlses.yaml
@@ -58,9 +58,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/docs/generated/openapi.yaml b/docs/generated/openapi.yaml
index 3bff3fe5c..83bc6dc20 100644
--- a/docs/generated/openapi.yaml
+++ b/docs/generated/openapi.yaml
@@ -10275,8 +10275,6 @@ components:
                         description: >-
                           Mode defines the behavior of inbound listeners with
                           regard to traffic encryption.
-
-                          Default: Strict.
                         enum:
                           - Permissive
                           - Strict
diff --git a/docs/generated/raw/crds/kuma.io_meshtlses.yaml b/docs/generated/raw/crds/kuma.io_meshtlses.yaml
index 82c2d1e92..4ddbfffcb 100644
--- a/docs/generated/raw/crds/kuma.io_meshtlses.yaml
+++ b/docs/generated/raw/crds/kuma.io_meshtlses.yaml
@@ -58,9 +58,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/pkg/core/resources/apis/mesh/validators.go b/pkg/core/resources/apis/mesh/validators.go
index ad77be1a8..3d3c50471 100644
--- a/pkg/core/resources/apis/mesh/validators.go
+++ b/pkg/core/resources/apis/mesh/validators.go
@@ -392,7 +392,7 @@ func ValidateTargetRef(
 		if len(ref.Labels) > 0 && ref.SectionName != "" {
 			err.AddViolation("sectionName", "sectionName should not be combined with labels")
 		}
-	case common_api.MeshServiceSubset, common_api.MeshGateway, common_api.MeshExternalService:
+	case common_api.MeshServiceSubset, common_api.MeshGateway:
 		err.Add(requiredField("name", ref.Name, ref.Kind))
 		err.Add(validateName(ref.Name, opts.AllowedInvalidNames))
 		err.Add(disallowedField("mesh", ref.Mesh, ref.Kind))
@@ -404,6 +404,17 @@ func ValidateTargetRef(
 		err.Add(disallowedField("labels", ref.Labels, ref.Kind))
 		err.Add(disallowedField("namespace", ref.Namespace, ref.Kind))
 		err.Add(disallowedField("sectionName", ref.SectionName, ref.Kind))
+	case common_api.MeshExternalService:
+		err.Add(requiredField("name", ref.Name, ref.Kind))
+		err.Add(validateName(ref.Name, opts.AllowedInvalidNames))
+		err.Add(disallowedField("mesh", ref.Mesh, ref.Kind))
+		err.Add(disallowedField("proxyTypes", ref.ProxyTypes, ref.Kind))
+		err.Add(ValidateTags(validators.RootedAt("tags"), ref.Tags, ValidateTagsOpts{}))
+		if ref.Kind == common_api.MeshGateway && len(ref.Tags) > 0 && !opts.GatewayListenerTagsAllowed {
+			err.Add(disallowedField("tags", ref.Tags, ref.Kind))
+		}
+		err.Add(disallowedField("labels", ref.Labels, ref.Kind))
+		err.Add(disallowedField("sectionName", ref.SectionName, ref.Kind))
 	}
 
 	return err
diff --git a/pkg/plugins/policies/meshtls/api/v1alpha1/meshtls.go b/pkg/plugins/policies/meshtls/api/v1alpha1/meshtls.go
index 66f1d6846..c1c3812d0 100644
--- a/pkg/plugins/policies/meshtls/api/v1alpha1/meshtls.go
+++ b/pkg/plugins/policies/meshtls/api/v1alpha1/meshtls.go
@@ -44,6 +44,5 @@ type Conf struct {
 	TlsCiphers common_tls.TlsCiphers `json:"tlsCiphers,omitempty"`
 
 	// Mode defines the behavior of inbound listeners with regard to traffic encryption.
-	// Default: Strict.
 	Mode *Mode `json:"mode,omitempty"`
 }
diff --git a/pkg/plugins/policies/meshtls/api/v1alpha1/schema.yaml b/pkg/plugins/policies/meshtls/api/v1alpha1/schema.yaml
index f475a0093..32ac5c8a5 100644
--- a/pkg/plugins/policies/meshtls/api/v1alpha1/schema.yaml
+++ b/pkg/plugins/policies/meshtls/api/v1alpha1/schema.yaml
@@ -31,9 +31,7 @@ properties:
                 'targetRef'
               properties:
                 mode:
-                  description: |-
-                    Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                    Default: Strict.
+                  description: Mode defines the behavior of inbound listeners with regard to traffic encryption.
                   enum:
                     - Permissive
                     - Strict
diff --git a/pkg/plugins/policies/meshtls/k8s/crd/kuma.io_meshtlses.yaml b/pkg/plugins/policies/meshtls/k8s/crd/kuma.io_meshtlses.yaml
index 82c2d1e92..4ddbfffcb 100644
--- a/pkg/plugins/policies/meshtls/k8s/crd/kuma.io_meshtlses.yaml
+++ b/pkg/plugins/policies/meshtls/k8s/crd/kuma.io_meshtlses.yaml
@@ -58,9 +58,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go
index bc20c1e30..fec29358e 100644
--- a/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go
+++ b/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go
@@ -48,6 +48,13 @@ func (p plugin) Apply(rs *core_xds.ResourceSet, ctx xds_context.Context, proxy *
 	if proxy.Dataplane == nil {
 		return nil
 	}
+	if !ctx.Mesh.Resource.MTLSEnabled() {
+		log.V(1).Info("skip applying MeshTLS, MTLS is disabled",
+			"proxyName", proxy.Dataplane.GetMeta().GetName(),
+			"mesh", ctx.Mesh.Resource.GetMeta().GetName())
+		return nil
+	}
+
 	log.V(1).Info("applying", "proxy-name", proxy.Dataplane.GetMeta().GetName())
 	policies, ok := proxy.Policies.Dynamic[api.MeshTLSType]
 	if !ok {
@@ -271,8 +278,7 @@ func configure(
 	xdsCtx xds_context.Context,
 ) (envoy_common.NamedResource, error) {
 	mesh := xdsCtx.Mesh.Resource
-	// Default Strict
-	mode := pointer.DerefOr(conf.Mode, api.ModeStrict)
+	mode := pointer.DerefOr(conf.Mode, getMeshTLSMode(mesh))
 	protocol := core_mesh.ParseProtocol(inbound.GetProtocol())
 	localClusterName := envoy_names.GetLocalClusterName(iface.WorkloadPort)
 	cluster := envoy_common.NewCluster(envoy_common.WithService(localClusterName))
@@ -311,3 +317,12 @@ func configure(
 	}
 	return listenerBuilder.Build()
 }
+
+func getMeshTLSMode(mesh *core_mesh.MeshResource) api.Mode {
+	switch mesh.GetEnabledCertificateAuthorityBackend().GetMode() {
+	case mesh_proto.CertificateAuthorityBackend_PERMISSIVE:
+		return api.ModePermissive
+	default:
+		return api.ModeStrict
+	}
+}


Test patch:
diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml
index 76be71db0..6fe291a9d 100644
--- a/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml
+++ b/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml
@@ -6589,9 +6589,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml
index 2860aab2f..693757646 100644
--- a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml
+++ b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml
@@ -6589,9 +6589,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml
index 466c5972f..38afad1dc 100644
--- a/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml
+++ b/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml
@@ -6609,9 +6609,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml b/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
index a56d63060..2ad58f5c7 100644
--- a/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
+++ b/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
@@ -8075,9 +8075,8 @@ spec:
                         'targetRef'
                       properties:
                         mode:
-                          description: |-
-                            Mode defines the behavior of inbound listeners with regard to traffic encryption.
-                            Default: Strict.
+                          description: Mode defines the behavior of inbound listeners
+                            with regard to traffic encryption.
                           enum:
                           - Permissive
                           - Strict
diff --git a/pkg/core/resources/apis/mesh/validators_test.go b/pkg/core/resources/apis/mesh/validators_test.go
index 6c33ce557..bdf76f0f2 100644
--- a/pkg/core/resources/apis/mesh/validators_test.go
+++ b/pkg/core/resources/apis/mesh/validators_test.go
@@ -273,6 +273,18 @@ sectionName: http-port
 				},
 			},
 		}),
+		Entry("MeshExternalService with name and namespace", testCase{
+			inputYaml: `
+kind: MeshExternalService
+name: backend
+namespace: test-ns
+`,
+			opts: &ValidateTargetRefOpts{
+				SupportedKinds: []common_api.TargetRefKind{
+					common_api.MeshExternalService,
+				},
+			},
+		}),
 	)
 
 	DescribeTable("should return as much individual errors as possible with",
diff --git a/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/permissive-no-mtls.listeners.golden.yaml b/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/permissive-no-mtls.listeners.golden.yaml
index fc92cdf5f..8f5832d8b 100644
--- a/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/permissive-no-mtls.listeners.golden.yaml
+++ b/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/permissive-no-mtls.listeners.golden.yaml
@@ -6,46 +6,32 @@ resources:
       socketAddress:
         address: 127.0.0.1
         portValue: 17777
-    bindToPort: false
     enableReusePort: false
     filterChains:
-    - filterChainMatch:
-        transportProtocol: raw_buffer
-      filters:
-      - name: envoy.filters.network.tcp_proxy
-        typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17777
-          idleTimeout: 7200s
-          statPrefix: localhost_17777
-    - filterChainMatch:
-        transportProtocol: tls
-      filters:
-      - name: envoy.filters.network.tcp_proxy
-        typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17777
-          idleTimeout: 7200s
-          statPrefix: localhost_17777
-    - filterChainMatch:
-        applicationProtocols:
-        - kuma
-        transportProtocol: tls
-      filters:
-      - name: envoy.filters.network.tcp_proxy
+    - filters:
+      - name: envoy.filters.network.http_connection_manager
         typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17777
-          idleTimeout: 7200s
-          statPrefix: localhost_17777
-    listenerFilters:
-    - name: envoy.filters.listener.tls_inspector
-      typedConfig:
-        '@type': type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
-    metadata:
-      filterMetadata:
-        io.kuma.tags:
-          kuma.io/service: backend
+          '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+          httpFilters:
+          - name: envoy.filters.http.router
+            typedConfig:
+              '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+          routeConfig:
+            name: inbound:backend
+            requestHeadersToRemove:
+            - x-kuma-tags
+            validateClusters: false
+            virtualHosts:
+            - domains:
+              - '*'
+              name: backend
+              routes:
+              - match:
+                  prefix: /
+                route:
+                  cluster: backend
+                  timeout: 0s
+          statPrefix: "127_0_0_1_17777"
     name: inbound:127.0.0.1:17777
     trafficDirection: INBOUND
 - name: inbound:127.0.0.1:17778
@@ -55,45 +41,13 @@ resources:
       socketAddress:
         address: 127.0.0.1
         portValue: 17778
-    bindToPort: false
     enableReusePort: false
     filterChains:
-    - filterChainMatch:
-        transportProtocol: raw_buffer
-      filters:
-      - name: envoy.filters.network.tcp_proxy
-        typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17778
-          idleTimeout: 7200s
-          statPrefix: localhost_17778
-    - filterChainMatch:
-        transportProtocol: tls
-      filters:
-      - name: envoy.filters.network.tcp_proxy
-        typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17778
-          idleTimeout: 7200s
-          statPrefix: localhost_17778
-    - filterChainMatch:
-        applicationProtocols:
-        - kuma
-        transportProtocol: tls
-      filters:
+    - filters:
       - name: envoy.filters.network.tcp_proxy
         typedConfig:
           '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17778
-          idleTimeout: 7200s
-          statPrefix: localhost_17778
-    listenerFilters:
-    - name: envoy.filters.listener.tls_inspector
-      typedConfig:
-        '@type': type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
-    metadata:
-      filterMetadata:
-        io.kuma.tags:
-          kuma.io/service: frontend
+          cluster: frontend
+          statPrefix: "127_0_0_1_17778"
     name: inbound:127.0.0.1:17778
     trafficDirection: INBOUND
diff --git a/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/strict-no-mtls.listeners.golden.yaml b/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/strict-no-mtls.listeners.golden.yaml
index 734cc6ddb..8f5832d8b 100644
--- a/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/strict-no-mtls.listeners.golden.yaml
+++ b/pkg/plugins/policies/meshtls/plugin/v1alpha1/testdata/strict-no-mtls.listeners.golden.yaml
@@ -6,20 +6,32 @@ resources:
       socketAddress:
         address: 127.0.0.1
         portValue: 17777
-    bindToPort: false
     enableReusePort: false
     filterChains:
     - filters:
-      - name: envoy.filters.network.tcp_proxy
+      - name: envoy.filters.network.http_connection_manager
         typedConfig:
-          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17777
-          idleTimeout: 7200s
-          statPrefix: localhost_17777
-    metadata:
-      filterMetadata:
-        io.kuma.tags:
-          kuma.io/service: backend
+          '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+          httpFilters:
+          - name: envoy.filters.http.router
+            typedConfig:
+              '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+          routeConfig:
+            name: inbound:backend
+            requestHeadersToRemove:
+            - x-kuma-tags
+            validateClusters: false
+            virtualHosts:
+            - domains:
+              - '*'
+              name: backend
+              routes:
+              - match:
+                  prefix: /
+                route:
+                  cluster: backend
+                  timeout: 0s
+          statPrefix: "127_0_0_1_17777"
     name: inbound:127.0.0.1:17777
     trafficDirection: INBOUND
 - name: inbound:127.0.0.1:17778
@@ -29,19 +41,13 @@ resources:
       socketAddress:
         address: 127.0.0.1
         portValue: 17778
-    bindToPort: false
     enableReusePort: false
     filterChains:
     - filters:
       - name: envoy.filters.network.tcp_proxy
         typedConfig:
           '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
-          cluster: localhost:17778
-          idleTimeout: 7200s
-          statPrefix: localhost_17778
-    metadata:
-      filterMetadata:
-        io.kuma.tags:
-          kuma.io/service: frontend
+          cluster: frontend
+          statPrefix: "127_0_0_1_17778"
     name: inbound:127.0.0.1:17778
     trafficDirection: INBOUND
diff --git a/test/e2e_env/kubernetes/gateway/delegated/meshhttproute.go b/test/e2e_env/kubernetes/gateway/delegated/meshhttproute.go
index bf7c8abd1..44b96cbcb 100644
--- a/test/e2e_env/kubernetes/gateway/delegated/meshhttproute.go
+++ b/test/e2e_env/kubernetes/gateway/delegated/meshhttproute.go
@@ -196,15 +196,15 @@ spec:
 					"demo-client",
 					fmt.Sprintf("http://%s/test-server", config.KicIP),
 					client.FromKubernetesPod(config.NamespaceOutsideMesh, "demo-client"),
-					client.WithNumberOfRequests(50),
+					client.WithNumberOfRequests(100),
 				)
 				g.Expect(err).ToNot(HaveOccurred())
 				g.Expect(response).To(HaveLen(4))
 				g.Expect(response).To(And(
-					HaveKeyWithValue(Equal(`test-server-0`), BeNumerically("~", 8, 3)),
-					HaveKeyWithValue(Equal(`test-server-1`), BeNumerically("~", 8, 3)),
-					HaveKeyWithValue(Equal(`test-server-2`), BeNumerically("~", 8, 3)),
-					HaveKeyWithValue(ContainSubstring(`external-service`), BeNumerically("~", 25, 6)),
+					HaveKey(Equal(`test-server-0`)),
+					HaveKey(Equal(`test-server-1`)),
+					HaveKey(Equal(`test-server-2`)),
+					HaveKeyWithValue(ContainSubstring(`external-service`), BeNumerically("~", 50, 15)),
 				))
 			}, "30s", "5s").Should(Succeed())
 		})