diff --git a/controllers/ssp_controller.go b/controllers/ssp_controller.go index fb09c150e..fc1b49135 100644 --- a/controllers/ssp_controller.go +++ b/controllers/ssp_controller.go @@ -19,7 +19,6 @@ package controllers import ( "context" "fmt" - "os" "reflect" "strconv" "strings" @@ -148,7 +147,6 @@ func (r *sspReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ct // Error reading the object - requeue the request. return ctrl.Result{}, err } - restartNeeded := r.isRestartNeeded(instance) r.clearCacheIfNeeded(instance) sspRequest := &common.Request{ @@ -163,10 +161,6 @@ func (r *sspReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ct CrdList: r.crdList, } - if restartNeeded { - r.restart(sspRequest) - } - if !isInitialized(sspRequest.Instance) { err := initialize(sspRequest) // No need to requeue here, because @@ -235,25 +229,6 @@ func (r *sspReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ct return ctrl.Result{}, nil } -func (r *sspReconciler) isRestartNeeded(sspObj *ssp.SSP) bool { - if reflect.DeepEqual(r.lastSspSpec, ssp.SSPSpec{}) { - return false - } - if !reflect.DeepEqual(r.lastSspSpec.TLSSecurityProfile, sspObj.Spec.TLSSecurityProfile) { - return true - } - return false -} - -func (r *sspReconciler) restart(request *common.Request) { - r.log.Info("TLSSecurityProfile changed, restarting") - err := setSspResourceDeploying(request) - if err != nil { - r.log.Info("Error at setSspResourceDeploying", "Error: ", err) - } - os.Exit(0) -} - func (r *sspReconciler) clearCacheIfNeeded(sspObj *ssp.SSP) { if !reflect.DeepEqual(r.lastSspSpec, sspObj.Spec) { r.subresourceCache = common.VersionCache{} diff --git a/internal/common/crypto_policy.go b/internal/common/crypto_policy.go index 3283626e4..fffe64e7a 100644 --- a/internal/common/crypto_policy.go +++ b/internal/common/crypto_policy.go @@ -1,16 +1,12 @@ package common import ( - "context" "crypto/tls" "fmt" "github.com/go-logr/logr" ocpv1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/crypto" - ssp "kubevirt.io/ssp-operator/api/v1beta2" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ) type SSPTLSOptions struct { @@ -65,8 +61,8 @@ func GetKnownCipherId(IANACipherName string) (uint16, bool) { return 0, false } -func (s *SSPTLSOptions) CipherIDs(logger *logr.Logger) (cipherSuites []uint16) { - for _, cipherName := range crypto.OpenSSLToIANACipherSuites(s.OpenSSLCipherNames) { +func CipherIDs(names []string, logger *logr.Logger) (cipherSuites []uint16) { + for _, cipherName := range crypto.OpenSSLToIANACipherSuites(names) { if id, ok := GetKnownCipherId(cipherName); ok { cipherSuites = append(cipherSuites, id) } else { @@ -78,33 +74,6 @@ func (s *SSPTLSOptions) CipherIDs(logger *logr.Logger) (cipherSuites []uint16) { return } -func GetSspTlsOptions(ctx context.Context) (*SSPTLSOptions, error) { - setupLog := ctrl.Log.WithName("setup tls options") - restConfig := ctrl.GetConfigOrDie() - apiReader, err := client.New(restConfig, client.Options{Scheme: Scheme}) - if err != nil { - return nil, err - } - - var sspList ssp.SSPList - if err := apiReader.List(ctx, &sspList, &client.ListOptions{}); err != nil { - return nil, err - } - - if len(sspList.Items) == 0 { - setupLog.Info("SSP CR not found, will use default tlsProfile") - return &SSPTLSOptions{}, nil - } - - ssp := sspList.Items[0] - - sspTLSOptions, err := NewSSPTLSOptions(ssp.Spec.TLSSecurityProfile, &setupLog) - if err != nil { - return nil, err - } - return sspTLSOptions, nil -} - func selectCipherSuitesAndMinTLSVersion(profile *ocpv1.TLSSecurityProfile) ([]string, ocpv1.TLSProtocolVersion) { if profile == nil { return nil, "" diff --git a/internal/template-validator/tlsinfo/tlsinfo.go b/internal/template-validator/tlsinfo/tlsinfo.go index 7f6dea818..cf5610719 100644 --- a/internal/template-validator/tlsinfo/tlsinfo.go +++ b/internal/template-validator/tlsinfo/tlsinfo.go @@ -192,7 +192,7 @@ func (ti *TLSInfo) CreateTlsConfig() *tls.Config { } if !ti.sspTLSOptions.IsEmpty() { - tlsConfig.CipherSuites = ti.sspTLSOptions.CipherIDs(nil) + tlsConfig.CipherSuites = common.CipherIDs(ti.sspTLSOptions.OpenSSLCipherNames, nil) minVersion, err := ti.sspTLSOptions.MinTLSVersionId() if err != nil { panic(fmt.Sprintf("TLS Configuration broken, min version misconfigured %v", err)) diff --git a/main.go b/main.go index cff2ea6b4..cca28a435 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "crypto/tls" "flag" "fmt" @@ -28,13 +29,18 @@ import ( // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" + ocpconfigv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/crypto" "github.com/prometheus/client_golang/prometheus/promhttp" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics" "sigs.k8s.io/controller-runtime/pkg/webhook" + ssp "kubevirt.io/ssp-operator/api/v1beta2" "kubevirt.io/ssp-operator/controllers" "kubevirt.io/ssp-operator/internal/common" common_templates "kubevirt.io/ssp-operator/internal/operands/common-templates" @@ -66,53 +72,121 @@ const ( webhookPort = 9443 ) -func runPrometheusServer(metricsAddr string, tlsOptions common.SSPTLSOptions) error { +// This callback executes on each client call returning a new config to be used +// please be aware that the APIServer is using http keepalive so this is going to +// be executed only after a while for fresh connections and not on existing ones +func getConfigForClient(ctx context.Context, cfg *tls.Config, cache cache.Cache) (*tls.Config, error) { + var sspList ssp.SSPList + err := cache.List(ctx, &sspList) + if err != nil { + return nil, err + } + + if len(sspList.Items) == 0 || sspList.Items[0].Spec.TLSSecurityProfile == nil { + cfg.MinVersion = crypto.DefaultTLSVersion() + cfg.CipherSuites = nil + return cfg, nil + } + + tlsProfile := sspList.Items[0].Spec.TLSSecurityProfile + if tlsProfile.Type == ocpconfigv1.TLSProfileCustomType { + minVersion, err := crypto.TLSVersion(string(tlsProfile.Custom.MinTLSVersion)) + if err != nil { + return nil, err + } + cfg.MinVersion = minVersion + cfg.CipherSuites = common.CipherIDs(tlsProfile.Custom.Ciphers, &ctrl.Log) + return cfg, nil + } + + minVersion, err := crypto.TLSVersion(string(ocpconfigv1.TLSProfiles[tlsProfile.Type].MinTLSVersion)) + if err != nil { + return nil, err + } + cfg.MinVersion = minVersion + cfg.CipherSuites = common.CipherIDs(ocpconfigv1.TLSProfiles[tlsProfile.Type].Ciphers, &ctrl.Log) + + return cfg, nil +} + +type prometheusServer struct { + cache cache.Cache + certPath string + keyPath string + serverAddress string +} + +// NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates +// the prometheus server doesn't need leader election. +func (s *prometheusServer) NeedLeaderElection() bool { + return false +} + +func (s *prometheusServer) Start(ctx context.Context) error { setupLog.Info("Starting Prometheus metrics endpoint server with TLS") - metrics.Registry.MustRegister(common_templates.CommonTemplatesRestored) - metrics.Registry.MustRegister(common.SSPOperatorReconcileSucceeded) handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{}) mux := http.NewServeMux() mux.Handle("/metrics", handler) - minTlsVersion, err := tlsOptions.MinTLSVersionId() - if err != nil { - return err + server := &http.Server{ + Addr: s.serverAddress, + Handler: mux, } - tlsConfig := tls.Config{ - CipherSuites: tlsOptions.CipherIDs(&setupLog), - MinVersion: minTlsVersion, + certWatcher, err := certwatcher.New(s.certPath, s.keyPath) + if err != nil { + return err } - server := http.Server{ - Addr: metricsAddr, - Handler: mux, - TLSConfig: &tlsConfig, - } + go func() { + // TODO: change context, so it can be closed when + // this function returns an error + if err := certWatcher.Start(ctx); err != nil { + setupLog.Error(err, "certificate watcher error") + } + }() + idleConnsClosed := make(chan struct{}) go func() { - err := server.ListenAndServeTLS(path.Join(sdkTLSDir, sdkTLSCrt), path.Join(sdkTLSDir, sdkTLSKey)) - if err != nil { - setupLog.Error(err, "Failed to start Prometheus metrics endpoint server") + // TODO: make sure that the goroutine finishes when + // this function returns an error + <-ctx.Done() + setupLog.Info("shutting down Prometheus metrics server") + + if err := server.Shutdown(context.Background()); err != nil { + setupLog.Error(err, "error shutting down the HTTP server") } + close(idleConnsClosed) }() + + server.TLSConfig = s.getPrometheusTLSConfig(ctx, certWatcher) + + if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && err != http.ErrServerClosed { + setupLog.Error(err, "Failed to start Prometheus metrics endpoint server") + return err + } + + <-idleConnsClosed return nil } -func getWebhookServer(sspTLSOptions common.SSPTLSOptions) *webhook.Server { - // If TLSSecurityProfile is empty, we want to return nil so that the default - // webhook server configuration is used. - if sspTLSOptions.IsEmpty() { - return nil +func (s *prometheusServer) getPrometheusTLSConfig(ctx context.Context, certWatcher *certwatcher.CertWatcher) *tls.Config { + return &tls.Config{ + GetConfigForClient: func(_ *tls.ClientHelloInfo) (*tls.Config, error) { + cfg := &tls.Config{} + cfg.GetCertificate = certWatcher.GetCertificate + return getConfigForClient(ctx, cfg, s.cache) + }, } +} - tlsCfgFunc := func(cfg *tls.Config) { - cfg.CipherSuites = sspTLSOptions.CipherIDs(&setupLog) - setupLog.Info("Configured ciphers", "ciphers", cfg.CipherSuites) +func newPrometheusServer(metricsAddr string, cache cache.Cache) *prometheusServer { + return &prometheusServer{ + certPath: path.Join(sdkTLSDir, sdkTLSCrt), + keyPath: path.Join(sdkTLSDir, sdkTLSKey), + cache: cache, + serverAddress: metricsAddr, } - - funcs := []func(*tls.Config){tlsCfgFunc} - return &webhook.Server{Port: webhookPort, TLSMinVersion: sspTLSOptions.MinTLSVersion, TLSOpts: funcs} } func main() { @@ -127,6 +201,8 @@ func main() { opts := zap.Options{} opts.BindFlags(flag.CommandLine) flag.Parse() + metrics.Registry.MustRegister(common_templates.CommonTemplatesRestored) + metrics.Registry.MustRegister(common.SSPOperatorReconcileSucceeded) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -138,26 +214,24 @@ func main() { ctx := ctrl.SetupSignalHandler() - tlsOptions, err := common.GetSspTlsOptions(ctx) - if err != nil { - setupLog.Error(err, "Error while getting tls profile") - os.Exit(1) - } + var mgr ctrl.Manager - err = runPrometheusServer(metricsAddr, *tlsOptions) - if err != nil { - setupLog.Error(err, "unable to start prometheus server") - os.Exit(1) + getTLSOptsFunc := func(cfg *tls.Config) { + cfg.GetConfigForClient = func(_ *tls.ClientHelloInfo) (*tls.Config, error) { + return getConfigForClient(ctx, cfg, mgr.GetCache()) + } } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + mgr, err = ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: common.Scheme, MetricsBindAddress: "0", HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: leaderElectionID, - // If WebhookServer is set to nil, a default one will be created. - WebhookServer: getWebhookServer(*tlsOptions), + WebhookServer: &webhook.Server{ + Port: webhookPort, + TLSOpts: []func(*tls.Config){getTLSOptsFunc}, + }, }) if err != nil { @@ -171,10 +245,23 @@ func main() { os.Exit(1) } } + + metricsServer := newPrometheusServer(metricsAddr, mgr.GetCache()) + if err != nil { + setupLog.Error(err, "unable create Prometheus server") + os.Exit(1) + } + + if err := mgr.Add(metricsServer); err != nil { + setupLog.Error(err, "unable to set up metrics") + os.Exit(1) + } + if err := mgr.AddReadyzCheck("check", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up ready check") os.Exit(1) } + if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") os.Exit(1) diff --git a/tests/crypto_policy_test.go b/tests/crypto_policy_test.go index 16cc494f5..cfccec02a 100644 --- a/tests/crypto_policy_test.go +++ b/tests/crypto_policy_test.go @@ -7,14 +7,12 @@ import ( "fmt" "net" "net/http" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ocpv1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/crypto" - apps "k8s.io/api/apps/v1" core "k8s.io/api/core/v1" apiregv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" ssp "kubevirt.io/ssp-operator/api/v1beta2" @@ -106,19 +104,6 @@ var _ = Describe("Crypto Policy", func() { AfterEach(func() { strategy.RevertToOriginalSspCr() - - // Because of bug[1], the SSP operator will move to CrashLoopBackOff state, - // so we need to wait until it is running. - // [1] - https://bugzilla.redhat.com/show_bug.cgi?id=2151248 - Eventually(func(g Gomega) { - deployment := &apps.Deployment{} - err := apiClient.Get(ctx, client.ObjectKey{ - Name: strategy.GetSSPDeploymentName(), - Namespace: strategy.GetSSPDeploymentNameSpace(), - }, deployment) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(deployment.Status.ReadyReplicas).To(BeNumerically(">=", 1)) - }, env.Timeout(), time.Second).Should(Succeed()) }) Context("setting Crypto Policy", func() {