EPMDEDP-17271: feat: validate Codebase deletion against deployment usage and return structured errors

cmd/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"crypto/tls"
 	"flag"
 	"os"
@@ -43,6 +44,7 @@ import (
 	"github.com/epam/edp-codebase-operator/v2/controllers/integrationsecret"
 	"github.com/epam/edp-codebase-operator/v2/controllers/jiraissuemetadata"
 	"github.com/epam/edp-codebase-operator/v2/controllers/jiraserver"
+	codebasePkg "github.com/epam/edp-codebase-operator/v2/pkg/codebase"
 	gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git"
 	"github.com/epam/edp-codebase-operator/v2/pkg/telemetry"
 	"github.com/epam/edp-codebase-operator/v2/pkg/util"
@@ -240,6 +242,13 @@ func main() {
 		os.Exit(1)
 	}
 
+	// Indexes have to be registered before the cache starts, and unconditionally: the
+	// deletion webhooks fail closed when an index they select on is missing.
+	if err = codebasePkg.RegisterFieldIndexes(context.Background(), mgr.GetFieldIndexer()); err != nil {
+		setupLog.Error(err, "failed to register field indexes")
+		os.Exit(1)
+	}
+
 	ctrlLog := ctrl.Log.WithName("controllers")
 
 	cdStageDeployCtrl := cdstagedeploy.NewReconcileCDStageDeploy(mgr.GetClient(), ctrlLog, chain.CreateChain)

controllers/codebasebranch/stalecheck/checker.go

@@ -10,6 +10,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1"
+	"github.com/epam/edp-codebase-operator/v2/pkg/codebasebranch"
+	"github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage"
 	gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git"
 	"github.com/epam/edp-codebase-operator/v2/pkg/util"
 )
@@ -88,6 +90,16 @@ func (c *Checker) sweep(ctx context.Context) {
 		return
 	}
 
