Fix health check fails when x509 files aren't configured (#263)

* Fix health check fails when x509 files or JWT bundle isn't configured

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>

* Omit status rather than "unconfigured"

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>

* Lint fixes

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>

* Cleanup

Signed-off-by: Faisal Memon <fymemon@yahoo.com>

* Avoid shadowing import

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>

* Add logic checking which SVIDs are configured

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>

---------

Signed-off-by: Keegan Witt <keeganwitt@gmail.com>
Signed-off-by: Faisal Memon <fymemon@yahoo.com>
Co-authored-by: Faisal Memon <fymemon@yahoo.com>

Author: keeganwitt

pkg/sidecar/sidecar.go

@@ -42,7 +42,7 @@ type Health struct {
 }
 
 type FileWriteStatuses struct {
-	X509WriteStatus string            `json:"x509_write_status"`
+	X509WriteStatus *string           `json:"x509_write_status,omitempty"`
 	JWTWriteStatus  map[string]string `json:"jwt_write_status"`
 }
 
@@ -59,20 +59,29 @@ func New(config *Config) *Sidecar {
 		certReadyChan: make(chan struct{}, 1),
 		health: Health{
 			FileWriteStatuses: FileWriteStatuses{
-				X509WriteStatus: writeStatusUnwritten,
-				JWTWriteStatus:  make(map[string]string),
+				JWTWriteStatus: make(map[string]string),
 			},
 		},
 	}
-	for _, jwtConfig := range config.JWTSVIDs {
-		jwtSVIDFilename := path.Join(config.CertDir, jwtConfig.JWTSVIDFilename)
-		sidecar.health.FileWriteStatuses.JWTWriteStatus[jwtSVIDFilename] = writeStatusUnwritten
-	}
-	jwtBundleFilePath := path.Join(config.CertDir, config.JWTBundleFilename)
-	sidecar.health.FileWriteStatuses.JWTWriteStatus[jwtBundleFilePath] = writeStatusUnwritten
+	sidecar.setupHealth()
 	return sidecar
 }
 
+func (s *Sidecar) setupHealth() {
+	if s.x509Enabled() {
+		writeStatus := writeStatusUnwritten
+		s.health.FileWriteStatuses.X509WriteStatus = &writeStatus
+	}
+	if s.jwtBundleEnabled() {
+		jwtBundleFilePath := path.Join(s.config.CertDir, s.config.JWTBundleFilename)
+		s.health.FileWriteStatuses.JWTWriteStatus[jwtBundleFilePath] = writeStatusUnwritten
+	}
+	for _, jwtConfig := range s.config.JWTSVIDs {
+		jwtSVIDFilename := path.Join(s.config.CertDir, jwtConfig.JWTSVIDFilename)
+		s.health.FileWriteStatuses.JWTWriteStatus[jwtSVIDFilename] = writeStatusUnwritten
+	}
+}
+
 // RunDaemon starts the main loop
 // Starts the workload API client to listen for new SVID updates
 // When a new SVID is received on the updateChan, the SVID certificates
@@ -184,10 +193,12 @@ func (s *Sidecar) updateCertificates(svidResponse *workloadapi.X509Context) {
 	s.config.Log.Debug("Updating X.509 certificates")
 	if err := disk.WriteX509Context(svidResponse, s.config.AddIntermediatesToBundle, s.config.IncludeFederatedDomains, s.config.CertDir, s.config.SVIDFileName, s.config.SVIDKeyFileName, s.config.SVIDBundleFileName, s.config.CertFileMode, s.config.KeyFileMode, s.config.Hint); err != nil {
 		s.config.Log.WithError(err).Error("Unable to dump bundle")
-		s.health.FileWriteStatuses.X509WriteStatus = writeStatusFailed
+		writeStatus := writeStatusFailed
+		s.health.FileWriteStatuses.X509WriteStatus = &writeStatus
 		return
 	}
-	s.health.FileWriteStatuses.X509WriteStatus = writeStatusWritten
+	writeStatus := writeStatusWritten
+	s.health.FileWriteStatuses.X509WriteStatus = &writeStatus
 	s.config.Log.Info("X.509 certificates updated")
 
 	if s.config.Cmd != "" {
@@ -460,7 +471,10 @@ func (s *Sidecar) CheckLiveness() bool {
 			return false
 		}
 	}
-	return s.health.FileWriteStatuses.X509WriteStatus != writeStatusFailed
+	if s.x509Enabled() && *s.health.FileWriteStatuses.X509WriteStatus == writeStatusFailed {
+		return false
+	}
+	return true
 }
 
 func (s *Sidecar) CheckReadiness() bool {
@@ -469,7 +483,7 @@ func (s *Sidecar) CheckReadiness() bool {
 			return false
 		}
 	}
