istio · rshriram · Apr 24, 2019 · Apr 11, 2019 · Apr 12, 2019 · Apr 15, 2019
diff --git a/pilot/cmd/pilot-agent/main.go b/pilot/cmd/pilot-agent/main.go
@@ -41,6 +41,7 @@ import (
 	"istio.io/istio/pilot/pkg/proxy"
 	"istio.io/istio/pilot/pkg/proxy/envoy"
 	"istio.io/istio/pilot/pkg/serviceregistry"
+	"istio.io/istio/pkg/bootstrap"
 	"istio.io/istio/pkg/cmd"
 	"istio.io/istio/pkg/collateral"
 	"istio.io/istio/pkg/features/pilot"
@@ -150,30 +151,18 @@ var (
 			role.TrustDomain = spiffe.GetTrustDomain()
 			log.Infof("Proxy role: %#v", role)
 
-			// Add cert paths as node metadata only if they differ from defaults
-			if tlsServerCertChain != model.DefaultCertChain {
-				role.Metadata[model.NodeMetadataTLSServerCertChain] = tlsServerCertChain
-			}
-			if tlsServerKey != model.DefaultKey {
-				role.Metadata[model.NodeMetadataTLSServerKey] = tlsServerKey
-			}
-			if tlsServerRootCert != model.DefaultRootCert {
-				role.Metadata[model.NodeMetadataTLSServerRootCert] = tlsServerRootCert
-			}
+			// Check for custom cert paths
+			tlsServerCertChain = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSServerCertChain, model.DefaultCertChain)
+			tlsServerKey = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSServerKey, model.DefaultKey)
+			tlsServerRootCert = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSServerRootCert, model.DefaultRootCert)
 
-			if tlsClientCertChain != model.DefaultCertChain {
-				role.Metadata[model.NodeMetadataTLSClientCertChain] = tlsClientCertChain
-			}
-			if tlsClientKey != model.DefaultKey {
-				role.Metadata[model.NodeMetadataTLSClientKey] = tlsClientKey
-			}
-			if tlsClientRootCert != model.DefaultRootCert {
-				role.Metadata[model.NodeMetadataTLSClientRootCert] = tlsClientRootCert
-			}
+			tlsClientCertChain = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSClientCertChain, model.DefaultCertChain)
+			tlsClientKey = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSClientKey, model.DefaultKey)
+			tlsClientRootCert = getEnvVarOrDefault(bootstrap.IstioMetaPrefix+model.NodeMetadataTLSClientRootCert, model.DefaultRootCert)
 
 			tlsCertsToWatch = []string{
 				tlsServerCertChain, tlsServerKey, tlsServerRootCert,
-				tlsClientCertChain, tlsClientKey, tlsClientCertChain,
+				tlsClientCertChain, tlsClientKey, tlsClientRootCert,
 			}
 
 			if role.Type == model.Ingress {
@@ -372,6 +361,15 @@ var (
 	}
 )
 
