Skip to content

Commit

Permalink
Change 404 checking when using grpc librarires
Browse files Browse the repository at this point in the history
It uses other error types, so to simplify we request the object first
and check the status on the response.
  • Loading branch information
thokra-nav committed Jul 2, 2024
1 parent e33e644 commit c1f167b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 8 deletions.
8 changes: 4 additions & 4 deletions internal/reconcilers/google/cdn/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"google.golang.org/api/googleapi"
"google.golang.org/api/iam/v1"
"google.golang.org/api/option"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/utils/ptr"
)

Expand Down Expand Up @@ -319,11 +321,9 @@ func (r *cdnReconciler) deleteBackendBucket(ctx context.Context, bucketName stri
Project: r.googleManagementProjectID,
})
if err != nil {
googleError, ok := err.(*googleapi.Error)
if !ok || googleError.Code != http.StatusNotFound {
return false, fmt.Errorf("get backend bucket, %w", err)
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
return false, nil
}
return false, nil
}

// remove entry from urlmap
Expand Down
12 changes: 8 additions & 4 deletions internal/reconcilers/google/gar/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,19 @@ func (r *garReconciler) Delete(ctx context.Context, client *apiclient.APIClient,
log.WithError(err).Errorf("GAR service account %q does not exist, nothing to delete", serviceAccountName)
}

// Check if repository exists, if it doesn't, no need to delete it
_, err := r.artifactRegistry.GetRepository(ctx, &artifactregistrypb.GetRepositoryRequest{Name: *naisTeam.GarRepository})
if err != nil {
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
return nil
}
}

req := &artifactregistrypb.DeleteRepositoryRequest{
Name: *naisTeam.GarRepository,
}
operation, err := r.artifactRegistry.DeleteRepository(ctx, req)
if err != nil {
googleError, ok := err.(*googleapi.Error)
if ok && googleError.Code == http.StatusNotFound {
return nil
}
return fmt.Errorf("delete GAR repository for team %q: %w", naisTeam.Slug, err)
}

Expand Down
41 changes: 41 additions & 0 deletions internal/reconcilers/google/gar/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,9 @@ func TestDelete(t *testing.T) {
delete: func(ctx context.Context, req *artifactregistrypb.DeleteRepositoryRequest) (*longrunningpb.Operation, error) {
return nil, fmt.Errorf("some error")
},
get: func(ctx context.Context, r *artifactregistrypb.GetRepositoryRequest) (*artifactregistrypb.Repository, error) {
return nil, nil
},
},
iam: test.HttpServerWithHandlers(t, []http.HandlerFunc{
func(w http.ResponseWriter, _ *http.Request) {
Expand Down Expand Up @@ -841,6 +844,9 @@ func TestDelete(t *testing.T) {
},
}, nil
},
get: func(ctx context.Context, r *artifactregistrypb.GetRepositoryRequest) (*artifactregistrypb.Repository, error) {
return nil, nil
},
},
iam: test.HttpServerWithHandlers(t, []http.HandlerFunc{
func(w http.ResponseWriter, _ *http.Request) {
Expand All @@ -860,6 +866,38 @@ func TestDelete(t *testing.T) {
}
})

t.Run("repository does not exist", func(t *testing.T) {
naisTeam := &protoapi.Team{
Slug: teamSlug,
GarRepository: ptr.To(repositoryName),
}

apiClient, _ := apiclient.NewMockClient(t)

mockedClients := mocks{
artifactRegistry: &fakeArtifactRegistry{
get: func(ctx context.Context, r *artifactregistrypb.GetRepositoryRequest) (*artifactregistrypb.Repository, error) {
return nil, status.Error(codes.NotFound, "not found")
},
},
iam: test.HttpServerWithHandlers(t, []http.HandlerFunc{
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
},
}),
}
garClient, iamService := mockedClients.start(t, ctx)

reconciler, err := google_gar_reconciler.New(ctx, serviceAccountEmail, managementProjectID, workloadIdentityPoolName, google_gar_reconciler.WithGarClient(garClient), google_gar_reconciler.WithIAMService(iamService))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if err = reconciler.Delete(ctx, apiClient, naisTeam, log); err != nil {
t.Errorf("unexpected error: %v", err)
}
})

t.Run("successful delete", func(t *testing.T) {
defer hook.Reset()

Expand Down Expand Up @@ -893,6 +931,9 @@ func TestDelete(t *testing.T) {
Result: &longrunningpb.Operation_Response{},
}, nil
},
get: func(ctx context.Context, r *artifactregistrypb.GetRepositoryRequest) (*artifactregistrypb.Repository, error) {
return nil, nil
},
},
iam: test.HttpServerWithHandlers(t, []http.HandlerFunc{
func(w http.ResponseWriter, _ *http.Request) {
Expand Down

0 comments on commit c1f167b

Please sign in to comment.