From 8c26aa5dce255b4d51b1f84d456ce33004f6480e Mon Sep 17 00:00:00 2001 From: Piero Rospigliosi Date: Sun, 9 Aug 2026 20:07:54 +0200 Subject: [PATCH] fix: handle archived repositories during collaborator reconciliation --- ...hubrepositorycollaborator_archived_test.go | 142 ++++++++++++++++++ ...githubrepositorycollaborator_controller.go | 76 +++++++++- .../controller/repository_access_helpers.go | 33 +++- 3 files changed, 244 insertions(+), 7 deletions(-) create mode 100644 internal/controller/githubrepositorycollaborator_archived_test.go diff --git a/internal/controller/githubrepositorycollaborator_archived_test.go b/internal/controller/githubrepositorycollaborator_archived_test.go new file mode 100644 index 0000000..a7e60d8 --- /dev/null +++ b/internal/controller/githubrepositorycollaborator_archived_test.go @@ -0,0 +1,142 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + 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/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + githubv1alpha1 "github.com/pierinho13/github-platform-operator/api/v1alpha1" + githubclient "github.com/pierinho13/github-platform-operator/internal/github" +) + +var _ = Describe("GitHubRepositoryCollaborator archived repository handling", func() { + const ( + providerName = "archived-collaborator-provider" + secretName = "archived-collaborator-credentials" + repositoryResourceName = "archived-collaborator-repository" + repositoryName = "archived-platform-worker" + collaboratorName = "archived-platform-worker-octocat" + ) + + ctx := context.Background() + collaboratorKey := types.NamespacedName{Name: collaboratorName, Namespace: testDefaultName} + + BeforeEach(func() { + createRepositoryAccessDependencies( + ctx, + providerName, + secretName, + repositoryResourceName, + repositoryName, + ) + + collaborator := &githubv1alpha1.GitHubRepositoryCollaborator{ + ObjectMeta: metav1.ObjectMeta{Name: collaboratorName, Namespace: testDefaultName}, + Spec: githubv1alpha1.GitHubRepositoryCollaboratorSpec{ + RepositoryRef: githubv1alpha1.GitHubRepositoryReference{Name: repositoryResourceName}, + Username: "octocat", + Permission: githubv1alpha1.RepositoryPermissionPush, + DeletionPolicy: githubv1alpha1.RepositoryAccessDeletionPolicyRevoke, + }, + } + Expect(k8sClient.Create(ctx, collaborator)).To(Succeed()) + }) + + AfterEach(func() { + collaborator := &githubv1alpha1.GitHubRepositoryCollaborator{} + if err := k8sClient.Get(ctx, collaboratorKey, collaborator); err == nil { + if controllerutil.ContainsFinalizer(collaborator, githubRepositoryCollaboratorFinalizer) { + controllerutil.RemoveFinalizer(collaborator, githubRepositoryCollaboratorFinalizer) + Expect(k8sClient.Update(ctx, collaborator)).To(Succeed()) + } + Expect(k8sClient.Delete(ctx, collaborator)).To(Succeed()) + } else { + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + } + + cleanupRepositoryAccessDependencies( + ctx, + providerName, + secretName, + repositoryResourceName, + ) + }) + + It("should pause instead of writing collaborator access when the repository is archived", func() { + fakeClient := newFakeRepositoryAccessClient() + fakeClient.repositories["k8sready/"+repositoryName] = &githubclient.Repository{ + ID: 30, + HTMLURL: "https://github.com/k8sready/" + repositoryName, + Visibility: string(githubv1alpha1.RepositoryVisibilityPrivate), + Archived: true, + } + factory := &fakeRepositoryAccessClientFactory{client: fakeClient} + reconciler := &GitHubRepositoryCollaboratorReconciler{ + Client: k8sClient, + APIReader: k8sClient, + Scheme: k8sClient.Scheme(), + GitHubClientFactory: factory, + } + request := reconcile.Request{NamespacedName: collaboratorKey} + + By("adding the finalizer") + _, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + + By("observing the archived repository without attempting a write") + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(repositoryAccessRequeueInterval)) + Expect(fakeClient.setCollaboratorCalls).To(Equal(0)) + Expect(fakeClient.updateInvitationCalls).To(Equal(0)) + + collaborator := &githubv1alpha1.GitHubRepositoryCollaborator{} + Expect(k8sClient.Get(ctx, collaboratorKey, collaborator)).To(Succeed()) + condition := meta.FindStatusCondition(collaborator.Status.Conditions, conditionTypeReady) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal("RepositoryArchived")) + Expect(condition.Message).To(ContainSubstring("archived and read-only")) + }) + + It("should classify only archived-repository forbidden responses as archived", func() { + Expect(isArchivedRepositoryError(&githubclient.APIError{ + StatusCode: http.StatusForbidden, + Body: `{"message":"Repository was archived so is read-only."}`, + })).To(BeTrue()) + + Expect(isArchivedRepositoryError(&githubclient.APIError{ + StatusCode: http.StatusForbidden, + Body: `{"message":"Resource not accessible by personal access token"}`, + })).To(BeFalse()) + + Expect(isArchivedRepositoryError(&githubclient.APIError{ + StatusCode: http.StatusInternalServerError, + Body: `{"message":"Repository was archived so is read-only."}`, + })).To(BeFalse()) + }) +}) diff --git a/internal/controller/githubrepositorycollaborator_controller.go b/internal/controller/githubrepositorycollaborator_controller.go index 6c613f7..9c76a9b 100644 --- a/internal/controller/githubrepositorycollaborator_controller.go +++ b/internal/controller/githubrepositorycollaborator_controller.go @@ -84,7 +84,8 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( if err != nil { return r.fail(ctx, &collaborator, nil, dependencyFailureReason(err), err) } - if err := verifyRemoteRepository(ctx, resolved); err != nil { + remoteRepository, err := getRemoteRepository(ctx, resolved) + if err != nil { return r.fail(ctx, &collaborator, resolved, "RepositoryUnavailable", err) } @@ -96,6 +97,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( ) switch { case errors.Is(err, githubclient.ErrNotFound): + if remoteRepository.Archived { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, nil) + } currentAccess, err = resolved.Client.SetCollaboratorPermission( ctx, resolved.Provider.Spec.Organization, @@ -104,6 +108,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( string(collaborator.Spec.Permission), ) if err != nil { + if isArchivedRepositoryError(err) { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, nil) + } return r.fail(ctx, &collaborator, resolved, "ReconciliationFailed", fmt.Errorf( "configure collaborator %q access to %s/%s: %w", collaborator.Spec.Username, @@ -113,6 +120,8 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( )) } return r.finishAccess(ctx, &collaborator, resolved, currentAccess, "AccessConfigured") + case isArchivedRepositoryError(err): + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, nil) case err != nil: return r.fail(ctx, &collaborator, resolved, "ReconciliationFailed", fmt.Errorf( "get collaborator %q access to %s/%s: %w", @@ -122,6 +131,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( err, )) case currentAccess.Permission != string(collaborator.Spec.Permission) && currentAccess.InvitationPending: + if remoteRepository.Archived { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, currentAccess) + } currentAccess, err = resolved.Client.UpdateRepositoryInvitation( ctx, resolved.Provider.Spec.Organization, @@ -130,6 +142,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( string(collaborator.Spec.Permission), ) if err != nil { + if isArchivedRepositoryError(err) { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, currentAccess) + } return r.fail(ctx, &collaborator, resolved, "ReconciliationFailed", fmt.Errorf( "update invitation for collaborator %q on %s/%s: %w", collaborator.Spec.Username, @@ -140,6 +155,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( } return r.finishAccess(ctx, &collaborator, resolved, currentAccess, "InvitationUpdated") case currentAccess.Permission != string(collaborator.Spec.Permission): + if remoteRepository.Archived { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, currentAccess) + } currentAccess, err = resolved.Client.SetCollaboratorPermission( ctx, resolved.Provider.Spec.Organization, @@ -148,6 +166,9 @@ func (r *GitHubRepositoryCollaboratorReconciler) Reconcile( string(collaborator.Spec.Permission), ) if err != nil { + if isArchivedRepositoryError(err) { + return r.pauseForArchivedRepository(ctx, &collaborator, resolved, currentAccess) + } return r.fail(ctx, &collaborator, resolved, "ReconciliationFailed", fmt.Errorf( "update collaborator %q access to %s/%s: %w", collaborator.Spec.Username, @@ -203,6 +224,40 @@ func (r *GitHubRepositoryCollaboratorReconciler) finishAccess( return ctrl.Result{RequeueAfter: repositoryAccessRequeueInterval}, nil } +func (r *GitHubRepositoryCollaboratorReconciler) pauseForArchivedRepository( + ctx context.Context, + collaborator *githubv1alpha1.GitHubRepositoryCollaborator, + resolved *resolvedRepositoryAccess, + access *githubclient.CollaboratorAccess, +) (ctrl.Result, error) { + logger := logf.FromContext(ctx) + logger.Info( + "repository is archived; skipping collaborator reconciliation", + "organization", resolved.Provider.Spec.Organization, + "repository", resolved.Repository.Spec.Name, + "username", collaborator.Spec.Username, + ) + + message := fmt.Sprintf( + "GitHub repository %s/%s is archived and read-only; collaborator reconciliation is paused", + resolved.Provider.Spec.Organization, + resolved.Repository.Spec.Name, + ) + if err := r.setReadyCondition( + ctx, + collaborator, + resolved, + access, + metav1.ConditionFalse, + "RepositoryArchived", + message, + ); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{RequeueAfter: repositoryAccessRequeueInterval}, nil +} + func (r *GitHubRepositoryCollaboratorReconciler) reconcileDelete( ctx context.Context, collaborator *githubv1alpha1.GitHubRepositoryCollaborator, @@ -257,6 +312,16 @@ func (r *GitHubRepositoryCollaboratorReconciler) reconcileDelete( invitationID = collaborator.Status.InvitationID } + if resolved.Repository.Status.Archived { + logger.Info( + "repository is archived; deferring collaborator revocation", + "organization", resolved.Provider.Spec.Organization, + "repository", resolved.Repository.Spec.Name, + "username", collaborator.Spec.Username, + ) + return ctrl.Result{RequeueAfter: repositoryAccessRequeueInterval}, nil + } + err = resolved.Client.RemoveCollaboratorAccess( ctx, resolved.Provider.Spec.Organization, @@ -265,6 +330,15 @@ func (r *GitHubRepositoryCollaboratorReconciler) reconcileDelete( invitationID, ) if err != nil && !errors.Is(err, githubclient.ErrNotFound) { + if isArchivedRepositoryError(err) { + logger.Info( + "repository became archived while revoking collaborator access; deferring revocation", + "organization", resolved.Provider.Spec.Organization, + "repository", resolved.Repository.Spec.Name, + "username", collaborator.Spec.Username, + ) + return ctrl.Result{RequeueAfter: repositoryAccessRequeueInterval}, nil + } if result, ok := githubDeferredResult(err); ok { return result, nil } diff --git a/internal/controller/repository_access_helpers.go b/internal/controller/repository_access_helpers.go index 4f8ca5d..c30f0fd 100644 --- a/internal/controller/repository_access_helpers.go +++ b/internal/controller/repository_access_helpers.go @@ -20,6 +20,8 @@ import ( "context" "errors" "fmt" + "net/http" + "strings" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -94,25 +96,25 @@ func resolveRepositoryAccess( }, nil } -func verifyRemoteRepository( +func getRemoteRepository( ctx context.Context, resolved *resolvedRepositoryAccess, -) error { - _, err := resolved.Client.GetRepository( +) (*githubclient.Repository, error) { + repository, err := resolved.Client.GetRepository( ctx, resolved.Provider.Spec.Organization, resolved.Repository.Spec.Name, ) if err != nil { if errors.Is(err, githubclient.ErrNotFound) { - return fmt.Errorf( + return nil, fmt.Errorf( "GitHub repository %s/%s does not exist", resolved.Provider.Spec.Organization, resolved.Repository.Spec.Name, ) } - return fmt.Errorf( + return nil, fmt.Errorf( "get GitHub repository %s/%s: %w", resolved.Provider.Spec.Organization, resolved.Repository.Spec.Name, @@ -120,5 +122,24 @@ func verifyRemoteRepository( ) } - return nil + return repository, nil +} + +func verifyRemoteRepository( + ctx context.Context, + resolved *resolvedRepositoryAccess, +) error { + _, err := getRemoteRepository(ctx, resolved) + return err +} + +func isArchivedRepositoryError(err error) bool { + var apiErr *githubclient.APIError + if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusForbidden { + return false + } + + body := strings.ToLower(apiErr.Body) + return strings.Contains(body, "repository was archived") || + strings.Contains(body, "repository is archived") }