Skip to content

Commit ce85681

Browse files
committed
fix: scope SpinAppExecutor finalizer check to executor's namespace
The handleDeletion() method lists dependent SpinApps by executor name across all namespaces. This causes a SpinApp in namespace B to block deletion of an unrelated SpinAppExecutor with the same name in namespace A — even though there are zero SpinApps in namespace A. This is particularly impactful because the common pattern (and the Helm chart bootstrap hook) uses the same executor name "containerd-shim-spin" in every namespace. All executors with that name become cross-coupled. The fix adds client.InNamespace(executor.Namespace) to scope the dependent SpinApp lookup to the executor's own namespace. Includes a regression test that creates same-named executors in two namespaces with a SpinApp only in one, then verifies deleting the executor in the other namespace succeeds. Fixes #480
1 parent 7082751 commit ce85681

2 files changed

Lines changed: 99 additions & 1 deletion

File tree

internal/controller/spinappexecutor_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (r *SpinAppExecutorReconciler) handleDeletion(ctx context.Context, executor
103103
log := logging.FromContext(ctx)
104104

105105
var spinApps spinv1alpha1.SpinAppList
106-
if err := r.Client.List(ctx, &spinApps, client.MatchingFields{spinAppExecutorKey: executor.Name}); err != nil {
106+
if err := r.Client.List(ctx, &spinApps, client.InNamespace(executor.Namespace), client.MatchingFields{spinAppExecutorKey: executor.Name}); err != nil {
107107
log.Error(err, "Unable to list SpinApps")
108108
return err
109109
}

internal/controller/spinappexecutor_controller_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ import (
2525
spinv1alpha1 "github.com/spinframework/spin-operator/api/v1alpha1"
2626
"github.com/spinframework/spin-operator/internal/generics"
2727
"github.com/stretchr/testify/require"
28+
corev1 "k8s.io/api/core/v1"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
ctrl "sigs.k8s.io/controller-runtime"
31+
"sigs.k8s.io/controller-runtime/pkg/client"
3032
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3133
controllerconfig "sigs.k8s.io/controller-runtime/pkg/config"
3234
"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -145,3 +147,99 @@ func testContainerdShimSpinExecutor() *spinv1alpha1.SpinAppExecutor {
145147
},
146148
}
147149
}
150+
151+
func TestSpinAppExecutorReconcile_CrossNamespaceDoesNotBlockDeletion(t *testing.T) {
152+
t.Parallel()
153+
154+
envTest, mgr, _ := setupExecutorController(t)
155+
156+
ctx, cancelFunc := context.WithTimeout(context.Background(), 10*time.Second)
157+
defer cancelFunc()
158+
159+
var wg sync.WaitGroup
160+
wg.Add(1)
161+
go func() {
162+
require.NoError(t, mgr.Start(ctx))
163+
wg.Done()
164+
}()
165+
166+
// Create two namespaces
167+
nsA := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-ns-a"}}
168+
nsB := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-ns-b"}}
169+
require.NoError(t, envTest.k8sClient.Create(ctx, nsA))
170+
require.NoError(t, envTest.k8sClient.Create(ctx, nsB))
171+
172+
// Create an executor with the same name in both namespaces
173+
executorA := &spinv1alpha1.SpinAppExecutor{
174+
ObjectMeta: metav1.ObjectMeta{
175+
Name: "containerd-shim-spin",
176+
Namespace: "test-ns-a",
177+
},
178+
Spec: spinv1alpha1.SpinAppExecutorSpec{
179+
CreateDeployment: true,
180+
DeploymentConfig: &spinv1alpha1.ExecutorDeploymentConfig{
181+
RuntimeClassName: generics.Ptr("wasmtime-spin-v2"),
182+
},
183+
},
184+
}
185+
executorB := &spinv1alpha1.SpinAppExecutor{
186+
ObjectMeta: metav1.ObjectMeta{
187+
Name: "containerd-shim-spin",
188+
Namespace: "test-ns-b",
189+
},
190+
Spec: spinv1alpha1.SpinAppExecutorSpec{
191+
CreateDeployment: true,
192+
DeploymentConfig: &spinv1alpha1.ExecutorDeploymentConfig{
193+
RuntimeClassName: generics.Ptr("wasmtime-spin-v2"),
194+
},
195+
},
196+
}
197+
require.NoError(t, envTest.k8sClient.Create(ctx, executorA))
198+
require.NoError(t, envTest.k8sClient.Create(ctx, executorB))
199+
200+
// Wait for finalizers to be added by the controller
201+
require.Eventually(t, func() bool {
202+
var exec spinv1alpha1.SpinAppExecutor
203+
if err := envTest.k8sClient.Get(ctx, client.ObjectKeyFromObject(executorA), &exec); err != nil {
204+
return false
205+
}
206+
return len(exec.Finalizers) > 0
207+
}, 5*time.Second, 100*time.Millisecond, "executor A should have finalizer")
208+
209+
require.Eventually(t, func() bool {
210+
var exec spinv1alpha1.SpinAppExecutor
211+
if err := envTest.k8sClient.Get(ctx, client.ObjectKeyFromObject(executorB), &exec); err != nil {
212+
return false
213+
}
214+
return len(exec.Finalizers) > 0
215+
}, 5*time.Second, 100*time.Millisecond, "executor B should have finalizer")
216+
217+
// Create a SpinApp in namespace B that references the executor by name
218+
spinApp := &spinv1alpha1.SpinApp{
219+
ObjectMeta: metav1.ObjectMeta{
220+
Name: "test-app",
221+
Namespace: "test-ns-b",
222+
},
223+
Spec: spinv1alpha1.SpinAppSpec{
224+
Executor: "containerd-shim-spin",
225+
Image: "ghcr.io/spinkube/containerd-shim-spin/examples/spin-rust-hello:v0.15.1",
226+
Replicas: 1,
227+
},
228+
}
229+
require.NoError(t, envTest.k8sClient.Create(ctx, spinApp))
230+
231+
// Delete executor in namespace A — this should succeed because
232+
// the SpinApp is in namespace B, not namespace A.
233+
require.NoError(t, envTest.k8sClient.Delete(ctx, executorA))
234+
235+
// The executor in namespace A should be fully deleted (finalizer removed)
236+
require.Eventually(t, func() bool {
237+
var exec spinv1alpha1.SpinAppExecutor
238+
err := envTest.k8sClient.Get(ctx, client.ObjectKeyFromObject(executorA), &exec)
239+
return err != nil // NotFound means it was deleted
240+
}, 5*time.Second, 100*time.Millisecond, "executor A should be deleted — SpinApp in namespace B should not block it")
241+
242+
// Verify executor in namespace B still exists (it has a dependent SpinApp)
243+
var execB spinv1alpha1.SpinAppExecutor
244+
require.NoError(t, envTest.k8sClient.Get(ctx, client.ObjectKeyFromObject(executorB), &execB))
245+
}

0 commit comments

Comments
 (0)