fix(policies): improve targetRef validation

- disallow setting name with `kind: Mesh` as it's unclear what it will mean - disallow setting mesh with all kinds as it's unclear what it will mean too - disallow using `MeshHTTPRoute` the thing is not even defined - rewrite validator for targetRef to use the same wording everwhere Fix #5165 Signed-off-by: Charly Molter <charly.molter@konghq.com> make targetRef compulsory
kumahq · Oct 21, 2022 · be831c6 · be831c6
1 parent 826cd4c
commit be831c6
Show file tree

Hide file tree

Showing 18 changed files with 370 additions and 374 deletions.
diff --git a/pkg/api-server/inspect_endpoints_test.go b/pkg/api-server/inspect_endpoints_test.go
@@ -339,6 +339,9 @@ var _ = Describe("Inspect WS", func() {
 						},
 						From: []*policies_api.MeshTrafficPermission_From{
 							{
+								TargetRef: &common_api.TargetRef{
+									Kind: "Mesh",
+								},
 								Default: &policies_api.MeshTrafficPermission_Conf{
 									Action: "ALLOW",
 								},
@@ -1105,6 +1108,9 @@ var _ = Describe("Inspect WS", func() {
 						},
 						From: []*policies_api.MeshTrafficPermission_From{
 							{
+								TargetRef: &common_api.TargetRef{
+									Kind: "Mesh",
+								},
 								Default: &policies_api.MeshTrafficPermission_Conf{
 									Action: "ALLOW",
 								},

diff --git a/pkg/api-server/testdata/inspect_dataplane.json b/pkg/api-server/testdata/inspect_dataplane.json
@@ -19,6 +19,9 @@
       },
       "from": [
        {
+        "targetRef": {
+         "kind": "Mesh"
+        },
         "default": {
          "action": "ALLOW"
         }

diff --git a/pkg/core/xds/rules.go b/pkg/core/xds/rules.go
@@ -218,10 +218,6 @@ func newConf(t reflect.Type) (proto.Message, error) {
 }
 
 func asSubset(tr *common_api.TargetRef) (Subset, error) {
-	if tr == nil {
-		// syntactic sugar, empty targetRef means targetRef{kind: Mesh}
-		return Subset{}, nil
-	}
 	switch tr.GetKindEnum() {
 	case common_api.TargetRef_Mesh:
 		return Subset{}, nil

diff --git a/pkg/core/xds/testdata/rules/06.policy.yaml b/pkg/core/xds/testdata/rules/06.policy.yaml
@@ -4,11 +4,10 @@ name: mt-1
 spec:
   targetRef:
     kind: Mesh
-    name: mesh-1
   default:
     tags:
       - name: team
         literal: core
     backends:
       - zipkin:
-          url: http://jaeger-collector.mesh-observability:9411/api/v2/spans
+          url: http://jaeger-collector.mesh-observability:9411/api/v2/spans
diff --git a/pkg/core/xds/testdata/rules/07.policy.yaml b/pkg/core/xds/testdata/rules/07.policy.yaml
@@ -4,7 +4,6 @@ name: mt-1
 spec:
   targetRef:
     kind: Mesh
-    name: mesh-1
   default:
     tags:
       - name: team
@@ -26,4 +25,4 @@ spec:
         literal: support
     backends:
       - datadog:
-          url: http://ingest.datadoghq.eu:8126
+          url: http://ingest.datadoghq.eu:8126
diff --git a/pkg/plugins/policies/matchers/match.go b/pkg/plugins/policies/matchers/match.go
@@ -33,8 +33,7 @@ func MatchedPolicies(rType core_model.ResourceType, dpp *core_mesh.DataplaneReso
 			return core_xds.TypedMatchingPolicies{}, errors.Errorf("resource type %v doesn't support TargetRef matching", rType)
 		}
 
-		targetRef := spec.GetTargetRef()
-		selectedInbounds := inboundsSelectedByTargetRef(targetRef, dpp, gateway)
+		selectedInbounds := inboundsSelectedByTargetRef(spec.GetTargetRef(), dpp, gateway)
 		if len(selectedInbounds) == 0 {
 			// DPP is not matched by the policy
 			continue

diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/01.golden.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/01.golden.yaml
@@ -9,7 +9,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: Mesh
   type: MeshTrafficPermission
@@ -23,7 +22,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshSubset
       tags:
@@ -39,7 +37,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshService
       name: web
@@ -54,7 +51,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshServiceSubset
       name: web

diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/01.policies.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/01.policies.yaml
@@ -11,7 +11,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -24,7 +23,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -38,7 +36,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -53,7 +50,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -67,7 +63,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -83,7 +78,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -99,6 +93,5 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/02.golden.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/02.golden.yaml
@@ -9,7 +9,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: Mesh
   type: MeshTrafficPermission
@@ -23,7 +22,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: Mesh
   type: MeshTrafficPermission
@@ -37,7 +35,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshSubset
       tags:
@@ -53,7 +50,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshSubset
       tags:
@@ -69,7 +65,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshService
       name: web
@@ -84,7 +79,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshService
       name: web
@@ -99,7 +93,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshServiceSubset
       name: web
@@ -116,7 +109,6 @@ items:
         action: ALLOW
       targetRef:
         kind: Mesh
-        name: default
     targetRef:
       kind: MeshServiceSubset
       name: web

diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/02.policies.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/02.policies.yaml
@@ -11,7 +11,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -27,7 +26,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -40,7 +38,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -53,7 +50,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -67,7 +63,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -81,7 +76,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -96,7 +90,6 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
 ---
@@ -111,6 +104,5 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/03.policies.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/03.policies.yaml
@@ -13,6 +13,5 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW
diff --git a/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/04.policies.yaml b/pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/04.policies.yaml
@@ -13,6 +13,5 @@ spec:
   from:
     - targetRef:
         kind: Mesh
-        name: default
       default:
         action: ALLOW