Flexible DNS names (istio#11986)

* WIP Flexible DNS names

* More fix

* Style filx

* Fix error

* Fix lint

* Fix lint

* fix lint

security/cmd/istio_ca/main.go

@@ -270,32 +270,10 @@ func runCA() {
 
 	verifyCommandLineOptions()
 
-	var webhooks map[string]controller.DNSNameEntry
+	var webhooks map[string]*controller.DNSNameEntry
 	if opts.appendDNSNames {
-		webhooks = make(map[string]controller.DNSNameEntry)
-		for i, svcAccount := range webhookServiceAccounts {
-			webhooks[svcAccount] = controller.DNSNameEntry{
-				ServiceName: webhookServiceNames[i],
-				Namespace:   opts.istioCaStorageNamespace,
-			}
-		}
-		if len(opts.customDNSNames) > 0 {
-			customNames := strings.Split(opts.customDNSNames, ",")
-			for _, customName := range customNames {
-				nameDomain := strings.Split(customName, ":")
-				if len(nameDomain) == 2 {
-					override, ok := webhooks[nameDomain[0]]
-					if ok {
-						override.CustomDomains = append(override.CustomDomains, nameDomain[1])
-					} else {
-						webhooks[nameDomain[0]] = controller.DNSNameEntry{
-							ServiceName:   nameDomain[0],
-							CustomDomains: []string{nameDomain[1]},
-						}
-					}
-				}
-			}
-		}
+		webhooks = controller.ConstructCustomDNSNames(webhookServiceAccounts,
+			webhookServiceNames, opts.istioCaStorageNamespace, opts.customDNSNames)
 	}
 
 	cs, err := kubelib.CreateClientset(opts.kubeConfigFile, "")

security/pkg/k8s/controller/customdnsname.go

@@ -0,0 +1,57 @@
+// Copyright 2019 Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package controller
+
+import (
+	"strings"
+
+	"istio.io/istio/pkg/log"
+)
+
+func ConstructCustomDNSNames(serviceAccounts []string, serviceNames []string,
+	namespace string, customDNSNames string) map[string]*DNSNameEntry {
+	result := make(map[string]*DNSNameEntry)
+	for i, svcAccount := range serviceAccounts {
+		result[svcAccount] = &DNSNameEntry{
+			ServiceName: serviceNames[i],
+			Namespace:   namespace,
+		}
+	}
+	if len(customDNSNames) > 0 {
+		customNames := strings.Split(customDNSNames, ",")
+		log.Infof("The custom-defined DNS name list is %v", customNames)
+		for _, customName := range customNames {
+			nameDomain := strings.Split(customName, ":")
+			if len(nameDomain) == 2 {
+				override, ok := result[nameDomain[0]]
+				if ok {
+					// There is already an entry for nameDomain[0], we just need to
+					// append current value.
+					override.CustomDomains = append(override.CustomDomains, nameDomain[1])
+				} else {
+					// There is no entry for nameDomain[0] in the map, create a new one.
+					result[nameDomain[0]] = &DNSNameEntry{
+						ServiceName:   nameDomain[0],
+						CustomDomains: []string{nameDomain[1]},
+					}
+				}
+			} else {
+				log.Warnf("Cannot process this invalid custom defined names %v, it "+
+					"should follow SERVICE_ACCOUNT.NAMESPACE:DOMAIN format", customName)
+			}
+		}
+	}
+	return result
+}

security/pkg/k8s/controller/customdnsname_test.go

@@ -0,0 +1,227 @@
+// Copyright 2019 Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package controller
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestConstructCustomDNSNames(t *testing.T) {
+	testCases := map[string]struct {
+		sas      []string
+		sns      []string
+		ns       string
+		dnsNames string
+		expected map[string]*DNSNameEntry
+	}{
+		"Valid input 1": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+			},
+			ns: "istio-system",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+			},
+		},
+		"Valid input 2": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns: "istio-system",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+			},
+		},
+		"Valid input 3": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns:       "istio-system",
+			dnsNames: "istio-galley-service-account.istio-system:istio-galley-ilb.istio-system.svc.us1.dog",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account.istio-system": &DNSNameEntry{
+					ServiceName:   "istio-galley-service-account.istio-system",
+					CustomDomains: []string{"istio-galley-ilb.istio-system.svc.us1.dog"},
+				},
+			},
+		},
+		"Valid input 4": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns: "istio-system",
+			dnsNames: "istio-galley-service-account.istio-system:istio-galley-ilb.istio-system.svc.us1," +
+				"istio-galley-service-account.istio-system:istio-galley-ilb.istio-system.svc.us2",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account.istio-system": &DNSNameEntry{
+					ServiceName:   "istio-galley-service-account.istio-system",
+					CustomDomains: []string{"istio-galley-ilb.istio-system.svc.us1", "istio-galley-ilb.istio-system.svc.us2"},
+				},
+			},
+		},
+		"Invalid input 1": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns: "istio-system",
+			dnsNames: "istio-galley-service-account.istio-systemistio-galley-ilb.istio-system.svc.us1," +
+				"istio-galley-service-account.istio-system:istio-galley-ilb.istio-system.svc.us2",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account.istio-system": &DNSNameEntry{
+					ServiceName:   "istio-galley-service-account.istio-system",
+					CustomDomains: []string{"istio-galley-ilb.istio-system.svc.us2"},
+				},
+			},
+		},
+		"Invalid input 2": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns:       "istio-system",
+			dnsNames: "istio-galley-service-account.istiley-ilb.is,istio-galley-service-account.istio-sysgalley-ilb.istio-system.svc.us2",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+			},
+		},
+		"Valid input 5": {
+			sas: []string{
+				"istio-sidecar-injector-service-account",
+				"istio-galley-service-account",
+			},
+			sns: []string{
+				"istio-sidecar-injector",
+				"istio-galley",
+			},
+			ns:       "istio-system",
+			dnsNames: "istio-mixer.istio-system:istio-mixer.istio-system.us1,istio-galley-service-account.istio-system:istio-galley-ilb.istio-system.svc.us2",
+			expected: map[string]*DNSNameEntry{
+				"istio-sidecar-injector-service-account": &DNSNameEntry{
+					ServiceName: "istio-sidecar-injector",
+					Namespace:   "istio-system",
+				},
+				"istio-galley-service-account": &DNSNameEntry{
+					ServiceName: "istio-galley",
+					Namespace:   "istio-system",
+				},
+				"istio-mixer.istio-system": &DNSNameEntry{
+					ServiceName:   "istio-mixer.istio-system",
+					CustomDomains: []string{"istio-mixer.istio-system.us1"},
+				},
+				"istio-galley-service-account.istio-system": &DNSNameEntry{
+					ServiceName:   "istio-galley-service-account.istio-system",
+					CustomDomains: []string{"istio-galley-ilb.istio-system.svc.us2"},
+				},
+			},
+		},
+	}
+
+	for k, tc := range testCases {
+		result := ConstructCustomDNSNames(tc.sas, tc.sns, tc.ns, tc.dnsNames)
+		if len(result) != len(tc.expected) {
+			t.Errorf("Test case %s: expected entry %v, actual %v", k, len(tc.expected), len(result))
+		}
+		for key, value := range tc.expected {
+			compareDNSNameEntry(t, k, value, result[key])
+		}
+	}
+}
+
+func compareDNSNameEntry(t *testing.T, k string, expected *DNSNameEntry, result *DNSNameEntry) {
+	if strings.Compare(expected.ServiceName, result.ServiceName) != 0 {
+		t.Errorf("Failure in test case %s: expected servicename %v, result %v", k, expected.ServiceName, result.ServiceName)
+	}
+	if strings.Compare(expected.Namespace, result.Namespace) != 0 {
+		t.Errorf("Failure in test case %s: expected namespace %v, result %v", k, expected.Namespace, result.Namespace)
+	}
+	if len(expected.CustomDomains) != len(result.CustomDomains) {
+		t.Errorf("Failure in test case %s: expected customDomain length %d, result %d", k, len(expected.CustomDomains), len(result.CustomDomains))
+	} else {
+		for i, cd := range expected.CustomDomains {
+			if strings.Compare(cd, result.CustomDomains[i]) != 0 {
+				t.Errorf("Failure in test case %s: expected customDomain[%d] be %v, result %v", k, i, cd, result.CustomDomains[i])
+			}
+		}
+	}
+}

