pilot: ensure initialization of secure grpc server also when serving … (

#42248) (#43352) * pilot: ensure initialization of secure grpc server also when serving certificates are loaded from non-custom locations * pilot: ensure initialization of secure grpc server also when serving certificates are loaded from non-custom locations Co-authored-by: Si Mon <85333972+siiimooon@users.noreply.github.com>
istio · Feb 15, 2023 · dff997a · dff997a
1 parent 934db1f
commit dff997a
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 31 deletions.
diff --git a/pilot/pkg/bootstrap/server.go b/pilot/pkg/bootstrap/server.go
@@ -1012,7 +1012,8 @@ func getDNSNames(args *PilotArgs, host string) []string {
 
 // createPeerCertVerifier creates a SPIFFE certificate verifier with the current istiod configuration.
 func (s *Server) createPeerCertVerifier(tlsOptions TLSOptions) (*spiffe.PeerCertVerifier, error) {
-	if tlsOptions.CaCertFile == "" && s.CA == nil && features.SpiffeBundleEndpoints == "" && !s.isCADisabled() {
+	customTLSCertsExists, _, _, _ := hasCustomTLSCerts(tlsOptions)
+	if !customTLSCertsExists && s.CA == nil && features.SpiffeBundleEndpoints == "" && !s.isCADisabled() {
 		// Running locally without configured certs - no TLS mode
 		return nil, nil
 	}

diff --git a/pilot/pkg/bootstrap/server_test.go b/pilot/pkg/bootstrap/server_test.go
@@ -104,14 +104,15 @@ func TestNewServerCertInit(t *testing.T) {
 	tlsArgcaCertFile := filepath.Join(tlsArgCertsDir, "ca-cert.pem")
 
 	cases := []struct {
-		name         string
-		FSCertsPaths TLSFSLoadPaths
-		tlsOptions   *TLSOptions
-		enableCA     bool
-		certProvider string
-		expNewCert   bool
-		expCert      []byte
-		expKey       []byte
+		name                      string
+		FSCertsPaths              TLSFSLoadPaths
+		tlsOptions                *TLSOptions
+		enableCA                  bool
+		certProvider              string
+		expNewCert                bool
+		expCert                   []byte
+		expKey                    []byte
+		expSecureDiscoveryService bool
 	}{
 		{
 			name:         "Load from existing DNS cert",
@@ -121,11 +122,12 @@ func TestNewServerCertInit(t *testing.T) {
 				KeyFile:    tlsArgkeyFile,
 				CaCertFile: tlsArgcaCertFile,
 			},
-			enableCA:     false,
-			certProvider: constants.CertProviderKubernetes,
-			expNewCert:   false,
-			expCert:      testcerts.ServerCert,
-			expKey:       testcerts.ServerKey,
+			enableCA:                  false,
+			certProvider:              constants.CertProviderKubernetes,
+			expNewCert:                false,
+			expCert:                   testcerts.ServerCert,
+			expKey:                    testcerts.ServerKey,
+			expSecureDiscoveryService: true,
 		},
 		{
 			name:         "Create new DNS cert using Istiod",
@@ -135,11 +137,12 @@ func TestNewServerCertInit(t *testing.T) {
 				KeyFile:    "",
 				CaCertFile: "",
 			},
-			enableCA:     true,
-			certProvider: constants.CertProviderIstiod,
-			expNewCert:   true,
-			expCert:      []byte{},
-			expKey:       []byte{},
+			enableCA:                  true,
+			certProvider:              constants.CertProviderIstiod,
+			expNewCert:                true,
+			expCert:                   []byte{},
+			expKey:                    []byte{},
+			expSecureDiscoveryService: true,
 		},
 		{
 			name:         "No DNS cert created because CA is disabled",
@@ -158,12 +161,13 @@ func TestNewServerCertInit(t *testing.T) {
 				constants.DefaultPilotTLSKey,
 				constants.DefaultPilotTLSCaCert,
 			},
-			tlsOptions:   &TLSOptions{},
-			enableCA:     false,
-			certProvider: constants.CertProviderNone,
-			expNewCert:   false,
-			expCert:      testcerts.ServerCert,
-			expKey:       testcerts.ServerKey,
+			tlsOptions:                &TLSOptions{},
+			enableCA:                  false,
+			certProvider:              constants.CertProviderNone,
+			expNewCert:                false,
+			expCert:                   testcerts.ServerCert,
+			expKey:                    testcerts.ServerKey,
+			expSecureDiscoveryService: true,
 		},
 		{
 			name: "DNS cert loaded from known location, even if CA is Disabled, with a fallback CA path",
@@ -172,12 +176,13 @@ func TestNewServerCertInit(t *testing.T) {
 				constants.DefaultPilotTLSKey,
 				constants.DefaultPilotTLSCaCertAlternatePath,
 			},
-			tlsOptions:   &TLSOptions{},
-			enableCA:     false,
-			certProvider: constants.CertProviderNone,
-			expNewCert:   false,
-			expCert:      testcerts.ServerCert,
-			expKey:       testcerts.ServerKey,
+			tlsOptions:                &TLSOptions{},
+			enableCA:                  false,
+			certProvider:              constants.CertProviderNone,
+			expNewCert:                false,
+			expCert:                   testcerts.ServerCert,
+			expKey:                    testcerts.ServerKey,
+			expSecureDiscoveryService: true,
 		},
 		{
 			name:         "No cert provider",
@@ -249,6 +254,12 @@ func TestNewServerCertInit(t *testing.T) {
 					}
 				}
 			}
+
+			if c.expSecureDiscoveryService {
+				if s.secureGrpcServer == nil {
+					t.Errorf("Istiod secure grpc server was not started.")
+				}
+			}
 		})
 	}
 }

diff --git a/releasenotes/notes/42248.yaml b/releasenotes/notes/42248.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: installation
+issue:
+  - 42249
+releaseNotes:
+  - |
+    **Fixed** initialization of secure gRPC server of Pilot, when serving certificates are provided in default location.