+	// One snapshot for the whole sweep: the deployment resources are the same for every
+	// branch. Going out of date mid-sweep is safe, because the snapshot only decides
+	// between deleting and marking, and the delete is re-validated against live data by
+	// the admission webhook.
+	usage, err := codebasebranch.NewBranchUsageIndex(ctx, c.client, c.namespace)
+	if err != nil {
+		log.Error(err, "Failed to index deployment usage, skipping staleness check")
+		return
+	}
+
 	branchesByCodebase := make(map[string][]*codebaseApi.CodebaseBranch)
 
 	for i := range branches.Items {
@@ -96,7 +108,7 @@ func (c *Checker) sweep(ctx context.Context) {
 	}
 
 	for codebaseName, codebaseBranches := range branchesByCodebase {
-		if err := c.checkCodebaseBranches(ctx, codebaseName, codebaseBranches); err != nil {
+		if err := c.checkCodebaseBranches(ctx, codebaseName, codebaseBranches, usage); err != nil {
 			log.Error(err, "Failed to check branches staleness", "codebase", codebaseName)
 		}
 	}
@@ -108,6 +120,7 @@ func (c *Checker) checkCodebaseBranches(
 	ctx context.Context,
 	codebaseName string,
 	branches []*codebaseApi.CodebaseBranch,
+	usage *codebasebranch.BranchUsageIndex,
 ) error {
 	log := ctrl.LoggerFrom(ctx).WithValues("codebase", codebaseName)
 
@@ -130,8 +143,11 @@ func (c *Checker) checkCodebaseBranches(
 		return fmt.Errorf("failed to get secret %s: %w", gitServer.Spec.NameSshKeySecret, err)
 	}
 
+	autoCleanup := codebase.Annotations[codebaseApi.BranchCleanupStrategyAnnotation] ==
+		codebaseApi.BranchCleanupStrategyAuto
+
 	action := c.markAction
-	if codebase.Annotations[codebaseApi.BranchCleanupStrategyAnnotation] == codebaseApi.BranchCleanupStrategyAuto {
+	if autoCleanup {
 		action = c.cleanupAction
 	}
 
@@ -156,7 +172,15 @@ func (c *Checker) checkCodebaseBranches(
 
 		_, exists := existsInGit[branch.Spec.BranchName]
 
-		if err := action.Apply(ctx, branch, Verdict{ExistsInGit: exists}); err != nil {
+		verdict := Verdict{ExistsInGit: exists}
+
+		// Only the cleanup strategy acts on RetainedBy, and a branch that is merely marked
+		// emits a different event when it is set.
+		if autoCleanup && !exists {
+			verdict.RetainedBy = deploymentusage.Join(usage.Find(branch))
+		}
+
+		if err := action.Apply(ctx, branch, verdict); err != nil {
 			log.Error(err, "Failed to apply staleness verdict", "branch", branch.Name)
 		}
 	}

controllers/codebasebranch/stalecheck/checker_test.go

@@ -2,18 +2,24 @@ package stalecheck
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+
+	pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1"
 
 	codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1"
 	gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git"
@@ -254,3 +260,163 @@ func TestMarkAction_IdempotentWhenAlreadyStale(t *testing.T) {
 	default:
 	}
 }
+
+// autoCleanupCodebase opts the codebase into the "auto" strategy, which is the only
+// strategy that acts on the deployment usage snapshot.
+func autoCleanupCodebase() *codebaseApi.Codebase {
+	codebase := newCodebase()
+	codebase.Annotations = map[string]string{
+		codebaseApi.BranchCleanupStrategyAnnotation: codebaseApi.BranchCleanupStrategyAuto,
+	}
+
+	return codebase
+}
+
+// The sweep resolves the retaining resource from a single namespace-wide snapshot, so a
+// branch a CDPipeline consumes must survive the sweep and be marked rather than deleted.
+func TestChecker_RetainsStaleBranchUsedByCDPipeline(t *testing.T) {
+	codebase := autoCleanupCodebase()
+	featureBranch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated)
+	gitServer, secret := newGitServerWithSecret()
+
+	pipeline := &pipelineApi.CDPipeline{}
+	pipeline.Name = "demo"
+	pipeline.Namespace = testNamespace
+	pipeline.Spec.InputDockerStreams = []string{"app-feature"}
+	pipeline.Spec.DeploymentType = "container"
+
+	scheme := newScheme(t)
+	require.NoError(t, pipelineApi.AddToScheme(scheme))
+
+	k8sClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(codebase, featureBranch, gitServer, secret, pipeline).
+		WithStatusSubresource(featureBranch).
+		Build()
+
+	gitClient := gitmocks.NewMockGit(t)
+	gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return([]string{"main"}, nil)
+
+	recorder := record.NewFakeRecorder(10)
+
+	newChecker(t, k8sClient, gitClient, recorder).sweep(context.Background())
+
+	retained := getBranch(t, k8sClient, "app-feature")
+	assert.True(t, meta.IsStatusConditionTrue(retained.Status.Conditions, codebaseApi.ConditionStale))
+
+	condition := meta.FindStatusCondition(retained.Status.Conditions, codebaseApi.ConditionStale)
+	require.NotNil(t, condition)
+	assert.Contains(t, condition.Message, "retained because it is used by CDPipeline demo")
+
+	select {
+	case event := <-recorder.Events:
+		assert.Contains(t, event, EventReasonStaleBranchRetained)
+	default:
+		t.Fatal("expected StaleBranchRetained event")
+	}
+}
+
+func TestChecker_DeletesUnusedStaleBranchUnderAutoStrategy(t *testing.T) {
+	codebase := autoCleanupCodebase()
+	featureBranch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated)
+	gitServer, secret := newGitServerWithSecret()
+
+	scheme := newScheme(t)
+	require.NoError(t, pipelineApi.AddToScheme(scheme))
+
+	k8sClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(codebase, featureBranch, gitServer, secret).
+		WithStatusSubresource(featureBranch).
+		Build()
+
+	gitClient := gitmocks.NewMockGit(t)
+	gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return([]string{"main"}, nil)
+
+	newChecker(t, k8sClient, gitClient, record.NewFakeRecorder(10)).sweep(context.Background())
+
+	err := k8sClient.Get(context.Background(),
+		client.ObjectKey{Namespace: testNamespace, Name: "app-feature"}, &codebaseApi.CodebaseBranch{})
+	assert.True(t, apierrors.IsNotFound(err), "unused stale branch must be deleted")
+}
+
+// The snapshot must not leak between strategies: a branch of a codebase that only marks
+// keeps the plain stale message even when a CDPipeline consumes it.
+func TestChecker_MarkStrategyIgnoresDeploymentUsage(t *testing.T) {
+	codebase := newCodebase()
+	featureBranch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated)
+	gitServer, secret := newGitServerWithSecret()
+
+	pipeline := &pipelineApi.CDPipeline{}
+	pipeline.Name = "demo"
+	pipeline.Namespace = testNamespace
+	pipeline.Spec.InputDockerStreams = []string{"app-feature"}
+	pipeline.Spec.DeploymentType = "container"
+
+	scheme := newScheme(t)
+	require.NoError(t, pipelineApi.AddToScheme(scheme))
+
+	k8sClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(codebase, featureBranch, gitServer, secret, pipeline).
+		WithStatusSubresource(featureBranch).
+		Build()
+
+	gitClient := gitmocks.NewMockGit(t)
+	gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return([]string{"main"}, nil)
+
+	recorder := record.NewFakeRecorder(10)
+
+	newChecker(t, k8sClient, gitClient, recorder).sweep(context.Background())
+
+	marked := getBranch(t, k8sClient, "app-feature")
+	condition := meta.FindStatusCondition(marked.Status.Conditions, codebaseApi.ConditionStale)
+	require.NotNil(t, condition)
+	assert.Equal(t, "Branch was not found in the git repository", condition.Message)
+
+	select {
+	case event := <-recorder.Events:
+		assert.Contains(t, event, EventReasonBranchStale)
+	default:
+		t.Fatal("expected BranchStale event")
+	}
+}
+
+// If deployment usage cannot be resolved the sweep must stop: acting on an unknown usage
+// set could delete a branch a CDPipeline still consumes.
+func TestChecker_SkipsSweepWhenUsageCannotBeResolved(t *testing.T) {
+	codebase := autoCleanupCodebase()
+	featureBranch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated)
+	gitServer, secret := newGitServerWithSecret()
+
+	scheme := newScheme(t)
+	require.NoError(t, pipelineApi.AddToScheme(scheme))
+
+	k8sClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(codebase, featureBranch, gitServer, secret).
+		WithStatusSubresource(featureBranch).
+		WithInterceptorFuncs(interceptor.Funcs{
+			List: func(
+				ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption,
+			) error {
+				if _, ok := list.(*pipelineApi.CDPipelineList); ok {
+					return apierrors.NewForbidden(
+						schema.GroupResource{Group: "v2.edp.epam.com", Resource: "cdpipelines"},
+						"", errors.New("no access"))
+				}
+
+				return c.List(ctx, list, opts...)
+			},
+		}).
+		Build()
+
+	gitClient := gitmocks.NewMockGit(t)
+
+	newChecker(t, k8sClient, gitClient, record.NewFakeRecorder(10)).sweep(context.Background())
+
+	// Neither deleted nor marked: the sweep returned before applying any verdict.
+	untouched := getBranch(t, k8sClient, "app-feature")
+	assert.Empty(t, untouched.Status.Conditions)
+	assert.NotContains(t, untouched.Labels, codebaseApi.StaleLabel)
+}

