From 77606dfe0bdf32d45bc5fe9ce584a9a38fd9b308 Mon Sep 17 00:00:00 2001 From: Sergiy Kulanov Date: Sun, 12 Jul 2026 10:21:58 +0300 Subject: [PATCH] EPMDEDP-17188: feat: Add stale branch detection and cleanup for codebase branches Periodically verify that every codebase branch still exists in the git repository, mark missing ones and optionally clean them up, so platforms with many branches do not accumulate CodebaseBranch resources for branches deleted from git. Detection (always on, configurable): - list remote branches without cloning (git ls-remote) via the git client - leader-elected periodic checker (BRANCH_STALE_CHECK_INTERVAL, default 24h, 0 disables) doing one listing per codebase repository - record the result as a Stale status condition (source of truth), mirror it with the app.edp.epam.com/stale label for selector-based filtering and emit events - never mark branches on git connectivity or authorization errors; skip default, not-yet-pushed and terminating branches - preserve status conditions in every CodebaseBranch status rewrite so the Stale condition survives regular reconciles of the branch Cleanup (opt-in per Codebase via app.edp.epam.com/branch-cleanup-strategy=auto): - delete stale branches not referenced by any CDPipeline inputDockerStreams or Stage autotest quality gate; the owned CodebaseImageStream is garbage-collected - keep referenced branches marked, recording the retaining resource in the condition message and a StaleBranchRetained event Protection: - the admission webhook now rejects deleting a CodebaseBranch that participates in a deployment (previously enforced only client-side by the portal), except during Codebase cascade deletion - works on clusters without cd-pipeline-operator installed Fix: release the CodebaseBranch finalizer when the owning Codebase is gone. When a Codebase is deleted, its branches are removed afterwards by garbage collection, at which point the owning Codebase no longer exists. Reconcile failed on the Codebase lookup before reaching the deletion flow, so such branches stayed terminating forever with their finalizer never released. Run the deletion flow directly when the branch is terminating and its Codebase is not found, letting cascade deletion complete. Signed-off-by: Sergiy Kulanov --- api/v1/annotations.go | 14 + api/v1/codebasebranch_types.go | 19 ++ api/v1/labels.go | 6 + api/v1/zz_generated.deepcopy.go | 8 + cmd/main.go | 44 +++ .../v2.edp.epam.com_codebasebranches.yaml | 68 +++++ .../codebasebranch/chain/check_reference.go | 1 + .../clean_tmp_directory.go | 1 + .../put_branch_in_git/put_branch_in_git.go | 2 + .../put_codebase_image_stream.go | 2 + .../codebasebranch_controller.go | 21 ++ .../codebasebranch_controller_test.go | 89 ++++++ .../codebasebranch/stalecheck/action.go | 151 +++++++++++ .../codebasebranch/stalecheck/checker.go | 180 ++++++++++++ .../codebasebranch/stalecheck/checker_test.go | 256 ++++++++++++++++++ .../stalecheck/cleanup_action.go | 60 ++++ .../stalecheck/cleanup_action_test.go | 109 ++++++++ deploy-templates/README.md | 1 + .../v2.edp.epam.com_codebasebranches.yaml | 68 +++++ deploy-templates/templates/deployment.yaml | 2 + deploy-templates/values.yaml | 5 + docs/api.md | 86 ++++++ pkg/codebasebranch/usage.go | 79 ++++++ pkg/codebasebranch/usage_test.go | 120 ++++++++ pkg/git/git.go | 4 + pkg/git/mocks/git_generated.mock.go | 68 +++++ pkg/git/provider.go | 46 ++++ pkg/git/provider_test.go | 47 ++++ pkg/webhook/codebasebranch_webhook.go | 38 +++ .../codebasebranch_webhook_delete_test.go | 120 ++++++++ 30 files changed, 1715 insertions(+) create mode 100644 controllers/codebasebranch/stalecheck/action.go create mode 100644 controllers/codebasebranch/stalecheck/checker.go create mode 100644 controllers/codebasebranch/stalecheck/checker_test.go create mode 100644 controllers/codebasebranch/stalecheck/cleanup_action.go create mode 100644 controllers/codebasebranch/stalecheck/cleanup_action_test.go create mode 100644 pkg/codebasebranch/usage.go create mode 100644 pkg/codebasebranch/usage_test.go create mode 100644 pkg/webhook/codebasebranch_webhook_delete_test.go diff --git a/api/v1/annotations.go b/api/v1/annotations.go index 0f06dd7a..05d80c4d 100644 --- a/api/v1/annotations.go +++ b/api/v1/annotations.go @@ -4,4 +4,18 @@ const ( // GitLabCITemplateAnnotation is an annotation on a Codebase CR that specifies the ConfigMap // name to use as the GitLab CI template. When absent, the operator falls back to "gitlab-ci-default". GitLabCITemplateAnnotation = "app.edp.epam.com/gitlab-ci-template" + + // BranchCleanupStrategyAnnotation is an annotation on a Codebase CR that defines what the + // operator does with CodebaseBranch resources whose branch no longer exists in git. + // Supported values: "mark" (default) - only mark the branch as stale; + // "auto" - delete the stale branch when it is not referenced by any CDPipeline/Stage. + BranchCleanupStrategyAnnotation = "app.edp.epam.com/branch-cleanup-strategy" +) + +const ( + // BranchCleanupStrategyMark marks stale branches for manual cleanup. + BranchCleanupStrategyMark = "mark" + + // BranchCleanupStrategyAuto deletes stale branches that are not used by any CDPipeline/Stage. + BranchCleanupStrategyAuto = "auto" ) diff --git a/api/v1/codebasebranch_types.go b/api/v1/codebasebranch_types.go index 3e86aa11..2efbedaa 100644 --- a/api/v1/codebasebranch_types.go +++ b/api/v1/codebasebranch_types.go @@ -6,6 +6,13 @@ import ( const ( CodebaseBranchGitStatusBranchCreated = "branch-created" + + // ConditionStale is a condition type indicating whether the branch + // no longer exists in the git repository ("True" means the branch is missing). + ConditionStale = "Stale" + + ReasonBranchNotFoundInGit = "BranchNotFoundInGit" + ReasonBranchFoundInGit = "BranchFoundInGit" ) // CodebaseBranchSpec defines the desired state of CodebaseBranch. @@ -83,6 +90,17 @@ type CodebaseBranchStatus struct { // Specifies a status of action for git. Git string `json:"git,omitempty"` + + // Conditions represent the latest available observations of an object's state. + // Each condition type is an independent observation (e.g. Stale); new types can be + // added over time without schema changes. + // +optional + // +nullable + // +listType=map + // +listMapKey=type + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metaV1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } // +kubebuilder:object:root=true @@ -94,6 +112,7 @@ type CodebaseBranchStatus struct { // +kubebuilder:printcolumn:name="Codebase Name",type="string",JSONPath=".spec.codebaseName",description="Owner of object" // +kubebuilder:printcolumn:name="Release",type="boolean",JSONPath=".spec.release",description="Is a release branch" // +kubebuilder:printcolumn:name="Branch",type="string",JSONPath=".spec.branchName",description="Name of branch" +// +kubebuilder:printcolumn:name="Stale",type="string",JSONPath=`.status.conditions[?(@.type=="Stale")].status`,description="Branch is missing in git" // CodebaseBranch is the Schema for the CodebaseBranches API. type CodebaseBranch struct { diff --git a/api/v1/labels.go b/api/v1/labels.go index 698ef831..481525b7 100644 --- a/api/v1/labels.go +++ b/api/v1/labels.go @@ -28,4 +28,10 @@ const ( // GitServerLabel is a label used to store the name of the GitServer in related resources. GitServerLabel = "app.edp.epam.com/gitServer" + + // StaleLabel marks a CodebaseBranch whose branch no longer exists in the git repository. + // It mirrors the Stale status condition so that resources can be filtered with a label + // selector; the condition remains the source of truth and the label is re-asserted by + // the operator on every staleness check. + StaleLabel = "app.edp.epam.com/stale" ) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index a2803ee0..613a8001 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -5,6 +5,7 @@ package v1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -251,6 +252,13 @@ func (in *CodebaseBranchStatus) DeepCopyInto(out *CodebaseBranchStatus) { *out = new(string) **out = **in } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CodebaseBranchStatus. diff --git a/cmd/main.go b/cmd/main.go index 55c287e8..656e4c7f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -37,11 +37,13 @@ import ( "github.com/epam/edp-codebase-operator/v2/controllers/cdstagedeploy/chain" "github.com/epam/edp-codebase-operator/v2/controllers/codebase" "github.com/epam/edp-codebase-operator/v2/controllers/codebasebranch" + "github.com/epam/edp-codebase-operator/v2/controllers/codebasebranch/stalecheck" "github.com/epam/edp-codebase-operator/v2/controllers/codebaseimagestream" "github.com/epam/edp-codebase-operator/v2/controllers/gitserver" "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" + 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" "github.com/epam/edp-codebase-operator/v2/pkg/webhook" @@ -59,6 +61,8 @@ const ( telemetryDefaultDelay = time.Hour telemetrySendEvery = time.Hour * 24 telemetryUrl = "https://telemetry.kuberocketci.io" + branchStaleCheckIntervalEnv = "BRANCH_STALE_CHECK_INTERVAL" + branchStaleCheckDefaultInterval = time.Hour * 24 ) func main() { @@ -322,6 +326,27 @@ func main() { os.Exit(1) } + if interval := getBranchStaleCheckInterval(); interval > 0 { + staleRecorder := mgr.GetEventRecorderFor("stale-branch-checker") + + markAction := stalecheck.NewMarkAction(mgr.GetClient(), staleRecorder) + + checker := stalecheck.NewChecker( + mgr.GetClient(), + ns, + interval, + gitproviderv2.DefaultGitProviderFactory, + markAction, + stalecheck.NewCleanupAction(mgr.GetClient(), staleRecorder, markAction), + ) + if err := mgr.Add(checker); err != nil { + setupLog.Error(err, "failed to add stale branch checker to manager") + os.Exit(1) + } + } else { + setupLog.Info("Stale branch checker is disabled", "env", branchStaleCheckIntervalEnv) + } + setupLog.Info("starting manager") ctx := ctrl.SetupSignalHandler() @@ -351,6 +376,25 @@ func getMaxConcurrentReconciles(envVar string) int { return int(n) } +// getBranchStaleCheckInterval accepts Go duration strings (e.g. "24h", "30m"); +// a zero or negative duration disables the check. +func getBranchStaleCheckInterval() time.Duration { + val, exists := os.LookupEnv(branchStaleCheckIntervalEnv) + if !exists { + return branchStaleCheckDefaultInterval + } + + d, err := time.ParseDuration(val) + if err != nil { + setupLog.Error(err, "Invalid stale branch check interval, using default", + "env", branchStaleCheckIntervalEnv, "value", val, "default", branchStaleCheckDefaultInterval) + + return branchStaleCheckDefaultInterval + } + + return d +} + func getTelemetryDelay() time.Duration { val, exists := os.LookupEnv("TELEMETRY_DELAY") if !exists { diff --git a/config/crd/bases/v2.edp.epam.com_codebasebranches.yaml b/config/crd/bases/v2.edp.epam.com_codebasebranches.yaml index e410a038..68368ac0 100644 --- a/config/crd/bases/v2.edp.epam.com_codebasebranches.yaml +++ b/config/crd/bases/v2.edp.epam.com_codebasebranches.yaml @@ -37,6 +37,10 @@ spec: jsonPath: .spec.branchName name: Branch type: string + - description: Branch is missing in git + jsonPath: .status.conditions[?(@.type=="Stale")].status + name: Stale + type: string name: v1 schema: openAPIV3Schema: @@ -109,6 +113,70 @@ spec: build: nullable: true type: string + conditions: + description: |- + Conditions represent the latest available observations of an object's state. + Each condition type is an independent observation (e.g. Stale); new types can be + added over time without schema changes. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + nullable: true + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map detailedMessage: description: |- Detailed information regarding action result diff --git a/controllers/codebasebranch/chain/check_reference.go b/controllers/codebasebranch/chain/check_reference.go index fc8092f9..4129effd 100644 --- a/controllers/codebasebranch/chain/check_reference.go +++ b/controllers/codebasebranch/chain/check_reference.go @@ -134,5 +134,6 @@ func (CheckReferenceExists) setFailedFields( Build: codebaseBranch.Status.Build, FailureCount: codebaseBranch.Status.FailureCount, Git: codebaseBranch.Status.Git, + Conditions: codebaseBranch.Status.Conditions, } } diff --git a/controllers/codebasebranch/chain/clean_tmp_directory/clean_tmp_directory.go b/controllers/codebasebranch/chain/clean_tmp_directory/clean_tmp_directory.go index 43ab1f6f..f8a93b8a 100644 --- a/controllers/codebasebranch/chain/clean_tmp_directory/clean_tmp_directory.go +++ b/controllers/codebasebranch/chain/clean_tmp_directory/clean_tmp_directory.go @@ -52,5 +52,6 @@ func setFailedFields(cb *codebaseApi.CodebaseBranch, a codebaseApi.ActionType, m Git: cb.Status.Git, VersionHistory: cb.Status.VersionHistory, Build: cb.Status.Build, + Conditions: cb.Status.Conditions, } } diff --git a/controllers/codebasebranch/chain/put_branch_in_git/put_branch_in_git.go b/controllers/codebasebranch/chain/put_branch_in_git/put_branch_in_git.go index 75ec691c..c3d32389 100644 --- a/controllers/codebasebranch/chain/put_branch_in_git/put_branch_in_git.go +++ b/controllers/codebasebranch/chain/put_branch_in_git/put_branch_in_git.go @@ -161,6 +161,7 @@ func (h PutBranchInGit) setIntermediateSuccessFields( Build: cb.Status.Build, FailureCount: cb.Status.FailureCount, Git: cb.Status.Git, + Conditions: cb.Status.Conditions, } err := h.Client.Status().Update(ctx, cb) @@ -185,6 +186,7 @@ func putGitBranchSetFailedFields(cb *codebaseApi.CodebaseBranch, message string) Build: cb.Status.Build, FailureCount: cb.Status.FailureCount, Git: cb.Status.Git, + Conditions: cb.Status.Conditions, } } diff --git a/controllers/codebasebranch/chain/put_codebase_image_stream/put_codebase_image_stream.go b/controllers/codebasebranch/chain/put_codebase_image_stream/put_codebase_image_stream.go index d79303bc..6da87dac 100644 --- a/controllers/codebasebranch/chain/put_codebase_image_stream/put_codebase_image_stream.go +++ b/controllers/codebasebranch/chain/put_codebase_image_stream/put_codebase_image_stream.go @@ -211,6 +211,7 @@ func setFailedFields(cb *codebaseApi.CodebaseBranch, a codebaseApi.ActionType, m Git: cb.Status.Git, VersionHistory: cb.Status.VersionHistory, Build: cb.Status.Build, + Conditions: cb.Status.Conditions, } } @@ -230,6 +231,7 @@ func (h PutCodebaseImageStream) setIntermediateSuccessFields( LastSuccessfulBuild: cb.Status.LastSuccessfulBuild, Build: cb.Status.Build, Git: cb.Status.Git, + Conditions: cb.Status.Conditions, } err := h.Client.Status().Update(ctx, cb) diff --git a/controllers/codebasebranch/codebasebranch_controller.go b/controllers/codebasebranch/codebasebranch_controller.go index f0bbd6b6..3b406e42 100644 --- a/controllers/codebasebranch/codebasebranch_controller.go +++ b/controllers/codebasebranch/codebasebranch_controller.go @@ -117,6 +117,22 @@ func (r *ReconcileCodebaseBranch) Reconcile(ctx context.Context, request reconci c := &codebaseApi.Codebase{} if err := r.client.Get(ctx, types.NamespacedName{Name: cb.Spec.CodebaseName, Namespace: cb.Namespace}, c); err != nil { + // When the owning Codebase is already gone the branch is being garbage-collected + // as part of a cascade deletion; the only work left is to run the deletion flow + // and release the finalizer, otherwise the branch stays terminating forever. + if k8sErrors.IsNotFound(err) && cb.DeletionTimestamp != nil { + result, delErr := r.tryToDeleteCodebaseBranch(ctx, cb, factory.GetDeletionChain()) + if delErr != nil { + return reconcile.Result{}, fmt.Errorf("failed to remove codebasebranch %v: %w", cb.Name, delErr) + } + + if result != nil { + return *result, nil + } + + return reconcile.Result{}, nil + } + return reconcile.Result{}, fmt.Errorf("failed to get Codebase: %w", err) } @@ -210,6 +226,7 @@ func (r *ReconcileCodebaseBranch) setSuccessStatus( LastSuccessfulBuild: cb.Status.LastSuccessfulBuild, Build: cb.Status.Build, Git: cb.Status.Git, + Conditions: cb.Status.Conditions, } return r.updateStatus(ctx, cb) @@ -223,7 +240,11 @@ func (r *ReconcileCodebaseBranch) updateStatus(ctx context.Context, cb *codebase return fmt.Errorf("failed to get CodebaseBranch: %w", err) } + // Conditions are owned by the stale branch checker, which may have patched them + // after cb was read; the freshly fetched value is always the authoritative one. + conditions := cbbranch.Status.Conditions cbbranch.Status = cb.Status + cbbranch.Status.Conditions = conditions if err := r.client.Status().Update(ctx, cbbranch); err != nil { return fmt.Errorf("failed to update CodebaseBranch status: %w", err) diff --git a/controllers/codebasebranch/codebasebranch_controller_test.go b/controllers/codebasebranch/codebasebranch_controller_test.go index fdf6efc8..6dfe64df 100644 --- a/controllers/codebasebranch/codebasebranch_controller_test.go +++ b/controllers/codebasebranch/codebasebranch_controller_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" coreV1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -79,6 +80,46 @@ func TestReconcileCodebaseBranch_Reconcile_ShouldFailNotFound(t *testing.T) { assert.Equal(t, time.Duration(0), res.RequeueAfter) } +func TestReconcileCodebaseBranch_Reconcile_ShouldReleaseFinalizerWhenCodebaseGone(t *testing.T) { + t.Setenv("WORKING_DIR", "/tmp/1") + + now := metaV1.Now() + cb := &codebaseApi.CodebaseBranch{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "app-main", + Namespace: "namespace", + DeletionTimestamp: &now, + Finalizers: []string{codebaseBranchOperatorFinalizerName}, + }, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: "app", + BranchName: "main", + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, codebaseApi.AddToScheme(scheme)) + fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(cb).Build() + + r := ReconcileCodebaseBranch{ + client: fakeCl, + log: logr.Discard(), + scheme: scheme, + } + + res, err := r.Reconcile(context.TODO(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "app-main", Namespace: "namespace"}, + }) + + require.NoError(t, err) + assert.Equal(t, time.Duration(0), res.RequeueAfter) + + // With the finalizer released, the fake client removes the terminating object entirely. + err = fakeCl.Get(context.TODO(), + types.NamespacedName{Name: "app-main", Namespace: "namespace"}, &codebaseApi.CodebaseBranch{}) + assert.True(t, k8sErrors.IsNotFound(err), "branch must be gone after finalizer release") +} + func TestReconcileCodebaseBranch_Reconcile_ShouldFailGetCodebase(t *testing.T) { t.Setenv("WORKING_DIR", "/tmp/1") @@ -741,3 +782,51 @@ func TestSetDefaultValues_ShouldSkipForGitLab(t *testing.T) { assert.False(t, changed) assert.Nil(t, cb.Spec.Pipelines) } + +func TestReconcileCodebaseBranch_setSuccessStatus_PreservesStaleCondition(t *testing.T) { + cb := &codebaseApi.CodebaseBranch{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "app-main", + Namespace: "namespace", + }, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: "app", + BranchName: "main", + }, + Status: codebaseApi.CodebaseBranchStatus{ + Conditions: []metaV1.Condition{{ + Type: codebaseApi.ConditionStale, + Status: metaV1.ConditionTrue, + Reason: codebaseApi.ReasonBranchNotFoundInGit, + Message: "Branch was not found in the git repository", + LastTransitionTime: metaV1.Now(), + }}, + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, codebaseApi.AddToScheme(scheme)) + + fakeCl := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(cb). + WithStatusSubresource(cb). + Build() + + r := ReconcileCodebaseBranch{ + client: fakeCl, + log: logr.Discard(), + scheme: scheme, + } + + require.NoError(t, r.setSuccessStatus(context.TODO(), cb.DeepCopy(), codebaseApi.CIConfiguration)) + + updated := &codebaseApi.CodebaseBranch{} + require.NoError(t, fakeCl.Get(context.TODO(), + types.NamespacedName{Name: "app-main", Namespace: "namespace"}, updated)) + + require.Len(t, updated.Status.Conditions, 1, + "the Stale condition must survive the success-status rewrite") + assert.Equal(t, codebaseApi.ConditionStale, updated.Status.Conditions[0].Type) + assert.Equal(t, metaV1.ConditionTrue, updated.Status.Conditions[0].Status) +} diff --git a/controllers/codebasebranch/stalecheck/action.go b/controllers/codebasebranch/stalecheck/action.go new file mode 100644 index 00000000..46c03c9b --- /dev/null +++ b/controllers/codebasebranch/stalecheck/action.go @@ -0,0 +1,151 @@ +package stalecheck + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" +) + +const ( + EventReasonBranchStale = "BranchStale" + EventReasonBranchStaleResolved = "BranchStaleResolved" + EventReasonStaleBranchDeleted = "StaleBranchDeleted" + EventReasonStaleBranchRetained = "StaleBranchRetained" +) + +// Verdict is the result of checking a single CodebaseBranch against the git repository. +type Verdict struct { + ExistsInGit bool + + // RetainedBy describes the deployment resource that prevents automatic cleanup + // of a missing branch (empty when the branch exists or is not retained). + RetainedBy string +} + +// StaleBranchAction applies a cleanup strategy decision to a CodebaseBranch based on a Verdict. +type StaleBranchAction interface { + Apply(ctx context.Context, branch *codebaseApi.CodebaseBranch, verdict Verdict) error +} + +// MarkAction marks/unmarks a CodebaseBranch as stale via the Stale status condition +// (source of truth) and the mirrored stale label (for label-selector filtering). +type MarkAction struct { + client client.Client + recorder record.EventRecorder +} + +func NewMarkAction(k8sClient client.Client, recorder record.EventRecorder) *MarkAction { + return &MarkAction{client: k8sClient, recorder: recorder} +} + +func (a *MarkAction) Apply(ctx context.Context, branch *codebaseApi.CodebaseBranch, verdict Verdict) error { + wasStale := meta.IsStatusConditionTrue(branch.Status.Conditions, codebaseApi.ConditionStale) + + if err := a.updateStatusCondition(ctx, branch, verdict); err != nil { + return err + } + + if err := a.updateStaleLabel(ctx, branch, verdict); err != nil { + return err + } + + a.emitTransitionEvent(branch, wasStale, verdict) + + return nil +} + +func (a *MarkAction) updateStatusCondition( + ctx context.Context, + branch *codebaseApi.CodebaseBranch, + verdict Verdict, +) error { + // Messages are deliberately static so that SetStatusCondition reports a change + // (and triggers a status write) only on real transitions, not on every periodic check. + condition := metav1.Condition{ + Type: codebaseApi.ConditionStale, + Status: metav1.ConditionFalse, + Reason: codebaseApi.ReasonBranchFoundInGit, + Message: "Branch exists in the git repository", + ObservedGeneration: branch.Generation, + } + + if !verdict.ExistsInGit { + condition.Status = metav1.ConditionTrue + condition.Reason = codebaseApi.ReasonBranchNotFoundInGit + condition.Message = "Branch was not found in the git repository" + + if verdict.RetainedBy != "" { + condition.Message = fmt.Sprintf( + "Branch was not found in the git repository; retained because it is used by %s", verdict.RetainedBy) + } + } + + original := branch.DeepCopy() + if !meta.SetStatusCondition(&branch.Status.Conditions, condition) { + return nil + } + + if err := a.client.Status().Patch(ctx, branch, client.MergeFrom(original)); err != nil { + return fmt.Errorf("failed to patch Stale condition on CodebaseBranch %s: %w", branch.Name, err) + } + + return nil +} + +func (a *MarkAction) updateStaleLabel(ctx context.Context, branch *codebaseApi.CodebaseBranch, verdict Verdict) error { + value, labeled := branch.Labels[codebaseApi.StaleLabel] + + upToDate := (verdict.ExistsInGit && !labeled) || (!verdict.ExistsInGit && value == "true") + if upToDate { + return nil + } + + original := branch.DeepCopy() + + if verdict.ExistsInGit { + delete(branch.Labels, codebaseApi.StaleLabel) + } else { + if branch.Labels == nil { + branch.Labels = make(map[string]string, 1) + } + + branch.Labels[codebaseApi.StaleLabel] = "true" + } + + if err := a.client.Patch(ctx, branch, client.MergeFrom(original)); err != nil { + return fmt.Errorf("failed to patch stale label on CodebaseBranch %s: %w", branch.Name, err) + } + + return nil +} + +func (a *MarkAction) emitTransitionEvent(branch *codebaseApi.CodebaseBranch, wasStale bool, verdict Verdict) { + if a.recorder == nil { + return + } + + if !wasStale && !verdict.ExistsInGit { + if verdict.RetainedBy != "" { + a.recorder.Eventf(branch, corev1.EventTypeWarning, EventReasonStaleBranchRetained, + "Branch %s was not found in the git repository; it is marked as stale but retained because it is used by %s", + branch.Spec.BranchName, verdict.RetainedBy) + + return + } + + a.recorder.Eventf(branch, corev1.EventTypeWarning, EventReasonBranchStale, + "Branch %s was not found in the git repository and is marked as stale", branch.Spec.BranchName) + } + + if wasStale && verdict.ExistsInGit { + a.recorder.Eventf(branch, corev1.EventTypeNormal, EventReasonBranchStaleResolved, + "Branch %s exists in the git repository again, stale mark removed", branch.Spec.BranchName) + } +} diff --git a/controllers/codebasebranch/stalecheck/checker.go b/controllers/codebasebranch/stalecheck/checker.go new file mode 100644 index 00000000..51acd284 --- /dev/null +++ b/controllers/codebasebranch/stalecheck/checker.go @@ -0,0 +1,180 @@ +package stalecheck + +import ( + "context" + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git" + "github.com/epam/edp-codebase-operator/v2/pkg/util" +) + +// GitClientFactory is the injection seam for tests; +// production wiring uses gitproviderv2.DefaultGitProviderFactory. +type GitClientFactory func(gitServer *codebaseApi.GitServer, secret *corev1.Secret) gitproviderv2.Git + +// Checker periodically verifies that every CodebaseBranch still has a corresponding +// branch in the real git repository and applies the configured cleanup strategy. +// +// It runs as a manager Runnable (leader-only) rather than a watch-driven controller +// because staleness is external state that no Kubernetes event reports. +type Checker struct { + client client.Client + namespace string + interval time.Duration + gitClientFactory GitClientFactory + markAction StaleBranchAction + cleanupAction StaleBranchAction +} + +func NewChecker( + k8sClient client.Client, + namespace string, + interval time.Duration, + gitClientFactory GitClientFactory, + markAction StaleBranchAction, + cleanupAction StaleBranchAction, +) *Checker { + return &Checker{ + client: k8sClient, + namespace: namespace, + interval: interval, + gitClientFactory: gitClientFactory, + markAction: markAction, + cleanupAction: cleanupAction, + } +} + +// Start implements manager.Runnable. It sweeps once on startup and then on every tick. +func (c *Checker) Start(ctx context.Context) error { + log := ctrl.Log.WithName("stale-branch-checker") + ctx = ctrl.LoggerInto(ctx, log) + + log.Info("Starting stale branch checker", "interval", c.interval) + + c.sweep(ctx) + + ticker := time.NewTicker(c.interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + log.Info("Stopping stale branch checker") + return nil + case <-ticker.C: + c.sweep(ctx) + } + } +} + +// NeedLeaderElection ensures only the elected leader talks to git servers. +func (c *Checker) NeedLeaderElection() bool { + return true +} + +func (c *Checker) sweep(ctx context.Context) { + log := ctrl.LoggerFrom(ctx) + log.Info("Checking codebase branches staleness") + + branches := &codebaseApi.CodebaseBranchList{} + if err := c.client.List(ctx, branches, client.InNamespace(c.namespace)); err != nil { + log.Error(err, "Failed to list codebase branches") + return + } + + branchesByCodebase := make(map[string][]*codebaseApi.CodebaseBranch) + + for i := range branches.Items { + branch := &branches.Items[i] + branchesByCodebase[branch.Spec.CodebaseName] = append(branchesByCodebase[branch.Spec.CodebaseName], branch) + } + + for codebaseName, codebaseBranches := range branchesByCodebase { + if err := c.checkCodebaseBranches(ctx, codebaseName, codebaseBranches); err != nil { + log.Error(err, "Failed to check branches staleness", "codebase", codebaseName) + } + } + + log.Info("Codebase branches staleness check finished", "codebases", len(branchesByCodebase)) +} + +func (c *Checker) checkCodebaseBranches( + ctx context.Context, + codebaseName string, + branches []*codebaseApi.CodebaseBranch, +) error { + log := ctrl.LoggerFrom(ctx).WithValues("codebase", codebaseName) + + codebase := &codebaseApi.Codebase{} + if err := c.client.Get(ctx, client.ObjectKey{Namespace: c.namespace, Name: codebaseName}, codebase); err != nil { + return fmt.Errorf("failed to get codebase %s: %w", codebaseName, err) + } + + gitServer := &codebaseApi.GitServer{} + if err := c.client.Get( + ctx, client.ObjectKey{Namespace: c.namespace, Name: codebase.Spec.GitServer}, gitServer, + ); err != nil { + return fmt.Errorf("failed to get git server %s: %w", codebase.Spec.GitServer, err) + } + + secret := &corev1.Secret{} + if err := c.client.Get( + ctx, client.ObjectKey{Namespace: c.namespace, Name: gitServer.Spec.NameSshKeySecret}, secret, + ); err != nil { + return fmt.Errorf("failed to get secret %s: %w", gitServer.Spec.NameSshKeySecret, err) + } + + action := c.markAction + if codebase.Annotations[codebaseApi.BranchCleanupStrategyAnnotation] == codebaseApi.BranchCleanupStrategyAuto { + action = c.cleanupAction + } + + repoURL := util.GetProjectGitUrl(gitServer, secret, codebase.Spec.GetProjectID()) + + // A failed listing means the repository state is unknown; branches are marked stale + // only on a successful listing that lacks them, never on connectivity/auth errors. + remoteBranches, err := c.gitClientFactory(gitServer, secret).ListRemoteBranches(ctx, repoURL) + if err != nil { + return fmt.Errorf("failed to list remote branches for %s, skipping staleness check: %w", repoURL, err) + } + + existsInGit := make(map[string]struct{}, len(remoteBranches)) + for _, name := range remoteBranches { + existsInGit[name] = struct{}{} + } + + for _, branch := range branches { + if !c.eligibleForCheck(branch, codebase) { + continue + } + + _, exists := existsInGit[branch.Spec.BranchName] + + if err := action.Apply(ctx, branch, Verdict{ExistsInGit: exists}); err != nil { + log.Error(err, "Failed to apply staleness verdict", "branch", branch.Name) + } + } + + return nil +} + +// eligibleForCheck filters out branches whose absence in git is expected or must not be acted on: +// branches not yet pushed by the operator, branches being deleted, and the codebase default +// branch (its disappearance almost always means repository migration/rename, not branch cleanup). +func (c *Checker) eligibleForCheck(branch *codebaseApi.CodebaseBranch, codebase *codebaseApi.Codebase) bool { + if branch.DeletionTimestamp != nil { + return false + } + + if branch.Status.Git != codebaseApi.CodebaseBranchGitStatusBranchCreated { + return false + } + + return branch.Spec.BranchName != codebase.Spec.DefaultBranch +} diff --git a/controllers/codebasebranch/stalecheck/checker_test.go b/controllers/codebasebranch/stalecheck/checker_test.go new file mode 100644 index 00000000..2a78cff3 --- /dev/null +++ b/controllers/codebasebranch/stalecheck/checker_test.go @@ -0,0 +1,256 @@ +package stalecheck + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git" + gitmocks "github.com/epam/edp-codebase-operator/v2/pkg/git/mocks" +) + +const testNamespace = "default" + +func newScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, codebaseApi.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + return scheme +} + +func newCodebase() *codebaseApi.Codebase { + return &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app", + Namespace: testNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + GitServer: "gitlab", + GitUrlPath: "/owner/app", + DefaultBranch: "main", + }, + } +} + +func newBranch(name, branchName, gitStatus string) *codebaseApi.CodebaseBranch { + return &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testNamespace, + }, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: "app", + BranchName: branchName, + }, + Status: codebaseApi.CodebaseBranchStatus{ + Git: gitStatus, + }, + } +} + +func newGitServerWithSecret() (*codebaseApi.GitServer, *corev1.Secret) { + gitServer := &codebaseApi.GitServer{ + ObjectMeta: metav1.ObjectMeta{Name: "gitlab", Namespace: testNamespace}, + Spec: codebaseApi.GitServerSpec{ + GitHost: "gitlab.example.com", + GitProvider: codebaseApi.GitProviderGitlab, + NameSshKeySecret: "gitlab-secret", + }, + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "gitlab-secret", Namespace: testNamespace}, + Data: map[string][]byte{"token": []byte("token"), "username": []byte("user")}, + } + + return gitServer, secret +} + +func newChecker( + t *testing.T, + k8sClient client.Client, + gitClient gitproviderv2.Git, + recorder record.EventRecorder, +) *Checker { + t.Helper() + + factory := func(_ *codebaseApi.GitServer, _ *corev1.Secret) gitproviderv2.Git { + return gitClient + } + + mark := NewMarkAction(k8sClient, recorder) + + return NewChecker(k8sClient, testNamespace, 0, factory, mark, NewCleanupAction(k8sClient, recorder, mark)) +} + +func getBranch(t *testing.T, k8sClient client.Client, name string) *codebaseApi.CodebaseBranch { + t.Helper() + + branch := &codebaseApi.CodebaseBranch{} + require.NoError(t, k8sClient.Get(context.Background(), client.ObjectKey{Namespace: testNamespace, Name: name}, branch)) + + return branch +} + +func TestChecker_MarksMissingBranchStale(t *testing.T) { + codebase := newCodebase() + mainBranch := newBranch("app-main", "main", codebaseApi.CodebaseBranchGitStatusBranchCreated) + featureBranch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + gitServer, secret := newGitServerWithSecret() + + k8sClient := fake.NewClientBuilder(). + WithScheme(newScheme(t)). + WithObjects(codebase, mainBranch, featureBranch, gitServer, secret). + WithStatusSubresource(mainBranch, 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()) + + stale := getBranch(t, k8sClient, "app-feature") + assert.True(t, meta.IsStatusConditionTrue(stale.Status.Conditions, codebaseApi.ConditionStale)) + assert.Equal(t, "true", stale.Labels[codebaseApi.StaleLabel]) + + condition := meta.FindStatusCondition(stale.Status.Conditions, codebaseApi.ConditionStale) + require.NotNil(t, condition) + assert.Equal(t, codebaseApi.ReasonBranchNotFoundInGit, condition.Reason) + + select { + case event := <-recorder.Events: + assert.Contains(t, event, EventReasonBranchStale) + default: + t.Fatal("expected BranchStale event") + } +} + +func TestChecker_ClearsStaleMarkWhenBranchReappears(t *testing.T) { + codebase := newCodebase() + + branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + branch.Labels = map[string]string{codebaseApi.StaleLabel: "true"} + branch.Status.Conditions = []metav1.Condition{{ + Type: codebaseApi.ConditionStale, + Status: metav1.ConditionTrue, + Reason: codebaseApi.ReasonBranchNotFoundInGit, + LastTransitionTime: metav1.Now(), + }} + + gitServer, secret := newGitServerWithSecret() + + k8sClient := fake.NewClientBuilder(). + WithScheme(newScheme(t)). + WithObjects(codebase, branch, gitServer, secret). + WithStatusSubresource(branch). + Build() + + gitClient := gitmocks.NewMockGit(t) + gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return([]string{"main", "feature"}, nil) + + recorder := record.NewFakeRecorder(10) + + newChecker(t, k8sClient, gitClient, recorder).sweep(context.Background()) + + updated := getBranch(t, k8sClient, "app-feature") + assert.False(t, meta.IsStatusConditionTrue(updated.Status.Conditions, codebaseApi.ConditionStale)) + assert.NotContains(t, updated.Labels, codebaseApi.StaleLabel) + + select { + case event := <-recorder.Events: + assert.Contains(t, event, EventReasonBranchStaleResolved) + default: + t.Fatal("expected BranchStaleResolved event") + } +} + +func TestChecker_DoesNotMarkOnGitError(t *testing.T) { + codebase := newCodebase() + branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + gitServer, secret := newGitServerWithSecret() + + k8sClient := fake.NewClientBuilder(). + WithScheme(newScheme(t)). + WithObjects(codebase, branch, gitServer, secret). + WithStatusSubresource(branch). + Build() + + gitClient := gitmocks.NewMockGit(t) + gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return(nil, assert.AnError) + + newChecker(t, k8sClient, gitClient, record.NewFakeRecorder(10)).sweep(context.Background()) + + updated := getBranch(t, k8sClient, "app-feature") + assert.Empty(t, updated.Status.Conditions) + assert.NotContains(t, updated.Labels, codebaseApi.StaleLabel) +} + +func TestChecker_SkipsDefaultAndUnpushedBranches(t *testing.T) { + codebase := newCodebase() + defaultBranch := newBranch("app-main", "main", codebaseApi.CodebaseBranchGitStatusBranchCreated) + unpushedBranch := newBranch("app-new", "new", "") + gitServer, secret := newGitServerWithSecret() + + k8sClient := fake.NewClientBuilder(). + WithScheme(newScheme(t)). + WithObjects(codebase, defaultBranch, unpushedBranch, gitServer, secret). + WithStatusSubresource(defaultBranch, unpushedBranch). + Build() + + // Remote listing is empty: both branches are missing in git, + // but neither is eligible for marking. + gitClient := gitmocks.NewMockGit(t) + gitClient.On("ListRemoteBranches", mock.Anything, mock.Anything).Return([]string{}, nil) + + newChecker(t, k8sClient, gitClient, record.NewFakeRecorder(10)).sweep(context.Background()) + + for _, name := range []string{"app-main", "app-new"} { + updated := getBranch(t, k8sClient, name) + assert.Empty(t, updated.Status.Conditions, name) + assert.NotContains(t, updated.Labels, codebaseApi.StaleLabel, name) + } +} + +func TestMarkAction_IdempotentWhenAlreadyStale(t *testing.T) { + branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + branch.Labels = map[string]string{codebaseApi.StaleLabel: "true"} + branch.Status.Conditions = []metav1.Condition{{ + Type: codebaseApi.ConditionStale, + Status: metav1.ConditionTrue, + Reason: codebaseApi.ReasonBranchNotFoundInGit, + Message: "Branch was not found in the git repository", + LastTransitionTime: metav1.Now(), + }} + + k8sClient := fake.NewClientBuilder(). + WithScheme(newScheme(t)). + WithObjects(branch). + WithStatusSubresource(branch). + Build() + + recorder := record.NewFakeRecorder(10) + action := NewMarkAction(k8sClient, recorder) + + require.NoError(t, action.Apply(context.Background(), branch, Verdict{ExistsInGit: false})) + + select { + case event := <-recorder.Events: + t.Fatalf("expected no events for already-stale branch, got %s", event) + default: + } +} diff --git a/controllers/codebasebranch/stalecheck/cleanup_action.go b/controllers/codebasebranch/stalecheck/cleanup_action.go new file mode 100644 index 00000000..2daf28c2 --- /dev/null +++ b/controllers/codebasebranch/stalecheck/cleanup_action.go @@ -0,0 +1,60 @@ +package stalecheck + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "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 +// participate in any deployment are deleted (the owned CodebaseImageStream is +// garbage-collected via its owner reference); branches referenced by a CDPipeline +// or Stage fall back to being marked, with the retaining resource recorded in the +// Stale condition message. +type CleanupAction struct { + client client.Client + recorder record.EventRecorder + mark *MarkAction +} + +func NewCleanupAction(k8sClient client.Client, recorder record.EventRecorder, mark *MarkAction) *CleanupAction { + return &CleanupAction{client: k8sClient, recorder: recorder, mark: mark} +} + +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 + + return a.mark.Apply(ctx, branch, verdict) + } + + if a.recorder != nil { + a.recorder.Eventf(branch, corev1.EventTypeNormal, EventReasonStaleBranchDeleted, + "Branch %s was not found in the git repository and is not used by any deployment, deleting", branch.Spec.BranchName) + } + + if err := a.client.Delete(ctx, branch); err != nil { + return fmt.Errorf("failed to delete stale CodebaseBranch %s: %w", branch.Name, err) + } + + ctrl.LoggerFrom(ctx).Info("Deleted stale codebase branch", + "codebasebranch", branch.Name, "branch", branch.Spec.BranchName) + + return nil +} diff --git a/controllers/codebasebranch/stalecheck/cleanup_action_test.go b/controllers/codebasebranch/stalecheck/cleanup_action_test.go new file mode 100644 index 00000000..163703ac --- /dev/null +++ b/controllers/codebasebranch/stalecheck/cleanup_action_test.go @@ -0,0 +1,109 @@ +package stalecheck + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/tools/record" + "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" +) + +func TestCleanupAction_DeletesUnusedStaleBranch(t *testing.T) { + branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + + scheme := newScheme(t) + require.NoError(t, pipelineApi.AddToScheme(scheme)) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + 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})) + + err := k8sClient.Get(context.Background(), + client.ObjectKey{Namespace: testNamespace, Name: "app-feature"}, &codebaseApi.CodebaseBranch{}) + assert.True(t, errors.IsNotFound(err), "stale branch must be deleted") + + select { + case event := <-recorder.Events: + assert.Contains(t, event, EventReasonStaleBranchDeleted) + default: + t.Fatal("expected StaleBranchDeleted event") + } +} + +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). + 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})) + + retained := getBranch(t, k8sClient, "app-feature") + assert.True(t, meta.IsStatusConditionTrue(retained.Status.Conditions, codebaseApi.ConditionStale)) + assert.Equal(t, "true", retained.Labels[codebaseApi.StaleLabel]) + + 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 TestCleanupAction_ClearsMarkWhenBranchExists(t *testing.T) { + branch := newBranch("app-feature", "feature", codebaseApi.CodebaseBranchGitStatusBranchCreated) + branch.Labels = map[string]string{codebaseApi.StaleLabel: "true"} + + scheme := newScheme(t) + require.NoError(t, pipelineApi.AddToScheme(scheme)) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + 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: true})) + + updated := getBranch(t, k8sClient, "app-feature") + assert.NotContains(t, updated.Labels, codebaseApi.StaleLabel) + assert.False(t, meta.IsStatusConditionTrue(updated.Status.Conditions, codebaseApi.ConditionStale)) +} diff --git a/deploy-templates/README.md b/deploy-templates/README.md index a76e4336..008756eb 100644 --- a/deploy-templates/README.md +++ b/deploy-templates/README.md @@ -23,6 +23,7 @@ A Helm chart for KubeRocketCI Codebase Operator |-----|------|---------|-------------| | affinity | object | `{}` | | | annotations | object | `{}` | | +| branchStaleCheckInterval | string | `"24h"` | How often the operator verifies that codebase branches still exist in git, marking missing ones with the Stale condition and the app.edp.epam.com/stale label. Accepts Go duration strings (e.g. 24h, 30m); "0" disables the check. | | enableWebhooks | bool | `true` | Enable webhook and cert-manager certificate resources. Webhooks require cert-manager to be installed in the cluster. | | envs[0].name | string | `"RECONCILATION_PERIOD"` | | | envs[0].value | string | `"360"` | | diff --git a/deploy-templates/crds/v2.edp.epam.com_codebasebranches.yaml b/deploy-templates/crds/v2.edp.epam.com_codebasebranches.yaml index e410a038..68368ac0 100644 --- a/deploy-templates/crds/v2.edp.epam.com_codebasebranches.yaml +++ b/deploy-templates/crds/v2.edp.epam.com_codebasebranches.yaml @@ -37,6 +37,10 @@ spec: jsonPath: .spec.branchName name: Branch type: string + - description: Branch is missing in git + jsonPath: .status.conditions[?(@.type=="Stale")].status + name: Stale + type: string name: v1 schema: openAPIV3Schema: @@ -109,6 +113,70 @@ spec: build: nullable: true type: string + conditions: + description: |- + Conditions represent the latest available observations of an object's state. + Each condition type is an independent observation (e.g. Stale); new types can be + added over time without schema changes. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + nullable: true + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map detailedMessage: description: |- Detailed information regarding action result diff --git a/deploy-templates/templates/deployment.yaml b/deploy-templates/templates/deployment.yaml index 1633e1de..08a903d6 100644 --- a/deploy-templates/templates/deployment.yaml +++ b/deploy-templates/templates/deployment.yaml @@ -73,6 +73,8 @@ spec: value: "{{ .Values.telemetryEnabled }}" - name: ENABLE_WEBHOOKS value: {{ .Values.enableWebhooks | quote }} + - name: BRANCH_STALE_CHECK_INTERVAL + value: {{ .Values.branchStaleCheckInterval | quote }} {{ toYaml .Values.envs | indent 12 }} resources: {{ toYaml .Values.resources | indent 12 }} diff --git a/deploy-templates/values.yaml b/deploy-templates/values.yaml index f2092f6a..100eeec0 100644 --- a/deploy-templates/values.yaml +++ b/deploy-templates/values.yaml @@ -83,3 +83,8 @@ telemetryEnabled: true # -- Enable webhook and cert-manager certificate resources. # Webhooks require cert-manager to be installed in the cluster. enableWebhooks: true + +# -- How often the operator verifies that codebase branches still exist in git, +# marking missing ones with the Stale condition and the app.edp.epam.com/stale label. +# Accepts Go duration strings (e.g. 24h, 30m); "0" disables the check. +branchStaleCheckInterval: 24h diff --git a/docs/api.md b/docs/api.md index 848e5831..8f17829c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -460,6 +460,15 @@ CodebaseBranchStatus defines the observed state of CodebaseBranch.
false + + conditions + []object + + Conditions represent the latest available observations of an object's state. +Each condition type is an independent observation (e.g. Stale); new types can be +added over time without schema changes.
+ + false detailedMessage string @@ -492,6 +501,83 @@ which were performed
+ +### CodebaseBranch.status.conditions[index] +[↩ Parent](#codebasebranchstatus) + + + +Condition contains details for one aspect of the current state of this API Resource. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
lastTransitionTimestring + lastTransitionTime is the last time the condition transitioned from one status to another. +This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
+
+ Format: date-time
+ 		true
messagestring + message is a human readable message indicating details about the transition. +This may be an empty string.
+ 		true
reasonstring + reason contains a programmatic identifier indicating the reason for the condition's last transition. +Producers of specific condition types may define expected values and meanings for this field, +and whether the values are considered a guaranteed API. +The value should be a CamelCase string. +This field may not be empty.
+ 		true
statusenum + status of the condition, one of True, False, Unknown.
+
+ Enum: True, False, Unknown
+ 		true
typestring + type of condition in CamelCase or in foo.example.com/CamelCase.
+ 		true
observedGenerationinteger + observedGeneration represents the .metadata.generation that the condition was set based upon. +For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date +with respect to the current state of the instance.
+
+ Format: int64
+ Minimum: 0
+ 		false
+ ## CodebaseImageStream [↩ Parent](#v2edpepamcomv1 ) diff --git a/pkg/codebasebranch/usage.go b/pkg/codebasebranch/usage.go new file mode 100644 index 00000000..3abb7497 --- /dev/null +++ b/pkg/codebasebranch/usage.go @@ -0,0 +1,79 @@ +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" +) + +// 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 + } + + return "", fmt.Errorf("failed to list CDPipelines: %w", err) + } + + for i := range pipelines.Items { + pipeline := &pipelines.Items[i] + + if pipeline.DeletionTimestamp != nil { + continue + } + + if slices.Contains(pipeline.Spec.InputDockerStreams, branch.Name) { + return fmt.Sprintf("CDPipeline %s (inputDockerStreams)", pipeline.Name), nil + } + } + + stages := &pipelineApi.StageList{} + if err := c.List(ctx, stages, client.InNamespace(branch.Namespace)); err != nil { + if isKindUnavailable(err) { + return "", nil + } + + return "", fmt.Errorf("failed to list Stages: %w", err) + } + + for i := range stages.Items { + stage := &stages.Items[i] + + if stage.DeletionTimestamp != nil { + 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 + } + } + } + + return "", nil +} + +func isKindUnavailable(err error) bool { + return apimeta.IsNoMatchError(err) || runtime.IsNotRegisteredError(err) +} diff --git a/pkg/codebasebranch/usage_test.go b/pkg/codebasebranch/usage_test.go new file mode 100644 index 00000000..4f1f2ca1 --- /dev/null +++ b/pkg/codebasebranch/usage_test.go @@ -0,0 +1,120 @@ +package codebasebranch + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "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" +) + +func usageScheme(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 +} + +func usageBranch() *codebaseApi.CodebaseBranch { + return &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-feature", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: "app", + BranchName: "feature", + }, + } +} + +func TestFindBranchUsage_InputDockerStreams(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + InputDockerStreams: []string{"other-main", "app-feature"}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() + + usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Equal(t, "CDPipeline demo (inputDockerStreams)", usage) +} + +func TestFindBranchUsage_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("feature"), + }}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(stage).Build() + + usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Equal(t, "Stage demo-dev of CDPipeline demo (autotest quality gate)", usage) +} + +func TestFindBranchUsage_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{ + InputDockerStreams: []string{"app-feature"}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() + + usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Empty(t, usage) +} + +func TestFindBranchUsage_Unused(t *testing.T) { + pipeline := &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{ + InputDockerStreams: []string{"other-main"}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(usageScheme(t, true)).WithObjects(pipeline).Build() + + usage, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) + require.NoError(t, err) + assert.Empty(t, usage) +} + +func TestFindBranchUsage_PipelineAPINotInstalled(t *testing.T) { + // CDPipeline/Stage kinds are absent from the scheme, emulating a cluster + // 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()) + require.NoError(t, err) + assert.Empty(t, usage) +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 29be0276..820d3f15 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -29,6 +29,10 @@ type Git interface { // CheckPermissions checks if the repository is accessible with current credentials. CheckPermissions(ctx context.Context, repoURL string) error + // ListRemoteBranches lists branch names that exist in the remote repository + // without cloning it (equivalent to git ls-remote --heads). + ListRemoteBranches(ctx context.Context, repoURL string) ([]string, error) + // CheckReference checks if a reference (branch or commit) exists in the repository. CheckReference(ctx context.Context, directory, refName string) error diff --git a/pkg/git/mocks/git_generated.mock.go b/pkg/git/mocks/git_generated.mock.go index 31ce4fb7..b03d6c6f 100644 --- a/pkg/git/mocks/git_generated.mock.go +++ b/pkg/git/mocks/git_generated.mock.go @@ -963,6 +963,74 @@ func (_c *MockGit_Init_Call) RunAndReturn(run func(ctx context.Context, director return _c } +// ListRemoteBranches provides a mock function for the type MockGit +func (_mock *MockGit) ListRemoteBranches(ctx context.Context, repoURL string) ([]string, error) { + ret := _mock.Called(ctx, repoURL) + + if len(ret) == 0 { + panic("no return value specified for ListRemoteBranches") + } + + var r0 []string + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string) ([]string, error)); ok { + return returnFunc(ctx, repoURL) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, string) []string); ok { + r0 = returnFunc(ctx, repoURL) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = returnFunc(ctx, repoURL) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// MockGit_ListRemoteBranches_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListRemoteBranches' +type MockGit_ListRemoteBranches_Call struct { + *mock.Call +} + +// ListRemoteBranches is a helper method to define mock.On call +// - ctx context.Context +// - repoURL string +func (_e *MockGit_Expecter) ListRemoteBranches(ctx interface{}, repoURL interface{}) *MockGit_ListRemoteBranches_Call { + return &MockGit_ListRemoteBranches_Call{Call: _e.mock.On("ListRemoteBranches", ctx, repoURL)} +} + +func (_c *MockGit_ListRemoteBranches_Call) Run(run func(ctx context.Context, repoURL string)) *MockGit_ListRemoteBranches_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *MockGit_ListRemoteBranches_Call) Return(strings []string, err error) *MockGit_ListRemoteBranches_Call { + _c.Call.Return(strings, err) + return _c +} + +func (_c *MockGit_ListRemoteBranches_Call) RunAndReturn(run func(ctx context.Context, repoURL string) ([]string, error)) *MockGit_ListRemoteBranches_Call { + _c.Call.Return(run) + return _c +} + // Push provides a mock function for the type MockGit func (_mock *MockGit) Push(ctx context.Context, directory string, refspecs ...string) error { // string diff --git a/pkg/git/provider.go b/pkg/git/provider.go index 1ce48f20..f5743b8c 100644 --- a/pkg/git/provider.go +++ b/pkg/git/provider.go @@ -431,6 +431,52 @@ func (p *GitProvider) GetCurrentBranchName(ctx context.Context, directory string return branchName, nil } +// ListRemoteBranches lists branch names that exist in the remote repository +// without cloning it (equivalent to git ls-remote --heads). +func (p *GitProvider) ListRemoteBranches(ctx context.Context, repoURL string) ([]string, error) { + log := ctrl.LoggerFrom(ctx).WithValues("repository", repoURL) + log.Info("Listing remote branches") + + auth, err := p.getAuth() + if err != nil { + return nil, fmt.Errorf("failed to get authentication: %w", err) + } + + repo, _ := git.Init(memory.NewStorage(), nil) + + remote, err := repo.CreateRemote(&config.RemoteConfig{ + Name: "origin", + URLs: []string{repoURL}, + }) + if err != nil { + return nil, fmt.Errorf("failed to create remote: %w", err) + } + + refs, err := remote.ListContext(ctx, &git.ListOptions{ + Auth: auth, + }) + if err != nil { + if errors.Is(err, transport.ErrEmptyRemoteRepository) { + log.Info("Repository is empty, no branches found") + return nil, nil + } + + return nil, fmt.Errorf("failed to list remote references: %w", err) + } + + branches := make([]string, 0, len(refs)) + + for _, ref := range refs { + if ref.Name().IsBranch() { + branches = append(branches, ref.Name().Short()) + } + } + + log.Info("Remote branches listed successfully", "count", len(branches)) + + return branches, nil +} + // CheckPermissions checks if the repository is accessible with current credentials. func (p *GitProvider) CheckPermissions(ctx context.Context, repoURL string) error { log := ctrl.LoggerFrom(ctx).WithValues("repository", repoURL) diff --git a/pkg/git/provider_test.go b/pkg/git/provider_test.go index bc9595e2..8dc52b25 100644 --- a/pkg/git/provider_test.go +++ b/pkg/git/provider_test.go @@ -74,6 +74,53 @@ func TestGitProvider_CheckPermissions_NoRefs(t *testing.T) { require.NoError(t, err, "v2 considers empty repos accessible") } +func TestGitProvider_ListRemoteBranches(t *testing.T) { + config := Config{ + Username: "user", + Token: "pass", + } + gp := NewGitProvider(config) + + // Same canned smart-HTTP ref advertisement as TestGitProvider_CheckPermissions: + // refs/heads/branch, refs/heads/master plus pull-request refs that must be filtered out. + bts, err := base64.StdEncoding.DecodeString(`MDAxZSMgc2VydmljZT1naXQtdXBsb2FkLXBhY2sKMDAwMDAxNTY2ZWNmMGVmMmMyZGZmYjc5NjAzM2U1YTAyMjE5YWY4NmVjNjU4NGU1IEhFQUQAbXVsdGlfYWNrIHRoaW4tcGFjayBzaWRlLWJhbmQgc2lkZS1iYW5kLTY0ayBvZnMtZGVsdGEgc2hhbGxvdyBkZWVwZW4tc2luY2UgZGVlcGVuLW5vdCBkZWVwZW4tcmVsYXRpdmUgbm8tcHJvZ3Jlc3MgaW5jbHVkZS10YWcgbXVsdGlfYWNrX2RldGFpbGVkIGFsbG93LXRpcC1zaGExLWluLXdhbnQgYWxsb3ctcmVhY2hhYmxlLXNoYTEtaW4td2FudCBuby1kb25lIHN5bXJlZj1IRUFEOnJlZnMvaGVhZHMvbWFzdGVyIGZpbHRlciBvYmplY3QtZm9ybWF0PXNoYTEgYWdlbnQ9Z2l0L2dpdGh1Yi1nNzhiNDUyNDEzZThiCjAwM2ZlOGQzZmZhYjU1Mjg5NWMxOWI5ZmNmN2FhMjY0ZDI3N2NkZTMzODgxIHJlZnMvaGVhZHMvYnJhbmNoCjAwM2Y2ZWNmMGVmMmMyZGZmYjc5NjAzM2U1YTAyMjE5YWY4NmVjNjU4NGU1IHJlZnMvaGVhZHMvbWFzdGVyCjAwM2ViOGU0NzFmNThiY2JjYTYzYjA3YmRhMjBlNDI4MTkwNDA5YzJkYjQ3IHJlZnMvcHVsbC8xL2hlYWQKMDAzZTk2MzJmMDI4MzNiMmY5NjEzYWZiNWU3NTY4MjEzMmIwYjIyZTRhMzEgcmVmcy9wdWxsLzIvaGVhZAowMDNmYzM3ZjU4YTEzMGNhNTU1ZTQyZmY5NmEwNzFjYjljY2IzZjQzNzUwNCByZWZzL3B1bGwvMi9tZXJnZQowMDAw`) // nolint:lll + require.NoError(t, err) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, err = w.Write(bts) + assert.NoError(t, err, "failed to write response") + })) + defer s.Close() + + branches, err := gp.ListRemoteBranches(context.Background(), s.URL) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"branch", "master"}, branches) +} + +func TestGitProvider_ListRemoteBranches_EmptyRepo(t *testing.T) { + config := Config{ + Username: "user", + Token: "pass", + } + gp := NewGitProvider(config) + + // Same empty-repo advertisement as TestGitProvider_CheckPermissions_NoRefs. + bts, err := base64.StdEncoding.DecodeString(`MDAxZSMgc2VydmljZT1naXQtdXBsb2FkLXBhY2sKMDAwMDAwZGUwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwIGNhcGFiaWxpdGllc157fQAgaW5jbHVkZS10YWcgbXVsdGlfYWNrX2RldGFpbGVkIG11bHRpX2FjayBvZnMtZGVsdGEgc2lkZS1iYW5kIHNpZGUtYmFuZC02NGsgdGhpbi1wYWNrIG5vLXByb2dyZXNzIHNoYWxsb3cgbm8tZG9uZSBhZ2VudD1KR2l0L3Y1LjkuMC4yMDIwMDkwODA1MDEtci00MS1nNWQ5MjVlY2JiCjAwMDA=`) // nolint:lll + require.NoError(t, err) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, err = w.Write(bts) + assert.NoError(t, err, "failed to write response") + })) + defer s.Close() + + branches, err := gp.ListRemoteBranches(context.Background(), s.URL) + require.NoError(t, err) + assert.Empty(t, branches) +} + func TestGitProvider_CreateChildBranch(t *testing.T) { tests := []struct { name string diff --git a/pkg/webhook/codebasebranch_webhook.go b/pkg/webhook/codebasebranch_webhook.go index f0898f05..0cdc1f62 100644 --- a/pkg/webhook/codebasebranch_webhook.go +++ b/pkg/webhook/codebasebranch_webhook.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -96,5 +97,42 @@ func (r *CodebaseBranchValidationWebhook) ValidateDelete( return nil, err } + deletedCodebaseBranch, ok := obj.(*v1.CodebaseBranch) + if !ok { + r.log.Info("The wrong object given, skipping validation") + + return nil, nil + } + + // When the owning Codebase is gone or terminating, the branch is being removed as part + // of a cascade delete; blocking it would leave garbage-collected branches stuck forever. + codebase := &v1.Codebase{} + if err = r.client.Get( + ctx, + client.ObjectKey{Namespace: deletedCodebaseBranch.Namespace, Name: deletedCodebaseBranch.Spec.CodebaseName}, + codebase, + ); err != nil { + if apierrors.IsNotFound(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to get codebase %s: %w", deletedCodebaseBranch.Spec.CodebaseName, err) + } + + if codebase.DeletionTimestamp != nil { + return nil, nil + } + + usage, 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) + } + return nil, nil } diff --git a/pkg/webhook/codebasebranch_webhook_delete_test.go b/pkg/webhook/codebasebranch_webhook_delete_test.go new file mode 100644 index 00000000..14de077f --- /dev/null +++ b/pkg/webhook/codebasebranch_webhook_delete_test.go @@ -0,0 +1,120 @@ +package webhook + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "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" +) + +func deleteWebhookScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, codebaseApi.AddToScheme(scheme)) + require.NoError(t, pipelineApi.AddToScheme(scheme)) + + return scheme +} + +func deleteWebhookBranch() *codebaseApi.CodebaseBranch { + return &codebaseApi.CodebaseBranch{ + ObjectMeta: metav1.ObjectMeta{Name: "app-feature", Namespace: "default"}, + Spec: codebaseApi.CodebaseBranchSpec{ + CodebaseName: "app", + BranchName: "feature", + }, + } +} + +func TestCodebaseBranchValidationWebhook_ValidateDelete_BranchInUse(t *testing.T) { + tests := []struct { + name string + objects []runtime.Object + wantErr string + }{ + { + name: "rejects deletion of branch used by CDPipeline", + 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"}}, + }, + }, + wantErr: "used by CDPipeline demo", + }, + { + name: "allows deletion of unused branch", + 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{"other-main"}}, + }, + }, + }, + { + name: "allows deletion when owning codebase is terminating", + objects: []runtime.Object{ + &codebaseApi.Codebase{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app", + Namespace: "default", + DeletionTimestamp: ptr.To(metav1.Now()), + Finalizers: []string{"keep"}, + }, + }, + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{InputDockerStreams: []string{"app-feature"}}, + }, + }, + }, + { + name: "allows deletion when owning codebase is gone", + objects: []runtime.Object{ + &pipelineApi.CDPipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: pipelineApi.CDPipelineSpec{InputDockerStreams: []string{"app-feature"}}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8sClient := fake.NewClientBuilder(). + WithScheme(deleteWebhookScheme(t)). + WithRuntimeObjects(tt.objects...). + Build() + + w := NewCodebaseBranchValidationWebhook(k8sClient, ctrl.Log) + + _, err := w.ValidateDelete(context.Background(), deleteWebhookBranch()) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + + return + } + + require.NoError(t, err) + }) + } +}