CMP-4601: metrics: wait for the serving cert before serving TLS

coverage-baseline.txt

@@ -10,7 +10,7 @@ github.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremedia
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancescan	43.8
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancesuite	23.7
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/customrule	65.7
-github.com/ComplianceAsCode/compliance-operator/pkg/controller/metrics	60.7
+github.com/ComplianceAsCode/compliance-operator/pkg/controller/metrics	62.2
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/profilebundle	14.5
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/scansettingbinding	53.6
 github.com/ComplianceAsCode/compliance-operator/pkg/controller/tailoredprofile	59.5

pkg/controller/metrics/metrics.go

@@ -5,12 +5,15 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/http"
+	"os"
+	"time"
 
 	"github.com/go-logr/logr"
 	libgocrypto "github.com/openshift/library-go/pkg/crypto"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"k8s.io/apimachinery/pkg/util/wait"
 	ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/ComplianceAsCode/compliance-operator/pkg/apis/compliance/v1alpha1"
@@ -38,6 +41,14 @@ const (
 	MetricsAddrListen            = ":8585"
 )
 
+// The metrics serving certificate is mounted from the secret minted by the
+// OpenShift service-ca operator. Package-level vars so tests can redirect
+// them to a temporary directory.
+var (
+	servingCertFile = "/var/run/secrets/serving-cert/tls.crt"
+	servingKeyFile  = "/var/run/secrets/serving-cert/tls.key"
+)
+
 const (
 	METRIC_STATE_COMPLIANT = iota
 	METRIC_STATE_NON_COMPLIANT
@@ -148,14 +159,39 @@ func (m *Metrics) Start(ctx context.Context) error {
 		TLSConfig: tlsConfig,
 	}
 
-	err := server.ListenAndServeTLS("/var/run/secrets/serving-cert/tls.crt", "/var/run/secrets/serving-cert/tls.key")
+	// The serving cert is minted asynchronously by the service-ca operator
+	// once the metrics Service exists, and this runnable can win that race
+	// on a fresh deployment. A one-shot ListenAndServeTLS would then fail
+	// and leave the endpoint dead for the life of the pod (the error below
+	// is deliberately not propagated), so wait for the files to show up.
+	if err := m.waitForServingCert(ctx, 5*time.Second, 5*time.Minute); err != nil {
+		// unhandled on purpose, we don't want to exit the operator.
+		m.log.Error(err, "Metrics service failed: serving cert never became available")
+		return nil
+	}
+
+	err := server.ListenAndServeTLS(servingCertFile, servingKeyFile)
 	if err != nil {
 		// unhandled on purpose, we don't want to exit the operator.
 		m.log.Error(err, "Metrics service failed")
 	}
 	return nil
 }
 
+// waitForServingCert polls until both the serving certificate and key exist,
+// the context is cancelled, or the timeout expires.
+func (m *Metrics) waitForServingCert(ctx context.Context, interval, timeout time.Duration) error {
+	return wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(context.Context) (bool, error) {
+		for _, f := range []string{servingCertFile, servingKeyFile} {
+			if _, err := os.Stat(f); err != nil {
+				m.log.Info("Waiting for the metrics serving cert", "file", f)
+				return false, nil
+			}
+		}
+		return true, nil
+	})
+}
+
 // IncComplianceScanStatus also increments error if necessary
 func (m *Metrics) IncComplianceScanStatus(name string, status v1alpha1.ComplianceScanStatus) {
 	m.metrics.metricComplianceScanStatus.With(prometheus.Labels{

pkg/controller/metrics/metrics_test.go

@@ -17,8 +17,12 @@ limitations under the License.
 package metrics
 
 import (
+	"context"
 	"errors"
+	"os"
+	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
@@ -186,3 +190,35 @@ func TestComplianceOperatorMetrics(t *testing.T) {
 		tc.then(sut)
 	}
 }
+
+func TestWaitForServingCert(t *testing.T) {
+	dir := t.TempDir()
+	certFile := filepath.Join(dir, "tls.crt")
+	keyFile := filepath.Join(dir, "tls.key")
+	origCert, origKey := servingCertFile, servingKeyFile
+	servingCertFile, servingKeyFile = certFile, keyFile
+	defer func() { servingCertFile, servingKeyFile = origCert, origKey }()
+
+	m := New()
+
+	// Times out while the cert and key are missing.
+	err := m.waitForServingCert(context.Background(), 5*time.Millisecond, 50*time.Millisecond)
+	require.Error(t, err)
+
+	// Recovers when the cert is minted late, as service-ca does on a
+	// fresh deployment.
+	go func() {
+		time.Sleep(20 * time.Millisecond)
+		_ = os.WriteFile(certFile, []byte("cert"), 0o600)
+		_ = os.WriteFile(keyFile, []byte("key"), 0o600)
+	}()
+	err = m.waitForServingCert(context.Background(), 5*time.Millisecond, 2*time.Second)
+	require.NoError(t, err)
+
+	// Honors context cancellation instead of waiting out the timeout.
+	require.NoError(t, os.Remove(certFile))
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	err = m.waitForServingCert(ctx, 5*time.Millisecond, time.Minute)
+	require.Error(t, err)
+}