controllers/codebasebranch/stalecheck/cleanup_action.go

@@ -10,7 +10,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1"
-	"github.com/epam/edp-codebase-operator/v2/pkg/codebasebranch"
 )
 
 // CleanupAction implements the "auto" cleanup strategy: stale branches that do not
@@ -28,19 +27,11 @@ func NewCleanupAction(k8sClient client.Client, recorder record.EventRecorder, ma
 	return &CleanupAction{client: k8sClient, recorder: recorder, mark: mark}
 }
 
+// Apply deletes the branch only when it is stale and unretained. Which deployment
+// resources retain it is decided by the Checker, which resolves RetainedBy from the
+// snapshot shared by every branch of the sweep.
 func (a *CleanupAction) Apply(ctx context.Context, branch *codebaseApi.CodebaseBranch, verdict Verdict) error {
-	if verdict.ExistsInGit {
-		return a.mark.Apply(ctx, branch, verdict)
-	}
-
-	usage, err := codebasebranch.FindBranchUsage(ctx, a.client, branch)
-	if err != nil {
-		return fmt.Errorf("failed to check CodebaseBranch %s usage: %w", branch.Name, err)
-	}
-
-	if usage != "" {
-		verdict.RetainedBy = usage
-
+	if verdict.ExistsInGit || verdict.RetainedBy != "" {
 		return a.mark.Apply(ctx, branch, verdict)
 	}
 

controllers/codebasebranch/stalecheck/cleanup_action_test.go

@@ -46,28 +46,25 @@ func TestCleanupAction_DeletesUnusedStaleBranch(t *testing.T) {
 	}
 }
 
+// The retaining resource is resolved by the Checker and arrives in the Verdict, so the
+// action honours it without looking anything up.
 func TestCleanupAction_RetainsBranchUsedByCDPipeline(t *testing.T) {
 	branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated)
 
-	pipeline := &pipelineApi.CDPipeline{}
-	pipeline.Name = "demo"
-	pipeline.Namespace = testNamespace
-	pipeline.Spec.InputDockerStreams = []string{"app-feature"}
-	pipeline.Spec.DeploymentType = "container"
-
 	scheme := newScheme(t)
 	require.NoError(t, pipelineApi.AddToScheme(scheme))
 
 	k8sClient := fake.NewClientBuilder().
 		WithScheme(scheme).
-		WithObjects(branch, pipeline).
+		WithObjects(branch).
 		WithStatusSubresource(branch).
 		Build()
 
 	recorder := record.NewFakeRecorder(10)
 	action := NewCleanupAction(k8sClient, recorder, NewMarkAction(k8sClient, recorder))
 
-	require.NoError(t, action.Apply(context.Background(), branch, Verdict{ExistsInGit: false}))
+	verdict := Verdict{ExistsInGit: false, RetainedBy: "CDPipeline demo (inputDockerStreams)"}
+	require.NoError(t, action.Apply(context.Background(), branch, verdict))
 
 	retained := getBranch(t, k8sClient, "app-feature")
 	assert.True(t, meta.IsStatusConditionTrue(retained.Status.Conditions, codebaseApi.ConditionStale))

pkg/codebase/index.go

@@ -0,0 +1,41 @@
+package codebase
+
+import (
+	"context"
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1"
+)
+
+// BranchCodebaseNameIndex indexes CodebaseBranch by the Codebase it belongs to, so that
+// a deletion check reads the branches of one codebase and not of the whole namespace.
+const BranchCodebaseNameIndex = "spec.codebaseName"
+
+// IndexBranchByCodebaseName extracts the index value of BranchCodebaseNameIndex.
+//
+// It is exported for fake clients, which are not a client.FieldIndexer and take the
+// extractor through fake.ClientBuilder.WithIndex; sharing it keeps the tested index
+// identical to the registered one.
+func IndexBranchByCodebaseName(o client.Object) []string {
+	branch, ok := o.(*codebaseApi.CodebaseBranch)
+	if !ok || branch.Spec.CodebaseName == "" {
+		return nil
+	}
+
+	return []string{branch.Spec.CodebaseName}
+}
+
+// RegisterFieldIndexes must be called before the manager cache starts. A List selecting
+// on an unregistered index fails outright, so a caller that skips this denies every
+// deletion rather than under-reporting usage.
+func RegisterFieldIndexes(ctx context.Context, indexer client.FieldIndexer) error {
+	if err := indexer.IndexField(
+		ctx, &codebaseApi.CodebaseBranch{}, BranchCodebaseNameIndex, IndexBranchByCodebaseName,
+	); err != nil {
+		return fmt.Errorf("failed to index CodebaseBranch by %s: %w", BranchCodebaseNameIndex, err)
+	}
+
+	return nil
+}