From 040094d2ffb00b318c4cea8fb35ba7ac0c8feb54 Mon Sep 17 00:00:00 2001
From: Yan Xue <3491507+yxue@users.noreply.github.com>
Date: Thu, 2 Jan 2020 16:42:23 -0800
Subject: [PATCH] skip port name check for system ns (#19888)

* skip port name check for system ns

* check istio control plane

* fix
---
 .../analysis/analyzers/analyzers_test.go      |  6 +++
 .../config/analysis/analyzers/auth/mtls.go    |  2 +-
 .../analysis/analyzers/service/portname.go    | 13 +++++
 ...service-no-port-name-system-namespace.yaml | 47 +++++++++++++++++++
 .../config/analysis/analyzers/util/config.go  |  5 ++
 5 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 galley/pkg/config/analysis/analyzers/testdata/service-no-port-name-system-namespace.yaml

diff --git a/galley/pkg/config/analysis/analyzers/analyzers_test.go b/galley/pkg/config/analysis/analyzers/analyzers_test.go
index f2de0a31461..1da947125d3 100644
--- a/galley/pkg/config/analysis/analyzers/analyzers_test.go
+++ b/galley/pkg/config/analysis/analyzers/analyzers_test.go
@@ -287,6 +287,12 @@ var testGrid = []testCase{
 		analyzer:   &service.PortNameAnalyzer{},
 		expected:   []message{},
 	},
+	{
+		name:       "unnamedPortInSystemNamespace",
+		inputFiles: []string{"testdata/service-no-port-name-system-namespace.yaml"},
+		analyzer:   &service.PortNameAnalyzer{},
+		expected:   []message{},
+	},
 	{
 		name:       "sidecarDefaultSelector",
 		inputFiles: []string{"testdata/sidecar-default-selector.yaml"},
diff --git a/galley/pkg/config/analysis/analyzers/auth/mtls.go b/galley/pkg/config/analysis/analyzers/auth/mtls.go
index 6079971edb8..4b669449135 100644
--- a/galley/pkg/config/analysis/analyzers/auth/mtls.go
+++ b/galley/pkg/config/analysis/analyzers/auth/mtls.go
@@ -115,7 +115,7 @@ func (s *MTLSAnalyzer) Analyze(c analysis.Context) {
 		// Skip the istio control plane, which doesn't obey Policy/MeshPolicy MTLS
 		// rules in general and instead is controlled by the mesh option
 		// 'controlPlaneSecurityEnabled'.
-		if _, ok := r.Metadata.Labels["istio"]; ok {
+		if util.IsIstioControlPlane(r) {
 			return true
 		}
 
diff --git a/galley/pkg/config/analysis/analyzers/service/portname.go b/galley/pkg/config/analysis/analyzers/service/portname.go
index 363a2ffe2a2..faeaf66d3bf 100644
--- a/galley/pkg/config/analysis/analyzers/service/portname.go
+++ b/galley/pkg/config/analysis/analyzers/service/portname.go
@@ -16,6 +16,7 @@ package service
 
 import (
 	"istio.io/istio/galley/pkg/config/analysis"
+	"istio.io/istio/galley/pkg/config/analysis/analyzers/util"
 	"istio.io/istio/galley/pkg/config/analysis/msg"
 	"istio.io/istio/galley/pkg/config/resource"
 	"istio.io/istio/galley/pkg/config/schema/collection"
@@ -44,6 +45,18 @@ func (s *PortNameAnalyzer) Metadata() analysis.Metadata {
 // Analyze implements Analyzer
 func (s *PortNameAnalyzer) Analyze(c analysis.Context) {
 	c.ForEach(collections.K8SCoreV1Services.Name(), func(r *resource.Instance) bool {
+		svcNs := r.Metadata.FullName.Namespace
+
+		// Skip system namespaces entirely
+		if util.IsSystemNamespace(svcNs) {
+			return true
+		}
+
+		// Skip port name check for istio control plane
+		if util.IsIstioControlPlane(r) {
+			return true
+		}
+
 		s.analyzeService(r, c)
 		return true
 	})
diff --git a/galley/pkg/config/analysis/analyzers/testdata/service-no-port-name-system-namespace.yaml b/galley/pkg/config/analysis/analyzers/testdata/service-no-port-name-system-namespace.yaml
new file mode 100644
index 00000000000..cb010d3f7ee
--- /dev/null
+++ b/galley/pkg/config/analysis/analyzers/testdata/service-no-port-name-system-namespace.yaml
@@ -0,0 +1,47 @@
+# If port is unnamed or port name doesn't follow <protocol>[-<suffix>], the analyzer will report warning.
+# If the service is in system namespace, i.e., kube-system, istio-system, kube-public, the check will be skipped.
+apiVersion: v1
+kind: Service
+metadata:
+  name: my-service1
+  namespace: kube-system
+spec:
+  selector:
+    app: my-service1
+  ports:
+    - protocol: TCP
+      port: 8080
+      targetPort: 8080
+    - protocol: TCP
+      port: 8081
+      targetPort: 8081
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: my-service2
+  namespace: istio-system
+  labels:
+    istio: xxx
+spec:
+  selector:
+    app: my-service2
+  ports:
+    - name: foo
+      protocol: TCP
+      port: 8080
+      targetPort: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: my-service3
+  namespace: kube-public
+spec:
+  selector:
+    app: my-service3
+  ports:
+    - name: bar
+      protocol: TCP
+      port: 8080
+      targetPort: 8080
\ No newline at end of file
diff --git a/galley/pkg/config/analysis/analyzers/util/config.go b/galley/pkg/config/analysis/analyzers/util/config.go
index d069ec5a849..316c94aaea7 100644
--- a/galley/pkg/config/analysis/analyzers/util/config.go
+++ b/galley/pkg/config/analysis/analyzers/util/config.go
@@ -39,3 +39,8 @@ func MeshConfig(ctx analysis.Context) *v1alpha1.MeshConfig {
 func IsSystemNamespace(ns resource.Namespace) bool {
 	return ns == "kube-system" || ns == "kube-public"
 }
+
+func IsIstioControlPlane(r *resource.Instance) bool {
+	_, ok := r.Metadata.Labels["istio"]
+	return ok
+}