-	return s.health.FileWriteStatuses.X509WriteStatus != writeStatusWritten
+	return !s.x509Enabled() || *s.health.FileWriteStatuses.X509WriteStatus == writeStatusWritten
 }
 
 func (s *Sidecar) GetHealth() Health {

pkg/sidecar/sidecar_test.go

@@ -96,7 +96,7 @@ func TestSidecar_RunDaemon(t *testing.T) {
 		certReadyChan: make(chan struct{}, 1),
 		health: Health{
 			FileWriteStatuses: FileWriteStatuses{
-				X509WriteStatus: writeStatusUnwritten,
+				X509WriteStatus: nil,
 				JWTWriteStatus:  make(map[string]string),
 			},
 		},
@@ -354,3 +354,85 @@ func TestSignalProcess(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 1, len(files))
 }
+
+func TestNew(t *testing.T) {
+	log, _ := test.NewNullLogger()
+	tmpdir := t.TempDir()
+	unwrittenStatus := writeStatusUnwritten
+	cases := []struct {
+		certDir                   string
+		svidFileName              string
+		svidKeyFileName           string
+		svidBundleFileName        string
+		jwtBundleFilename         string
+		jwtSVIDs                  []JWTConfig
+		expectedErr               string
+		expectedFileWriteStatuses FileWriteStatuses
+	}{
+		{
+			certDir:            tmpdir,
+			svidFileName:       "svid.pem",
+			svidKeyFileName:    "svid_key.pem",
+			svidBundleFileName: "svid_bundle.pem",
+			jwtBundleFilename:  "jwt_bundle.json",
+			jwtSVIDs: []JWTConfig{
+				{
+					JWTAudience:     "my-audience",
+					JWTSVIDFilename: "jwt_svid.jwt",
+				},
+			},
+			expectedFileWriteStatuses: FileWriteStatuses{
+				X509WriteStatus: &unwrittenStatus,
+				JWTWriteStatus: map[string]string{
+					path.Join(tmpdir, "jwt_bundle.json"): writeStatusUnwritten,
+					path.Join(tmpdir, "jwt_svid.jwt"):    writeStatusUnwritten,
+				},
+			},
+		},
+		{
+			jwtSVIDs: []JWTConfig{
+				{
+					JWTAudience:     "my-audience",
+					JWTSVIDFilename: "jwt_svid.jwt",
+				},
+			},
+			expectedFileWriteStatuses: FileWriteStatuses{
+				X509WriteStatus: nil,
+				JWTWriteStatus: map[string]string{
+					path.Join(tmpdir, "jwt_svid.jwt"): writeStatusUnwritten,
+				},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		c := c
+		t.Run("New Sidecar", func(t *testing.T) {
+			config := &Config{
+				CertDir:            tmpdir,
+				SVIDFileName:       c.svidFileName,
+				SVIDKeyFileName:    c.svidKeyFileName,
+				SVIDBundleFileName: c.svidBundleFileName,
+				JWTBundleFilename:  c.jwtBundleFilename,
+				JWTSVIDs:           c.jwtSVIDs,
+				Log:                log,
+			}
+			sidecar := New(config)
+			assert.NotNil(t, sidecar)
+			assert.Equal(t, config, sidecar.config)
+			assert.Equal(t, c.expectedFileWriteStatuses, sidecar.health.FileWriteStatuses)
+		})
+	}
+}
+
+func Test_CheckReadiness(t *testing.T) {
+	sidecar := Sidecar{
+		config: &Config{},
+		health: Health{
+			FileWriteStatuses: FileWriteStatuses{
+				X509WriteStatus: nil,
+			},
+		},
+	}
+	assert.True(t, sidecar.CheckReadiness())
+}