diff --git a/.github/actions/deploy-lifecycle-manager-e2e/action.yml b/.github/actions/deploy-lifecycle-manager-e2e/action.yml index a3e195662c..393b77d759 100644 --- a/.github/actions/deploy-lifecycle-manager-e2e/action.yml +++ b/.github/actions/deploy-lifecycle-manager-e2e/action.yml @@ -80,9 +80,28 @@ runs: cat requeue-interval-patch.yaml kustomize edit add patch --path requeue-interval-patch.yaml --kind Deployment popd + - name: Patch KLM deployment for watcher zero downtime + if: ${{matrix.e2e-test == 'watcher-zero-downtime'}} + working-directory: lifecycle-manager + shell: bash + run: | + pushd config/watcher_local_test + echo \ + "- op: replace + path: /spec/template/spec/containers/0/args/16 + value: --kyma-requeue-success-interval=10s + - op: add + path: /spec/template/spec/containers/0/args/- + value: --istio-gateway-cert-switch-before-expiration-time=58m30s + - op: add + path: /spec/template/spec/containers/0/args/- + value: --istio-gateway-secret-requeue-success-interval=6s" >> requeue-interval-patch.yaml + cat requeue-interval-patch.yaml + kustomize edit add patch --path requeue-interval-patch.yaml --kind Deployment + popd - name: Patch CA certificate renewBefore - if: ${{matrix.e2e-test == 'ca-certificate-rotation' || - matrix.e2e-test == 'istio-gateway-secret-rotation'}} + if: ${{matrix.e2e-test == 'legacy-istio-gateway-secret-rotation' || + matrix.e2e-test == 'watcher-zero-downtime'}} working-directory: lifecycle-manager shell: bash run: | @@ -97,6 +116,19 @@ runs: cat certificate_renewal.yaml kustomize edit add patch --path certificate_renewal.yaml --kind Certificate --group cert-manager.io --version v1 --name watcher-serving popd + - name: Use legacy istio gateway secret rotation strategy + if: ${{matrix.e2e-test == 'legacy-istio-gateway-secret-rotation'}} + working-directory: lifecycle-manager + shell: bash + run: | + pushd config/watcher_local_test + echo \ + "- op: add + path: /spec/template/spec/containers/0/args/- + value: --legacy-strategy-for-istio-gateway-secret=true" >> legacy-secret-rotation.yaml + cat legacy-secret-rotation.yaml + kustomize edit add patch --path legacy-secret-rotation.yaml --kind Deployment + popd - name: Create and use maintenance window policy if: ${{matrix.e2e-test == 'maintenance-windows' || matrix.e2e-test == 'maintenance-windows-initial-installation' || diff --git a/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml b/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml index 4afbbd5738..d2ac0cea34 100644 --- a/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml +++ b/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml @@ -21,7 +21,8 @@ runs: - name: Create and apply Template Operator ModuleTemplate from the latest release working-directory: template-operator if: ${{ matrix.e2e-test != 'mandatory-module' && - matrix.e2e-test != 'mandatory-module-metrics' + matrix.e2e-test != 'mandatory-module-metrics' && + matrix.e2e-test != 'watcher-zero-downtime' }} shell: bash run: | @@ -32,7 +33,8 @@ runs: - name: Create and apply Template Operator ModuleTemplate with ModuleDeploymentNameInOlderVersion working-directory: template-operator if: ${{ matrix.e2e-test != 'mandatory-module' && - matrix.e2e-test != 'mandatory-module-metrics' + matrix.e2e-test != 'mandatory-module-metrics' && + matrix.e2e-test != 'watcher-zero-downtime' }} shell: bash run: | @@ -42,7 +44,8 @@ runs: - name: Create and apply Template Operator ModuleTemplate with ModuleDeploymentNameInNewerVersion working-directory: template-operator if: ${{ matrix.e2e-test != 'mandatory-module' && - matrix.e2e-test != 'mandatory-module-metrics' + matrix.e2e-test != 'mandatory-module-metrics' && + matrix.e2e-test != 'watcher-zero-downtime' }} shell: bash run: | diff --git a/.github/workflows/test-e2e-with-modulereleasemeta.yml b/.github/workflows/test-e2e-with-modulereleasemeta.yml index 89b49ef491..e19bb7e99a 100644 --- a/.github/workflows/test-e2e-with-modulereleasemeta.yml +++ b/.github/workflows/test-e2e-with-modulereleasemeta.yml @@ -54,8 +54,7 @@ jobs: - modulereleasemeta-module-upgrade-new-version - unmanage-module - skip-manifest-reconciliation - - ca-certificate-rotation - - istio-gateway-secret-rotation + - legacy-istio-gateway-secret-rotation - self-signed-certificate-rotation - mandatory-module - mandatory-module-metrics @@ -70,6 +69,7 @@ jobs: - maintenance-windows - maintenance-windows-initial-installation - maintenance-windows-skip + - watcher-zero-downtime runs-on: ubuntu-latest timeout-minutes: 20 diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 7dbdf594a9..98770c3012 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -55,8 +55,7 @@ jobs: - unmanage-module - module-install-by-version - skip-manifest-reconciliation - - ca-certificate-rotation - - istio-gateway-secret-rotation + - legacy-istio-gateway-secret-rotation - self-signed-certificate-rotation - mandatory-module-with-old-naming-pattern - mandatory-module-metrics-with-old-naming-pattern diff --git a/.golangci.yaml b/.golangci.yaml index dc487c7f74..2e51a71891 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -134,6 +134,8 @@ linters-settings: alias: watcherctrl - pkg: github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/client alias: gatewaysecretclient + - pkg: github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/handler + alias: gatewaysecrethandler ireturn: allow: - anon diff --git a/.vscode/tasks.json b/.vscode/tasks.json index fd0692612c..41f0de1574 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -117,7 +117,6 @@ "module-upgrade-new-version", "unmanage-module", "skip-manifest-reconciliation", - "ca-certificate-rotation", "self-signed-certificate-rotation", "mandatory-module", "mandatory-module-metrics", diff --git a/internal/controller/istiogatewaysecret/controller.go b/internal/controller/istiogatewaysecret/controller.go index 4e22d6393b..1adae8028a 100644 --- a/internal/controller/istiogatewaysecret/controller.go +++ b/internal/controller/istiogatewaysecret/controller.go @@ -11,6 +11,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/kyma-project/lifecycle-manager/pkg/log" + "github.com/kyma-project/lifecycle-manager/pkg/queue" ) var ErrSecretNotFound = errors.New("root secret not found") @@ -25,12 +26,14 @@ type ( type Reconciler struct { getRootSecret GetterFunc handler Handler + intervals queue.RequeueIntervals } -func NewReconciler(getSecretFunc GetterFunc, handler Handler) *Reconciler { +func NewReconciler(getSecretFunc GetterFunc, handler Handler, intervals queue.RequeueIntervals) *Reconciler { return &Reconciler{ getRootSecret: getSecretFunc, handler: handler, + intervals: intervals, } } @@ -42,13 +45,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("failed to get istio gateway root secret: %w", err) } if rootSecret == nil { - return ctrl.Result{}, ErrSecretNotFound + return ctrl.Result{Requeue: true, RequeueAfter: r.intervals.Error}, ErrSecretNotFound } err = r.handler.ManageGatewaySecret(ctx, rootSecret) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to manage gateway secret: %w", err) + return ctrl.Result{Requeue: true, RequeueAfter: r.intervals.Error}, + fmt.Errorf("failed to manage gateway secret: %w", err) } - return ctrl.Result{}, nil + return ctrl.Result{Requeue: true, RequeueAfter: r.intervals.Success}, nil } diff --git a/internal/controller/istiogatewaysecret/controller_test.go b/internal/controller/istiogatewaysecret/controller_test.go index 0af19ae1b6..dcf9689a69 100644 --- a/internal/controller/istiogatewaysecret/controller_test.go +++ b/internal/controller/istiogatewaysecret/controller_test.go @@ -12,6 +12,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "github.com/kyma-project/lifecycle-manager/internal/controller/istiogatewaysecret" + "github.com/kyma-project/lifecycle-manager/pkg/queue" ) func TestReconcile_WhenGetSecretFuncReturnsError_ReturnError(t *testing.T) { @@ -20,7 +21,7 @@ func TestReconcile_WhenGetSecretFuncReturnsError_ReturnError(t *testing.T) { return nil, errors.New("some-error") } mockHandler := &mockHandler{} - reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler) + reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler, queue.RequeueIntervals{}) // ACT _, err := reconciler.Reconcile(context.TODO(), ctrl.Request{}) @@ -36,7 +37,7 @@ func TestReconcile_WhenGetSecretFuncReturnsNoErrorAndSecretIsNil_ReturnError(t * return nil, nil } mockHandler := &mockHandler{} - reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler) + reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler, queue.RequeueIntervals{}) // ACT _, err := reconciler.Reconcile(context.TODO(), ctrl.Request{}) @@ -56,7 +57,7 @@ func TestReconcile_WhenGetSecretFuncIsCalled_IsCalledWithRequestNamespacedName(t assert.Equal(t, request.Name, name.Name) return nil, nil } - reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, &mockHandler{}) + reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, &mockHandler{}, queue.RequeueIntervals{}) // ACT // ASSERT @@ -70,7 +71,7 @@ func TestReconcile_WhenGetSecretFuncReturnsSecret_HandlerManageGatewaySecretIsCa return secret, nil } mockHandler := &mockHandler{} - reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler) + reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler, queue.RequeueIntervals{}) // ACT _, err := reconciler.Reconcile(context.TODO(), ctrl.Request{}) @@ -87,7 +88,7 @@ func TestReconcile_WhenHandlerManageGatewaySecretReturnsError_ReturnError(t *tes return secret, nil } mockHandler := &mockHandler{err: errors.New("some-error")} - reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler) + reconciler := istiogatewaysecret.NewReconciler(stubGetterFunc, mockHandler, queue.RequeueIntervals{}) // ACT _, err := reconciler.Reconcile(context.TODO(), ctrl.Request{}) diff --git a/internal/controller/istiogatewaysecret/setup.go b/internal/controller/istiogatewaysecret/setup.go index 94af8cc825..5d7bc389a1 100644 --- a/internal/controller/istiogatewaysecret/setup.go +++ b/internal/controller/istiogatewaysecret/setup.go @@ -16,8 +16,11 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/cabundle" gatewaysecretclient "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/client" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/legacy" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" + "github.com/kyma-project/lifecycle-manager/pkg/queue" ) const ( @@ -25,21 +28,30 @@ const ( kcpRootSecretName = "klm-watcher" ) -var errCouldNotGetLastModifiedAt = errors.New("getting lastModifiedAt time failed") +var errCouldNotGetTimeFromAnnotation = errors.New("getting time from annotation failed") func SetupReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options ctrlruntime.Options) error { options.MaxConcurrentReconciles = flagVar.MaxConcurrentWatcherReconciles clnt := gatewaysecretclient.NewGatewaySecretRotationClient(mgr.GetConfig()) - var parseLastModifiedFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { - if gwSecretLastModifiedAtValue, ok := secret.Annotations[shared.LastModifiedAtAnnotation]; ok { - if gwSecretLastModifiedAt, err := time.Parse(time.RFC3339, gwSecretLastModifiedAtValue); err == nil { - return gwSecretLastModifiedAt, nil + var parseLastModifiedFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { + if strValue, ok := secret.Annotations[annotation]; ok { + if time, err := time.Parse(time.RFC3339, strValue); err == nil { + return time, nil } } - return time.Time{}, errCouldNotGetLastModifiedAt + return time.Time{}, fmt.Errorf("%w: %s", errCouldNotGetTimeFromAnnotation, annotation) + } + + var handler gatewaysecret.Handler + if flagVar.UseLegacyStrategyForIstioGatewaySecret { + handler = legacy.NewGatewaySecretHandler(clnt, parseLastModifiedFunc) + } else { + handler = cabundle.NewGatewaySecretHandler(clnt, parseLastModifiedFunc, + flagVar.IstioGatewayCertSwitchBeforeExpirationTime) } - handler := gatewaysecret.NewGatewaySecretHandler(clnt, parseLastModifiedFunc) var getSecretFunc GetterFunc = func(ctx context.Context, name types.NamespacedName) (*apicorev1.Secret, error) { secret := &apicorev1.Secret{} @@ -51,7 +63,10 @@ func SetupReconciler(mgr ctrl.Manager, flagVar *flags.FlagVar, options ctrlrunti return secret, nil } - return NewReconciler(getSecretFunc, handler).setupWithManager(mgr, options) + return NewReconciler(getSecretFunc, handler, queue.RequeueIntervals{ + Success: flagVar.IstioGatewaySecretRequeueSuccessInterval, + Error: flagVar.IstioGatewaySecretRequeueErrInterval, + }).setupWithManager(mgr, options) } func (r *Reconciler) setupWithManager(mgr ctrl.Manager, opts ctrlruntime.Options) error { diff --git a/internal/gatewaysecret/cabundle/handler.go b/internal/gatewaysecret/cabundle/handler.go new file mode 100644 index 0000000000..6dd3b85283 --- /dev/null +++ b/internal/gatewaysecret/cabundle/handler.go @@ -0,0 +1,150 @@ +package cabundle + +import ( + "context" + "errors" + "slices" + "time" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + apicorev1 "k8s.io/api/core/v1" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" + "github.com/kyma-project/lifecycle-manager/pkg/util" +) + +var ErrCACertificateNotReady = errors.New("watcher-serving ca certificate is not ready") + +const ( + caBundleTempCertKey = "temp.ca.crt" + CurrentCAExpirationAnnotation = "currentCAExpiration" +) + +type Handler struct { + client gatewaysecret.Client + parseTimeFromAnnotationFunc gatewaysecret.TimeParserFunc + switchCertBeforeExpirationTime time.Duration +} + +func NewGatewaySecretHandler(client gatewaysecret.Client, timeParserFunc gatewaysecret.TimeParserFunc, + switchCertBeforeExpirationTime time.Duration, +) *Handler { + return &Handler{ + client: client, + parseTimeFromAnnotationFunc: timeParserFunc, + switchCertBeforeExpirationTime: switchCertBeforeExpirationTime, + } +} + +func (h *Handler) ManageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error { + caCert, err := h.client.GetWatcherServingCert(ctx) + if err != nil { + return err + } + if caCert.Status.NotBefore == nil || caCert.Status.NotAfter == nil { + return ErrCACertificateNotReady + } + + gwSecret, err := h.client.GetGatewaySecret(ctx) + if util.IsNotFound(err) { + return h.createGatewaySecretFromRootSecret(ctx, rootSecret, caCert) + } else if err != nil { + return err + } + + // this is for the case when we switch existing secret from legacy to new rotation mechanism + bootstrapLegacyGatewaySecret(gwSecret, rootSecret, caCert) + + if h.requiresBundling(gwSecret, caCert) { + bundleCACrt(gwSecret, rootSecret) + setLastModifiedToNow(gwSecret) + } + if h.requiresCertSwitching(gwSecret) { + switchCertificate(gwSecret, rootSecret) + setCurrentCAExpiration(gwSecret, caCert) + } + return h.client.UpdateGatewaySecret(ctx, gwSecret) +} + +func (h *Handler) createGatewaySecretFromRootSecret(ctx context.Context, rootSecret *apicorev1.Secret, + caCert *certmanagerv1.Certificate, +) error { + newSecret := &apicorev1.Secret{ + TypeMeta: apimetav1.TypeMeta{ + Kind: gatewaysecret.SecretKind, + APIVersion: apicorev1.SchemeGroupVersion.String(), + }, + ObjectMeta: apimetav1.ObjectMeta{ + Name: shared.GatewaySecretName, + Namespace: shared.IstioNamespace, + }, + } + + newSecret.Data = make(map[string][]byte) + newSecret.Data[gatewaysecret.TLSCrt] = rootSecret.Data[gatewaysecret.TLSCrt] + newSecret.Data[gatewaysecret.TLSKey] = rootSecret.Data[gatewaysecret.TLSKey] + newSecret.Data[gatewaysecret.CACrt] = rootSecret.Data[gatewaysecret.CACrt] + + newSecret.Data[caBundleTempCertKey] = rootSecret.Data[gatewaysecret.CACrt] + setLastModifiedToNow(newSecret) + setCurrentCAExpiration(newSecret, caCert) + + return h.client.CreateGatewaySecret(ctx, newSecret) +} + +func (h *Handler) requiresBundling(gwSecret *apicorev1.Secret, caCert *certmanagerv1.Certificate) bool { + // If the last modified time of the gateway secret is after the notBefore time of the CA certificate, + // then we don't need to update the gateway secret + if lastModified, err := h.parseTimeFromAnnotationFunc(gwSecret, shared.LastModifiedAtAnnotation); err == nil { + if lastModified.After(caCert.Status.NotBefore.Time) { + return false + } + } + return true +} + +func (h *Handler) requiresCertSwitching(gwSecret *apicorev1.Secret) bool { + // If the current CA is about to expire, then we need to switch the certificate and private key + caExpirationTime, err := h.parseTimeFromAnnotationFunc(gwSecret, CurrentCAExpirationAnnotation) + return err != nil || time.Now().After(caExpirationTime.Add(-h.switchCertBeforeExpirationTime)) +} + +func bootstrapLegacyGatewaySecret(gwSecret *apicorev1.Secret, rootSecret *apicorev1.Secret, + caCert *certmanagerv1.Certificate, +) { + if _, ok := gwSecret.Annotations[CurrentCAExpirationAnnotation]; !ok { + setCurrentCAExpiration(gwSecret, caCert) + } + if _, ok := gwSecret.Data[caBundleTempCertKey]; !ok { + gwSecret.Data[caBundleTempCertKey] = rootSecret.Data[gatewaysecret.CACrt] + } +} + +func setLastModifiedToNow(secret *apicorev1.Secret) { + if secret.Annotations == nil { + secret.Annotations = make(map[string]string) + } + secret.Annotations[shared.LastModifiedAtAnnotation] = apimetav1.Now().Format(time.RFC3339) +} + +func setCurrentCAExpiration(secret *apicorev1.Secret, caCert *certmanagerv1.Certificate) { + if secret.Annotations == nil { + secret.Annotations = make(map[string]string) + } + secret.Annotations[CurrentCAExpirationAnnotation] = caCert.Status.NotAfter.Time.Format(time.RFC3339) +} + +func bundleCACrt(gatewaySecret *apicorev1.Secret, rootSecret *apicorev1.Secret) { + gatewaySecret.Data[gatewaysecret.CACrt] = slices.Clone(rootSecret.Data[gatewaysecret.CACrt]) + gatewaySecret.Data[gatewaysecret.CACrt] = append(gatewaySecret.Data[gatewaysecret.CACrt], + gatewaySecret.Data[caBundleTempCertKey]...) + + gatewaySecret.Data[caBundleTempCertKey] = rootSecret.Data[gatewaysecret.CACrt] +} + +func switchCertificate(gatewaySecret *apicorev1.Secret, rootSecret *apicorev1.Secret) { + gatewaySecret.Data[gatewaysecret.TLSCrt] = rootSecret.Data[gatewaysecret.TLSCrt] + gatewaySecret.Data[gatewaysecret.TLSKey] = rootSecret.Data[gatewaysecret.TLSKey] +} diff --git a/internal/gatewaysecret/cabundle/handler_test.go b/internal/gatewaysecret/cabundle/handler_test.go new file mode 100644 index 0000000000..bf27fafca1 --- /dev/null +++ b/internal/gatewaysecret/cabundle/handler_test.go @@ -0,0 +1,320 @@ +package cabundle_test + +import ( + "context" + "errors" + "testing" + "time" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + apicorev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/cabundle" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/testutils" +) + +const gatewaySwitchCertBeforeExpirationTime = 1 * time.Hour + +func TestManageGatewaySecret_WhenGetWatcherServingCertReturnsError_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + someError := errors.New("some-error") + mockClient.On("GetWatcherServingCert", mock.Anything).Return(nil, someError) + + handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewaySwitchCertBeforeExpirationTime) + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) + + // ASSERT + require.Error(t, err) + require.ErrorIs(t, err, someError) + mockClient.AssertNumberOfCalls(t, "GetWatcherServingCert", 1) +} + +func TestManageGatewaySecret_WhenGetWatcherServingCertReturnsIncompleteStatus_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(&certmanagerv1.Certificate{ + Status: certmanagerv1.CertificateStatus{}, + }, nil) + + handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewaySwitchCertBeforeExpirationTime) + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) + + // ASSERT + require.Error(t, err) + require.ErrorIs(t, err, cabundle.ErrCACertificateNotReady) + mockClient.AssertNumberOfCalls(t, "GetWatcherServingCert", 1) +} + +func TestManageGatewaySecret_WhenGetGatewaySecretReturnsError_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + someError := errors.New("some-error") + mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, someError) + + handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewaySwitchCertBeforeExpirationTime) + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) + + // ASSERT + require.Error(t, err) + require.ErrorIs(t, err, someError) + mockClient.AssertNumberOfCalls(t, "GetWatcherServingCert", 1) + mockClient.AssertNumberOfCalls(t, "GetGatewaySecret", 1) +} + +func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGatewaySecretFromRootSecret(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, notFoundError()) + mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(nil) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("value3"), + }, + } + handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewaySwitchCertBeforeExpirationTime) + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), rootSecret) + + // ASSERT + require.NoError(t, err) + mockClient.AssertNumberOfCalls(t, "GetGatewaySecret", 1) + mockClient.AssertNumberOfCalls(t, "CreateGatewaySecret", 1) + + expectedNamespace := "istio-system" + expectedName := "klm-istio-gateway" + mockClient.AssertCalled(t, "CreateGatewaySecret", mock.Anything, mock.MatchedBy( + func(secret *apicorev1.Secret) bool { + return secret.ObjectMeta.Name == expectedName && + secret.ObjectMeta.Namespace == expectedNamespace && + string(secret.Data["tls.crt"]) == string(rootSecret.Data["tls.crt"]) && + string(secret.Data["tls.key"]) == string(rootSecret.Data["tls.key"]) && + string(secret.Data["ca.crt"]) == string(rootSecret.Data["ca.crt"]) && + string(secret.Data["temp.ca.crt"]) == string(rootSecret.Data["ca.crt"]) && + secret.Annotations[shared.LastModifiedAtAnnotation] != "" && + secret.Annotations[cabundle.CurrentCAExpirationAnnotation] != "" + })) +} + +func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundErrorAndCreationFailed_ReturnError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, notFoundError()) + expectedError := errors.New("some-error") + mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) + + handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewaySwitchCertBeforeExpirationTime) + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) + + // ASSERT + require.Error(t, err) + require.ErrorIs(t, err, expectedError) + mockClient.AssertNumberOfCalls(t, "GetGatewaySecret", 1) + mockClient.AssertNumberOfCalls(t, "CreateGatewaySecret", 1) +} + +func TestManageGatewaySecret_WhenLegacySecret_BootstrapsLegacyGatewaySecret(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("value3"), + }, + }, nil) + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) + timeParserFunction := getTimeParserFunction(false, false) + handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewaySwitchCertBeforeExpirationTime) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("value3"), + }, + } + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), rootSecret) + + // ASSERT + require.NoError(t, err) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) + mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( + func(secret *apicorev1.Secret) bool { + return secret.Annotations[cabundle.CurrentCAExpirationAnnotation] != "" && + string(secret.Data["temp.ca.crt"]) == "value3" + })) +} + +//nolint:dupl // the tests may contain similar code but they test different scenarios +func TestManageGatewaySecret_WhenRequiresBundling_BundlesGatewaySecretWithRootSecretCA(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("value3"), + "temp.ca.crt": []byte("value3"), + }, + }, nil) + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) + timeParserFunction := getTimeParserFunction(true, false) + handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewaySwitchCertBeforeExpirationTime) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("new-value1"), + "tls.key": []byte("new-value2"), + "ca.crt": []byte("new-value3"), + }, + } + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), rootSecret) + + // ASSERT + require.NoError(t, err) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) + mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( + func(secret *apicorev1.Secret) bool { + return string(secret.Data["tls.crt"]) == "value1" && + string(secret.Data["tls.key"]) == "value2" && + string(secret.Data["ca.crt"]) == "new-value3value3" && + string(secret.Data["temp.ca.crt"]) == "new-value3" + })) +} + +func TestManageGatewaySecret_WhenUpdateSecretFails_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("value3"), + "temp.ca.crt": []byte("value3"), + }, + }, nil) + expectedError := errors.New("some-error") + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) + timeParserFunction := getTimeParserFunction(true, false) + handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewaySwitchCertBeforeExpirationTime) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("new-value1"), + "tls.key": []byte("new-value2"), + "ca.crt": []byte("new-value3"), + }, + } + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), rootSecret) + + // ASSERT + require.Error(t, err) + require.ErrorIs(t, err, expectedError) + mockClient.AssertNumberOfCalls(t, "GetWatcherServingCert", 1) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) +} + +//nolint:dupl // the tests may contain similar code but they test different scenarios +func TestManageGatewaySecret_WhenRequiresCertSwitching_SwitchesTLSCertAndKeyWithRootSecret(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCert", mock.Anything).Return(getActiveCertificate(), nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("value1"), + "tls.key": []byte("value2"), + "ca.crt": []byte("new-value3value3"), + "temp.ca.crt": []byte("new-value3"), + }, + }, nil) + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) + timeParserFunction := getTimeParserFunction(false, true) + handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewaySwitchCertBeforeExpirationTime) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("new-value1"), + "tls.key": []byte("new-value2"), + "ca.crt": []byte("new-value3"), + }, + } + + // ACT + err := handler.ManageGatewaySecret(context.TODO(), rootSecret) + + // ASSERT + require.NoError(t, err) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) + mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( + func(secret *apicorev1.Secret) bool { + return string(secret.Data["tls.crt"]) == "new-value1" && + string(secret.Data["tls.key"]) == "new-value2" && + string(secret.Data["ca.crt"]) == "new-value3value3" && + string(secret.Data["temp.ca.crt"]) == "new-value3" + })) +} + +func getActiveCertificate() *certmanagerv1.Certificate { + return &certmanagerv1.Certificate{ + Status: certmanagerv1.CertificateStatus{ + NotBefore: &apimetav1.Time{ + Time: time.Now().Add(-1 * time.Hour), + }, + NotAfter: &apimetav1.Time{ + Time: time.Now().Add(2 * time.Hour), + }, + }, + } +} + +func getTimeParserFunction(bundlingRequired, certSwitchRequired bool) gatewaysecret.TimeParserFunc { + var lastModifiedAt, currentCAExpiration time.Time + + if bundlingRequired { + lastModifiedAt = time.Now().Add(-2 * time.Hour) + } else { + lastModifiedAt = time.Now() + } + if certSwitchRequired { + currentCAExpiration = time.Now().Add(30 * time.Minute) + } else { + currentCAExpiration = time.Now().Add(2 * time.Hour) + } + + return func(secret *apicorev1.Secret, annotation string) (time.Time, error) { + if annotation == shared.LastModifiedAtAnnotation { + return lastModifiedAt, nil + } + return currentCAExpiration, nil + } +} + +func notFoundError() error { + return apierrors.NewNotFound(apicorev1.Resource("secrets"), "not-found") +} diff --git a/internal/gatewaysecret/handler.go b/internal/gatewaysecret/handler.go index c0c7052ad8..dd954077fc 100644 --- a/internal/gatewaysecret/handler.go +++ b/internal/gatewaysecret/handler.go @@ -6,10 +6,13 @@ import ( certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" apicorev1 "k8s.io/api/core/v1" - apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) - "github.com/kyma-project/lifecycle-manager/api/shared" - "github.com/kyma-project/lifecycle-manager/pkg/util" +const ( + TLSCrt = "tls.crt" + TLSKey = "tls.key" + CACrt = "ca.crt" + SecretKind = "Secret" ) type Client interface { @@ -19,91 +22,8 @@ type Client interface { UpdateGatewaySecret(ctx context.Context, secret *apicorev1.Secret) error } -type TimeParserFunc func(secret *apicorev1.Secret) (time.Time, error) - -type Handler struct { - client Client - parseLastModifiedTime TimeParserFunc -} - -func NewGatewaySecretHandler(client Client, timeParserFunc TimeParserFunc) *Handler { - return &Handler{ - client: client, - parseLastModifiedTime: timeParserFunc, - } -} - -const ( - tlsCrt = "tls.crt" - tlsKey = "tls.key" - caCrt = "ca.crt" - secretKind = "Secret" -) - -func (h *Handler) ManageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error { - gwSecret, err := h.client.GetGatewaySecret(ctx) - if util.IsNotFound(err) { - return h.createGatewaySecretFromRootSecret(ctx, rootSecret) - } else if err != nil { - return err - } - - caCert, err := h.client.GetWatcherServingCert(ctx) - if err != nil { - return err - } - - if h.requiresUpdate(gwSecret, caCert) { - copyDataFromRootSecret(gwSecret, rootSecret) - setLastModifiedToNow(gwSecret) - - return h.client.UpdateGatewaySecret(ctx, gwSecret) - } - - return nil -} - -func (h *Handler) createGatewaySecretFromRootSecret(ctx context.Context, rootSecret *apicorev1.Secret) error { - newSecret := &apicorev1.Secret{ - TypeMeta: apimetav1.TypeMeta{ - Kind: secretKind, - APIVersion: apicorev1.SchemeGroupVersion.String(), - }, - ObjectMeta: apimetav1.ObjectMeta{ - Name: shared.GatewaySecretName, - Namespace: shared.IstioNamespace, - }, - } - - copyDataFromRootSecret(newSecret, rootSecret) - setLastModifiedToNow(newSecret) - - return h.client.CreateGatewaySecret(ctx, newSecret) -} - -func (h *Handler) requiresUpdate(gwSecret *apicorev1.Secret, caCert *certmanagerv1.Certificate) bool { - // If the last modified time of the gateway secret is after the notBefore time of the CA certificate, - // then we don't need to update the gateway secret - if lastModified, err := h.parseLastModifiedTime(gwSecret); err == nil { - if caCert.Status.NotBefore != nil && lastModified.After(caCert.Status.NotBefore.Time) { - return false - } - } - return true -} - -func setLastModifiedToNow(secret *apicorev1.Secret) { - if secret.Annotations == nil { - secret.Annotations = make(map[string]string) - } - secret.Annotations[shared.LastModifiedAtAnnotation] = apimetav1.Now().Format(time.RFC3339) -} +type TimeParserFunc func(secret *apicorev1.Secret, annotation string) (time.Time, error) -func copyDataFromRootSecret(secret *apicorev1.Secret, rootSecret *apicorev1.Secret) { - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - secret.Data[tlsCrt] = rootSecret.Data[tlsCrt] - secret.Data[tlsKey] = rootSecret.Data[tlsKey] - secret.Data[caCrt] = rootSecret.Data[caCrt] +type Handler interface { + ManageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error } diff --git a/internal/gatewaysecret/legacy/handler.go b/internal/gatewaysecret/legacy/handler.go new file mode 100644 index 0000000000..73e5a96eae --- /dev/null +++ b/internal/gatewaysecret/legacy/handler.go @@ -0,0 +1,94 @@ +package legacy + +import ( + "context" + "time" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + apicorev1 "k8s.io/api/core/v1" + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" + "github.com/kyma-project/lifecycle-manager/pkg/util" +) + +type Handler struct { + client gatewaysecret.Client + parseLastModifiedTime gatewaysecret.TimeParserFunc +} + +func NewGatewaySecretHandler(client gatewaysecret.Client, timeParserFunc gatewaysecret.TimeParserFunc) *Handler { + return &Handler{ + client: client, + parseLastModifiedTime: timeParserFunc, + } +} + +func (h *Handler) ManageGatewaySecret(ctx context.Context, rootSecret *apicorev1.Secret) error { + gwSecret, err := h.client.GetGatewaySecret(ctx) + if util.IsNotFound(err) { + return h.createGatewaySecretFromRootSecret(ctx, rootSecret) + } else if err != nil { + return err + } + + caCert, err := h.client.GetWatcherServingCert(ctx) + if err != nil { + return err + } + + if h.requiresUpdate(gwSecret, caCert) { + copyDataFromRootSecret(gwSecret, rootSecret) + setLastModifiedToNow(gwSecret) + + return h.client.UpdateGatewaySecret(ctx, gwSecret) + } + + return nil +} + +func (h *Handler) createGatewaySecretFromRootSecret(ctx context.Context, rootSecret *apicorev1.Secret) error { + newSecret := &apicorev1.Secret{ + TypeMeta: apimetav1.TypeMeta{ + Kind: gatewaysecret.SecretKind, + APIVersion: apicorev1.SchemeGroupVersion.String(), + }, + ObjectMeta: apimetav1.ObjectMeta{ + Name: shared.GatewaySecretName, + Namespace: shared.IstioNamespace, + }, + } + + copyDataFromRootSecret(newSecret, rootSecret) + setLastModifiedToNow(newSecret) + + return h.client.CreateGatewaySecret(ctx, newSecret) +} + +func (h *Handler) requiresUpdate(gwSecret *apicorev1.Secret, caCert *certmanagerv1.Certificate) bool { + // If the last modified time of the gateway secret is after the notBefore time of the CA certificate, + // then we don't need to update the gateway secret + if lastModified, err := h.parseLastModifiedTime(gwSecret, shared.LastModifiedAtAnnotation); err == nil { + if caCert.Status.NotBefore != nil && lastModified.After(caCert.Status.NotBefore.Time) { + return false + } + } + return true +} + +func setLastModifiedToNow(secret *apicorev1.Secret) { + if secret.Annotations == nil { + secret.Annotations = make(map[string]string) + } + secret.Annotations[shared.LastModifiedAtAnnotation] = apimetav1.Now().Format(time.RFC3339) +} + +func copyDataFromRootSecret(secret *apicorev1.Secret, rootSecret *apicorev1.Secret) { + if secret.Data == nil { + secret.Data = make(map[string][]byte) + } + secret.Data[gatewaysecret.TLSCrt] = rootSecret.Data[gatewaysecret.TLSCrt] + secret.Data[gatewaysecret.TLSKey] = rootSecret.Data[gatewaysecret.TLSKey] + secret.Data[gatewaysecret.CACrt] = rootSecret.Data[gatewaysecret.CACrt] +} diff --git a/internal/gatewaysecret/handler_test.go b/internal/gatewaysecret/legacy/handler_test.go similarity index 86% rename from internal/gatewaysecret/handler_test.go rename to internal/gatewaysecret/legacy/handler_test.go index 17ccc316a2..49c746783b 100644 --- a/internal/gatewaysecret/handler_test.go +++ b/internal/gatewaysecret/legacy/handler_test.go @@ -1,4 +1,4 @@ -package gatewaysecret_test +package legacy_test import ( "context" @@ -15,15 +15,17 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/legacy" + "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/testutils" ) func TestManageGatewaySecret_WhenGetGatewaySecretReturnsError_ReturnsError(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} someError := errors.New("some-error") mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, someError) - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, nil) + handler := legacy.NewGatewaySecretHandler(mockClient, nil) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -36,7 +38,7 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsError_ReturnsError(t *te func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGatewaySecretFromRootSecret(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, notFoundError()) mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(nil) rootSecret := &apicorev1.Secret{ @@ -46,7 +48,7 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGat "ca.crt": []byte("value3"), }, } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, nil) + handler := legacy.NewGatewaySecretHandler(mockClient, nil) // ACT err := handler.ManageGatewaySecret(context.TODO(), rootSecret) @@ -70,12 +72,12 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGat func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundErrorAndCreationFailed_ReturnError(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, notFoundError()) expectedError := errors.New("some-error") mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, nil) + handler := legacy.NewGatewaySecretHandler(mockClient, nil) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -89,11 +91,11 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundErrorAndCreation func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGatewaySecretWithLastModifiedAnnotation(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, notFoundError()) mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(nil) - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, nil) + handler := legacy.NewGatewaySecretHandler(mockClient, nil) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -110,12 +112,12 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGat func TestManageGatewaySecret_WhenWatcherServingCertReturnsError_ReturnsError(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{}, nil) expectedError := errors.New("some-error") mockClient.On("GetWatcherServingCert", mock.Anything).Return(nil, expectedError) - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, nil) + handler := legacy.NewGatewaySecretHandler(mockClient, nil) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -129,7 +131,7 @@ func TestManageGatewaySecret_WhenWatcherServingCertReturnsError_ReturnsError(t * func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithRootSecretData(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{}, nil) cert := &certmanagerv1.Certificate{ Status: certmanagerv1.CertificateStatus{ @@ -140,10 +142,12 @@ func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithRootSecr } mockClient.On("GetWatcherServingCert", mock.Anything).Return(cert, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) - var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { + var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { return time.Now(), nil } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, mockFunc) + handler := legacy.NewGatewaySecretHandler(mockClient, mockFunc) rootSecret := &apicorev1.Secret{ Data: map[string][]byte{ "tls.crt": []byte("value1"), @@ -168,7 +172,7 @@ func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithRootSecr func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithUpdatedModifiedNowAnnotation(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} originalTime := time.Now().Add(-time.Hour) gwSecret := &apicorev1.Secret{ ObjectMeta: apimetav1.ObjectMeta{ @@ -187,10 +191,12 @@ func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithUpdatedM } mockClient.On("GetWatcherServingCert", mock.Anything).Return(cert, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) - var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { + var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { return time.Now(), nil } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, mockFunc) + handler := legacy.NewGatewaySecretHandler(mockClient, mockFunc) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -212,7 +218,7 @@ func TestManageGatewaySecret_WhenRequiresUpdate_UpdatesGatewaySecretWithUpdatedM func TestManageGatewaySecret_WhenRequiresUpdateAndUpdateFails_ReturnsError(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{}, nil) cert := &certmanagerv1.Certificate{ Status: certmanagerv1.CertificateStatus{ @@ -224,10 +230,12 @@ func TestManageGatewaySecret_WhenRequiresUpdateAndUpdateFails_ReturnsError(t *te mockClient.On("GetWatcherServingCert", mock.Anything).Return(cert, nil) expectedError := errors.New("some-error") mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) - var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { + var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { return time.Now(), nil } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, mockFunc) + handler := legacy.NewGatewaySecretHandler(mockClient, mockFunc) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -241,7 +249,7 @@ func TestManageGatewaySecret_WhenRequiresUpdateAndUpdateFails_ReturnsError(t *te func TestManageGatewaySecret_WhenRequiresUpdateIsFalse_DoesNotUpdateGatewaySecret(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{}, nil) cert := &certmanagerv1.Certificate{ Status: certmanagerv1.CertificateStatus{ @@ -252,10 +260,12 @@ func TestManageGatewaySecret_WhenRequiresUpdateIsFalse_DoesNotUpdateGatewaySecre } mockClient.On("GetWatcherServingCert", mock.Anything).Return(cert, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) - var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { + var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { return time.Now(), nil } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, mockFunc) + handler := legacy.NewGatewaySecretHandler(mockClient, mockFunc) // ACT err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) @@ -267,7 +277,7 @@ func TestManageGatewaySecret_WhenRequiresUpdateIsFalse_DoesNotUpdateGatewaySecre func TestManageGatewaySecret_WhenTimeParserFuncReturnsError_UpdatesGatewaySecret(t *testing.T) { // ARRANGE - mockClient := &ClientMock{} + mockClient := &testutils.ClientMock{} mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{}, nil) cert := &certmanagerv1.Certificate{ Status: certmanagerv1.CertificateStatus{ @@ -278,12 +288,14 @@ func TestManageGatewaySecret_WhenTimeParserFuncReturnsError_UpdatesGatewaySecret } mockClient.On("GetWatcherServingCert", mock.Anything).Return(cert, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) - var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret) (time.Time, error) { + var mockFunc gatewaysecret.TimeParserFunc = func(secret *apicorev1.Secret, + annotation string, + ) (time.Time, error) { return time.Time{}, errors.New("some-error") } - handler := gatewaysecret.NewGatewaySecretHandler(mockClient, mockFunc) + handler := legacy.NewGatewaySecretHandler(mockClient, mockFunc) - // ACT + // ACT″ err := handler.ManageGatewaySecret(context.TODO(), &apicorev1.Secret{}) // ASSERT diff --git a/internal/gatewaysecret/mock_Client_test.go b/internal/gatewaysecret/testutils/mock_Client.go similarity index 99% rename from internal/gatewaysecret/mock_Client_test.go rename to internal/gatewaysecret/testutils/mock_Client.go index bb54f2243b..d609fc10c9 100644 --- a/internal/gatewaysecret/mock_Client_test.go +++ b/internal/gatewaysecret/testutils/mock_Client.go @@ -1,6 +1,6 @@ // Code generated by mockery v2.50.0. DO NOT EDIT. -package gatewaysecret_test +package testutils import ( context "context" diff --git a/internal/pkg/flags/flags.go b/internal/pkg/flags/flags.go index a54a556030..4db872f32f 100644 --- a/internal/pkg/flags/flags.go +++ b/internal/pkg/flags/flags.go @@ -48,6 +48,9 @@ const ( DefaultSelfSignedCertRenewBefore time.Duration = 60 * 24 * time.Hour DefaultSelfSignedCertificateRenewBuffer = 24 * time.Hour DefaultSelfSignedCertKeySize = 4096 + DefaultIstioGatewayCertSwitchBeforeExpirationTime = 24 * time.Hour + DefaultIstioGatewaySecretRequeueSuccessInterval = 5 * time.Minute + DefaultIstioGatewaySecretRequeueErrInterval = 2 * time.Second DefaultRemoteSyncNamespace = shared.DefaultRemoteNamespace DefaultMetricsAddress = ":8080" DefaultProbeAddress = ":8081" @@ -218,6 +221,17 @@ func DefineFlagVar() *FlagVar { "The buffer duration to wait before confirm self-signed certificate not renewed") flag.IntVar(&flagVar.SelfSignedCertKeySize, "self-signed-cert-key-size", DefaultSelfSignedCertKeySize, "The key size for the self-signed certificate") + flag.DurationVar(&flagVar.IstioGatewayCertSwitchBeforeExpirationTime, + "istio-gateway-cert-switch-before-expiration-time", DefaultIstioGatewayCertSwitchBeforeExpirationTime, + "Time before the expiration of the current CA certificate when the Gateway certificate should be switched") + flag.DurationVar(&flagVar.IstioGatewaySecretRequeueSuccessInterval, + "istio-gateway-secret-requeue-success-interval", DefaultIstioGatewaySecretRequeueSuccessInterval, + "determines the duration after which the istio gateway secret is enqueued after successful reconciliation.") + flag.DurationVar(&flagVar.IstioGatewaySecretRequeueErrInterval, + "istio-gateway-secret-requeue-error-interval", DefaultIstioGatewaySecretRequeueErrInterval, + "determines the duration after which the istio gateway secret is enqueued after unsuccessful reconciliation.") + flag.BoolVar(&flagVar.UseLegacyStrategyForIstioGatewaySecret, "legacy-strategy-for-istio-gateway-secret", + false, "Use the legacy strategy (with downtime) for the Istio Gateway Secret") flag.BoolVar(&flagVar.IsKymaManaged, "is-kyma-managed", false, "indicates whether Kyma is managed") flag.StringVar(&flagVar.DropCrdStoredVersionMap, "drop-crd-stored-version-map", DefaultDropCrdStoredVersionMap, "Specify the API versions to be dropped from the storage version. The input format should be a "+ @@ -281,35 +295,39 @@ type FlagVar struct { // ListenerPortOverwrite is used to enable the user to overwrite the port // used to expose the KCP cluster for the watcher. By default, it will be // fetched from the specified gateway. - ListenerPortOverwrite string - Pprof bool - PprofAddr string - PprofServerTimeout time.Duration - FailureBaseDelay, FailureMaxDelay time.Duration - RateLimiterBurst, RateLimiterFrequency int - CacheSyncTimeout time.Duration - LogLevel int - InKCPMode bool - PurgeFinalizerTimeout time.Duration - SkipPurgingFor string - RemoteSyncNamespace string - CaCertName string - IsKymaManaged bool - SelfSignedCertDuration time.Duration - SelfSignedCertRenewBefore time.Duration - SelfSignedCertRenewBuffer time.Duration - SelfSignedCertKeySize int - DropCrdStoredVersionMap string - WatcherImageTag string - WatcherImageName string - WatcherImageRegistry string - WatcherResourceLimitsMemory string - WatcherResourceLimitsCPU string - WatcherResourcesPath string - MetricsCleanupIntervalInMinutes int - ManifestRequeueJitterProbability float64 - ManifestRequeueJitterPercentage float64 - MinMaintenanceWindowSize time.Duration + ListenerPortOverwrite string + Pprof bool + PprofAddr string + PprofServerTimeout time.Duration + FailureBaseDelay, FailureMaxDelay time.Duration + RateLimiterBurst, RateLimiterFrequency int + CacheSyncTimeout time.Duration + LogLevel int + InKCPMode bool + PurgeFinalizerTimeout time.Duration + SkipPurgingFor string + RemoteSyncNamespace string + CaCertName string + IsKymaManaged bool + SelfSignedCertDuration time.Duration + SelfSignedCertRenewBefore time.Duration + SelfSignedCertRenewBuffer time.Duration + SelfSignedCertKeySize int + UseLegacyStrategyForIstioGatewaySecret bool + DropCrdStoredVersionMap string + WatcherImageTag string + WatcherImageName string + WatcherImageRegistry string + WatcherResourceLimitsMemory string + WatcherResourceLimitsCPU string + WatcherResourcesPath string + MetricsCleanupIntervalInMinutes int + ManifestRequeueJitterProbability float64 + ManifestRequeueJitterPercentage float64 + IstioGatewayCertSwitchBeforeExpirationTime time.Duration + IstioGatewaySecretRequeueSuccessInterval time.Duration + IstioGatewaySecretRequeueErrInterval time.Duration + MinMaintenanceWindowSize time.Duration } func (f FlagVar) Validate() error { diff --git a/internal/pkg/flags/flags_test.go b/internal/pkg/flags/flags_test.go index ef86265758..6d45e74445 100644 --- a/internal/pkg/flags/flags_test.go +++ b/internal/pkg/flags/flags_test.go @@ -195,6 +195,21 @@ func Test_ConstantFlags(t *testing.T) { constValue: strconv.Itoa(int(DefaultSelfSignedCertKeySize)), expectedValue: "4096", }, + { + constName: "DefaultIstioGatewayCertSwitchBeforeExpirationTime", + constValue: DefaultIstioGatewayCertSwitchBeforeExpirationTime.String(), + expectedValue: (24 * time.Hour).String(), + }, + { + constName: "DefaultIstioGatewaySecretRequeueSuccessInterval", + constValue: DefaultIstioGatewaySecretRequeueSuccessInterval.String(), + expectedValue: (5 * time.Minute).String(), + }, + { + constName: "DefaultIstioGatewaySecretRequeueErrInterval", + constValue: DefaultIstioGatewaySecretRequeueErrInterval.String(), + expectedValue: (2 * time.Second).String(), + }, { constName: "DefaultMetricsAddress", constValue: DefaultMetricsAddress, diff --git a/pkg/testutils/kyma.go b/pkg/testutils/kyma.go index d430ff615e..53c169bb52 100644 --- a/pkg/testutils/kyma.go +++ b/pkg/testutils/kyma.go @@ -28,6 +28,10 @@ var ( ErrModuleMessageInStatusIsIncorrect = errors.New("status.modules.message is incorrect") ) +const ( + FastChannel = "fast" +) + func NewTestKyma(name string) *v1beta2.Kyma { return NewKymaWithSyncLabel(name, ControlPlaneNamespace, v1beta2.DefaultChannel) } diff --git a/pkg/testutils/utils.go b/pkg/testutils/utils.go index c260bc0227..c166215829 100644 --- a/pkg/testutils/utils.go +++ b/pkg/testutils/utils.go @@ -180,3 +180,17 @@ func parseResourcesFromYAML(yamlFilePath string, clnt client.Client) ([]*unstruc } return resources, nil } + +func PatchServiceToTypeLoadBalancer(ctx context.Context, clnt client.Client, serviceName, namespace string) error { + service := &apicorev1.Service{} + if err := clnt.Get(ctx, client.ObjectKey{Name: serviceName, Namespace: namespace}, service); err != nil { + return err + } + + service.Spec.Type = apicorev1.ServiceTypeLoadBalancer + if err := clnt.Update(ctx, service); err != nil { + return err + } + + return nil +} diff --git a/scripts/tests/create_test_clusters.sh b/scripts/tests/create_test_clusters.sh index 68a3145316..35a24184c1 100755 --- a/scripts/tests/create_test_clusters.sh +++ b/scripts/tests/create_test_clusters.sh @@ -54,6 +54,7 @@ else k3d cluster create skr \ -p 10080:80@loadbalancer \ -p 10443:443@loadbalancer \ + -p 2112:2112@loadbalancer \ --k3s-arg --tls-san="skr.cluster.local@server:*" \ --image rancher/k3s:v${K8S_VERSION}-k3s1 \ --k3s-arg --disable="traefik@server:*" \ diff --git a/tests/e2e/Makefile b/tests/e2e/Makefile index bda03c58d1..f331e7b7f6 100644 --- a/tests/e2e/Makefile +++ b/tests/e2e/Makefile @@ -136,11 +136,8 @@ modulereleasemeta-module-upgrade-new-version: module-install-by-version: go test -timeout 20m -ginkgo.v -ginkgo.focus "Module Install By Version" -ca-certificate-rotation: - go test -timeout 20m -ginkgo.v -ginkgo.focus "CA Certificate Rotation" - -istio-gateway-secret-rotation: - go test -timeout 20m -ginkgo.v -ginkgo.focus "Istio Gateway Secret Rotation" +legacy-istio-gateway-secret-rotation: + go test -timeout 20m -ginkgo.v -ginkgo.focus "Legacy Istio Gateway Secret Rotation" self-signed-certificate-rotation: go test -timeout 20m -ginkgo.v -ginkgo.focus "Self Signed Certificate Rotation" @@ -180,3 +177,6 @@ maintenance-windows-initial-installation: maintenance-windows-skip: go test -timeout 20m -ginkgo.v -ginkgo.focus "Maintenance Windows - No Wait for Maintenance Widnow on Skip" + +watcher-zero-downtime: + go test -timeout 20m -ginkgo.v -ginkgo.focus "Watcher Zero Downtime" diff --git a/tests/e2e/ca_certificate_rotation_test.go b/tests/e2e/ca_certificate_rotation_test.go deleted file mode 100644 index 9b18d93eaa..0000000000 --- a/tests/e2e/ca_certificate_rotation_test.go +++ /dev/null @@ -1,70 +0,0 @@ -package e2e_test - -import ( - "time" - - certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - "k8s.io/apimachinery/pkg/types" - - "github.com/kyma-project/lifecycle-manager/api/v1beta2" - "github.com/kyma-project/lifecycle-manager/pkg/watcher" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - . "github.com/kyma-project/lifecycle-manager/pkg/testutils" - . "github.com/kyma-project/lifecycle-manager/tests/e2e/commontestutils" -) - -var _ = Describe("CA Certificate Rotation", Ordered, func() { - kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) - InitEmptyKymaBeforeAll(kyma) - CleanupKymaAfterAll(kyma) - - var caCertificate *certmanagerv1.Certificate - caCertName := "klm-watcher-serving" - - Context("Given KCP Cluster and rotated CA certificate", func() { - kcpSecretName := types.NamespacedName{ - Name: kyma.Name + "-webhook-tls", - Namespace: IstioNamespace, - } - skrSecretName := types.NamespacedName{ - Name: watcher.SkrTLSName, - Namespace: RemoteNamespace, - } - It("Then KCP TLS Certificate is removed", func() { - var err error - namespacedSecretName := types.NamespacedName{ - Name: watcher.ResolveTLSCertName(kyma.Name), - Namespace: IstioNamespace, - } - tlsSecret, err := GetTLSSecret(ctx, namespacedSecretName, kcpClient) - Expect(err).NotTo(HaveOccurred()) - - // The timeout used is 4 minutes bec the certificate gets rotated every 1 minute - Eventually(TLSSecretRotated, 4*time.Minute). - WithContext(ctx). - WithArguments(tlsSecret.CreationTimestamp.Time, namespacedSecretName, kcpClient). - Should(Succeed()) - - By("And new TLS Certificate is created") - namespacedCertName := types.NamespacedName{ - Name: caCertName, - Namespace: IstioNamespace, - } - caCertificate, err = GetCACertificate(ctx, namespacedCertName, kcpClient) - Expect(err).NotTo(HaveOccurred()) - Eventually(CertificateSecretIsCreatedAfter). - WithContext(ctx). - WithArguments(kcpSecretName, kcpClient, caCertificate.Status.NotBefore). - Should(Succeed()) - - By("And new TLS Certificate is synced to SKR Cluster") - Eventually(CertificateSecretIsSyncedToSkrCluster). - WithContext(ctx). - WithArguments(kcpSecretName, kcpClient, skrSecretName, skrClient). - Should(Succeed()) - }) - }) -}) diff --git a/tests/e2e/commontestutils/metrics.go b/tests/e2e/commontestutils/metrics.go index 5181a7162a..879be3e05e 100644 --- a/tests/e2e/commontestutils/metrics.go +++ b/tests/e2e/commontestutils/metrics.go @@ -13,10 +13,15 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/pkg/metrics" ) +const ( + kcpMetricsPort = 9081 + skrMetricsPort = 2112 +) + var ErrMetricNotFound = errors.New("metric was not found") func GetMaintenanceWindowGauge(ctx context.Context) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -26,7 +31,7 @@ func GetMaintenanceWindowGauge(ctx context.Context) (int, error) { } func GetKymaStateMetricCount(ctx context.Context, kymaName string, state shared.State) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -42,7 +47,7 @@ func getKymaStateMetricRegex(kymaName string, state shared.State) *regexp.Regexp } func AssertKymaStateMetricNotFound(ctx context.Context, kymaName string, state shared.State) error { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return err } @@ -59,7 +64,7 @@ func AssertKymaStateMetricNotFound(ctx context.Context, kymaName string, state s func GetRequeueReasonCount(ctx context.Context, requeueReason, requeueType string, ) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -73,7 +78,7 @@ func GetRequeueReasonCount(ctx context.Context, func IsManifestRequeueReasonCountIncreased(ctx context.Context, requeueReason, requeueType string) (bool, error, ) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return false, err } @@ -86,7 +91,7 @@ func IsManifestRequeueReasonCountIncreased(ctx context.Context, requeueReason, r } func GetModuleStateMetricCount(ctx context.Context, kymaName, moduleName string, state shared.State) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -103,7 +108,7 @@ func PurgeMetricsAreAsExpected(ctx context.Context, ) bool { correctCount := false correctTime := false - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return false } @@ -131,7 +136,7 @@ func PurgeMetricsAreAsExpected(ctx context.Context, } func GetSelfSignedCertNotRenewMetricsGauge(ctx context.Context, kymaName string) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -143,7 +148,7 @@ func GetSelfSignedCertNotRenewMetricsGauge(ctx context.Context, kymaName string) } func GetMandatoryModuleTemplateCountMetric(ctx context.Context) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -153,7 +158,7 @@ func GetMandatoryModuleTemplateCountMetric(ctx context.Context) (int, error) { } func GetMandatoryModuleStateMetric(ctx context.Context, kymaName, moduleName, state string) (int, error) { - bodyString, err := getMetricsBody(ctx) + bodyString, err := getKCPMetricsBody(ctx) if err != nil { return 0, err } @@ -163,9 +168,27 @@ func GetMandatoryModuleStateMetric(ctx context.Context, kymaName, moduleName, st return parseCount(re, bodyString) } -func getMetricsBody(ctx context.Context) (string, error) { +func GetWatcherFailedKcpTotalMetric(ctx context.Context) (int, error) { + metricsBody, err := getSKRMetricsBody(ctx) + if err != nil { + return 0, err + } + regex := regexp.MustCompile(`watcher_failed_kcp_total{error_reason="failed-request"} (\d+)`) + return parseCount(regex, metricsBody) +} + +func getKCPMetricsBody(ctx context.Context) (string, error) { + return getMetricsBody(ctx, kcpMetricsPort) +} + +func getSKRMetricsBody(ctx context.Context) (string, error) { + return getMetricsBody(ctx, skrMetricsPort) +} + +func getMetricsBody(ctx context.Context, port int) (string, error) { clnt := &http.Client{} - request, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:9081/metrics", nil) + url := fmt.Sprintf("http://localhost:%d/metrics", port) + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return "", fmt.Errorf("request to metrics endpoint :%w", err) } diff --git a/tests/e2e/istio_gateway_secret_rotation_test.go b/tests/e2e/legacy_istio_gateway_secret_rotation_test.go similarity index 95% rename from tests/e2e/istio_gateway_secret_rotation_test.go rename to tests/e2e/legacy_istio_gateway_secret_rotation_test.go index b979553003..945a8e02fe 100644 --- a/tests/e2e/istio_gateway_secret_rotation_test.go +++ b/tests/e2e/legacy_istio_gateway_secret_rotation_test.go @@ -12,7 +12,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Istio Gateway Secret Rotation", Ordered, func() { +var _ = Describe("Legacy Istio Gateway Secret Rotation", Ordered, func() { kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) InitEmptyKymaBeforeAll(kyma) CleanupKymaAfterAll(kyma) diff --git a/tests/e2e/maintenance_windows_initial_installation_test.go b/tests/e2e/maintenance_windows_initial_installation_test.go index 62784c9ee9..c4002b196e 100644 --- a/tests/e2e/maintenance_windows_initial_installation_test.go +++ b/tests/e2e/maintenance_windows_initial_installation_test.go @@ -17,7 +17,6 @@ Maintenance Windows are defined as such: */ var _ = Describe("Maintenance Windows - No Wait for Maintenance Window on Initial Installation", Ordered, func() { - const fastChannel = "fast" const europe = "europe" kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) @@ -41,7 +40,7 @@ var _ = Describe("Maintenance Windows - No Wait for Maintenance Window on Initia Context("Given SKR Cluster; Kyma CR .spec.skipMaintenanceWindows=false; NO active maintenance window", func() { It("When module in fast channel is enabled (requiresDowntime=true)", func() { - module.Channel = fastChannel + module.Channel = FastChannel Eventually(EnableModule). WithContext(ctx). WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace, module). diff --git a/tests/e2e/maintenance_windows_skip_test.go b/tests/e2e/maintenance_windows_skip_test.go index f776e351aa..a3bd8007b6 100644 --- a/tests/e2e/maintenance_windows_skip_test.go +++ b/tests/e2e/maintenance_windows_skip_test.go @@ -17,7 +17,6 @@ Maintenance Windows are defined as such: */ var _ = Describe("Maintenance Windows - No Wait for Maintenance Widnow on Skip", Ordered, func() { - const fastChannel = "fast" const europe = "europe" kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) @@ -64,7 +63,7 @@ var _ = Describe("Maintenance Windows - No Wait for Maintenance Widnow on Skip", }) It("When module channel is changed to fast (requiresDowntime=true)", func() { - module.Channel = fastChannel + module.Channel = FastChannel Eventually(UpdateKymaModuleChannel). WithContext(ctx). WithArguments(skrClient, shared.DefaultRemoteKymaName, shared.DefaultRemoteNamespace, module.Channel). diff --git a/tests/e2e/maintenance_windows_wait_test.go b/tests/e2e/maintenance_windows_wait_test.go index 5c580bd320..0897ff135f 100644 --- a/tests/e2e/maintenance_windows_wait_test.go +++ b/tests/e2e/maintenance_windows_wait_test.go @@ -17,7 +17,6 @@ Maintenance Windows are defined as such: */ var _ = Describe("Maintenance Windows - Wait for Maintenance Window", Ordered, func() { - const fastChannel = "fast" const europe = "europe" const asia = "asia" @@ -65,7 +64,7 @@ var _ = Describe("Maintenance Windows - Wait for Maintenance Window", Ordered, f }) It("When module channel is changed to fast (requiresDowntime=true)", func() { - module.Channel = fastChannel + module.Channel = FastChannel Eventually(UpdateKymaModuleChannel). WithContext(ctx). WithArguments(skrClient, shared.DefaultRemoteKymaName, shared.DefaultRemoteNamespace, module.Channel). diff --git a/tests/e2e/module_deletion_test.go b/tests/e2e/module_deletion_test.go index 8387af78c6..8bbd524fe4 100644 --- a/tests/e2e/module_deletion_test.go +++ b/tests/e2e/module_deletion_test.go @@ -97,7 +97,7 @@ var _ = Describe("Non Blocking Kyma Module Deletion", Ordered, func() { }) It("When Kyma Module is re-enabled in different Module Distribution Channel", func() { - module.Channel = "fast" + module.Channel = FastChannel Eventually(EnableModule). WithContext(ctx). WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace, module). diff --git a/tests/e2e/module_upgrade_channel_switch_test.go b/tests/e2e/module_upgrade_channel_switch_test.go index eeaec78526..6c96db811a 100644 --- a/tests/e2e/module_upgrade_channel_switch_test.go +++ b/tests/e2e/module_upgrade_channel_switch_test.go @@ -53,7 +53,7 @@ var _ = Describe("Module Upgrade By Channel Switch", Ordered, func() { It("When upgrade version by switch Channel", func() { Eventually(UpdateKymaModuleChannel). WithContext(ctx). - WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace, "fast"). + WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace, FastChannel). Should(Succeed()) }) diff --git a/tests/e2e/watcher_test.go b/tests/e2e/watcher_test.go index c00cf7cf5a..35d15435e6 100644 --- a/tests/e2e/watcher_test.go +++ b/tests/e2e/watcher_test.go @@ -83,7 +83,7 @@ var _ = Describe("Enqueue Event from Watcher", Ordered, func() { timeNow := &apimetav1.Time{Time: time.Now()} It("When spec of SKR Kyma CR is changed", func() { GinkgoWriter.Println(fmt.Sprintf("Spec watching logs since %s: ", timeNow)) - switchedChannel := "fast" + switchedChannel := FastChannel Eventually(changeRemoteKymaChannel). WithContext(ctx). WithArguments(RemoteNamespace, switchedChannel, skrClient). diff --git a/tests/e2e/watcher_zero_downtime_test.go b/tests/e2e/watcher_zero_downtime_test.go new file mode 100644 index 0000000000..3f4f899727 --- /dev/null +++ b/tests/e2e/watcher_zero_downtime_test.go @@ -0,0 +1,72 @@ +package e2e_test + +import ( + "context" + "errors" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + . "github.com/kyma-project/lifecycle-manager/pkg/testutils" + . "github.com/kyma-project/lifecycle-manager/tests/e2e/commontestutils" +) + +var _ = Describe("Watcher Zero Downtime", Ordered, func() { + kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) + + InitEmptyKymaBeforeAll(kyma) + CleanupKymaAfterAll(kyma) + + Context("Given SKR Cluster", func() { + It("When SKR metrics service is exposed", func() { + Expect(PatchServiceToTypeLoadBalancer(ctx, skrClient, + "skr-webhook-metrics", "kyma-system")). + To(Succeed()) + time.Sleep(1 * time.Second) + }) + + It("Then no downtime errors can be observed", func() { + Consistently(triggerWatcherAndCheckDowntime). + WithContext(ctx). + WithArguments(skrClient, defaultRemoteKymaName, RemoteNamespace). + WithTimeout(4 * time.Minute). + WithPolling(10 * time.Second). + Should(Succeed()) + }) + }) +}) + +func triggerWatcherAndCheckDowntime(ctx context.Context, skrClient client.Client, + kymaName, kymaNamespace string, +) error { + // Triggering watcher request + kyma, err := GetKyma(ctx, skrClient, kymaName, kymaNamespace) + if err != nil { + return err + } + if kyma.Spec.Channel == v1beta2.DefaultChannel { + kyma.Spec.Channel = FastChannel + } else { + kyma.Spec.Channel = v1beta2.DefaultChannel + } + if err := skrClient.Update(ctx, kyma); err != nil && !strings.Contains(err.Error(), + "the object has been modified") { + return err + } + + time.Sleep(1 * time.Second) + + // Checking that failed KCP error metrics is not increasing + count, err := GetWatcherFailedKcpTotalMetric(ctx) + if err != nil && !strings.Contains(err.Error(), "EOF") { + return err + } + if count > 0 { + return errors.New("watcher is experiencing downtime") + } + return nil +} diff --git a/tests/integration/controller/kyma/kyma_module_channel_test.go b/tests/integration/controller/kyma/kyma_module_channel_test.go index 3208e96a7c..9e93a5a518 100644 --- a/tests/integration/controller/kyma/kyma_module_channel_test.go +++ b/tests/integration/controller/kyma/kyma_module_channel_test.go @@ -20,9 +20,7 @@ import ( ) const ( - FastChannel = "fast" ValidChannel = "valid" - InvalidNoneChannel = string(shared.NoneChannel) InValidChannel = "Invalid01" // lower case characters from a to z InValidMinLengthChannel = "ch" // minlength = 3 InValidMaxLengthChannel = "averylongchannelwhichlargerthanallowedmaxlength" // maxlength = 32 diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 78717fb221..e169979d10 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -1,10 +1,11 @@ packages: - internal/controller/istiogatewaysecret: 31 + internal/controller/istiogatewaysecret: 28.9 internal/crd: 92.1 internal/descriptor/cache: 92.3 internal/descriptor/provider: 66.7 internal/event: 100 - internal/gatewaysecret: 100 + internal/gatewaysecret/legacy: 100 + internal/gatewaysecret/cabundle: 97.6 internal/manifest: 15.8 internal/manifest/statecheck: 71 internal/manifest/filemutex: 100