diff --git a/coverage-baseline.txt b/coverage-baseline.txt index 7f599b52bc..cdc77c6f4a 100644 --- a/coverage-baseline.txt +++ b/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 diff --git a/pkg/controller/metrics/metrics.go b/pkg/controller/metrics/metrics.go index 099d61f6e0..e643711684 100644 --- a/pkg/controller/metrics/metrics.go +++ b/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,7 +159,18 @@ 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") @@ -156,6 +178,20 @@ func (m *Metrics) Start(ctx context.Context) error { 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{ diff --git a/pkg/controller/metrics/metrics_test.go b/pkg/controller/metrics/metrics_test.go index 6abb135c64..b3dcf0cd09 100644 --- a/pkg/controller/metrics/metrics_test.go +++ b/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) +}