+// getEnvVarOrDefault gets the 'key' env var, or returns 'defaultVal' if the env var is missing or blank.
+func getEnvVarOrDefault(key, defaultVal string) string {
+	v := os.Getenv(key)
+	if v == "" {
+		return defaultVal
+	}
+	return v
+}
+
 func dedupeStrings(in []string) []string {
 	stringMap := map[string]bool{}
 	for _, c := range in {
@@ -520,22 +518,6 @@ func init() {
 	proxyCmd.PersistentFlags().BoolVar(&controlPlaneBootstrap, "controlPlaneBootstrap", true,
 		"Process bootstrap provided via templateFile to be used by control plane components.")
 
-	// server certs
-	proxyCmd.PersistentFlags().StringVar(&tlsServerCertChain, "tlsServerCertChain",
-		model.DefaultCertChain, "Absolute path to server cert-chain file used for istio mTLS")
-	proxyCmd.PersistentFlags().StringVar(&tlsServerKey, "tlsServerKey",
-		model.DefaultKey, "Absolute path to server private key file used for istio mTLS")
-	proxyCmd.PersistentFlags().StringVar(&tlsServerRootCert, "tlsServerRootCert",
-		model.DefaultRootCert, "Absolute path to server root cert file used for istio mTLS")
-
-	// client certs
-	proxyCmd.PersistentFlags().StringVar(&tlsClientCertChain, "tlsClientCertChain",
-		model.DefaultCertChain, "Absolute path to client cert-chain file used for istio mTLS")
-	proxyCmd.PersistentFlags().StringVar(&tlsClientKey, "tlsSClientKey",
-		model.DefaultKey, "Absolute path to client key file used for istio mTLS")
-	proxyCmd.PersistentFlags().StringVar(&tlsClientRootCert, "tlsClientRootCert",
-		model.DefaultRootCert, "Absolute path to client root cert file used for istio mTLS")
-
 	// Attach the Istio logging options to the command.
 	loggingOptions.AttachCobraFlags(rootCmd)
 

diff --git a/pilot/cmd/pilot-agent/main_test.go b/pilot/cmd/pilot-agent/main_test.go
@@ -178,3 +178,58 @@ func TestDedupeStrings(t *testing.T) {
 
 	g.Expect(actual).To(gomega.ConsistOf(expected))
 }
+
+func TestGetEnvVarOrDefault(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+
+	testCases := []struct {
+		name          string
+		key           string
+		envVarValue   *string
+		defaultValue  string
+		expectedValue string
+	}{
+		{
+			name:          "env var with value set",
+			key:           "ENV_VAR_WITH_VALUE",
+			envVarValue:   strPtr("non-default value"),
+			defaultValue:  "default",
+			expectedValue: "non-default value",
+		},
+		{
+			name:          "env var with blank value",
+			key:           "ENV_VAR_WITH_BLANK_VALUE",
+			envVarValue:   strPtr(""),
+			defaultValue:  "default",
+			expectedValue: "default",
+		},
+		{
+			name:          "env var with unset value",
+			key:           "ENV_VAR_WITH_UNSET_VALUE",
+			envVarValue:   nil,
+			defaultValue:  "default",
+			expectedValue: "default",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.envVarValue != nil {
+				os.Setenv(tc.key, *tc.envVarValue)
+			} else {
+				os.Unsetenv(tc.key)
+			}
+
+			actual := getEnvVarOrDefault(tc.key, tc.defaultValue)
+			g.Expect(actual).To(gomega.Equal(tc.expectedValue))
+
+			if tc.envVarValue != nil {
+				os.Unsetenv(tc.key)
+			}
+		})
+	}
+}
+
+func strPtr(s string) *string {
+	return &s
+}
diff --git a/pilot/pkg/networking/core/v1alpha3/cluster.go b/pilot/pkg/networking/core/v1alpha3/cluster.go
@@ -893,7 +893,7 @@ func applyUpstreamTLSSettings(env *model.Environment, cluster *apiv2.Cluster, tl
 	if len(tls.CaCertificates) != 0 {
 		trustedCa = &core.DataSource{
 			Specifier: &core.DataSource_Filename{
-				Filename: tls.CaCertificates,
+				Filename: model.GetOrDefaultFromMap(metadata, model.NodeMetadataTLSClientRootCert, tls.CaCertificates),
 			},
 		}
 	}
@@ -943,12 +943,12 @@ func applyUpstreamTLSSettings(env *model.Environment, cluster *apiv2.Cluster, tl
 				{
 					CertificateChain: &core.DataSource{
 						Specifier: &core.DataSource_Filename{
-							Filename: tls.ClientCertificate,
+							Filename: model.GetOrDefaultFromMap(metadata, model.NodeMetadataTLSClientCertChain, tls.ClientCertificate),
 						},
 					},
 					PrivateKey: &core.DataSource{
 						Specifier: &core.DataSource_Filename{
-							Filename: tls.PrivateKey,
+							Filename: model.GetOrDefaultFromMap(metadata, model.NodeMetadataTLSClientKey, tls.PrivateKey),
 						},
 					},
 				},

diff --git a/pilot/pkg/networking/core/v1alpha3/cluster_test.go b/pilot/pkg/networking/core/v1alpha3/cluster_test.go
@@ -378,6 +378,72 @@ func TestBuildSidecarClustersWithIstioMutualAndSNI(t *testing.T) {
 	g.Expect(cluster.TlsContext.GetSni()).To(Equal("outbound_.8080_.foobar_.foo.example.org"))
 }
 
+func TestBuildClustersWithMutualTlsAndNodeMetadataCertfileOverrides(t *testing.T) {
+	expectedClientKeyPath := "/clientKeyFromNodeMetadata.pem"
+	expectedClientCertPath := "/clientCertFromNodeMetadata.pem"
+	expectedRootCertPath := "/clientRootCertFromNodeMetadata.pem"
+
+	g := NewGomegaWithT(t)
+
+	envoyMetadata := map[string]string{
+		model.NodeMetadataTLSClientCertChain: expectedClientCertPath,
+		model.NodeMetadataTLSClientKey:       expectedClientKeyPath,
+		model.NodeMetadataTLSClientRootCert:  expectedRootCertPath,
+	}
+
+	destRule := &networking.DestinationRule{
+		Host: "*.example.org",
+		TrafficPolicy: &networking.TrafficPolicy{
+			Tls: &networking.TLSSettings{
+				Mode:              networking.TLSSettings_MUTUAL,
+				ClientCertificate: "/defaultCert.pem",
+				PrivateKey:        "/defaultPrivateKey.pem",
+				CaCertificates:    "/defaultCaCert.pem",
+			},
+		},
+		Subsets: []*networking.Subset{
+			{
+				Name:   "foobar",
+				Labels: map[string]string{"foo": "bar"},
+				TrafficPolicy: &networking.TrafficPolicy{
+					PortLevelSettings: []*networking.TrafficPolicy_PortTrafficPolicy{
+						{
+							Port: &networking.PortSelector{
+								Port: &networking.PortSelector_Number{Number: 8080},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	clusters, err := buildTestClustersWithProxyMetadata("foo.example.org", model.ClientSideLB, model.SidecarProxy,
+		nil, testMesh, destRule, envoyMetadata)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	g.Expect(clusters).To(HaveLen(5))
+
+	expectedOutboundClusterCount := 2
+	actualOutboundClusterCount := 0
+
+	for _, c := range clusters {
+		if strings.Contains(c.Name, "outbound") {
+			actualOutboundClusterCount++
+			tlsContext := c.TlsContext.CommonTlsContext
+			g.Expect(tlsContext).NotTo(BeNil())
+
+			tlsCerts := tlsContext.TlsCertificates
+			g.Expect(tlsCerts).To(HaveLen(1))
+
+			g.Expect(tlsCerts[0].PrivateKey.GetFilename()).To(Equal(expectedClientKeyPath))
+			g.Expect(tlsCerts[0].CertificateChain.GetFilename()).To(Equal(expectedClientCertPath))
+			g.Expect(tlsContext.GetValidationContext().TrustedCa.GetFilename()).To(Equal(expectedRootCertPath))
+		}
+	}
+	g.Expect(actualOutboundClusterCount).To(Equal(expectedOutboundClusterCount))
+}
+
 func buildSniTestClusters(sniValue string) ([]*apiv2.Cluster, error) {
 	return buildSniTestClustersWithMetadata(sniValue, make(map[string]string))
 }