From 831d005df8f245ba2cc98028524488f6d0a7442c Mon Sep 17 00:00:00 2001
From: Mariano Cano <mariano.cano@gmail.com>
Date: Wed, 9 Jul 2025 12:20:29 -0700
Subject: [PATCH] Fix gcp unit tests

This PR fixes the gcp unit tests and delays the initialization of the
cloud resource manager client.
---
 authority/provisioner/gcp.go                  |  5 +-
 authority/provisioner/gcp/projectvalidator.go | 30 +++----
 .../provisioner/gcp/projectvalidator_test.go  | 84 +++++++++++++++++--
 3 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index 4a9361cd..ff84f13a 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -251,10 +251,7 @@ func (p *GCP) Init(config Config) (err error) {
 		return
 	}
 
-	if p.projectValidator, err = gcp.NewOrganizationValidator(context.Background(), p.ProjectIDs, p.OrganizationID); err != nil {
-		return
-	}
-
+	p.projectValidator = gcp.NewOrganizationValidator(p.ProjectIDs, p.OrganizationID)
 	return
 }
 
diff --git a/authority/provisioner/gcp/projectvalidator.go b/authority/provisioner/gcp/projectvalidator.go
index 92e9d698..2b92c5b6 100644
--- a/authority/provisioner/gcp/projectvalidator.go
+++ b/authority/provisioner/gcp/projectvalidator.go
@@ -4,9 +4,9 @@ import (
 	"context"
 	"net/http"
 
-	"github.com/smallstep/certificates/errs"
-
 	"google.golang.org/api/cloudresourcemanager/v1"
+
+	"github.com/smallstep/certificates/errs"
 )
 
 type ProjectValidator struct {
@@ -29,23 +29,14 @@ func (p *ProjectValidator) ValidateProject(_ context.Context, projectID string)
 
 type OrganizationValidator struct {
 	*ProjectValidator
-
-	OrganizationID  string
-	projectsService *cloudresourcemanager.ProjectsService
+	OrganizationID string
 }
 
-func NewOrganizationValidator(ctx context.Context, projectIDs []string, organizationID string) (*OrganizationValidator, error) {
-	crm, err := cloudresourcemanager.NewService(ctx)
-
-	if err != nil {
-		return nil, err
-	}
-
+func NewOrganizationValidator(projectIDs []string, organizationID string) *OrganizationValidator {
 	return &OrganizationValidator{
 		&ProjectValidator{projectIDs},
 		organizationID,
-		crm.Projects,
-	}, nil
+	}
 }
 
 func (p *OrganizationValidator) ValidateProject(ctx context.Context, projectID string) error {
@@ -53,7 +44,16 @@ func (p *OrganizationValidator) ValidateProject(ctx context.Context, projectID s
 		return err
 	}
 
-	ancestry, err := p.projectsService.
+	if p.OrganizationID == "" {
+		return nil
+	}
+
+	crm, err := cloudresourcemanager.NewService(ctx)
+	if err != nil {
+		return err
+	}
+
+	ancestry, err := crm.Projects.
 		GetAncestry(projectID, &cloudresourcemanager.GetAncestryRequest{}).
 		Context(ctx).
 		Do()
diff --git a/authority/provisioner/gcp/projectvalidator_test.go b/authority/provisioner/gcp/projectvalidator_test.go
index 9233a1ba..1b68813c 100644
--- a/authority/provisioner/gcp/projectvalidator_test.go
+++ b/authority/provisioner/gcp/projectvalidator_test.go
@@ -3,16 +3,84 @@ package gcp
 import (
 	"context"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/api/cloudresourcemanager/v1"
 )
 
-func TestProjectValidator(t *testing.T) {
-	validator := &ProjectValidator{ProjectIDs: []string{"allowed-1", "allowed-2"}}
-
-	if err := validator.ValidateProject(context.Background(), "not-allowed"); err == nil {
-		t.Errorf("ProjectValidator.ValidateProject() = nil, want err")
+func TestProjectValidator_ValidateProject(t *testing.T) {
+	ctx := context.Background()
+	type fields struct {
+		ProjectIDs []string
 	}
-
-	if err := validator.ValidateProject(context.Background(), "allowed-2"); err != nil {
-		t.Errorf("ProjectValidator.ValidateProject() = %v, want nil", err)
+	type args struct {
+		in0       context.Context
+		projectID string
+	}
+	tests := []struct {
+		name      string
+		fields    fields
+		args      args
+		assertion assert.ErrorAssertionFunc
+	}{
+		{"allowed-1", fields{[]string{"allowed-1", "allowed-2"}}, args{ctx, "allowed-1"}, assert.NoError},
+		{"allowed-2", fields{[]string{"allowed-1", "allowed-2"}}, args{ctx, "allowed-2"}, assert.NoError},
+		{"empty", fields{nil}, args{ctx, "allowed-1"}, assert.NoError},
+		{"not allowed", fields{[]string{"allowed-1", "allowed-2"}}, args{ctx, "not-allowed"}, assert.Error},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &ProjectValidator{
+				ProjectIDs: tt.fields.ProjectIDs,
+			}
+			tt.assertion(t, p.ValidateProject(tt.args.in0, tt.args.projectID))
+		})
+	}
+}
+
+func TestNewOrganizationValidator(t *testing.T) {
+	got := NewOrganizationValidator([]string{"project-1", "project-2"}, "organization")
+	assert.Equal(t, &OrganizationValidator{
+		ProjectValidator: &ProjectValidator{ProjectIDs: []string{"project-1", "project-2"}},
+		OrganizationID:   "organization",
+	}, got)
+}
+
+func TestOrganizationValidator_ValidateProject(t *testing.T) {
+	ctx := context.Background()
+	_, err := cloudresourcemanager.NewService(ctx)
+	skip := (err != nil)
+
+	type fields struct {
+		ProjectValidator *ProjectValidator
+		OrganizationID   string
+	}
+	type args struct {
+		ctx       context.Context
+		projectID string
+	}
+	tests := []struct {
+		name      string
+		skip      bool
+		fields    fields
+		args      args
+		assertion assert.ErrorAssertionFunc
+	}{
+		{"ok projects", false, fields{&ProjectValidator{ProjectIDs: []string{"allowed"}}, ""}, args{ctx, "allowed"}, assert.NoError},
+		{"fail projects", false, fields{&ProjectValidator{ProjectIDs: []string{"allowed"}}, "organization"}, args{ctx, "not-allowed"}, assert.Error},
+		{"fail organization", skip, fields{&ProjectValidator{ProjectIDs: []string{"allowed"}}, "fake-organization"}, args{ctx, "allowed"}, assert.Error},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &OrganizationValidator{
+				ProjectValidator: tt.fields.ProjectValidator,
+				OrganizationID:   tt.fields.OrganizationID,
+			}
+			if tt.skip {
+				t.SkipNow()
+				return
+			}
+			tt.assertion(t, p.ValidateProject(tt.args.ctx, tt.args.projectID))
+		})
 	}
 }