diff --git a/.golangci.yaml b/.golangci.yaml index 6e3a796b0c..f3afd53d92 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -54,6 +54,8 @@ linters-settings: no-unaliased: true no-extra-aliases: true alias: + - pkg: github.com/cert-manager/cert-manager/pkg/client/clientset/versioned + alias: certmanagerclientset - pkg: github.com/kyma-project/template-operator/api/v1alpha1 alias: templatev1alpha1 - pkg: github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1 diff --git a/cmd/main.go b/cmd/main.go index b2e0dfd446..e805077093 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -36,7 +36,6 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" machineryruntime "k8s.io/apimachinery/pkg/runtime" machineryutilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/client-go/kubernetes" k8sclientscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/util/workqueue" @@ -204,7 +203,7 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma go cleanupStoredVersions(flagVar.DropCrdStoredVersionMap, mgr, setupLog) go scheduleMetricsCleanup(kymaMetrics, flagVar.MetricsCleanupIntervalInMinutes, mgr, setupLog) - go setupIstioGatewaySecretRotation(config, kcpClient, setupLog) + startCAWatch(config, setupLog) if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") @@ -212,11 +211,12 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma } } -func setupIstioGatewaySecretRotation(config *rest.Config, kcpClient *remote.ConfigAndClient, setupLog logr.Logger) { - kcpClientset := kubernetes.NewForConfigOrDie(config) - gatewaySecretHandler := gatewaysecret.NewGatewaySecretHandler(kcpClient) - - gatewaySecretHandler.StartRootCertificateWatch(kcpClientset, setupLog) +func startCAWatch(config *rest.Config, setupLog logr.Logger) { + if err := gatewaysecret.NewGatewaySecretHandler(config, setupLog). + StartRootCertificateWatch(); err != nil { + setupLog.Error(err, "unable to start root certificate watch") + os.Exit(bootstrapFailedExitCode) + } } func addHealthChecks(mgr manager.Manager, setupLog logr.Logger) { diff --git a/config/rbac/namespace_bindings/watcher_certmanager_role.yaml b/config/rbac/namespace_bindings/watcher_certmanager_role.yaml index e6bc636d36..05b512e6a5 100644 --- a/config/rbac/namespace_bindings/watcher_certmanager_role.yaml +++ b/config/rbac/namespace_bindings/watcher_certmanager_role.yaml @@ -13,6 +13,7 @@ rules: verbs: - list - watch + - get - create - delete - update diff --git a/pkg/gatewaysecret/certmanagerclient.go b/pkg/gatewaysecret/certmanagerclient.go new file mode 100644 index 0000000000..6313066985 --- /dev/null +++ b/pkg/gatewaysecret/certmanagerclient.go @@ -0,0 +1,38 @@ +package gatewaysecret + +import ( + "context" + "fmt" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanagerclientset "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned" + apicorev1 "k8s.io/api/core/v1" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" +) + +type CertManagerClient struct { + clientset certmanagerclientset.Clientset +} + +func NewCertManagerClient(config *rest.Config) *CertManagerClient { + return &CertManagerClient{clientset: *certmanagerclientset.NewForConfigOrDie(config)} +} + +func (h *CertManagerClient) GetRootCACertificate(ctx context.Context) (*certmanagerv1.Certificate, error) { + caCert, err := h.clientset.CertmanagerV1().Certificates(istioNamespace).Get(ctx, kcpCACertName, + apimetav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get CA certificate: %w", err) + } + return caCert, nil +} + +func RequiresUpdate(gwSecret *apicorev1.Secret, caCert *certmanagerv1.Certificate) bool { + if gwSecretLastModifiedAt, err := GetValidLastModifiedAt(gwSecret); err == nil { + if caCert.Status.NotBefore != nil && gwSecretLastModifiedAt.After(caCert.Status.NotBefore.Time) { + return false + } + } + return true +} diff --git a/pkg/gatewaysecret/handler.go b/pkg/gatewaysecret/handler.go index 2eac5c02aa..b967c0ed54 100644 --- a/pkg/gatewaysecret/handler.go +++ b/pkg/gatewaysecret/handler.go @@ -6,13 +6,13 @@ import ( "fmt" "time" - certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/go-logr/logr" apicorev1 "k8s.io/api/core/v1" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kyma-project/lifecycle-manager/pkg/util" @@ -28,46 +28,52 @@ const ( var errCouldNotGetLastModifiedAt = errors.New("getting lastModifiedAt time failed") -type GatewaySecretHandler struct { - kcpClient client.Client +type Handler struct { + certManagerClient *CertManagerClient + kcpClientset *kubernetes.Clientset + log logr.Logger } -func NewGatewaySecretHandler(kcpClient client.Client) *GatewaySecretHandler { - return &GatewaySecretHandler{ - kcpClient: kcpClient, +func NewGatewaySecretHandler(config *rest.Config, + log logr.Logger, +) *Handler { + return &Handler{ + certManagerClient: NewCertManagerClient(config), + kcpClientset: kubernetes.NewForConfigOrDie(config), + log: log, } } -func (gsh *GatewaySecretHandler) manageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error { - gwSecret, err := gsh.FindGatewaySecret(ctx) +func (h *Handler) manageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error { + gwSecret, err := h.findGatewaySecret(ctx) if util.IsNotFound(err) { - return gsh.handleNonExisting(ctx, rootSecret) + return h.handleNonExisting(ctx, rootSecret) } if err != nil { return err } - return gsh.handleExisting(ctx, rootSecret, gwSecret) + return h.handleExisting(ctx, rootSecret, gwSecret) } -func (gsh *GatewaySecretHandler) handleNonExisting(ctx context.Context, rootSecret *apicorev1.Secret) error { +func (h *Handler) handleNonExisting(ctx context.Context, rootSecret *apicorev1.Secret) error { gwSecret := NewGatewaySecret(rootSecret) - return gsh.Create(ctx, gwSecret) + return h.Create(ctx, gwSecret) } -func (gsh *GatewaySecretHandler) handleExisting(ctx context.Context, +func (h *Handler) handleExisting(ctx context.Context, rootSecret *apicorev1.Secret, gwSecret *apicorev1.Secret, ) error { - caCert, err := gsh.GetRootCACertificate(ctx) + caCert, err := h.certManagerClient.GetRootCACertificate(ctx) if err != nil { return err } - if !GatewaySecretRequiresUpdate(gwSecret, caCert) { + if !RequiresUpdate(gwSecret, caCert) { return nil } CopyRootSecretDataIntoGatewaySecret(gwSecret, rootSecret) - return gsh.Update(ctx, gwSecret) + return h.Update(ctx, gwSecret) } func CopyRootSecretDataIntoGatewaySecret(gwSecret *apicorev1.Secret, rootSecret *apicorev1.Secret) { @@ -76,15 +82,6 @@ func CopyRootSecretDataIntoGatewaySecret(gwSecret *apicorev1.Secret, rootSecret gwSecret.Data["ca.crt"] = rootSecret.Data["ca.crt"] } -func GatewaySecretRequiresUpdate(gwSecret *apicorev1.Secret, caCert certmanagerv1.Certificate) bool { - if gwSecretLastModifiedAt, err := GetValidLastModifiedAt(gwSecret); err == nil { - if caCert.Status.NotBefore != nil && gwSecretLastModifiedAt.After(caCert.Status.NotBefore.Time) { - return false - } - } - return true -} - func GetValidLastModifiedAt(secret *apicorev1.Secret) (time.Time, error) { if gwSecretLastModifiedAtValue, ok := secret.Annotations[LastModifiedAtAnnotation]; ok { if gwSecretLastModifiedAt, err := time.Parse(time.RFC3339, gwSecretLastModifiedAtValue); err == nil { @@ -94,37 +91,34 @@ func GetValidLastModifiedAt(secret *apicorev1.Secret) (time.Time, error) { return time.Time{}, errCouldNotGetLastModifiedAt } -func (gsh *GatewaySecretHandler) FindGatewaySecret(ctx context.Context) (*apicorev1.Secret, error) { - return GetGatewaySecret(ctx, gsh.kcpClient) +func (h *Handler) findGatewaySecret(ctx context.Context) (*apicorev1.Secret, error) { + secret, err := h.kcpClientset.CoreV1().Secrets(istioNamespace).Get(ctx, gatewaySecretName, + apimetav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get secret %s: %w", gatewaySecretName, err) + } + return secret, nil } -func (gsh *GatewaySecretHandler) Create(ctx context.Context, secret *apicorev1.Secret) error { - gsh.updateLastModifiedAt(secret) - if err := gsh.kcpClient.Create(ctx, secret); err != nil { +func (h *Handler) Create(ctx context.Context, secret *apicorev1.Secret) error { + h.updateLastModifiedAt(secret) + if _, err := h.kcpClientset.CoreV1().Secrets(istioNamespace).Create(ctx, secret, + apimetav1.CreateOptions{}); err != nil { return fmt.Errorf("failed to create secret %s: %w", secret.Name, err) } return nil } -func (gsh *GatewaySecretHandler) Update(ctx context.Context, secret *apicorev1.Secret) error { - gsh.updateLastModifiedAt(secret) - if err := gsh.kcpClient.Update(ctx, secret); err != nil { +func (h *Handler) Update(ctx context.Context, secret *apicorev1.Secret) error { + h.updateLastModifiedAt(secret) + if _, err := h.kcpClientset.CoreV1().Secrets(istioNamespace).Update(ctx, secret, + apimetav1.UpdateOptions{}); err != nil { return fmt.Errorf("failed to update secret %s: %w", secret.Name, err) } return nil } -func (gsh *GatewaySecretHandler) GetRootCACertificate(ctx context.Context) (certmanagerv1.Certificate, error) { - caCert := certmanagerv1.Certificate{} - if err := gsh.kcpClient.Get(ctx, - client.ObjectKey{Namespace: istioNamespace, Name: kcpCACertName}, - &caCert); err != nil { - return certmanagerv1.Certificate{}, fmt.Errorf("failed to get CA certificate: %w", err) - } - return caCert, nil -} - -func (gsh *GatewaySecretHandler) updateLastModifiedAt(secret *apicorev1.Secret) { +func (h *Handler) updateLastModifiedAt(secret *apicorev1.Secret) { if secret.Annotations == nil { secret.Annotations = make(map[string]string) } @@ -161,26 +155,51 @@ func GetGatewaySecret(ctx context.Context, clnt client.Client) (*apicorev1.Secre return secret, nil } -func (gsh *GatewaySecretHandler) StartRootCertificateWatch(clientset *kubernetes.Clientset, - log logr.Logger, -) { - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() +func (h *Handler) StartRootCertificateWatch() error { + ctx, cancel := context.WithCancel(context.Background()) - secretWatch, err := clientset.CoreV1().Secrets(istioNamespace).Watch(ctx, apimetav1.ListOptions{ + if err := h.handleAlreadyCreatedRootCertificate(ctx); err != nil { + cancel() + return err + } + if err := h.handleNewRootCertificates(ctx, cancel); err != nil { + cancel() + return err + } + return nil +} + +func (h *Handler) handleAlreadyCreatedRootCertificate(ctx context.Context) error { + rootCASecret, err := h.kcpClientset.CoreV1().Secrets(istioNamespace).Get(ctx, kcpRootSecretName, + apimetav1.GetOptions{}) + if err != nil { + if util.IsNotFound(err) { + return nil + } + h.log.Error(err, "unable to get root certificate") + return fmt.Errorf("unable to get root certificate: %w", err) + } + return h.manageGatewaySecret(ctx, rootCASecret) +} + +func (h *Handler) handleNewRootCertificates(ctx context.Context, cancel context.CancelFunc) error { + secretWatch, err := h.kcpClientset.CoreV1().Secrets(istioNamespace).Watch(ctx, apimetav1.ListOptions{ FieldSelector: fields.OneTermEqualSelector(apimetav1.ObjectNameField, kcpRootSecretName).String(), }) if err != nil { - log.Error(err, "unable to start watching root certificate") - panic(err) + h.log.Error(err, "unable to start watching root certificate") + return fmt.Errorf("unable to start watching root certificate: %w", err) } - WatchEvents(ctx, secretWatch.ResultChan(), gsh.manageGatewaySecret, log) + go WatchEvents(ctx, cancel, secretWatch.ResultChan(), h.manageGatewaySecret, h.log) + return nil } -func WatchEvents(ctx context.Context, watchEvents <-chan watch.Event, +func WatchEvents(ctx context.Context, cancel context.CancelFunc, watchEvents <-chan watch.Event, manageGatewaySecretFunc func(context.Context, *apicorev1.Secret) error, log logr.Logger, ) { + defer cancel() + for event := range watchEvents { rootCASecret, _ := event.Object.(*apicorev1.Secret) diff --git a/pkg/gatewaysecret/handler_test.go b/pkg/gatewaysecret/handler_test.go index 661ea3950b..83d37f7f53 100644 --- a/pkg/gatewaysecret/handler_test.go +++ b/pkg/gatewaysecret/handler_test.go @@ -206,9 +206,9 @@ func TestGatewaySecretRequiresUpdate(t *testing.T) { } for _, testcase := range tests { t.Run(testcase.name, func(t *testing.T) { - if got := gatewaysecret.GatewaySecretRequiresUpdate( - testcase.args.gwSecret, testcase.args.caCert); got != testcase.want { - t.Errorf("GatewaySecretRequiresUpdate() = %v, want %v", got, testcase.want) + if got := gatewaysecret.RequiresUpdate( + testcase.args.gwSecret, &testcase.args.caCert); got != testcase.want { + t.Errorf("RequiresUpdate() = %v, want %v", got, testcase.want) } }) } @@ -244,7 +244,6 @@ func TestWatchEvents(t *testing.T) { t.Parallel() ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() var waitGroup sync.WaitGroup waitGroup.Add(1) @@ -258,7 +257,7 @@ func TestWatchEvents(t *testing.T) { events := make(chan watch.Event, 1) go func() { defer waitGroup.Done() - gatewaysecret.WatchEvents(ctx, events, mockManageSecretFunc, logr.Logger{}) + gatewaysecret.WatchEvents(ctx, cancel, events, mockManageSecretFunc, logr.Logger{}) }() events <- watch.Event{ diff --git a/tests/e2e/rbac_privileges_test.go b/tests/e2e/rbac_privileges_test.go index 2184178ad1..0eb7b51b31 100644 --- a/tests/e2e/rbac_privileges_test.go +++ b/tests/e2e/rbac_privileges_test.go @@ -178,7 +178,7 @@ var _ = Describe("RBAC Privileges", func() { { APIGroups: []string{""}, Resources: []string{"secrets"}, - Verbs: []string{"list", "watch", "create", "delete", "update"}, + Verbs: []string{"list", "watch", "get", "create", "delete", "update"}, }, { APIGroups: []string{"cert-manager.io"}, diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 1e9cb284d5..3e2973feb9 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -17,4 +17,4 @@ packages: internal/remote: 13.2 internal/util/collections: 86 pkg/templatelookup: 77.1 - pkg/gatewaysecret: 27.7 + pkg/gatewaysecret: 22.6