diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go index ef8051a0c9..5afd90fb11 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go @@ -3495,3 +3495,102 @@ func TestCreateJob_AlreadyExistsIsIgnored(t *testing.T) { t.Fatal("createJob should return false when job already exists") } } + +// Test_PropagateAcceptedFailureToReady_SetsReadyFalse verifies that when the +// Accepted condition is False, propagateAcceptedFailureToReady forces Ready to +// False with the same message and reason. This is the core fix for +// https://github.com/rancher/fleet/issues/4865. +func Test_PropagateAcceptedFailureToReady_SetsReadyFalse(t *testing.T) { + gitrepo := &fleetv1.GitRepo{} + gitrepo.Status.Conditions = []genericcondition.GenericCondition{ + { + Type: fleetv1.GitRepoAcceptedCondition, + Status: "False", + Message: "failed to look up HelmSecretNameForPaths, error: secret not found", + Reason: "Error", + }, + { + Type: "Ready", + Status: "True", + }, + } + + propagateAcceptedFailureToReady(gitrepo) + + readyCond, found := getCondition(gitrepo, "Ready") + if !found { + t.Fatal("expected Ready condition to be present") + } + if readyCond.Status != "False" { + t.Errorf("expected Ready=False, got Ready=%s", readyCond.Status) + } + if readyCond.Message != "failed to look up HelmSecretNameForPaths, error: secret not found" { + t.Errorf("unexpected Ready message: %s", readyCond.Message) + } +} + +// Test_PropagateAcceptedFailureToReady_AddsReadyWhenMissing verifies that when +// Ready is absent and Accepted=False, a Ready=False condition is added. +func Test_PropagateAcceptedFailureToReady_AddsReadyWhenMissing(t *testing.T) { + gitrepo := &fleetv1.GitRepo{} + gitrepo.Status.Conditions = []genericcondition.GenericCondition{ + { + Type: fleetv1.GitRepoAcceptedCondition, + Status: "False", + Message: "missing cabundle secret", + Reason: "Error", + }, + } + + propagateAcceptedFailureToReady(gitrepo) + + readyCond, found := getCondition(gitrepo, "Ready") + if !found { + t.Fatal("expected Ready condition to be added") + } + if readyCond.Status != "False" { + t.Errorf("expected Ready=False, got Ready=%s", readyCond.Status) + } + if readyCond.Message != "missing cabundle secret" { + t.Errorf("unexpected Ready message: %s", readyCond.Message) + } +} + +// Test_PropagateAcceptedFailureToReady_NoOpWhenAcceptedTrue verifies that when +// Accepted=True, the Ready condition is not changed by propagateAcceptedFailureToReady. +func Test_PropagateAcceptedFailureToReady_NoOpWhenAcceptedTrue(t *testing.T) { + gitrepo := &fleetv1.GitRepo{} + gitrepo.Status.Conditions = []genericcondition.GenericCondition{ + { + Type: fleetv1.GitRepoAcceptedCondition, + Status: "True", + }, + { + Type: "Ready", + Status: "True", + }, + } + + propagateAcceptedFailureToReady(gitrepo) + + readyCond, found := getCondition(gitrepo, "Ready") + if !found { + t.Fatal("expected Ready condition to be present") + } + if readyCond.Status != "True" { + t.Errorf("expected Ready=True (unchanged), got Ready=%s", readyCond.Status) + } +} + +// Test_PropagateAcceptedFailureToReady_NoOpWhenNoAccepted verifies that when +// no Accepted condition is present, propagateAcceptedFailureToReady is a no-op. +func Test_PropagateAcceptedFailureToReady_NoOpWhenNoAccepted(t *testing.T) { + gitrepo := &fleetv1.GitRepo{} + // No Accepted condition, no Ready condition - simulates fresh GitRepo with 0/0 bundles. + propagateAcceptedFailureToReady(gitrepo) + + _, found := getCondition(gitrepo, "Ready") + if found { + t.Error("expected no Ready condition to be added when no Accepted condition is present") + } +} diff --git a/internal/cmd/controller/gitops/reconciler/status_controller.go b/internal/cmd/controller/gitops/reconciler/status_controller.go index 21b6580d32..4d7099f237 100644 --- a/internal/cmd/controller/gitops/reconciler/status_controller.go +++ b/internal/cmd/controller/gitops/reconciler/status_controller.go @@ -145,6 +145,12 @@ func (r *StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } + // If the Accepted condition is False (e.g. missing secret, cabundle failure, + // restriction violation), propagate that failure to the Ready condition. + // Without this, a GitRepo with no bundles (because the job never ran) would + // report Ready=True because the empty bundle summary satisfies 0/0 == ready. + propagateAcceptedFailureToReady(gitrepo) + if err := r.updateStatus(ctx, orig, gitrepo); err != nil { logger.Error(err, "Reconcile failed update to git repo status", "status", gitrepo.Status) return ctrl.Result{RequeueAfter: durations.GitRepoStatusDelay}, nil @@ -247,6 +253,49 @@ bundles: return nil } +// propagateAcceptedFailureToReady ensures the Ready condition reflects an +// Accepted=False error. When a pre-job error (missing secret, cabundle failure, +// restriction violation) prevents any bundle from being created, the bundle +// summary is 0/0, which would normally resolve to Ready=True. By copying the +// Accepted condition's message onto Ready we surface the real error. +func propagateAcceptedFailureToReady(gitrepo *fleet.GitRepo) { + var acceptedMsg string + var acceptedReason string + acceptedFalse := false + for _, c := range gitrepo.Status.Conditions { + if c.Type == fleet.GitRepoAcceptedCondition && c.Status == v1.ConditionFalse { + acceptedFalse = true + acceptedMsg = c.Message + acceptedReason = c.Reason + break + } + } + if !acceptedFalse { + return + } + + found := false + newConditions := make([]genericcondition.GenericCondition, 0, len(gitrepo.Status.Conditions)) + for _, c := range gitrepo.Status.Conditions { + if c.Type == string(fleet.Ready) { + c.Status = v1.ConditionFalse + c.Message = acceptedMsg + c.Reason = acceptedReason + found = true + } + newConditions = append(newConditions, c) + } + if !found { + newConditions = append(newConditions, genericcondition.GenericCondition{ + Type: string(fleet.Ready), + Status: v1.ConditionFalse, + Message: acceptedMsg, + Reason: acceptedReason, + }) + } + gitrepo.Status.Conditions = newConditions +} + type forcedDelayingSource[R comparable] struct { source.TypedSource[R] delay time.Duration