diff --git a/pkg/controller/external_secrets/certificate.go b/pkg/controller/external_secrets/certificate.go index b972aa263..d37ca4ed2 100644 --- a/pkg/controller/external_secrets/certificate.go +++ b/pkg/controller/external_secrets/certificate.go @@ -71,10 +71,9 @@ func (r *Reconciler) createOrApplyCertificate(esc *operatorv1alpha1.ExternalSecr r.log.V(4).Info("certificate resource already exists and is in expected state", "name", certificateName) } if !exist { - if err := r.Create(r.ctx, desired); err != nil { - return common.FromClientError(err, "failed to create %s certificate resource", certificateName) + if err := r.createWithFallback(desired, resourceMetadata, certificateName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "certificate resource %s created", certificateName) } return nil diff --git a/pkg/controller/external_secrets/certificate_test.go b/pkg/controller/external_secrets/certificate_test.go index ced01c305..d20b5039e 100644 --- a/pkg/controller/external_secrets/certificate_test.go +++ b/pkg/controller/external_secrets/certificate_test.go @@ -268,7 +268,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { esc.Spec.ControllerConfig.CertProvider.CertManager.IssuerRef.Name = testIssuerName }, recon: false, - wantErr: fmt.Sprintf("failed to create %s/%s certificate resource: %s", commontest.TestExternalSecretsNamespace, testValidateCertificateResourceName, commontest.ErrTestClient), + wantErr: fmt.Sprintf("failed to create Certificate %s/%s: %s", commontest.TestExternalSecretsNamespace, testValidateCertificateResourceName, commontest.ErrTestClient), }, { name: "successful webhook certificate creation", diff --git a/pkg/controller/external_secrets/configmap.go b/pkg/controller/external_secrets/configmap.go index b4d89654e..73ab6f485 100644 --- a/pkg/controller/external_secrets/configmap.go +++ b/pkg/controller/external_secrets/configmap.go @@ -5,6 +5,7 @@ import ( "maps" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -57,25 +58,31 @@ func (r *Reconciler) ensureTrustedCABundleConfigMap(esc *operatorv1alpha1.Extern } if !exist { - // Create the ConfigMap + // NOTE: This ConfigMap cannot use the generic createWithFallback helper because + // the Data/BinaryData fields are managed by CNO (not by this operator). On + // AlreadyExists we use a MergePatch that touches only metadata, leaving + // CNO-injected CA certificates untouched. if err := r.Create(r.ctx, desiredConfigMap); err != nil { - return common.FromClientError(err, "failed to create %s trusted CA bundle ConfigMap resource", configMapName) + if !apierrors.IsAlreadyExists(err) { + return common.FromClientError(err, "failed to create %s trusted CA bundle ConfigMap resource", configMapName) + } + r.log.V(1).Info("trusted CA bundle ConfigMap exists on API server but absent from label-filtered cache, patching metadata", "name", configMapName) + if patchErr := r.patchResourceMetadata(desiredConfigMap, resourceMetadata); patchErr != nil { + return common.FromClientError(patchErr, "failed to patch %s trusted CA bundle ConfigMap metadata", configMapName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "trusted CA bundle ConfigMap resource %s restored to desired state", configMapName) + return nil } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "trusted CA bundle ConfigMap resource %s created", configMapName) return nil } - // ConfigMap exists, ensure it has the correct labels and annotations. - // Do not update the data of the ConfigMap since it is managed by CNO. - // MergeFetchedAnnotations preserves external annotations (e.g., CNO's openshift.io/owning-component). + // ConfigMap exists in cache — ensure it has the correct labels and annotations. + // Use a metadata-only patch so CNO-managed Data/BinaryData are never touched. if exist && common.ObjectMetadataModified(desiredConfigMap, existingConfigMap, &resourceMetadata) { - r.log.V(1).Info("trusted CA bundle ConfigMap has been modified, updating to desired state", "name", configMapName) - // Preserve data from existing ConfigMap (managed by CNO) - desiredConfigMap.Data = existingConfigMap.Data - desiredConfigMap.BinaryData = existingConfigMap.BinaryData - common.RemoveObsoleteAnnotations(desiredConfigMap, resourceMetadata) - if err := r.UpdateWithRetry(r.ctx, desiredConfigMap); err != nil { - return common.FromClientError(err, "failed to update %s trusted CA bundle ConfigMap resource", configMapName) + r.log.V(1).Info("trusted CA bundle ConfigMap has been modified, patching metadata to desired state", "name", configMapName) + if err := r.patchResourceMetadata(desiredConfigMap, resourceMetadata); err != nil { + return common.FromClientError(err, "failed to patch %s trusted CA bundle ConfigMap metadata", configMapName) } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "trusted CA bundle ConfigMap resource %s reconciled back to desired state", configMapName) } else { diff --git a/pkg/controller/external_secrets/configmap_test.go b/pkg/controller/external_secrets/configmap_test.go new file mode 100644 index 000000000..c728b0bda --- /dev/null +++ b/pkg/controller/external_secrets/configmap_test.go @@ -0,0 +1,259 @@ +package external_secrets + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" + "github.com/openshift/external-secrets-operator/pkg/controller/client/fakes" + "github.com/openshift/external-secrets-operator/pkg/controller/common" + "github.com/openshift/external-secrets-operator/pkg/controller/commontest" +) + +// testESCWithProxy returns an ExternalSecretsConfig with a proxy configured so that +// ensureTrustedCABundleConfigMap proceeds past the proxy-nil guard. +func testESCWithProxy() *operatorv1alpha1.ExternalSecretsConfig { + esc := commontest.TestExternalSecretsConfig() + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPProxy: "http://proxy.example.com:3128", + } + return esc +} + +func TestEnsureTrustedCABundleConfigMap(t *testing.T) { + cnoData := map[string]string{"ca-bundle.crt": "cert-data"} + + tests := []struct { + name string + resourceMetadata common.ResourceMetadata + preReq func(r *Reconciler, cached *fakes.FakeCtrlClient, uncached *fakes.FakeCtrlClient) + wantErr string + wantCreate bool + wantPatch bool + noProxy bool + }{ + { + name: "ConfigMap created when it does not exist", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, _ client.Object) (bool, error) { + return false, nil + }) + cached.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + t.Errorf("expected ConfigMap, got %T", obj) + } + if cm.Name != trustedCABundleConfigMapName { + t.Errorf("expected name %s, got %s", trustedCABundleConfigMapName, cm.Name) + } + if cm.Labels[trustedCABundleInjectLabel] != "true" { + t.Errorf("expected inject label to be 'true', got %q", cm.Labels[trustedCABundleInjectLabel]) + } + return nil + }) + }, + wantCreate: true, + }, + { + name: "ConfigMap exists in correct state, no update needed", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { + existing := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCABundleConfigMapName, + Namespace: commontest.TestExternalSecretsNamespace, + Labels: getTrustedCABundleLabels(controllerDefaultResourceLabels), + }, + Data: cnoData, + } + existing.DeepCopyInto(obj.(*corev1.ConfigMap)) + return true, nil + }) + }, + }, + { + name: "ConfigMap exists with wrong labels, patch triggered", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { + existing := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCABundleConfigMapName, + Namespace: commontest.TestExternalSecretsNamespace, + Labels: map[string]string{"app": "something-else"}, + }, + Data: cnoData, + } + existing.DeepCopyInto(obj.(*corev1.ConfigMap)) + return true, nil + }) + cached.PatchCalls(func(_ context.Context, obj client.Object, patch client.Patch, _ ...client.PatchOption) error { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + t.Errorf("expected ConfigMap, got %T", obj) + } + if cm.Labels[trustedCABundleInjectLabel] != "true" { + t.Errorf("expected inject label in patch target, got %q", cm.Labels[trustedCABundleInjectLabel]) + } + if patch.Type() != types.JSONPatchType { + t.Errorf("expected JSONPatch, got %s", patch.Type()) + } + return nil + }) + }, + wantPatch: true, + }, + { + // Regression test for: labels updating with app: external-secrets of cm + // external-secrets-trusted-ca-bundle will block the reconcile in the http_proxy env. + // + // When the managed label (app=external-secrets) is removed from the ConfigMap, + // the object falls out of the label-filtered cache. The cached Exists() returns + // false, Create() fails with AlreadyExists, and the controller must patch only + // the metadata (labels/annotations), leaving CNO-managed Data/BinaryData untouched. + name: "Create returns AlreadyExists (label-filtered cache miss): patches metadata to restore labels", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, _ client.Object) (bool, error) { + return false, nil + }) + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "configmaps"}, trustedCABundleConfigMapName) + }) + cached.PatchCalls(func(_ context.Context, obj client.Object, patch client.Patch, _ ...client.PatchOption) error { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + t.Errorf("expected ConfigMap, got %T", obj) + } + if cm.Labels["app"] != "external-secrets" { + t.Errorf("expected app=external-secrets label in patch target, got %q", cm.Labels["app"]) + } + if cm.Labels[trustedCABundleInjectLabel] != "true" { + t.Errorf("expected inject label in patch target, got %q", cm.Labels[trustedCABundleInjectLabel]) + } + if patch.Type() != types.JSONPatchType { + t.Errorf("expected JSONPatch, got %s", patch.Type()) + } + return nil + }) + }, + wantPatch: true, + }, + { + name: "proxy not configured: ConfigMap reconcile skipped", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + noProxy: true, + }, + { + name: "cached Exists check fails", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, _ client.Object) (bool, error) { + return false, commontest.ErrTestClient + }) + }, + wantErr: "failed to check external-secrets/external-secrets-trusted-ca-bundle trusted CA bundle ConfigMap resource already exists: test client error", + }, + { + name: "Create fails with non-AlreadyExists error", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, _ client.Object) (bool, error) { + return false, nil + }) + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to create external-secrets/external-secrets-trusted-ca-bundle trusted CA bundle ConfigMap resource: test client error", + }, + { + name: "Patch fails after AlreadyExists from Create", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, _ client.Object) (bool, error) { + return false, nil + }) + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "configmaps"}, trustedCABundleConfigMapName) + }) + cached.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to patch external-secrets/external-secrets-trusted-ca-bundle trusted CA bundle ConfigMap metadata: test client error", + }, + { + name: "cached Patch fails when ConfigMap has wrong labels", + resourceMetadata: testResourceMetadata(commontest.TestExternalSecretsConfig()), + preReq: func(_ *Reconciler, cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { + existing := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCABundleConfigMapName, + Namespace: commontest.TestExternalSecretsNamespace, + Labels: nil, + }, + } + existing.DeepCopyInto(obj.(*corev1.ConfigMap)) + return true, nil + }) + cached.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to patch external-secrets/external-secrets-trusted-ca-bundle trusted CA bundle ConfigMap metadata: test client error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + cached := &fakes.FakeCtrlClient{} + uncached := &fakes.FakeCtrlClient{} + if tt.preReq != nil { + tt.preReq(r, cached, uncached) + } + r.CtrlClient = cached + r.UncachedClient = uncached + + var esc *operatorv1alpha1.ExternalSecretsConfig + if tt.noProxy { + esc = commontest.TestExternalSecretsConfig() + } else { + esc = testESCWithProxy() + } + + err := r.ensureTrustedCABundleConfigMap(esc, tt.resourceMetadata) + + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Errorf("ensureTrustedCABundleConfigMap() err = %v, wantErr = %v", err, tt.wantErr) + } + return + } + if err != nil { + t.Errorf("ensureTrustedCABundleConfigMap() unexpected error: %v", err) + } + + if tt.wantCreate && cached.CreateCallCount() == 0 { + t.Error("expected Create to be called, but it wasn't") + } + if tt.wantPatch && cached.PatchCallCount() == 0 { + t.Error("expected Patch to be called, but it wasn't") + } + if !tt.wantPatch && cached.PatchCallCount() > 0 { + t.Error("expected no Patch call, but one was made") + } + }) + } +} diff --git a/pkg/controller/external_secrets/controller.go b/pkg/controller/external_secrets/controller.go index 49035797c..3be983d48 100644 --- a/pkg/controller/external_secrets/controller.go +++ b/pkg/controller/external_secrets/controller.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -256,29 +257,43 @@ func checkAndRegisterCertificates(ctx context.Context, mgr ctrl.Manager, r *Reco func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { mapFunc := func(ctx context.Context, obj client.Object) []reconcile.Request { r.log.V(4).Info("received reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) - - objLabels := obj.GetLabels() - if objLabels != nil { - if objLabels[requestEnqueueLabelKey] == requestEnqueueLabelValue { - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Name: common.ExternalSecretsConfigObjectName, - }, - }, - } - } + return []reconcile.Request{ + { + NamespacedName: types.NamespacedName{ + Name: common.ExternalSecretsConfigObjectName, + }, + }, } - r.log.V(4).Info("object not of interest, ignoring reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) - return []reconcile.Request{} } - // predicate function to ignore events for objects not managed by controller. - managedResources := predicate.NewPredicateFuncs(func(object client.Object) bool { - return object.GetLabels() != nil && object.GetLabels()[requestEnqueueLabelKey] == requestEnqueueLabelValue - }) + isManagedResource := func(object client.Object) bool { + labels := object.GetLabels() + matches := labels != nil && labels[requestEnqueueLabelKey] == requestEnqueueLabelValue + r.log.V(4).Info("predicate evaluation", "object", fmt.Sprintf("%T", object), "name", object.GetName(), "namespace", object.GetNamespace(), "labels", labels, "matches", matches) + return matches + } + + // On updates, check both old and new objects so that events where the managed + // label is removed externally still trigger reconciliation and label restoration. + managedResources := predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return isManagedResource(e.Object) + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return isManagedResource(e.ObjectOld) || isManagedResource(e.ObjectNew) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return isManagedResource(e.Object) + }, + GenericFunc: func(e event.GenericEvent) bool { + return isManagedResource(e.Object) + }, + } - withIgnoreStatusUpdatePredicates := builder.WithPredicates(predicate.GenerationChangedPredicate{}, managedResources) + withIgnoreStatusUpdatePredicates := builder.WithPredicates( + predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{}), + managedResources, + ) managedResourcePredicate := builder.WithPredicates(managedResources) mgrBuilder := ctrl.NewControllerManagedBy(mgr). diff --git a/pkg/controller/external_secrets/deployments.go b/pkg/controller/external_secrets/deployments.go index f6d52b930..a9ad9973b 100644 --- a/pkg/controller/external_secrets/deployments.go +++ b/pkg/controller/external_secrets/deployments.go @@ -88,10 +88,9 @@ func (r *Reconciler) createOrApplyDeploymentFromAsset(esc *operatorv1alpha1.Exte } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "deployment resource %s updated", deploymentName) case !exist: - if err := r.Create(r.ctx, deployment); err != nil { - return common.FromClientError(err, "failed to create %s deployment resource", deploymentName) + if err := r.createWithFallback(deployment, resourceMetadata, deploymentName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "deployment resource %s created", deploymentName) default: r.log.V(4).Info("deployment resource already exists and is in expected state", "name", deploymentName) } diff --git a/pkg/controller/external_secrets/networkpolicy.go b/pkg/controller/external_secrets/networkpolicy.go index 7f111a0b4..53d8d1f76 100644 --- a/pkg/controller/external_secrets/networkpolicy.go +++ b/pkg/controller/external_secrets/networkpolicy.go @@ -144,10 +144,9 @@ func (r *Reconciler) createOrApplyCustomNetworkPolicy(esc *operatorv1alpha1.Exte } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s updated", networkPolicyName) case !exists: - if err := r.Create(r.ctx, networkPolicy); err != nil { - return common.FromClientError(err, "failed to create network policy %s", networkPolicyName) + if err := r.createWithFallback(networkPolicy, resourceMetadata, networkPolicyName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s created", networkPolicyName) default: r.log.V(4).Info("NetworkPolicy already up-to-date", "name", networkPolicyName) } @@ -183,10 +182,9 @@ func (r *Reconciler) createOrApplyNetworkPolicyFromAsset(esc *operatorv1alpha1.E } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s updated", networkPolicyName) case !exists: - if err := r.Create(r.ctx, networkPolicy); err != nil { - return common.FromClientError(err, "failed to create network policy %s", networkPolicyName) + if err := r.createWithFallback(networkPolicy, resourceMetadata, networkPolicyName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s created", networkPolicyName) default: r.log.V(4).Info("NetworkPolicy already up-to-date", "name", networkPolicyName) } @@ -288,10 +286,9 @@ func (r *Reconciler) reconcileProxyEgressPolicy(esc *operatorv1alpha1.ExternalSe } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "proxy egress NetworkPolicy %s updated", npName) case !exists: - if err := r.Create(r.ctx, np); err != nil { - return common.FromClientError(err, "failed to create proxy egress network policy %s", npName) + if err := r.createWithFallback(np, resourceMetadata, npName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "proxy egress NetworkPolicy %s created", npName) if recon { r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "proxy egress NetworkPolicy %s already exists", npName) } diff --git a/pkg/controller/external_secrets/networkpolicy_test.go b/pkg/controller/external_secrets/networkpolicy_test.go index 1f49620f9..ab5937679 100644 --- a/pkg/controller/external_secrets/networkpolicy_test.go +++ b/pkg/controller/external_secrets/networkpolicy_test.go @@ -146,7 +146,7 @@ func TestCreateOrApplyStaticNetworkPolicies(t *testing.T) { return nil }) }, - wantErr: "failed to create network policy external-secrets/eso-sys-deny-all-traffic: test client error", + wantErr: "failed to create NetworkPolicy external-secrets/eso-sys-deny-all-traffic: test client error", }, { name: "network policy exists check fails", @@ -344,7 +344,7 @@ func TestCreateOrApplyCustomNetworkPolicies(t *testing.T) { }, } }, - wantErr: "failed to create network policy external-secrets/eso-user-test-fail-policy: test client error", + wantErr: "failed to create NetworkPolicy external-secrets/eso-user-test-fail-policy: test client error", }, { name: "custom network policy updated successfully", diff --git a/pkg/controller/external_secrets/rbacs.go b/pkg/controller/external_secrets/rbacs.go index b094b5689..fabc7cba4 100644 --- a/pkg/controller/external_secrets/rbacs.go +++ b/pkg/controller/external_secrets/rbacs.go @@ -125,10 +125,9 @@ func (r *Reconciler) createOrApplyClusterRole(esc *operatorv1alpha1.ExternalSecr r.log.V(4).Info("clusterrole resource already exists and is in expected state", "name", clusterRoleName) } if !exist { - if err := r.Create(r.ctx, obj); err != nil { - return common.FromClientError(err, "failed to create %s clusterrole resource", clusterRoleName) + if err := r.createWithFallback(obj, resourceMetadata, clusterRoleName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrole resource %s created", clusterRoleName) } return nil @@ -171,10 +170,9 @@ func (r *Reconciler) createOrApplyClusterRoleBinding(esc *operatorv1alpha1.Exter r.log.V(4).Info("clusterrolebinding resource already exists and is in expected state", "name", clusterRoleBindingName) } if !exist { - if err := r.Create(r.ctx, obj); err != nil { - return common.FromClientError(err, "failed to create %s clusterrolebinding resource", clusterRoleBindingName) + if err := r.createWithFallback(obj, resourceMetadata, clusterRoleBindingName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "clusterrolebinding resource %s created", clusterRoleBindingName) } return nil @@ -215,10 +213,9 @@ func (r *Reconciler) createOrApplyRole(esc *operatorv1alpha1.ExternalSecretsConf r.log.V(4).Info("role resource already exists and is in expected state", "name", roleName) } if !exist { - if err := r.Create(r.ctx, obj); err != nil { - return common.FromClientError(err, "failed to create %s role resource", roleName) + if err := r.createWithFallback(obj, resourceMetadata, roleName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "role resource %s created", roleName) } return nil @@ -258,10 +255,9 @@ func (r *Reconciler) createOrApplyRoleBinding(esc *operatorv1alpha1.ExternalSecr r.log.V(4).Info("rolebinding resource already exists and is in expected state", "name", roleBindingName) } if !exist { - if err := r.Create(r.ctx, obj); err != nil { - return common.FromClientError(err, "failed to create %s rolebinding resource", roleBindingName) + if err := r.createWithFallback(obj, resourceMetadata, roleBindingName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s created", roleBindingName) } return nil diff --git a/pkg/controller/external_secrets/rbacs_test.go b/pkg/controller/external_secrets/rbacs_test.go index 136e7f3f3..352b870b0 100644 --- a/pkg/controller/external_secrets/rbacs_test.go +++ b/pkg/controller/external_secrets/rbacs_test.go @@ -124,7 +124,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { return nil }) }, - wantErr: `failed to create external-secrets-cert-controller clusterrolebinding resource: test client error`, + wantErr: `failed to create ClusterRoleBinding external-secrets-cert-controller: test client error`, }, { name: "clusterrole reconciliation updating to desired state fails", @@ -164,7 +164,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { return nil }) }, - wantErr: `failed to create external-secrets-cert-controller clusterrole resource: test client error`, + wantErr: `failed to create ClusterRole external-secrets-cert-controller: test client error`, }, { name: "role reconciliation updating to desired state fails", @@ -202,7 +202,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { return nil }) }, - wantErr: `failed to create external-secrets/external-secrets-leaderelection role resource: test client error`, + wantErr: `failed to create Role external-secrets/external-secrets-leaderelection: test client error`, }, { name: "rolebindings reconciliation updating to desired state fails", @@ -240,7 +240,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { return nil }) }, - wantErr: `failed to create external-secrets/external-secrets-leaderelection rolebinding resource: test client error`, + wantErr: `failed to create RoleBinding external-secrets/external-secrets-leaderelection: test client error`, }, { name: "clusterroles creation successful", diff --git a/pkg/controller/external_secrets/secret.go b/pkg/controller/external_secrets/secret.go index 7bd24cc0c..4c2f6be41 100644 --- a/pkg/controller/external_secrets/secret.go +++ b/pkg/controller/external_secrets/secret.go @@ -4,6 +4,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" @@ -33,10 +34,9 @@ func (r *Reconciler) createOrApplySecret(esc *operatorv1alpha1.ExternalSecretsCo } if exist && common.ObjectMetadataModified(desired, fetched, &resourceMetadata) { - r.log.V(1).Info("secret has been modified, updating to desired state", "name", secretName) - common.RemoveObsoleteAnnotations(desired, resourceMetadata) - if err := r.UpdateWithRetry(r.ctx, desired); err != nil { - return common.FromClientError(err, "failed to update %s secret resource", secretName) + r.log.V(1).Info("secret has been modified, patching metadata to desired state", "name", secretName) + if err := r.patchResourceMetadata(desired, resourceMetadata); err != nil { + return common.FromClientError(err, "failed to patch %s secret resource metadata", secretName) } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s reconciled back to desired state", secretName) } else { @@ -44,8 +44,20 @@ func (r *Reconciler) createOrApplySecret(esc *operatorv1alpha1.ExternalSecretsCo } if !exist { + // NOTE: This Secret cannot use the generic createWithFallback helper because + // its Data field is managed by the external-secrets cert-controller, which injects + // TLS content at runtime. On AlreadyExists we use a MergePatch that touches only + // metadata, leaving cert-controller-managed TLS certificates untouched. if err := r.Create(r.ctx, desired); err != nil { - return common.FromClientError(err, "failed to create %s secret resource", secretName) + if !apierrors.IsAlreadyExists(err) { + return common.FromClientError(err, "failed to create %s secret resource", secretName) + } + r.log.V(1).Info("secret exists on API server but absent from label-filtered cache, patching metadata", "name", secretName) + if patchErr := r.patchResourceMetadata(desired, resourceMetadata); patchErr != nil { + return common.FromClientError(patchErr, "failed to patch %s secret resource metadata", secretName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s restored to desired state", secretName) + return nil } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "secret resource %s created", secretName) } diff --git a/pkg/controller/external_secrets/secret_test.go b/pkg/controller/external_secrets/secret_test.go index bfebdb6f6..6a75d97b8 100644 --- a/pkg/controller/external_secrets/secret_test.go +++ b/pkg/controller/external_secrets/secret_test.go @@ -6,6 +6,8 @@ import ( "testing" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -87,7 +89,7 @@ func TestCreateOrApplySecret(t *testing.T) { wantErr: fmt.Sprintf("failed to check %s/%s secret resource already exists: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient), }, { - name: "reconciliation of secret fails while restoring to expected state", + name: "reconciliation of secret fails while patching metadata to expected state", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { if o, ok := obj.(*corev1.Secret); ok { @@ -104,14 +106,52 @@ func TestCreateOrApplySecret(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { if _, ok := obj.(*corev1.Secret); ok { return commontest.ErrTestClient } return nil }) }, - wantErr: fmt.Sprintf("failed to update %s/%s secret resource: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient), + wantErr: fmt.Sprintf("failed to patch %s/%s secret resource metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient), + }, + { + // Regression test: when the managed label is removed from the Secret, the object + // falls out of the label-filtered cache. cached Exists() returns false, Create() + // fails with AlreadyExists, and the controller must patch only the metadata, + // leaving cert-controller-injected TLS data untouched. + name: "Create returns AlreadyExists (label-filtered cache miss): patches metadata to restore labels", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "secrets"}, testValidateSecretResourceName) + }) + m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if _, ok := obj.(*corev1.Secret); ok { + if obj.GetLabels()["app"] != "external-secrets" { + t.Errorf("expected app=external-secrets label in patch target, got %v", obj.GetLabels()) + } + } + return nil + }) + }, + }, + { + name: "Patch fails after AlreadyExists from Create", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "secrets"}, testValidateSecretResourceName) + }) + m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: fmt.Sprintf("failed to patch %s/%s secret resource metadata: %s", commontest.TestExternalSecretsNamespace, testValidateSecretResourceName, commontest.ErrTestClient), }, { name: "reconciliation of secret which already exists in expected state", diff --git a/pkg/controller/external_secrets/service_test.go b/pkg/controller/external_secrets/service_test.go index cb5c0b430..67f06b7c0 100644 --- a/pkg/controller/external_secrets/service_test.go +++ b/pkg/controller/external_secrets/service_test.go @@ -62,7 +62,7 @@ func TestCreateOrApplyServices(t *testing.T) { }, } }, - wantErr: `failed to create service external-secrets/bitwarden-sdk-server: test client error`, + wantErr: `failed to create Service external-secrets/bitwarden-sdk-server: test client error`, }, { @@ -107,7 +107,7 @@ func TestCreateOrApplyServices(t *testing.T) { return commontest.ErrTestClient }) }, - wantErr: `failed to create service external-secrets/external-secrets-webhook: test client error`, + wantErr: `failed to create Service external-secrets/external-secrets-webhook: test client error`, }, { name: "service with custom annotations applied successfully", diff --git a/pkg/controller/external_secrets/serviceaccounts.go b/pkg/controller/external_secrets/serviceaccounts.go index f04067024..0c9d99be1 100644 --- a/pkg/controller/external_secrets/serviceaccounts.go +++ b/pkg/controller/external_secrets/serviceaccounts.go @@ -69,10 +69,9 @@ func (r *Reconciler) createOrApplyServiceAccounts(esc *operatorv1alpha1.External } if !exist { - if err := r.Create(r.ctx, desired); err != nil { - return common.FromClientError(err, "failed to create serviceaccount %s", serviceAccountName) + if err := r.createWithFallback(desired, resourceMetadata, serviceAccountName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "Created serviceaccount %s", serviceAccountName) } } diff --git a/pkg/controller/external_secrets/serviceaccounts_test.go b/pkg/controller/external_secrets/serviceaccounts_test.go index 5b57466f6..7eece199b 100644 --- a/pkg/controller/external_secrets/serviceaccounts_test.go +++ b/pkg/controller/external_secrets/serviceaccounts_test.go @@ -119,7 +119,7 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { return nil }) }, - wantErr: "failed to create serviceaccount external-secrets/external-secrets: test client error", + wantErr: "failed to create ServiceAccount external-secrets/external-secrets: test client error", }, { name: "serviceaccount with custom annotations applied successfully", diff --git a/pkg/controller/external_secrets/services.go b/pkg/controller/external_secrets/services.go index 0dd93d000..ab2329ae5 100644 --- a/pkg/controller/external_secrets/services.go +++ b/pkg/controller/external_secrets/services.go @@ -74,10 +74,9 @@ func (r *Reconciler) createOrApplyServiceFromAsset(esc *operatorv1alpha1.Externa } r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "Service %s updated", serviceName) case !exists: - if err := r.Create(r.ctx, service); err != nil { - return common.FromClientError(err, "failed to create service %s", serviceName) + if err := r.createWithFallback(service, resourceMetadata, serviceName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "Service %s created", serviceName) default: r.log.V(4).Info("Service already up-to-date", "name", serviceName) } diff --git a/pkg/controller/external_secrets/test_utils.go b/pkg/controller/external_secrets/test_utils.go index 77f82e6b9..87a40f77b 100644 --- a/pkg/controller/external_secrets/test_utils.go +++ b/pkg/controller/external_secrets/test_utils.go @@ -11,6 +11,8 @@ import ( rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" "github.com/go-logr/logr/testr" @@ -36,8 +38,12 @@ func testResourceMetadata(esc *operatorv1alpha1.ExternalSecretsConfig) common.Re // testReconciler returns a sample Reconciler instance. func testReconciler(t *testing.T) *Reconciler { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(certmanagerv1.AddToScheme(scheme)) + return &Reconciler{ - Scheme: runtime.NewScheme(), + Scheme: scheme, ctx: context.Background(), eventRecorder: record.NewFakeRecorder(100), log: testr.New(t), diff --git a/pkg/controller/external_secrets/utils.go b/pkg/controller/external_secrets/utils.go index 51291276d..bd566cf3d 100644 --- a/pkg/controller/external_secrets/utils.go +++ b/pkg/controller/external_secrets/utils.go @@ -2,13 +2,18 @@ package external_secrets import ( "context" + "encoding/json" "fmt" "net/url" "os" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "go.uber.org/zap/zapcore" @@ -203,3 +208,84 @@ func validateProxy(rawURL string) error { return nil } + +// createWithFallback attempts to create a resource and handles the AlreadyExists error that +// occurs when the resource exists on the API server but is invisible to the label-filtered +// informer cache (e.g. because the managed label app=external-secrets was externally removed). +// +// When Create returns AlreadyExists, this method bypasses the stale cache by using the +// uncached client to update the resource directly on the API server, restoring the managed +// labels and annotations to the desired state. +// +// It records a "created" event on success, or a "restored" event when the AlreadyExists +// fallback path is taken. +func (r *Reconciler) createWithFallback(desired client.Object, resourceMetadata common.ResourceMetadata, resourceName string, esc *operatorv1alpha1.ExternalSecretsConfig) error { + kind := desired.GetObjectKind().GroupVersionKind().Kind + if kind == "" { + gvk, gvkErr := apiutil.GVKForObject(desired, r.Scheme) + if gvkErr != nil { + r.log.V(5).Info("could not determine GVK, falling back to Go type name", + "type", fmt.Sprintf("%T", desired), "err", gvkErr) + kind = fmt.Sprintf("%T", desired) + } else { + kind = gvk.Kind + } + } + + if err := r.Create(r.ctx, desired); err != nil { + if !apierrors.IsAlreadyExists(err) { + return common.FromClientError(err, "failed to create %s %s", kind, resourceName) + } + + r.log.V(1).Info("resource exists on API server but absent from label-filtered cache, restoring desired state", + "kind", kind, "name", resourceName) + common.RemoveObsoleteAnnotations(desired, resourceMetadata) + if err := r.UncachedClient.UpdateWithRetry(r.ctx, desired); err != nil { + return common.FromClientError(err, "failed to restore %s %s to desired state", kind, resourceName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "%s resource %s restored to desired state", kind, resourceName) + return nil + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "%s resource %s created", kind, resourceName) + return nil +} + +// jsonPatchOp is a single JSON Patch operation. +type jsonPatchOp struct { + Op string `json:"op"` + Path string `json:"path"` + Value interface{} `json:"value"` +} + +// patchResourceMetadata fully replaces the labels and annotations on an object using a +// JSON Patch, leaving all other fields untouched. This is safe for co-managed +// resources where this operator owns all metadata but data fields are owned by an external +// controller (e.g. CNO-managed ConfigMap data, cert-controller-managed TLS Secret data). +// +// JSON Patch "add" on an existing path replaces the entire value, so the resulting +// labels/annotations on the server exactly match desired. +// +// RemoveObsoleteAnnotations is called defensively to strip any deleted annotation keys +// from desired before building the patch, regardless of whether the caller already did so. +func (r *Reconciler) patchResourceMetadata(desired client.Object, resourceMetadata common.ResourceMetadata) error { + common.RemoveObsoleteAnnotations(desired, resourceMetadata) + + annotations := desired.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + labels := desired.GetLabels() + if labels == nil { + labels = map[string]string{} + } + + ops := []jsonPatchOp{ + {Op: "add", Path: "/metadata/labels", Value: labels}, + {Op: "add", Path: "/metadata/annotations", Value: annotations}, + } + patchBytes, err := json.Marshal(ops) + if err != nil { + return fmt.Errorf("failed to marshal metadata patch: %w", err) + } + return r.CtrlClient.Patch(r.ctx, desired, client.RawPatch(types.JSONPatchType, patchBytes)) +} diff --git a/pkg/controller/external_secrets/utils_test.go b/pkg/controller/external_secrets/utils_test.go new file mode 100644 index 000000000..a18760e15 --- /dev/null +++ b/pkg/controller/external_secrets/utils_test.go @@ -0,0 +1,172 @@ +package external_secrets + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/external-secrets-operator/pkg/controller/client/fakes" + "github.com/openshift/external-secrets-operator/pkg/controller/common" + "github.com/openshift/external-secrets-operator/pkg/controller/commontest" +) + +func TestCreateWithFallback(t *testing.T) { + resourceMetadata := common.ResourceMetadata{ + Labels: controllerDefaultResourceLabels, + } + + tests := []struct { + name string + preReq func(cached *fakes.FakeCtrlClient, uncached *fakes.FakeCtrlClient) + customDesired func() client.Object + wantErr string + wantUpdate bool + }{ + { + name: "resource created successfully", + preReq: func(cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return nil + }) + }, + }, + { + name: "create fails with non-AlreadyExists error", + preReq: func(cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to create ServiceAccount test-ns/test-sa: test client error", + }, + { + name: "GVK resolved from object TypeMeta when pre-set, skipping scheme lookup", + preReq: func(cached *fakes.FakeCtrlClient, _ *fakes.FakeCtrlClient) { + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return commontest.ErrTestClient + }) + }, + customDesired: func() client.Object { + return &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{Kind: "ServiceAccount", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: controllerDefaultResourceLabels, + }, + } + }, + wantErr: "failed to create ServiceAccount test-ns/test-sa: test client error", + }, + { + name: "AlreadyExists triggers uncached update to restore labels", + preReq: func(cached *fakes.FakeCtrlClient, uncached *fakes.FakeCtrlClient) { + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "serviceaccounts"}, "test-sa") + }) + uncached.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { + if obj.GetLabels()["app"] != "external-secrets" { + t.Errorf("expected managed label app=external-secrets to be present, got %v", obj.GetLabels()) + } + return nil + }) + }, + wantUpdate: true, + }, + { + name: "AlreadyExists but uncached update fails", + preReq: func(cached *fakes.FakeCtrlClient, uncached *fakes.FakeCtrlClient) { + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "serviceaccounts"}, "test-sa") + }) + uncached.UpdateWithRetryCalls(func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to restore ServiceAccount test-ns/test-sa to desired state: test client error", + wantUpdate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + cached := &fakes.FakeCtrlClient{} + uncached := &fakes.FakeCtrlClient{} + if tt.preReq != nil { + tt.preReq(cached, uncached) + } + r.CtrlClient = cached + r.UncachedClient = uncached + + var desired client.Object + if tt.customDesired != nil { + desired = tt.customDesired() + } else { + desired = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: controllerDefaultResourceLabels, + }, + } + } + + err := r.createWithFallback(desired, resourceMetadata, "test-ns/test-sa", commontest.TestExternalSecretsConfig()) + + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Errorf("createWithFallback() err = %v, wantErr = %v", err, tt.wantErr) + } + } else if err != nil { + t.Errorf("createWithFallback() unexpected error: %v", err) + } + + if tt.wantUpdate && uncached.UpdateWithRetryCallCount() == 0 { + t.Error("expected UncachedClient.UpdateWithRetry to be called, but it wasn't") + } + if !tt.wantUpdate && uncached.UpdateWithRetryCallCount() > 0 { + t.Error("expected no UncachedClient.UpdateWithRetry call, but one was made") + } + }) + } +} + +// TestCreateWithFallback_GVKResolutionFallback verifies that when the scheme has no +// registered types, createWithFallback falls back to the Go type name in log and error messages. +func TestCreateWithFallback_GVKResolutionFallback(t *testing.T) { + resourceMetadata := common.ResourceMetadata{ + Labels: controllerDefaultResourceLabels, + } + + r := testReconciler(t) + // Use an empty scheme so apiutil.GVKForObject cannot resolve the type. + r.Scheme = runtime.NewScheme() + + cached := &fakes.FakeCtrlClient{} + cached.CreateCalls(func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return commontest.ErrTestClient + }) + r.CtrlClient = cached + r.UncachedClient = &fakes.FakeCtrlClient{} + + desired := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: controllerDefaultResourceLabels, + }, + } + + err := r.createWithFallback(desired, resourceMetadata, "test-ns/test-sa", commontest.TestExternalSecretsConfig()) + wantErr := "failed to create *v1.ServiceAccount test-ns/test-sa: test client error" + if err == nil || err.Error() != wantErr { + t.Errorf("createWithFallback() err = %v, wantErr = %v", err, wantErr) + } +} diff --git a/pkg/controller/external_secrets/validatingwebhook.go b/pkg/controller/external_secrets/validatingwebhook.go index 0c8fc0d03..190235fc8 100644 --- a/pkg/controller/external_secrets/validatingwebhook.go +++ b/pkg/controller/external_secrets/validatingwebhook.go @@ -39,10 +39,9 @@ func (r *Reconciler) createOrApplyValidatingWebhookConfiguration(esc *operatorv1 } if !exist { - if err := r.Create(r.ctx, desired); err != nil { - return common.FromClientError(err, "failed to create validatingWebhook resource %s", validatingWebhookName) + if err := r.createWithFallback(desired, resourceMetadata, validatingWebhookName, esc); err != nil { + return err } - r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "validatingWebhook resource %s created", validatingWebhookName) } } return nil diff --git a/pkg/controller/external_secrets/validatingwebhook_test.go b/pkg/controller/external_secrets/validatingwebhook_test.go index 4617b0619..19eafaa18 100644 --- a/pkg/controller/external_secrets/validatingwebhook_test.go +++ b/pkg/controller/external_secrets/validatingwebhook_test.go @@ -80,7 +80,7 @@ func TestCreateOrApplyValidatingWebhookConfiguration(t *testing.T) { return nil }) }, - wantErr: fmt.Sprintf("failed to create validatingWebhook resource %s: %s", testValidateWebhookConfigurationResourceName, commontest.ErrTestClient), + wantErr: fmt.Sprintf("failed to create ValidatingWebhookConfiguration %s: %s", testValidateWebhookConfigurationResourceName, commontest.ErrTestClient), }, { name: "validatingWebhookConfiguration creation successful", diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 229b59038..c29dba34a 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -1056,6 +1056,677 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, }) + // Context("Managed Annotation Restoration") tests that when a managed annotation is + // externally removed from a resource, the operator detects the drift and restores it. + // Resources intentionally excluded: + // - Deployment: Kubernetes adds deployment.kubernetes.io/revision on every rollout, + // so annotation events are suppressed to avoid infinite reconcile loops. + Context("Managed Annotation Restoration", Ordered, Label("Platform:AWS"), func() { + const ( + restorationAnnotationKey = "example.com/restoration-test" + restorationAnnotationValue = "managed-value" + ) + + BeforeAll(func() { + By("Adding the restoration test annotation to ExternalSecretsConfig spec") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + cr := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, cr); err != nil { + return err + } + if cr.Spec.ControllerConfig.Annotations == nil { + cr.Spec.ControllerConfig.Annotations = make(map[string]string) + } + cr.Spec.ControllerConfig.Annotations[restorationAnnotationKey] = restorationAnnotationValue + return runtimeClient.Update(ctx, cr) + }) + Expect(err).NotTo(HaveOccurred(), "should add restoration annotation to ExternalSecretsConfig") + }) + + AfterAll(func() { + By("Removing the restoration test annotation from ExternalSecretsConfig spec") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + cr := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, cr); err != nil { + return err + } + delete(cr.Spec.ControllerConfig.Annotations, restorationAnnotationKey) + return runtimeClient.Update(ctx, cr) + }) + Expect(err).NotTo(HaveOccurred(), "should remove restoration annotation from ExternalSecretsConfig") + + By("Verifying ExternalSecretsConfig is Ready after annotation restoration tests") + Expect(utils.WaitForExternalSecretsConfigReady(ctx, dynamicClient, "cluster", 2*time.Minute)).To(Succeed()) + + By("Verifying operand pods are still ready after annotation restoration tests") + Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ + operandCoreControllerPodPrefix, + operandCertControllerPodPrefix, + operandWebhookPodPrefix, + })).To(Succeed()) + }) + + It("should restore a managed annotation on a ServiceAccount after external removal", func() { + saName := "external-secrets" + + By("Verifying ServiceAccount has the managed annotation initially") + Eventually(func(g Gomega) { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(sa.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the ServiceAccount") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(sa.Annotations, restorationAnnotationKey) + _, err = clientset.CoreV1().ServiceAccounts(operandNamespace).Update(ctx, sa, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(sa.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on ServiceAccount %s", saName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a Role after external removal", func() { + roleName := "external-secrets-leaderelection" + + By("Verifying Role has the managed annotation initially") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(role.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the Role") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(role.Annotations, restorationAnnotationKey) + _, err = clientset.RbacV1().Roles(operandNamespace).Update(ctx, role, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(role.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on Role %s", roleName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a RoleBinding after external removal", func() { + roleBindingName := "external-secrets-leaderelection" + + By("Verifying RoleBinding has the managed annotation initially") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(rb.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the RoleBinding") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(rb.Annotations, restorationAnnotationKey) + _, err = clientset.RbacV1().RoleBindings(operandNamespace).Update(ctx, rb, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(rb.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on RoleBinding %s", roleBindingName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a ClusterRole after external removal", func() { + clusterRoleName := "external-secrets-controller" + + By("Verifying ClusterRole has the managed annotation initially") + Eventually(func(g Gomega) { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cr.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the ClusterRole") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(cr.Annotations, restorationAnnotationKey) + _, err = clientset.RbacV1().ClusterRoles().Update(ctx, cr, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cr.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on ClusterRole %s", clusterRoleName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a ClusterRoleBinding after external removal", func() { + clusterRoleBindingName := "external-secrets-controller" + + By("Verifying ClusterRoleBinding has the managed annotation initially") + Eventually(func(g Gomega) { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(crb.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the ClusterRoleBinding") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(crb.Annotations, restorationAnnotationKey) + _, err = clientset.RbacV1().ClusterRoleBindings().Update(ctx, crb, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(crb.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on ClusterRoleBinding %s", clusterRoleBindingName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a Service after external removal", func() { + serviceName := "external-secrets-webhook" + + By("Verifying Service has the managed annotation initially") + Eventually(func(g Gomega) { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(svc.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the Service") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(svc.Annotations, restorationAnnotationKey) + _, err = clientset.CoreV1().Services(operandNamespace).Update(ctx, svc, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(svc.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on Service %s", serviceName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore a managed annotation on a NetworkPolicy after external removal", func() { + npName := "eso-sys-deny-all-traffic" + + By("Verifying NetworkPolicy has the managed annotation initially") + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(np.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Externally removing the managed annotation from the NetworkPolicy") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(np.Annotations, restorationAnnotationKey) + _, err = clientset.NetworkingV1().NetworkPolicies(operandNamespace).Update(ctx, np, metav1.UpdateOptions{}) + return err + })).To(Succeed()) + + By("Waiting for operator to restore the managed annotation") + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(np.Annotations).To(HaveKeyWithValue(restorationAnnotationKey, restorationAnnotationValue), + "operator should restore annotation on NetworkPolicy %s", npName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + }) + + // Context("Custom Labels") tests that labels added via esc.Spec.ControllerConfig.Labels + // are propagated to all managed resources, and that removing a label from the spec + // causes the operator to remove it from all resources. This exercises the full + // label lifecycle across every resource type, including the co-managed Secret whose + // metadata is patched via JSON Patch rather than a full update. + Context("Custom Labels", Label("Platform:AWS"), func() { + AfterEach(func() { + By("Verifying ExternalSecretsConfig is Ready and not Degraded after label lifecycle test") + Expect(utils.WaitForExternalSecretsConfigReady(ctx, dynamicClient, "cluster", 2*time.Minute)).To(Succeed(), + "ExternalSecretsConfig should remain Ready after custom label add/remove") + + By("Verifying operand pods are still ready after label lifecycle test") + Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ + operandCoreControllerPodPrefix, + operandCertControllerPodPrefix, + operandWebhookPodPrefix, + })).To(Succeed(), "operand pods should still be running after custom label lifecycle test") + }) + + It("should apply and remove custom labels to all managed resources", func() { + testLabels := map[string]string{ + "mycompany.io/env": "staging", + } + + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + Expect(runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR)).To(Succeed(), + "should get ExternalSecretsConfig to capture initial state") + originalLabels := maps.Clone(existingCR.Spec.ControllerConfig.Labels) + + defer func() { + By("Restoring original labels on ExternalSecretsConfig CR") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + currentCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, currentCR); err != nil { + return err + } + currentCR.Spec.ControllerConfig.Labels = originalLabels + return runtimeClient.Update(ctx, currentCR) + }) + Expect(err).NotTo(HaveOccurred(), "should restore labels on ExternalSecretsConfig") + }() + + By("Updating ExternalSecretsConfig with custom labels") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + currentCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, currentCR); err != nil { + return err + } + merged := make(map[string]string) + if originalLabels != nil { + maps.Copy(merged, originalLabels) + } + maps.Copy(merged, testLabels) + currentCR.Spec.ControllerConfig.Labels = merged + return runtimeClient.Update(ctx, currentCR) + }) + Expect(err).NotTo(HaveOccurred(), "should update ExternalSecretsConfig with custom labels") + + By("Waiting for external-secrets operand pods to be ready") + Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ + operandCoreControllerPodPrefix, + operandCertControllerPodPrefix, + operandWebhookPodPrefix, + })).To(Succeed()) + + for _, resourceType := range getResourceTypesToVerify() { + By(fmt.Sprintf("Verifying custom labels are applied to %s resources", resourceType.name)) + Eventually(func(g Gomega) { + objects, err := resourceType.listFunc(ctx, clientset, operandNamespace, g) + g.Expect(err).NotTo(HaveOccurred(), "should list %s", resourceType.name) + + for _, obj := range objects { + if !strings.HasPrefix(obj.GetName(), "external-secrets") { + continue + } + for k, v := range testLabels { + g.Expect(obj.GetLabels()).To(HaveKeyWithValue(k, v), + "%s %s should have label %s=%s", resourceType.name, obj.GetName(), k, v) + } + } + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + } + + By("Removing custom labels from ExternalSecretsConfig CR") + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + currentCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, currentCR); err != nil { + return err + } + for k := range testLabels { + delete(currentCR.Spec.ControllerConfig.Labels, k) + } + return runtimeClient.Update(ctx, currentCR) + }) + Expect(err).NotTo(HaveOccurred(), "should remove custom labels from ExternalSecretsConfig") + + for _, resourceType := range getResourceTypesToVerify() { + By(fmt.Sprintf("Verifying custom labels are removed from %s resources", resourceType.name)) + Eventually(func(g Gomega) { + objects, err := resourceType.listFunc(ctx, clientset, operandNamespace, g) + g.Expect(err).NotTo(HaveOccurred(), "should list %s", resourceType.name) + + for _, obj := range objects { + if !strings.HasPrefix(obj.GetName(), "external-secrets") { + continue + } + for k := range testLabels { + g.Expect(obj.GetLabels()).NotTo(HaveKey(k), + "%s %s should NOT have label %s after removal", resourceType.name, obj.GetName(), k) + } + } + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + } + }) + }) + + Context("Managed Label Restoration", Label("Platform:AWS"), func() { + const ( + managedLabelKey = "app" + managedLabelValue = "external-secrets" + ) + + AfterEach(func() { + By("Verifying ExternalSecretsConfig is Ready and not Degraded after label restoration") + Expect(utils.WaitForExternalSecretsConfigReady(ctx, dynamicClient, "cluster", 2*time.Minute)).To(Succeed(), + "ExternalSecretsConfig should remain Ready after label tampering and restoration") + + By("Verifying operand pods are still ready after label restoration") + Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ + operandCoreControllerPodPrefix, + operandCertControllerPodPrefix, + operandWebhookPodPrefix, + })).To(Succeed(), "operand pods should still be running after label restoration") + }) + + It("should restore the app=external-secrets label on a ServiceAccount after external removal", func() { + saName := "external-secrets" + + By("Verifying ServiceAccount has the managed label initially") + Eventually(func(g Gomega) { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(sa.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the ServiceAccount") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(sa.Labels, managedLabelKey) + _, err = clientset.CoreV1().ServiceAccounts(operandNamespace).Update(ctx, sa, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + sa, err := clientset.CoreV1().ServiceAccounts(operandNamespace).Get(ctx, saName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(sa.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on ServiceAccount %s", saName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a Role after external removal", func() { + roleName := "external-secrets-leaderelection" + + By("Verifying Role has the managed label initially") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(role.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the Role") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(role.Labels, managedLabelKey) + _, err = clientset.RbacV1().Roles(operandNamespace).Update(ctx, role, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(operandNamespace).Get(ctx, roleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(role.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on Role %s", roleName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a Deployment after external removal", func() { + depName := "external-secrets" + + By("Verifying Deployment has the managed label initially") + Eventually(func(g Gomega) { + dep, err := clientset.AppsV1().Deployments(operandNamespace).Get(ctx, depName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dep.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the Deployment") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + dep, err := clientset.AppsV1().Deployments(operandNamespace).Get(ctx, depName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(dep.Labels, managedLabelKey) + _, err = clientset.AppsV1().Deployments(operandNamespace).Update(ctx, dep, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + dep, err := clientset.AppsV1().Deployments(operandNamespace).Get(ctx, depName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dep.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on Deployment %s", depName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + // Secret uses patchResourceMetadata (JSON Patch) rather than UpdateWithRetry because + // its Data field is co-managed by the cert-controller. This test specifically exercises + // that the metadata-only patch path correctly restores the managed label. + It("should restore the app=external-secrets label on a Secret after external removal", func() { + secretName := "external-secrets-webhook" + + By("Verifying Secret has the managed label initially") + Eventually(func(g Gomega) { + secret, err := clientset.CoreV1().Secrets(operandNamespace).Get(ctx, secretName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(secret.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the Secret") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + secret, err := clientset.CoreV1().Secrets(operandNamespace).Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(secret.Labels, managedLabelKey) + _, err = clientset.CoreV1().Secrets(operandNamespace).Update(ctx, secret, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + secret, err := clientset.CoreV1().Secrets(operandNamespace).Get(ctx, secretName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(secret.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on Secret %s", secretName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a Service after external removal", func() { + serviceName := "external-secrets-webhook" + + By("Verifying Service has the managed label initially") + Eventually(func(g Gomega) { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(svc.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the Service") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(svc.Labels, managedLabelKey) + _, err = clientset.CoreV1().Services(operandNamespace).Update(ctx, svc, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + svc, err := clientset.CoreV1().Services(operandNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(svc.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on Service %s", serviceName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a RoleBinding after external removal", func() { + roleBindingName := "external-secrets-leaderelection" + + By("Verifying RoleBinding has the managed label initially") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(rb.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the RoleBinding") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(rb.Labels, managedLabelKey) + _, err = clientset.RbacV1().RoleBindings(operandNamespace).Update(ctx, rb, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(operandNamespace).Get(ctx, roleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(rb.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on RoleBinding %s", roleBindingName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a ClusterRole after external removal", func() { + clusterRoleName := "external-secrets-controller" + + By("Verifying ClusterRole has the managed label initially") + Eventually(func(g Gomega) { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cr.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the ClusterRole") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(cr.Labels, managedLabelKey) + _, err = clientset.RbacV1().ClusterRoles().Update(ctx, cr, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + cr, err := clientset.RbacV1().ClusterRoles().Get(ctx, clusterRoleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cr.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on ClusterRole %s", clusterRoleName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a ClusterRoleBinding after external removal", func() { + clusterRoleBindingName := "external-secrets-controller" + + By("Verifying ClusterRoleBinding has the managed label initially") + Eventually(func(g Gomega) { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(crb.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the ClusterRoleBinding") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(crb.Labels, managedLabelKey) + _, err = clientset.RbacV1().ClusterRoleBindings().Update(ctx, crb, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + crb, err := clientset.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBindingName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(crb.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on ClusterRoleBinding %s", clusterRoleBindingName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should restore the app=external-secrets label on a NetworkPolicy after external removal", func() { + npName := "eso-sys-deny-all-traffic" + + By("Verifying NetworkPolicy has the managed label initially") + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(np.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue)) + }, time.Minute, 5*time.Second).Should(Succeed()) + + By("Removing the managed label from the NetworkPolicy") + Expect(retry.RetryOnConflict(retry.DefaultRetry, func() error { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + if err != nil { + return err + } + delete(np.Labels, managedLabelKey) + _, err = clientset.NetworkingV1().NetworkPolicies(operandNamespace).Update(ctx, np, metav1.UpdateOptions{}) + return err + })).To(Succeed(), "should remove the managed label") + + By("Waiting for operator to restore the managed label") + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, npName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(np.Labels).To(HaveKeyWithValue(managedLabelKey, managedLabelValue), + "operator should restore app=external-secrets on NetworkPolicy %s", npName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + }) + AfterAll(func() { By("Deleting the externalsecrets.openshift.operator.io/cluster CR") loader.DeleteFromFile(testassets.ReadFile, externalSecretsFile, "") diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go index b3a09d13a..dda844a91 100644 --- a/test/e2e/helpers_test.go +++ b/test/e2e/helpers_test.go @@ -155,6 +155,36 @@ func getResourceTypesToVerify() []resourceType { return objects, nil }, }, + { + // ClusterRoles are cluster-scoped; the namespace parameter is unused. + name: "ClusterRole", + listFunc: func(ctx context.Context, clientset *kubernetes.Clientset, _ string, g Gomega) ([]metav1.Object, error) { + clusterRoles, err := clientset.RbacV1().ClusterRoles().List(ctx, listOnlyManagedResources) + if err != nil { + return nil, err + } + objects := make([]metav1.Object, 0, len(clusterRoles.Items)) + for i := range clusterRoles.Items { + objects = append(objects, &clusterRoles.Items[i]) + } + return objects, nil + }, + }, + { + // ClusterRoleBindings are cluster-scoped; the namespace parameter is unused. + name: "ClusterRoleBinding", + listFunc: func(ctx context.Context, clientset *kubernetes.Clientset, _ string, g Gomega) ([]metav1.Object, error) { + crbs, err := clientset.RbacV1().ClusterRoleBindings().List(ctx, listOnlyManagedResources) + if err != nil { + return nil, err + } + objects := make([]metav1.Object, 0, len(crbs.Items)) + for i := range crbs.Items { + objects = append(objects, &crbs.Items[i]) + } + return objects, nil + }, + }, } }