diff --git a/cmd/main.go b/cmd/main.go index 656e4c7f..fdf13132 100644 --- a/cmd/main.go +++ b/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) diff --git a/controllers/codebasebranch/stalecheck/checker.go b/controllers/codebasebranch/stalecheck/checker.go index 51acd284..38ac9732 100644 --- a/controllers/codebasebranch/stalecheck/checker.go +++ b/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) } } diff --git a/controllers/codebasebranch/stalecheck/checker_test.go b/controllers/codebasebranch/stalecheck/checker_test.go index 2a78cff3..f9f8c46e 100644 --- a/controllers/codebasebranch/stalecheck/checker_test.go +++ b/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) +} diff --git a/controllers/codebasebranch/stalecheck/cleanup_action.go b/controllers/codebasebranch/stalecheck/cleanup_action.go index 2daf28c2..e89ee40e 100644 --- a/controllers/codebasebranch/stalecheck/cleanup_action.go +++ b/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) } diff --git a/controllers/codebasebranch/stalecheck/cleanup_action_test.go b/controllers/codebasebranch/stalecheck/cleanup_action_test.go index 163703ac..faf443c3 100644 --- a/controllers/codebasebranch/stalecheck/cleanup_action_test.go +++ b/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)) diff --git a/pkg/codebase/index.go b/pkg/codebase/index.go new file mode 100644 index 00000000..a67b249c --- /dev/null +++ b/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 +} diff --git a/pkg/codebase/index_test.go b/pkg/codebase/index_test.go new file mode 100644 index 00000000..ab11459f --- /dev/null +++ b/pkg/codebase/index_test.go @@ -0,0 +1,83 @@ +package codebase + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" +) + +type stubFieldIndexer struct { + fields []string + err error +} + +func (s *stubFieldIndexer) IndexField(_ context.Context, _ client.Object, field string, _ client.IndexerFunc) error { + if s.err != nil { + return s.err + } + + s.fields = append(s.fields, field) + + return nil +} + +func TestIndexBranchByCodebaseName(t *testing.T) { + tests := []struct { + name string + obj client.Object + want []string + }{ + { + name: "indexes branch by its codebase", + obj: &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-main", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{CodebaseName: "app"}, + }, + want: []string{"app"}, + }, + { + // An empty key would collect every branch with an unset codebaseName under one + // index entry, so such branches are left out of the index entirely. + name: "skips branch without a codebase", + obj: &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "orphan", Namespace: "default"}, + }, + want: nil, + }, + { + name: "skips object of another kind", + obj: &codebaseApi.Codebase{ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}}, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IndexBranchByCodebaseName(tt.obj)) + }) + } +} + +func TestRegisterFieldIndexes(t *testing.T) { + indexer := &stubFieldIndexer{} + + require.NoError(t, RegisterFieldIndexes(context.Background(), indexer)) + assert.Equal(t, []string{BranchCodebaseNameIndex}, indexer.fields) +} + +// A failure to register must stop the operator rather than leave the deletion check +// selecting on an index that does not exist. +func TestRegisterFieldIndexes_Error(t *testing.T) { + indexer := &stubFieldIndexer{err: errors.New("cache already started")} + + err := RegisterFieldIndexes(context.Background(), indexer) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to index CodebaseBranch by spec.codebaseName") +} diff --git a/pkg/codebase/usage.go b/pkg/codebase/usage.go new file mode 100644 index 00000000..4d408ec9 --- /dev/null +++ b/pkg/codebase/usage.go @@ -0,0 +1,135 @@ +package codebase + +import ( + "context" + "fmt" + "slices" + + "sigs.k8s.io/controller-runtime/pkg/client" + + pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage" +) + +// FindCodebaseUsage returns every deployment resource that references the +// given Codebase, or an empty slice when the codebase is unused. +// +// A codebase participates in deployments in three ways: +// - CDPipeline.Spec.Applications (or ApplicationsToPromote) lists Codebase names +// (application components); +// - CDPipeline.Spec.InputDockerStreams lists the names of the codebase's branches; +// - Stage.Spec.QualityGates references a Codebase name via AutotestName +// (autotest components). +// +// Resources that are being deleted are not counted as usage. +func FindCodebaseUsage( + ctx context.Context, + c client.Client, + codebase *codebaseApi.Codebase, +) ([]deploymentusage.Reference, error) { + var refs []deploymentusage.Reference + + pipelines, err := deploymentusage.ListActiveCDPipelines(ctx, c, codebase.Namespace) + if err != nil { + return nil, err + } + + branchNames, err := listBranchNames(ctx, c, codebase) + if err != nil { + return nil, err + } + + for i := range pipelines { + pipeline := &pipelines[i] + + field, reason, ok := matchPipeline(pipeline, codebase.Name, branchNames) + if !ok { + continue + } + + refs = append(refs, deploymentusage.Reference{ + Kind: deploymentusage.KindCDPipeline, + Name: pipeline.Name, + Field: field, + Reason: reason, + }) + } + + gates, err := deploymentusage.FindAutotestGates(ctx, c, codebase.Namespace) + if err != nil { + return nil, err + } + + // A codebase is the autotest itself, so the gate's branch is not part of the match. + for _, gate := range gates { + if gate.AutotestName == codebase.Name { + refs = append(refs, gate.Reference) + } + } + + return refs, nil +} + +// matchPipeline reports how the pipeline references the codebase, if it does. +// +// applicationsToPromote is normally a subset of applications, but nothing enforces that, +// so both are checked. inputDockerStreams holds branch names rather than the codebase +// name: deleting the codebase cascades to its branches, and the CodebaseBranch webhook +// lets that cascade through, so a pipeline consuming any of them blocks the codebase too. +// A pipeline is reported once, naming the field it was matched on. +func matchPipeline( + pipeline *pipelineApi.CDPipeline, + codebaseName string, + branchNames map[string]struct{}, +) (field, reason string, ok bool) { + switch { + case slices.Contains(pipeline.Spec.Applications, codebaseName): + return deploymentusage.FieldApplications, "applications", true + case slices.Contains(pipeline.Spec.ApplicationsToPromote, codebaseName): + return deploymentusage.FieldApplicationsToPromote, "applicationsToPromote", true + } + + for _, stream := range pipeline.Spec.InputDockerStreams { + if _, isBranch := branchNames[stream]; isBranch { + return deploymentusage.FieldInputDockerStreams, + fmt.Sprintf("branch %s in inputDockerStreams", stream), + true + } + } + + return "", "", false +} + +// listBranchNames returns the resource names of the codebase's branches, which is what +// CDPipeline.Spec.InputDockerStreams holds. +func listBranchNames( + ctx context.Context, + c client.Client, + codebase *codebaseApi.Codebase, +) (map[string]struct{}, error) { + branches := &codebaseApi.CodebaseBranchList{} + if err := c.List( + ctx, + branches, + client.InNamespace(codebase.Namespace), + client.MatchingFields{BranchCodebaseNameIndex: codebase.Name}, + ); err != nil { + // Without the CodebaseBranch kind there are no branches, so nothing can reference + // one. Any other failure means usage could not be verified and must surface. + if deploymentusage.IsKindUnavailable(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to list CodebaseBranches: %w", err) + } + + names := make(map[string]struct{}, len(branches.Items)) + + for i := range branches.Items { + names[branches.Items[i].Name] = struct{}{} + } + + return names, nil +} diff --git a/pkg/codebase/usage_bench_test.go b/pkg/codebase/usage_bench_test.go new file mode 100644 index 00000000..cf5c9588 --- /dev/null +++ b/pkg/codebase/usage_bench_test.go @@ -0,0 +1,68 @@ +package codebase + +import ( + "fmt" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" +) + +// BenchmarkCodebaseBranchListDeepCopy measures what a cached List costs: the informer +// cache deep-copies every object it returns. BranchCodebaseNameIndex shrinks that set to +// the branches of one codebase, so this is the per-object price it saves. +// +// The result is a lower bound, as cached objects also carry managedFields. +func BenchmarkCodebaseBranchListDeepCopy(b *testing.B) { + for _, branches := range []int{10, 500, 5000} { + b.Run(fmt.Sprintf("branches=%d", branches), func(b *testing.B) { + list := benchBranchList(branches) + + b.ReportAllocs() + + for b.Loop() { + _ = list.DeepCopy() + } + }) + } +} + +func benchBranchList(size int) *codebaseApi.CodebaseBranchList { + list := &codebaseApi.CodebaseBranchList{Items: make([]codebaseApi.CodebaseBranch, 0, size)} + + for i := range size { + list.Items = append(list.Items, codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("app-%d-main", i), + Namespace: "krci", + ResourceVersion: "123456", + Labels: map[string]string{"app.edp.epam.com/codebaseName": fmt.Sprintf("app-%d", i)}, + }, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: fmt.Sprintf("app-%d", i), + BranchName: "main", + Pipelines: map[string]string{"review": "review-pipeline", "build": "build-pipeline"}, + }, + Status: codebaseApi.CodebaseBranchStatus{ + LastTimeUpdated: metav1.Now(), + Status: "created", + Value: "active", + Action: codebaseApi.ActionType("codebase_branch_provisioning"), + Result: codebaseApi.Success, + Username: "system", + VersionHistory: []string{"0.1.0-SNAPSHOT", "0.2.0-SNAPSHOT"}, + Conditions: []metav1.Condition{{ + Type: "Stale", + Status: metav1.ConditionFalse, + Reason: "ExistsInGit", + Message: "Branch exists in the git repository", + LastTransitionTime: metav1.NewTime(time.Unix(0, 0)), + }}, + }, + }) + } + + return list +} diff --git a/pkg/codebase/usage_test.go b/pkg/codebase/usage_test.go new file mode 100644 index 00000000..0fa28920 --- /dev/null +++ b/pkg/codebase/usage_test.go @@ -0,0 +1,313 @@ +package codebase + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + 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" + "k8s.io/utils/ptr" + "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" + "github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage" +) + +func codebaseUsageScheme(t *testing.T, withPipelineAPI bool) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, codebaseApi.AddToScheme(scheme)) + + if withPipelineAPI { + require.NoError(t, pipelineApi.AddToScheme(scheme)) + } + + return scheme +} + +// codebaseUsageClientBuilder returns a fake client builder with BranchCodebaseNameIndex +// registered. A fake client is not a client.FieldIndexer, so it takes the index through +// the builder; a List selecting on an unregistered index fails. +func codebaseUsageClientBuilder(t *testing.T, withPipelineAPI bool) *fake.ClientBuilder { + t.Helper() + + return fake.NewClientBuilder(). + WithScheme(codebaseUsageScheme(t, withPipelineAPI)). + WithIndex(&codebaseApi.CodebaseBranch{}, BranchCodebaseNameIndex, IndexBranchByCodebaseName) +} + +func usageCodebase() *codebaseApi.Codebase { + return &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + } +} + +func TestFindCodebaseUsage_Applications(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"other-app", "app"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (applications)", refs[0].String()) +} + +func TestFindCodebaseUsage_ApplicationsToPromote(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"other-app"}, + ApplicationsToPromote: []string{"app"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (applicationsToPromote)", refs[0].String()) +} + +func TestFindCodebaseUsage_ApplicationsTakesPrecedence(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"app"}, + ApplicationsToPromote: []string{"app"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (applications)", refs[0].String()) +} + +// Deleting a Codebase cascades to its branches, and the CodebaseBranch webhook lets that +// cascade through, so a pipeline consuming one of those branches must block the Codebase. +func TestFindCodebaseUsage_BranchInInputDockerStreams(t *testing.T) { + branch := &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-main", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{CodebaseName: "app", BranchName: "main"}, + } + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"other-app"}, + InputDockerStreams: []string{"app-main"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true). + WithObjects(branch, pipeline). + Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (branch app-main in inputDockerStreams)", refs[0].String()) + assert.Equal(t, deploymentusage.FieldInputDockerStreams, refs[0].Field) +} + +func TestFindCodebaseUsage_ForeignBranchInInputDockerStreams(t *testing.T) { + branch := &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "other-app-main", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{CodebaseName: "other-app", BranchName: "main"}, + } + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"other-app"}, + InputDockerStreams: []string{"other-app-main"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true). + WithObjects(branch, pipeline). + Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +// A List failure that is not "CRD absent" must surface, so the webhook denies rather than +// silently treating an unverifiable codebase as unused. +func TestFindCodebaseUsage_ListErrorIsSurfaced(t *testing.T) { + k8sClient := codebaseUsageClientBuilder(t, true). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(_ context.Context, _ client.WithWatch, _ client.ObjectList, _ ...client.ListOption) error { + return apierrors.NewForbidden( + schema.GroupResource{Group: "v2.edp.epam.com", Resource: "cdpipelines"}, "", errors.New("no access")) + }, + }). + Build() + + _, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to list") +} + +// The branch lookup must stay scoped to the codebase through BranchCodebaseNameIndex. +// The refs are the same whether or not it is scoped, so the selector itself is the only +// thing that can be asserted on. +func TestFindCodebaseUsage_BranchLookupIsIndexed(t *testing.T) { + var branchListOpts []client.ListOption + + k8sClient := codebaseUsageClientBuilder(t, true). + WithObjects(&codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-main", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{CodebaseName: "app", BranchName: "main"}, + }). + WithInterceptorFuncs(interceptor.Funcs{ + List: func( + ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption, + ) error { + if _, ok := list.(*codebaseApi.CodebaseBranchList); ok { + branchListOpts = opts + } + + return c.List(ctx, list, opts...) + }, + }). + Build() + + _, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Contains(t, branchListOpts, client.MatchingFields{BranchCodebaseNameIndex: "app"}) +} + +func TestFindCodebaseUsage_AutotestQualityGate(t *testing.T) { + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("main"), + }}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(stage).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 1) + assert.Equal(t, "Stage demo-dev of CDPipeline demo (autotest quality gate)", refs[0].String()) +} + +// A manual quality gate leaves AutotestName unset and must never be treated as a match. +func TestFindCodebaseUsage_ManualQualityGateNotMatched(t *testing.T) { + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "manual", + StepName: "approve", + }}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(stage).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +func TestFindCodebaseUsage_MultipleReferences(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"app"}, + }, + } + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("main"), + }}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline, stage).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + require.Len(t, refs, 2) + + descriptions := []string{refs[0].String(), refs[1].String()} + assert.Contains(t, descriptions, "CDPipeline demo (applications)") + assert.Contains(t, descriptions, "Stage demo-dev of CDPipeline demo (autotest quality gate)") +} + +func TestFindCodebaseUsage_TerminatingPipelineIgnored(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + DeletionTimestamp: ptr.To(metav1.Now()), + Finalizers: []string{"keep"}, + }, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"app"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +func TestFindCodebaseUsage_Unused(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + Applications: []string{"other-app"}, + }, + } + + k8sClient := codebaseUsageClientBuilder(t, true).WithObjects(pipeline).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +func TestFindCodebaseUsage_PipelineAPINotInstalled(t *testing.T) { + // CDPipeline/Stage kinds are absent from the scheme, emulating a cluster + // without edp-cd-pipeline-operator: the codebase must be reported as unused. + k8sClient := codebaseUsageClientBuilder(t, false).Build() + + refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) + require.NoError(t, err) + assert.Empty(t, refs) +} diff --git a/pkg/codebasebranch/usage.go b/pkg/codebasebranch/usage.go index 3abb7497..49a4b49c 100644 --- a/pkg/codebasebranch/usage.go +++ b/pkg/codebasebranch/usage.go @@ -2,78 +2,124 @@ package codebasebranch import ( "context" - "fmt" - "slices" - apimeta "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage" ) -// FindBranchUsage returns a human-readable description of the first deployment resource -// that references the given CodebaseBranch, or an empty string when the branch is unused. -// -// A branch participates in deployments in two ways: -// - CDPipeline.Spec.InputDockerStreams lists CodebaseBranch resource names -// (resolved to CodebaseImageStreams via the codebasebranch label); -// - Stage.Spec.QualityGates references autotest codebase name + git branch name. -// -// Resources that are being deleted are not counted as usage. -func FindBranchUsage(ctx context.Context, c client.Client, branch *codebaseApi.CodebaseBranch) (string, error) { - pipelines := &pipelineApi.CDPipelineList{} - if err := c.List(ctx, pipelines, client.InNamespace(branch.Namespace)); err != nil { - // The CD pipeline CRDs are owned by edp-cd-pipeline-operator, which may not be - // installed alongside this operator; in that case nothing can reference the branch. - if isKindUnavailable(err) { - return "", nil - } +// autotestGate identifies the autotest a Stage quality gate runs: a codebase and one of +// its git branches. Stage.Spec.QualityGates references both, never the CodebaseBranch +// resource name. +type autotestGate struct { + codebaseName string + branchName string +} - return "", fmt.Errorf("failed to list CDPipelines: %w", err) - } +// BranchUsageIndex answers "which deployment resources reference this CodebaseBranch" +// for many branches from a single read of the CDPipelines and Stages, which is what the +// stale branch sweep needs: it asks per branch while the answer set is the same for all. +type BranchUsageIndex struct { + // byStreamName is keyed by CodebaseBranch resource name, as held in + // CDPipeline.Spec.InputDockerStreams. + byStreamName map[string][]deploymentusage.Reference + byAutotest map[autotestGate][]deploymentusage.Reference +} - for i := range pipelines.Items { - pipeline := &pipelines.Items[i] +// NewBranchUsageIndex reads the deployment resources of the namespace once and indexes +// the references they hold. Resources that are being deleted are not counted as usage. +// +// The snapshot is never refreshed, so callers that must not act on stale data build one +// per request. +func NewBranchUsageIndex(ctx context.Context, c client.Client, namespace string) (*BranchUsageIndex, error) { + index := &BranchUsageIndex{ + byStreamName: make(map[string][]deploymentusage.Reference), + byAutotest: make(map[autotestGate][]deploymentusage.Reference), + } - if pipeline.DeletionTimestamp != nil { - continue - } + pipelines, err := deploymentusage.ListActiveCDPipelines(ctx, c, namespace) + if err != nil { + return nil, err + } - if slices.Contains(pipeline.Spec.InputDockerStreams, branch.Name) { - return fmt.Sprintf("CDPipeline %s (inputDockerStreams)", pipeline.Name), nil - } + for i := range pipelines { + index.addPipeline(&pipelines[i]) } - stages := &pipelineApi.StageList{} - if err := c.List(ctx, stages, client.InNamespace(branch.Namespace)); err != nil { - if isKindUnavailable(err) { - return "", nil - } + gates, err := deploymentusage.FindAutotestGates(ctx, c, namespace) + if err != nil { + return nil, err + } - return "", fmt.Errorf("failed to list Stages: %w", err) + for _, gate := range gates { + key := autotestGate{codebaseName: gate.AutotestName, branchName: gate.BranchName} + index.byAutotest[key] = append(index.byAutotest[key], gate.Reference) } - for i := range stages.Items { - stage := &stages.Items[i] + return index, nil +} + +func (i *BranchUsageIndex) addPipeline(pipeline *pipelineApi.CDPipeline) { + // A pipeline listing the same stream twice must still be reported once. + seen := make(map[string]struct{}, len(pipeline.Spec.InputDockerStreams)) - if stage.DeletionTimestamp != nil { + for _, stream := range pipeline.Spec.InputDockerStreams { + if _, duplicate := seen[stream]; duplicate { continue } - for _, gate := range stage.Spec.QualityGates { - if gate.AutotestName != nil && *gate.AutotestName == branch.Spec.CodebaseName && - gate.BranchName != nil && *gate.BranchName == branch.Spec.BranchName { - return fmt.Sprintf("Stage %s of CDPipeline %s (autotest quality gate)", stage.Name, stage.Spec.CdPipeline), nil - } - } + seen[stream] = struct{}{} + + i.byStreamName[stream] = append(i.byStreamName[stream], deploymentusage.Reference{ + Kind: deploymentusage.KindCDPipeline, + Name: pipeline.Name, + Field: deploymentusage.FieldInputDockerStreams, + Reason: "inputDockerStreams", + }) + } +} + +// Find returns every deployment resource that references the given CodebaseBranch, or an +// empty slice when the branch is unused. +// +// A branch participates in deployments in two ways: +// - CDPipeline.Spec.InputDockerStreams lists CodebaseBranch resource names +// (resolved to CodebaseImageStreams via the codebasebranch label); +// - Stage.Spec.QualityGates references autotest codebase name + git branch name. +func (i *BranchUsageIndex) Find(branch *codebaseApi.CodebaseBranch) []deploymentusage.Reference { + pipelineRefs := i.byStreamName[branch.Name] + gateRefs := i.byAutotest[autotestGate{ + codebaseName: branch.Spec.CodebaseName, + branchName: branch.Spec.BranchName, + }] + + if len(pipelineRefs) == 0 && len(gateRefs) == 0 { + return nil } - return "", nil + // A fresh slice: appending to an indexed one writes into the index's own storage. + refs := make([]deploymentusage.Reference, 0, len(pipelineRefs)+len(gateRefs)) + refs = append(refs, pipelineRefs...) + + return append(refs, gateRefs...) } -func isKindUnavailable(err error) bool { - return apimeta.IsNoMatchError(err) || runtime.IsNotRegisteredError(err) +// FindBranchUsage returns every deployment resource that references the given +// CodebaseBranch, or an empty slice when the branch is unused. It reads the deployment +// resources on every call, as admission requires; callers checking many branches at once +// should build a BranchUsageIndex. +func FindBranchUsage( + ctx context.Context, + c client.Client, + branch *codebaseApi.CodebaseBranch, +) ([]deploymentusage.Reference, error) { + index, err := NewBranchUsageIndex(ctx, c, branch.Namespace) + if err != nil { + return nil, err + } + + return index.Find(branch), nil } diff --git a/pkg/codebasebranch/usage_test.go b/pkg/codebasebranch/usage_test.go index 4f1f2ca1..4b7acd99 100644 --- a/pkg/codebasebranch/usage_test.go +++ b/pkg/codebasebranch/usage_test.go @@ -9,11 +9,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage" ) func usageScheme(t *testing.T, withPipelineAPI bool) *runtime.Scheme { @@ -49,9 +51,10 @@ func TestFindBranchUsage_InputDockerStreams(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() - usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) require.NoError(t, err) - assert.Equal(t, "CDPipeline demo (inputDockerStreams)", usage) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (inputDockerStreams)", refs[0].String()) } func TestFindBranchUsage_AutotestQualityGate(t *testing.T) { @@ -69,9 +72,61 @@ func TestFindBranchUsage_AutotestQualityGate(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(stage).Build() - usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) require.NoError(t, err) - assert.Equal(t, "Stage demo-dev of CDPipeline demo (autotest quality gate)", usage) + require.Len(t, refs, 1) + assert.Equal(t, "Stage demo-dev of CDPipeline demo (autotest quality gate)", refs[0].String()) +} + +// A manual quality gate leaves AutotestName/BranchName unset, and must never be treated +// as a match just because the nil pointers happen to compare equal. +func TestFindBranchUsage_ManualQualityGateNotMatched(t *testing.T) { + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "manual", + StepName: "approve", + }}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(stage).Build() + + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +func TestFindBranchUsage_MultipleReferences(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + InputDockerStreams: []string{"app-feature"}, + }, + } + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("feature"), + }}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline, stage).Build() + + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + require.Len(t, refs, 2) + + descriptions := []string{refs[0].String(), refs[1].String()} + assert.Contains(t, descriptions, "CDPipeline demo (inputDockerStreams)") + assert.Contains(t, descriptions, "Stage demo-dev of CDPipeline demo (autotest quality gate)") } func TestFindBranchUsage_TerminatingPipelineIgnored(t *testing.T) { @@ -89,9 +144,9 @@ func TestFindBranchUsage_TerminatingPipelineIgnored(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() - usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) require.NoError(t, err) - assert.Empty(t, usage) + assert.Empty(t, refs) } func TestFindBranchUsage_Unused(t *testing.T) { @@ -104,9 +159,9 @@ func TestFindBranchUsage_Unused(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() - usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) require.NoError(t, err) - assert.Empty(t, usage) + assert.Empty(t, refs) } func TestFindBranchUsage_PipelineAPINotInstalled(t *testing.T) { @@ -114,7 +169,124 @@ func TestFindBranchUsage_PipelineAPINotInstalled(t *testing.T) { // without edp-cd-pipeline-operator: the branch must be reported as unused. k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, false)).Build() - usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Empty(t, refs) +} + +// One snapshot must answer for many branches, which is the whole point of the index: +// the sweep asks per branch while the deployment resources are read once. +func TestBranchUsageIndex_AnswersManyBranchesFromOneRead(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + InputDockerStreams: []string{"app-feature"}, + }, + } + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-qa", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("autotest"), + }}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline, stage).Build() + + index, err := NewBranchUsageIndex(context.Background(), k8sClient, "default") + require.NoError(t, err) + + usedByPipeline := index.Find(usageBranch()) + require.Len(t, usedByPipeline, 1) + assert.Equal(t, "CDPipeline demo (inputDockerStreams)", usedByPipeline[0].String()) + + autotestBranch := usageBranch() + autotestBranch.Name = "app-autotest" + autotestBranch.Spec.BranchName = "autotest" + + usedByGate := index.Find(autotestBranch) + require.Len(t, usedByGate, 1) + assert.Equal(t, "Stage demo-qa of CDPipeline demo (autotest quality gate)", usedByGate[0].String()) + + unused := usageBranch() + unused.Name = "app-orphan" + unused.Spec.BranchName = "orphan" + assert.Empty(t, index.Find(unused)) +} + +// Find must not hand out the slice the index stores. Three pipelines leave that slice +// with spare capacity, so appending the gate reference onto it writes into the index's +// own backing array, letting a later lookup overwrite a result the caller still holds. +func TestBranchUsageIndex_FindDoesNotAliasStoredReferences(t *testing.T) { + pipelineNames := []string{"demo-a", "demo-b", "demo-c"} + objects := make([]client.Object, 0, len(pipelineNames)+1) + + for _, name := range pipelineNames { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{InputDockerStreams: []string{"app-feature"}}, + } + objects = append(objects, pipeline) + } + + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-qa", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("feature"), + }}, + }, + } + objects = append(objects, stage) + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(objects...).Build() + + index, err := NewBranchUsageIndex(context.Background(), k8sClient, "default") + require.NoError(t, err) + + first := index.Find(usageBranch()) + require.Len(t, first, 4) + + sentinel := deploymentusage.Reference{Kind: "sentinel"} + first[len(first)-1] = sentinel + + // A second lookup must not reach back into the slice the first caller is holding. + index.Find(usageBranch()) + + assert.Equal(t, sentinel, first[len(first)-1], + "Find returned a slice backed by the index, so a later lookup corrupted an earlier result") +} + +// A pipeline may list the same stream twice; it is still one blocking resource. +func TestBranchUsageIndex_DeduplicatesRepeatedStream(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + InputDockerStreams: []string{"app-feature", "app-feature"}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() + + index, err := NewBranchUsageIndex(context.Background(), k8sClient, "default") + require.NoError(t, err) + + assert.Len(t, index.Find(usageBranch()), 1) +} + +// Without the CD pipeline CRDs nothing can reference a branch, and the index must stay +// usable instead of failing the caller. +func TestBranchUsageIndex_NoPipelineCRDs(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, false)).Build() + + index, err := NewBranchUsageIndex(context.Background(), k8sClient, "default") require.NoError(t, err) - assert.Empty(t, usage) + assert.Empty(t, index.Find(usageBranch())) } diff --git a/pkg/deploymentusage/deploymentusage.go b/pkg/deploymentusage/deploymentusage.go new file mode 100644 index 00000000..02e4ebec --- /dev/null +++ b/pkg/deploymentusage/deploymentusage.go @@ -0,0 +1,177 @@ +// Package deploymentusage provides shared helpers for detecting whether a +// Codebase or CodebaseBranch is still referenced by CD pipeline deployment +// resources (CDPipeline, Stage). It backs the admission webhooks that block +// deletion of resources that are still in use. +package deploymentusage + +import ( + "context" + "fmt" + "strings" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" +) + +// Kinds of the deployment resources that can hold a reference. Reference.String +// branches on these, so they are not free-form. +const ( + KindCDPipeline = "CDPipeline" + KindStage = "Stage" +) + +// Spec paths reported as Reference.Field, and in turn as metav1.StatusCause.Field. +// Clients key off these instead of parsing the message. +const ( + FieldApplications = "spec.applications" + FieldApplicationsToPromote = "spec.applicationsToPromote" + FieldInputDockerStreams = "spec.inputDockerStreams" + FieldQualityGateAutotest = "spec.qualityGates.autotestName" +) + +// Reference describes a single deployment resource that references a +// Codebase or CodebaseBranch, blocking its deletion. +type Reference struct { + Kind string + Name string + // Field is the JSON path holding the reference, e.g. "spec.inputDockerStreams". + // It is surfaced as the Field of a metav1.StatusCause. + Field string + // Reason is a human-readable phrase, e.g. "inputDockerStreams" or + // "autotest quality gate". + Reason string + // ParentCDPipeline is set only when Kind is KindStage. + ParentCDPipeline string +} + +// String renders the reference as it appears in the denial message shown to users. +func (r Reference) String() string { + if r.Kind == KindStage { + return fmt.Sprintf("%s %s of %s %s (%s)", KindStage, r.Name, KindCDPipeline, r.ParentCDPipeline, r.Reason) + } + + return fmt.Sprintf("%s %s (%s)", r.Kind, r.Name, r.Reason) +} + +// Join renders references as a single description, in the form used by both the +// admission denial message and the stale-branch retention verdict. +func Join(refs []Reference) string { + descriptions := make([]string, 0, len(refs)) + + for _, ref := range refs { + descriptions = append(descriptions, ref.String()) + } + + return strings.Join(descriptions, "; ") +} + +// ListActiveCDPipelines returns the CDPipelines in the given namespace that +// are not being deleted. When the CD pipeline CRDs are not installed in the +// cluster, it returns an empty, non-error result. +func ListActiveCDPipelines(ctx context.Context, c client.Client, namespace string) ([]pipelineApi.CDPipeline, error) { + pipelines := &pipelineApi.CDPipelineList{} + if err := c.List(ctx, pipelines, client.InNamespace(namespace)); err != nil { + if IsKindUnavailable(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to list CDPipelines: %w", err) + } + + active := make([]pipelineApi.CDPipeline, 0, len(pipelines.Items)) + + for i := range pipelines.Items { + if pipelines.Items[i].DeletionTimestamp != nil { + continue + } + + active = append(active, pipelines.Items[i]) + } + + return active, nil +} + +// AutotestGate is a Stage quality gate that runs an autotest, together with the Reference +// describing the Stage holding it. Codebase and CodebaseBranch are both referenced through +// this one gate field and key on different parts of it. +type AutotestGate struct { + Reference + + // AutotestName is the referenced Codebase name; never empty. + AutotestName string + // BranchName is the referenced git branch name, empty when the gate leaves it unset. + BranchName string +} + +// FindAutotestGates returns every autotest quality gate of the namespace. Manual gates +// leave AutotestName unset and are dropped here, so no caller can mistake one for a +// match by comparing unset fields. +func FindAutotestGates(ctx context.Context, c client.Client, namespace string) ([]AutotestGate, error) { + stages, err := listActiveStages(ctx, c, namespace) + if err != nil { + return nil, err + } + + var gates []AutotestGate + + for i := range stages { + stage := &stages[i] + + for _, gate := range stage.Spec.QualityGates { + if gate.AutotestName == nil || *gate.AutotestName == "" { + continue + } + + gates = append(gates, AutotestGate{ + Reference: Reference{ + Kind: KindStage, + Name: stage.Name, + Field: FieldQualityGateAutotest, + Reason: "autotest quality gate", + ParentCDPipeline: stage.Spec.CdPipeline, + }, + AutotestName: *gate.AutotestName, + BranchName: ptr.Deref(gate.BranchName, ""), + }) + } + } + + return gates, nil +} + +// listActiveStages returns the Stages in the given namespace that are not +// being deleted. When the CD pipeline CRDs are not installed in the cluster, +// it returns an empty, non-error result. +func listActiveStages(ctx context.Context, c client.Client, namespace string) ([]pipelineApi.Stage, error) { + stages := &pipelineApi.StageList{} + if err := c.List(ctx, stages, client.InNamespace(namespace)); err != nil { + if IsKindUnavailable(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to list Stages: %w", err) + } + + active := make([]pipelineApi.Stage, 0, len(stages.Items)) + + for i := range stages.Items { + if stages.Items[i].DeletionTimestamp != nil { + continue + } + + active = append(active, stages.Items[i]) + } + + return active, nil +} + +// IsKindUnavailable reports whether err indicates that a CRD kind is not +// registered/installed in the cluster (e.g. edp-cd-pipeline-operator is not +// deployed alongside this operator). +func IsKindUnavailable(err error) bool { + return apimeta.IsNoMatchError(err) || runtime.IsNotRegisteredError(err) +} diff --git a/pkg/deploymentusage/deploymentusage_test.go b/pkg/deploymentusage/deploymentusage_test.go new file mode 100644 index 00000000..7b6b0a6d --- /dev/null +++ b/pkg/deploymentusage/deploymentusage_test.go @@ -0,0 +1,169 @@ +package deploymentusage + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + 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" + "k8s.io/utils/ptr" + "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" +) + +// The rendered form reaches clients verbatim: it is the message of the admission denial +// and of each StatusCause. +func TestReference_String(t *testing.T) { + tests := []struct { + name string + ref Reference + want string + }{ + { + name: "CDPipeline reference", + ref: Reference{ + Kind: KindCDPipeline, + Name: "demo", + Field: "spec.inputDockerStreams", + Reason: "inputDockerStreams", + }, + want: "CDPipeline demo (inputDockerStreams)", + }, + { + name: "Stage reference names its parent CDPipeline", + ref: Reference{ + Kind: KindStage, + Name: "demo-dev", + Field: "spec.qualityGates.autotestName", + Reason: "autotest quality gate", + ParentCDPipeline: "demo", + }, + want: "Stage demo-dev of CDPipeline demo (autotest quality gate)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.ref.String()) + }) + } +} + +func TestJoin(t *testing.T) { + cdPipeline := Reference{Kind: KindCDPipeline, Name: "demo", Reason: "applications"} + stage := Reference{Kind: KindStage, Name: "demo-dev", Reason: "autotest quality gate", ParentCDPipeline: "demo"} + + tests := []struct { + name string + refs []Reference + want string + }{ + { + name: "no references", + refs: nil, + want: "", + }, + { + name: "single reference is rendered without a separator", + refs: []Reference{cdPipeline}, + want: "CDPipeline demo (applications)", + }, + { + name: "multiple references are separated by a semicolon", + refs: []Reference{cdPipeline, stage}, + want: "CDPipeline demo (applications); Stage demo-dev of CDPipeline demo (autotest quality gate)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, Join(tt.refs)) + }) + } +} + +func gateScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, pipelineApi.AddToScheme(scheme)) + + return scheme +} + +// A Stage on its way out no longer deploys anything, so its gates must not keep a +// Codebase or CodebaseBranch alive. +func TestFindAutotestGates_TerminatingStageIgnored(t *testing.T) { + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo-dev", + Namespace: "default", + DeletionTimestamp: ptr.To(metav1.Now()), + Finalizers: []string{"keep"}, + }, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("main"), + }}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(gateScheme(t)).WithObjects(stage).Build() + + gates, err := FindAutotestGates(context.Background(), k8sClient, "default") + require.NoError(t, err) + assert.Empty(t, gates) +} + +// A gate that names no autotest carries no reference, and must not reach callers as one +// with an empty AutotestName. +func TestFindAutotestGates_SkipsGatesWithoutAutotest(t *testing.T) { + stage := &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{ + {QualityGateType: "manual", StepName: "approve"}, + {QualityGateType: "autotests", AutotestName: ptr.To("")}, + {QualityGateType: "autotests", AutotestName: ptr.To("app")}, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(gateScheme(t)).WithObjects(stage).Build() + + gates, err := FindAutotestGates(context.Background(), k8sClient, "default") + require.NoError(t, err) + require.Len(t, gates, 1) + assert.Equal(t, "app", gates[0].AutotestName) + // An unset BranchName must not be reported as a branch of its own. + assert.Empty(t, gates[0].BranchName) +} + +// A Stage listing failure means usage could not be verified and must surface. +func TestFindAutotestGates_ListErrorIsSurfaced(t *testing.T) { + k8sClient := fake.NewClientBuilder(). + WithScheme(gateScheme(t)). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(_ context.Context, _ client.WithWatch, _ client.ObjectList, _ ...client.ListOption) error { + return apierrors.NewForbidden( + schema.GroupResource{Group: "v2.edp.epam.com", Resource: "stages"}, "", errors.New("no access")) + }, + }). + Build() + + _, err := FindAutotestGates(context.Background(), k8sClient, "default") + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to list Stages") +} diff --git a/pkg/webhook/codebase_webhook.go b/pkg/webhook/codebase_webhook.go index 2fdeec7a..d2bf26c7 100644 --- a/pkg/webhook/codebase_webhook.go +++ b/pkg/webhook/codebase_webhook.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" v1 "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/codebase" "github.com/epam/edp-codebase-operator/v2/pkg/util" ) @@ -144,5 +145,26 @@ func (r *CodebaseValidationWebhook) ValidateDelete( return nil, err } + deletedCodebase, ok := obj.(*v1.Codebase) + if !ok { + r.log.Info("the wrong object given, skipping validation") + + return nil, nil + } + + refs, err := codebase.FindCodebaseUsage(ctx, r.client, deletedCodebase) + if err != nil { + return nil, fmt.Errorf("failed to check Codebase usage: %w", err) + } + + if len(refs) > 0 { + return nil, newBlockedByUsageError( + v1.GroupVersion.WithResource("codebases").GroupResource(), + codebaseKind, + deletedCodebase.Name, + refs, + ) + } + return nil, nil } diff --git a/pkg/webhook/codebase_webhook_delete_test.go b/pkg/webhook/codebase_webhook_delete_test.go new file mode 100644 index 00000000..38aa9cbc --- /dev/null +++ b/pkg/webhook/codebase_webhook_delete_test.go @@ -0,0 +1,269 @@ +package webhook + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + 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" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + admissionv1 "k8s.io/api/admission/v1" + + pipelineApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/codebase" +) + +// codebaseDeleteClientBuilder mirrors the production wiring: the Codebase deletion check +// selects branches on BranchCodebaseNameIndex, which a fake client only serves when the +// index is registered on the builder. +func codebaseDeleteClientBuilder(t *testing.T) *fake.ClientBuilder { + t.Helper() + + return fake.NewClientBuilder(). + WithScheme(deleteWebhookScheme(t)). + WithIndex(&codebaseApi.CodebaseBranch{}, codebase.BranchCodebaseNameIndex, codebase.IndexBranchByCodebaseName) +} + +func deleteWebhookCodebase() *codebaseApi.Codebase { + return &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + } +} + +func codebaseDeleteAdmissionCtx() context.Context { + return admission.NewContextWithRequest(context.Background(), admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "app", + Namespace: "default", + }, + }) +} + +func TestCodebaseValidationWebhook_ValidateDelete_CodebaseInUse(t *testing.T) { + tests := []struct { + name string + objects []runtime.Object + wantErr string + }{ + { + name: "rejects deletion of application component used by CDPipeline", + objects: []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{Applications: []string{"app"}}, + }, + }, + wantErr: "used by CDPipeline demo", + }, + { + name: "rejects deletion of autotest component used by a Stage quality gate", + objects: []runtime.Object{ + &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("main"), + }}, + }, + }, + }, + wantErr: "used by Stage demo-dev of CDPipeline demo", + }, + { + // A manual quality gate leaves AutotestName unset and must never be + // mistaken for a match. + name: "allows deletion when only a manual quality gate exists", + objects: []runtime.Object{ + &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "manual", + StepName: "approve", + }}, + }, + }, + }, + }, + { + name: "allows deletion of unused codebase", + objects: []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{Applications: []string{"other-app"}}, + }, + }, + }, + { + name: "allows deletion when referencing CDPipeline is terminating", + objects: []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + DeletionTimestamp: ptr.To(metav1.Now()), + Finalizers: []string{"keep"}, + }, + Spec: pipelineApi.CDPipelineSpec{Applications: []string{"app"}}, + }, + }, + }, + { + name: "allows deletion when CD pipeline CRDs are not installed", + objects: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8sClient := codebaseDeleteClientBuilder(t). + WithRuntimeObjects(tt.objects...). + Build() + + w := NewCodebaseValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(codebaseDeleteAdmissionCtx(), deleteWebhookCodebase()) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + + return + } + + require.NoError(t, err) + }) + } +} + +func TestCodebaseValidationWebhook_ValidateDelete_StatusError(t *testing.T) { + objects := []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{Applications: []string{"app"}}, + }, + } + + k8sClient := codebaseDeleteClientBuilder(t). + WithRuntimeObjects(objects...). + Build() + + w := NewCodebaseValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(codebaseDeleteAdmissionCtx(), deleteWebhookCodebase()) + require.Error(t, err) + + var statusErr *apierrors.StatusError + require.True(t, errors.As(err, &statusErr)) + + status := statusErr.Status() + assert.Equal(t, int32(http.StatusForbidden), status.Code) + assert.Equal(t, metav1.StatusReasonForbidden, status.Reason) + assert.Equal(t, + "Codebase app cannot be deleted because it is used by CDPipeline demo (applications); "+ + "remove it from the deployment first", + status.Message, + ) + require.NotNil(t, status.Details) + require.Len(t, status.Details.Causes, 1) + assert.Equal(t, metav1.CauseTypeForbidden, status.Details.Causes[0].Type) + assert.Equal(t, "CDPipeline demo (applications)", status.Details.Causes[0].Message) + // Field is the machine-readable half of the cause; clients key off it instead of + // parsing Message. + assert.Equal(t, "spec.applications", status.Details.Causes[0].Field) +} + +// Every blocking reference is reported at once, so the user sees the full set to clean up +// rather than discovering them one delete at a time. +func TestCodebaseValidationWebhook_ValidateDelete_StatusErrorMultipleReferences(t *testing.T) { + objects := []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{Applications: []string{"app"}}, + }, + &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("main"), + }}, + }, + }, + } + + k8sClient := codebaseDeleteClientBuilder(t). + WithRuntimeObjects(objects...). + Build() + + w := NewCodebaseValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(codebaseDeleteAdmissionCtx(), deleteWebhookCodebase()) + require.Error(t, err) + + var statusErr *apierrors.StatusError + require.True(t, errors.As(err, &statusErr)) + + status := statusErr.Status() + assert.Equal(t, + "Codebase app cannot be deleted because it is used by CDPipeline demo (applications); "+ + "Stage demo-dev of CDPipeline demo (autotest quality gate); "+ + "remove it from the deployment first", + status.Message, + ) + require.NotNil(t, status.Details) + require.Len(t, status.Details.Causes, 2) + assert.Equal(t, "spec.applications", status.Details.Causes[0].Field) + assert.Equal(t, "spec.qualityGates.autotestName", status.Details.Causes[1].Field) +} + +// An object of another kind cannot be checked for usage; validation is skipped rather +// than denied, since the request is not one this webhook governs. +func TestCodebaseValidationWebhook_ValidateDelete_WrongObjectType(t *testing.T) { + w := NewCodebaseValidationWebhook(codebaseDeleteClientBuilder(t).Build(), ctrl.Log) + + warnings, err := w.ValidateDelete(codebaseDeleteAdmissionCtx(), &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-main", Namespace: "default"}, + }) + require.NoError(t, err) + assert.Nil(t, warnings) +} + +// When usage cannot be determined the deletion must be refused: treating an unverifiable +// codebase as unused is what this webhook exists to prevent. +func TestCodebaseValidationWebhook_ValidateDelete_UsageCheckErrorDenies(t *testing.T) { + k8sClient := codebaseDeleteClientBuilder(t). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(_ context.Context, _ client.WithWatch, _ client.ObjectList, _ ...client.ListOption) error { + return apierrors.NewForbidden( + schema.GroupResource{Group: "v2.edp.epam.com", Resource: "cdpipelines"}, "", errors.New("no access")) + }, + }). + Build() + + w := NewCodebaseValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(codebaseDeleteAdmissionCtx(), deleteWebhookCodebase()) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to check Codebase usage") +} diff --git a/pkg/webhook/codebasebranch_webhook.go b/pkg/webhook/codebasebranch_webhook.go index 0cdc1f62..ff915570 100644 --- a/pkg/webhook/codebasebranch_webhook.go +++ b/pkg/webhook/codebasebranch_webhook.go @@ -123,15 +123,18 @@ func (r *CodebaseBranchValidationWebhook) ValidateDelete( return nil, nil } - usage, err := codebasebranch.FindBranchUsage(ctx, r.client, deletedCodebaseBranch) + refs, err := codebasebranch.FindBranchUsage(ctx, r.client, deletedCodebaseBranch) if err != nil { return nil, fmt.Errorf("failed to check CodebaseBranch usage: %w", err) } - if usage != "" { - return nil, fmt.Errorf( - "CodebaseBranch %s cannot be deleted because it is used by %s; remove it from the deployment first", - deletedCodebaseBranch.Name, usage) + if len(refs) > 0 { + return nil, newBlockedByUsageError( + v1.GroupVersion.WithResource("codebasebranches").GroupResource(), + codebaseBranchKind, + deletedCodebaseBranch.Name, + refs, + ) } return nil, nil diff --git a/pkg/webhook/codebasebranch_webhook_delete_test.go b/pkg/webhook/codebasebranch_webhook_delete_test.go index 14de077f..f46b34b8 100644 --- a/pkg/webhook/codebasebranch_webhook_delete_test.go +++ b/pkg/webhook/codebasebranch_webhook_delete_test.go @@ -2,10 +2,13 @@ package webhook import ( "context" + "errors" + "net/http" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" @@ -118,3 +121,87 @@ func TestCodebaseBranchValidationWebhook_ValidateDelete_BranchInUse(t *testing.T }) } } + +func TestCodebaseBranchValidationWebhook_ValidateDelete_StatusError(t *testing.T) { + t.Run("single reference: message wording is preserved and causes are populated", func(t *testing.T) { + objects := []runtime.Object{ + &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + }, + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{InputDockerStreams: []string{"app-feature"}}, + }, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(deleteWebhookScheme(t)). + WithRuntimeObjects(objects...). + Build() + + w := NewCodebaseBranchValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(context.Background(), deleteWebhookBranch()) + require.Error(t, err) + + var statusErr *apierrors.StatusError + require.True(t, errors.As(err, &statusErr)) + + status := statusErr.Status() + assert.Equal(t, int32(http.StatusForbidden), status.Code) + assert.Equal(t, metav1.StatusReasonForbidden, status.Reason) + assert.Equal(t, + "CodebaseBranch app-feature cannot be deleted because it is used by "+ + "CDPipeline demo (inputDockerStreams); remove it from the deployment first", + status.Message, + ) + require.NotNil(t, status.Details) + require.Len(t, status.Details.Causes, 1) + assert.Equal(t, metav1.CauseTypeForbidden, status.Details.Causes[0].Type) + assert.Equal(t, "CDPipeline demo (inputDockerStreams)", status.Details.Causes[0].Message) + }) + + t.Run("multiple references: all are reported as separate causes", func(t *testing.T) { + objects := []runtime.Object{ + &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "default"}, + }, + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{InputDockerStreams: []string{"app-feature"}}, + }, + &pipelineApi.Stage{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-dev", Namespace: "default"}, + Spec: pipelineApi.StageSpec{ + CdPipeline: "demo", + QualityGates: []pipelineApi.QualityGate{{ + QualityGateType: "autotests", + AutotestName: ptr.To("app"), + BranchName: ptr.To("feature"), + }}, + }, + }, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(deleteWebhookScheme(t)). + WithRuntimeObjects(objects...). + Build() + + w := NewCodebaseBranchValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(context.Background(), deleteWebhookBranch()) + require.Error(t, err) + + var statusErr *apierrors.StatusError + require.True(t, errors.As(err, &statusErr)) + + status := statusErr.Status() + require.NotNil(t, status.Details) + require.Len(t, status.Details.Causes, 2) + + messages := []string{status.Details.Causes[0].Message, status.Details.Causes[1].Message} + assert.Contains(t, messages, "CDPipeline demo (inputDockerStreams)") + assert.Contains(t, messages, "Stage demo-dev of CDPipeline demo (autotest quality gate)") + }) +} diff --git a/pkg/webhook/usage_error.go b/pkg/webhook/usage_error.go new file mode 100644 index 00000000..11bc0e85 --- /dev/null +++ b/pkg/webhook/usage_error.go @@ -0,0 +1,65 @@ +package webhook + +import ( + "fmt" + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/epam/edp-codebase-operator/v2/pkg/deploymentusage" +) + +const ( + codebaseKind = "Codebase" + codebaseBranchKind = "CodebaseBranch" +) + +// newBlockedByUsageError builds a StatusError that denies deletion of a +// resource still referenced by deployment resources. +// +// It intentionally hand-builds the status rather than using +// apierrors.NewForbidden/NewInvalid: NewForbidden does not carry +// Details.Causes, and NewInvalid returns HTTP 422/"Invalid" semantics which +// is wrong here (the object being deleted is not invalid, it is blocked by +// external references) and would change the HTTP code clients rely on. +// controller-runtime's admission handler unwraps *apierrors.StatusError via +// errors.As and forwards its full metav1.Status, so Details.Causes reaches +// the client intact. +func newBlockedByUsageError( + gr schema.GroupResource, + kind string, + name string, + refs []deploymentusage.Reference, +) *apierrors.StatusError { + causes := make([]metav1.StatusCause, 0, len(refs)) + + for _, ref := range refs { + causes = append(causes, metav1.StatusCause{ + Type: metav1.CauseTypeForbidden, + Field: ref.Field, + Message: ref.String(), + }) + } + + message := fmt.Sprintf( + "%s %s cannot be deleted because it is used by %s; remove it from the deployment first", + kind, name, deploymentusage.Join(refs), + ) + + return &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusForbidden, + Reason: metav1.StatusReasonForbidden, + Message: message, + Details: &metav1.StatusDetails{ + Group: gr.Group, + Kind: gr.Resource, + Name: name, + Causes: causes, + }, + }, + } +}