security/pkg/k8s/controller/workloadsecret.go

@@ -95,8 +95,8 @@ type SecretController struct {
 	// Whether the certificates are for CAs.
 	forCA bool
 
-	// DNS-enabled service account/service pair
-	dnsNames map[string]DNSNameEntry
+	// DNS-enabled serviceAccount.namespace to service pair
+	dnsNames map[string]*DNSNameEntry
 
 	// Controller and store for service account objects.
 	saController cache.Controller
@@ -112,7 +112,8 @@ type SecretController struct {
 // NewSecretController returns a pointer to a newly constructed SecretController instance.
 func NewSecretController(ca ca.CertificateAuthority, certTTL time.Duration,
 	gracePeriodRatio float32, minGracePeriod time.Duration, dualUse bool,
-	core corev1.CoreV1Interface, forCA bool, namespace string, dnsNames map[string]DNSNameEntry) (*SecretController, error) {
+	core corev1.CoreV1Interface, forCA bool, namespace string,
+	dnsNames map[string]*DNSNameEntry) (*SecretController, error) {
 
 	if gracePeriodRatio < 0 || gracePeriodRatio > 1 {
 		return nil, fmt.Errorf("grace period ratio %f should be within [0, 1]", gracePeriodRatio)
@@ -312,13 +313,14 @@ func (sc *SecretController) generateKeyAndCert(saName string, saNamespace string
 				id += "," + fmt.Sprintf("%s.%s", e.ServiceName, e.Namespace)
 			}
 		}
-		// Custom overrides using CLI
+		// Custom adds more DNS entries using CLI
 		if e, ok := sc.dnsNames[saName+"."+saNamespace]; ok {
 			for _, d := range e.CustomDomains {
 				id += "," + d
 			}
 		}
 	}
+
 	options := util.CertOptions{
 		Host:       id,
 		RSAKeySize: keySize,

security/pkg/k8s/controller/workloadsecret_test.go

@@ -155,8 +155,8 @@ func TestSecretController(t *testing.T) {
 			})
 		}
 
-		webhooks := map[string]DNSNameEntry{
-			sidecarInjectorSvcAccount: {
+		webhooks := map[string]*DNSNameEntry{
+			sidecarInjectorSvcAccount: &DNSNameEntry{
 				ServiceName: sidecarInjectorSvc,
 				Namespace:   "test-ns",
 			},
@@ -204,7 +204,7 @@ func TestSecretContent(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	controller, err := NewSecretController(createFakeCA(), defaultTTL,
 		defaultGracePeriodRatio, defaultMinGracePeriod, false, client.CoreV1(), false,
-		metav1.NamespaceAll, map[string]DNSNameEntry{})
+		metav1.NamespaceAll, map[string]*DNSNameEntry{})
 	if err != nil {
 		t.Errorf("Failed to create secret controller: %v", err)
 	}