diff --git a/pkg/operator/encryption/controllers/migrators/inprocess.go b/pkg/operator/encryption/controllers/migrators/inprocess.go index 2a9c1e6022..1d54d4a014 100644 --- a/pkg/operator/encryption/controllers/migrators/inprocess.go +++ b/pkg/operator/encryption/controllers/migrators/inprocess.go @@ -134,9 +134,32 @@ func (m *InProcessMigrator) runMigration(gvr schema.GroupVersionResource, writeK listProcessor := newListProcessor(ctx, m.dynamicClient, func(obj *unstructured.Unstructured) error { for { _, updateErr := d.Namespace(obj.GetNamespace()).Update(ctx, obj, metav1.UpdateOptions{}) - if updateErr == nil || errors.IsNotFound(updateErr) || errors.IsConflict(updateErr) { + if updateErr == nil || errors.IsConflict(updateErr) { return nil } + if errors.IsNotFound(updateErr) { + // NotFound on Update can mean either: + // (a) the object was deleted between List and Update, or + // (b) the namespace was deleted but the object persists in etcd + // (NamespaceLifecycle admission rejects Updates to non-existent namespaces). + // + // GET bypasses admission and reads directly from storage. + // If the object still exists, it is an orphan that cannot be re-encrypted. + // Failing the migration prevents the state machine from pruning the old + // encryption key, which would make orphaned objects permanently undecryptable. + _, getErr := d.Namespace(obj.GetNamespace()).Get(ctx, obj.GetName(), metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + return nil + } + if getErr != nil { + return fmt.Errorf("failed to verify existence of %s/%s after update returned NotFound: %v", + obj.GetNamespace(), obj.GetName(), getErr) + } + return fmt.Errorf("cannot migrate %s/%s: the object exists in etcd but its namespace %q does not, "+ + "blocking migration to prevent the old encryption key from being pruned "+ + "(delete the orphaned object from etcd to unblock, see https://access.redhat.com/solutions/6769801)", + obj.GetNamespace(), obj.GetName(), obj.GetNamespace()) + } if retryable := canRetry(updateErr); retryable == nil || *retryable == false { klog.Warningf("Update of %s/%s failed: %v", obj.GetNamespace(), obj.GetName(), updateErr) return updateErr // not retryable or we don't know. Return error and controller will restart migration. diff --git a/pkg/operator/encryption/controllers/migrators/inprocess_test.go b/pkg/operator/encryption/controllers/migrators/inprocess_test.go index 32598b0ab1..a87f127154 100644 --- a/pkg/operator/encryption/controllers/migrators/inprocess_test.go +++ b/pkg/operator/encryption/controllers/migrators/inprocess_test.go @@ -10,6 +10,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -138,6 +139,117 @@ func TestInProcessMigrator(t *testing.T) { } } +func TestInProcessMigratorOrphanedNamespace(t *testing.T) { + apiResources := []metav1.APIResource{ + { + Name: "configmaps", + Namespaced: true, + Group: "", + Version: "v1", + }, + } + gr := schema.GroupResource{Resource: "configmaps"} + gvrToListKind := map[schema.GroupVersionResource]string{ + gr.WithVersion("v1"): "ConfigMapList", + } + + testCases := []struct { + name string + getReturnsNot bool + expectError bool + }{ + { + name: "orphaned object in deleted namespace blocks migration", + getReturnsNot: false, + expectError: true, + }, + { + name: "genuinely deleted object is skipped", + getReturnsNot: true, + expectError: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset() + + scheme := runtime.NewScheme() + resources := []runtime.Object{ + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm1", Namespace: "existing-ns"}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "orphan-cm", Namespace: "deleted-ns"}}, + } + var unstructuredObjs []runtime.Object + for _, rawObject := range resources { + rawUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(rawObject.DeepCopyObject()) + if err != nil { + t.Fatal(err) + } + unstructured.SetNestedField(rawUnstructured, "v1", "apiVersion") + unstructured.SetNestedField(rawUnstructured, reflect.TypeOf(rawObject).Elem().Name(), "kind") + unstructuredObjs = append(unstructuredObjs, &unstructured.Unstructured{Object: rawUnstructured}) + } + dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind, unstructuredObjs...) + + // Simulate NamespaceLifecycle rejecting Updates in "deleted-ns". + dynamicClient.PrependReactor("update", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) { + updateAction := action.(clientgotesting.UpdateAction) + if updateAction.GetNamespace() == "deleted-ns" { + return true, nil, errors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, "deleted-ns") + } + return false, nil, nil + }) + + if tc.getReturnsNot { + // Simulate the object being genuinely deleted from etcd. + dynamicClient.PrependReactor("get", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) { + getAction := action.(clientgotesting.GetAction) + if getAction.GetNamespace() == "deleted-ns" { + return true, nil, errors.NewNotFound(gr, getAction.GetName()) + } + return false, nil, nil + }) + } + + discoveryClient := &fakeDisco{ + delegate: fakeKubeClient.Discovery(), + serverPreferredRes: []*metav1.APIResourceList{ + { + APIResources: apiResources, + }, + }, + } + + handler := &fakeHandler{} + m := NewInProcessMigrator(dynamicClient, discoveryClient) + m.AddEventHandler(handler) + + err := wait.PollImmediate(100*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { + finished, result, _, err := m.EnsureMigration(gr, "1") + if err != nil { + return false, err + } + if !finished { + return false, nil + } + if tc.expectError { + if result == nil { + return false, fmt.Errorf("expected migration to fail for orphaned namespace object, but it succeeded") + } + t.Logf("migration correctly failed: %v", result) + } else { + if result != nil { + return false, fmt.Errorf("unexpected migration error: %v", result) + } + } + return true, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + func validateMigratedResources(ts *testing.T, actions []clientgotesting.Action, unstructuredObjs []runtime.Object, targetGRs []schema.GroupResource) { ts.Helper()