From 4d09657883267aa4cb7d16e897dfd470ff9f0cee Mon Sep 17 00:00:00 2001 From: Bharath B Date: Wed, 3 Apr 2024 20:46:28 +0530 Subject: [PATCH] CORS-3254: Update google.golang.org/api/cloudresourcemanager library version --- pkg/asset/cluster/tfvars/tfvars.go | 3 +- pkg/asset/installconfig/gcp/client.go | 60 +++++++++++-- .../gcp/mock/gcpclient_generated.go | 32 ++++++- .../installconfig/gcp/mock/usertags_mock.go | 67 -------------- pkg/asset/installconfig/gcp/usertags.go | 87 +++---------------- pkg/asset/installconfig/gcp/usertags_test.go | 16 ++-- pkg/asset/installconfig/gcp/validation.go | 2 +- .../installconfig/gcp/validation_test.go | 2 +- pkg/constants/gcp/gcp.go | 13 +++ pkg/destroy/gcp/gcp.go | 9 +- pkg/destroy/gcp/policybinding.go | 12 ++- 11 files changed, 132 insertions(+), 171 deletions(-) delete mode 100644 pkg/asset/installconfig/gcp/mock/usertags_mock.go create mode 100644 pkg/constants/gcp/gcp.go diff --git a/pkg/asset/cluster/tfvars/tfvars.go b/pkg/asset/cluster/tfvars/tfvars.go index 9c80141a3b6..7a5f2a4b5d2 100644 --- a/pkg/asset/cluster/tfvars/tfvars.go +++ b/pkg/asset/cluster/tfvars/tfvars.go @@ -554,8 +554,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { return fmt.Errorf("%s: No GCP build found", st.FormatPrefix(archName)) } - tags, err := gcpconfig.GetUserTags(ctx, - gcpconfig.NewTagManager(client), + tags, err := gcpconfig.NewTagManager(client).GetUserTags(ctx, installConfig.Config.Platform.GCP.ProjectID, installConfig.Config.Platform.GCP.UserTags) if err != nil { diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 7ac9caef040..3403d9f1142 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -8,7 +8,7 @@ import ( "github.com/pkg/errors" googleoauth "golang.org/x/oauth2/google" - "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/api/cloudresourcemanager/v3" compute "google.golang.org/api/compute/v1" dns "google.golang.org/api/dns/v1" "google.golang.org/api/googleapi" @@ -16,6 +16,8 @@ import ( "google.golang.org/api/option" "google.golang.org/api/serviceusage/v1" "k8s.io/apimachinery/pkg/util/sets" + + gcpconsts "github.com/openshift/installer/pkg/constants/gcp" ) //go:generate mockgen -source=./client.go -destination=./mock/gcpclient_generated.go -package=mock @@ -48,6 +50,8 @@ type API interface { GetProjectPermissions(ctx context.Context, project string, permissions []string) (sets.Set[string], error) GetProjectByID(ctx context.Context, project string) (*cloudresourcemanager.Project, error) ValidateServiceAccountHasPermissions(ctx context.Context, project string, permissions []string) (bool, error) + GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) + GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) } // Client makes calls to the GCP API. @@ -317,9 +321,9 @@ func (c *Client) GetProjects(ctx context.Context) (map[string]string, error) { return nil, err } - req := svc.Projects.List() + req := svc.Projects.Search() projects := make(map[string]string) - if err := req.Pages(ctx, func(page *cloudresourcemanager.ListProjectsResponse) error { + if err := req.Pages(ctx, func(page *cloudresourcemanager.SearchProjectsResponse) error { for _, project := range page.Projects { projects[project.ProjectId] = project.Name } @@ -340,7 +344,7 @@ func (c *Client) GetProjectByID(ctx context.Context, project string) (*cloudreso return nil, err } - return svc.Projects.Get(project).Context(ctx).Do() + return svc.Projects.Get(fmt.Sprintf(gcpconsts.ProjectNameFmt, project)).Context(ctx).Do() } // GetRegions gets the regions that are valid for the project. An error is returned when unsuccessful @@ -485,7 +489,7 @@ func (c *Client) getPermissions(ctx context.Context, project string, permissions projectsService := cloudresourcemanager.NewProjectsService(service) rb := &cloudresourcemanager.TestIamPermissionsRequest{Permissions: permissions} - response, err := projectsService.TestIamPermissions(project, rb).Context(ctx).Do() + response, err := projectsService.TestIamPermissions(fmt.Sprintf(gcpconsts.ProjectNameFmt, project), rb).Context(ctx).Do() if err != nil { return nil, errors.Wrapf(err, "failed to get Iam permissions") } @@ -513,3 +517,49 @@ func (c *Client) ValidateServiceAccountHasPermissions(ctx context.Context, proje } return validPermissions.Len() == len(permissions), nil } + +// GetProjectTags returns the list of effective tags attached to the provided project resource. +func (c *Client) GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) { + service, err := c.getCloudResourceService(ctx) + if err != nil { + return nil, fmt.Errorf("failed to create cloud resource service: %w", err) + } + + effectiveTags := sets.New[string]() + effectiveTagsService := cloudresourcemanager.NewEffectiveTagsService(service) + effectiveTagsRequest := effectiveTagsService.List(). + Context(ctx). + Parent(fmt.Sprintf(gcpconsts.ProjectParentPathFmt, projectID)) + + if err := effectiveTagsRequest.Pages(ctx, func(page *cloudresourcemanager.ListEffectiveTagsResponse) error { + for _, effectiveTag := range page.EffectiveTags { + effectiveTags.Insert(effectiveTag.NamespacedTagValue) + } + return nil + }); err != nil { + return nil, fmt.Errorf("failed to fetch tags attached to %s project: %w", projectID, err) + } + + return effectiveTags, nil +} + +// GetNamespacedTagValue returns the Tag Value metadata fetched using the tag's NamespacedName. +func (c *Client) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) { + service, err := c.getCloudResourceService(ctx) + if err != nil { + return nil, fmt.Errorf("failed to create cloud resource service: %w", err) + } + + tagValuesService := cloudresourcemanager.NewTagValuesService(service) + + tagValue, err := tagValuesService.GetNamespaced(). + Context(ctx). + Name(tagNamespacedName). + Do() + + if err != nil { + return nil, fmt.Errorf("failed to fetch %s tag value: %w", tagNamespacedName, err) + } + + return tagValue, nil +} diff --git a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go index b2c3ed69cbc..e2203798e7f 100644 --- a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go +++ b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go @@ -10,7 +10,7 @@ import ( gomock "github.com/golang/mock/gomock" google "golang.org/x/oauth2/google" - cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1" + cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v3" compute "google.golang.org/api/compute/v1" dns "google.golang.org/api/dns/v1" sets "k8s.io/apimachinery/pkg/util/sets" @@ -144,6 +144,21 @@ func (mr *MockAPIMockRecorder) GetMachineTypeWithZones(ctx, project, region, mac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMachineTypeWithZones", reflect.TypeOf((*MockAPI)(nil).GetMachineTypeWithZones), ctx, project, region, machineType) } +// GetNamespacedTagValue mocks base method. +func (m *MockAPI) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNamespacedTagValue", ctx, tagNamespacedName) + ret0, _ := ret[0].(*cloudresourcemanager.TagValue) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNamespacedTagValue indicates an expected call of GetNamespacedTagValue. +func (mr *MockAPIMockRecorder) GetNamespacedTagValue(ctx, tagNamespacedName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNamespacedTagValue", reflect.TypeOf((*MockAPI)(nil).GetNamespacedTagValue), ctx, tagNamespacedName) +} + // GetNetwork mocks base method. func (m *MockAPI) GetNetwork(ctx context.Context, network, project string) (*compute.Network, error) { m.ctrl.T.Helper() @@ -189,6 +204,21 @@ func (mr *MockAPIMockRecorder) GetProjectPermissions(ctx, project, permissions i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProjectPermissions", reflect.TypeOf((*MockAPI)(nil).GetProjectPermissions), ctx, project, permissions) } +// GetProjectTags mocks base method. +func (m *MockAPI) GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProjectTags", ctx, projectID) + ret0, _ := ret[0].(sets.Set[string]) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetProjectTags indicates an expected call of GetProjectTags. +func (mr *MockAPIMockRecorder) GetProjectTags(ctx, projectID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProjectTags", reflect.TypeOf((*MockAPI)(nil).GetProjectTags), ctx, projectID) +} + // GetProjects mocks base method. func (m *MockAPI) GetProjects(ctx context.Context) (map[string]string, error) { m.ctrl.T.Helper() diff --git a/pkg/asset/installconfig/gcp/mock/usertags_mock.go b/pkg/asset/installconfig/gcp/mock/usertags_mock.go deleted file mode 100644 index cf799a016e2..00000000000 --- a/pkg/asset/installconfig/gcp/mock/usertags_mock.go +++ /dev/null @@ -1,67 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ./usertags.go - -// Package mock is a generated GoMock package. -package mock - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v3" - sets "k8s.io/apimachinery/pkg/util/sets" -) - -// MockTagManager is a mock of TagManager interface. -type MockTagManager struct { - ctrl *gomock.Controller - recorder *MockTagManagerMockRecorder -} - -// MockTagManagerMockRecorder is the mock recorder for MockTagManager. -type MockTagManagerMockRecorder struct { - mock *MockTagManager -} - -// NewMockTagManager creates a new mock instance. -func NewMockTagManager(ctrl *gomock.Controller) *MockTagManager { - mock := &MockTagManager{ctrl: ctrl} - mock.recorder = &MockTagManagerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockTagManager) EXPECT() *MockTagManagerMockRecorder { - return m.recorder -} - -// GetNamespacedTagValue mocks base method. -func (m *MockTagManager) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetNamespacedTagValue", ctx, tagNamespacedName) - ret0, _ := ret[0].(*cloudresourcemanager.TagValue) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetNamespacedTagValue indicates an expected call of GetNamespacedTagValue. -func (mr *MockTagManagerMockRecorder) GetNamespacedTagValue(ctx, tagNamespacedName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNamespacedTagValue", reflect.TypeOf((*MockTagManager)(nil).GetNamespacedTagValue), ctx, tagNamespacedName) -} - -// GetProjectTags mocks base method. -func (m *MockTagManager) GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetProjectTags", ctx, projectID) - ret0, _ := ret[0].(sets.Set[string]) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetProjectTags indicates an expected call of GetProjectTags. -func (mr *MockTagManagerMockRecorder) GetProjectTags(ctx, projectID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProjectTags", reflect.TypeOf((*MockTagManager)(nil).GetProjectTags), ctx, projectID) -} diff --git a/pkg/asset/installconfig/gcp/usertags.go b/pkg/asset/installconfig/gcp/usertags.go index 923564d0f37..2a6f32b5f3d 100644 --- a/pkg/asset/installconfig/gcp/usertags.go +++ b/pkg/asset/installconfig/gcp/usertags.go @@ -10,15 +10,11 @@ import ( "github.com/googleapis/gax-go/v2/apierror" "github.com/sirupsen/logrus" - tags "google.golang.org/api/cloudresourcemanager/v3" - "google.golang.org/api/option" "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/installer/pkg/types/gcp" ) -//go:generate mockgen -source=./usertags.go -destination=./mock/usertags_mock.go -package=mock - const ( // maxUserTagLimit is the maximum userTags that can be configured as defined in openshift/api. // https://github.com/openshift/api/commit/ae73a19d05c35068af16c9aeff375d0b7c936a8a#diff-07b264a49084976b670fb699badaca1795027d6ea732a99226a5388104f6174fR604-R613 @@ -39,17 +35,11 @@ type processedUserTags struct { sync.Mutex } -// tagManager handles resource tagging. -type tagManager struct { +// TagManager handles resource tagging. +type TagManager struct { client API } -// TagManager is the interface that wraps methods for resource tag operations. -type TagManager interface { - GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) - GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*tags.TagValue, error) -} - // newProcessedUserTags is for initializing an instance of processedUserTags. func newProcessedUserTags() *processedUserTags { return &processedUserTags{} @@ -100,16 +90,16 @@ func (p *processedUserTags) copy() map[string]string { return t } -// NewTagManager creates a tagManager instance. -func NewTagManager(client API) TagManager { - return &tagManager{client: client} +// NewTagManager creates a TagManager instance. +func NewTagManager(client API) *TagManager { + return &TagManager{client: client} } // GetUserTags returns the processed list of user provided tags if already available, // else validates, persists in-memory and returns the processed tags. -func GetUserTags(ctx context.Context, mgr TagManager, projectID string, userTags []gcp.UserTag) (map[string]string, error) { +func (t *TagManager) GetUserTags(ctx context.Context, projectID string, userTags []gcp.UserTag) (map[string]string, error) { if !processedTags.isProcessed() { - if err := validateAndPersistUserTags(ctx, mgr, projectID, userTags); err != nil { + if err := t.validateAndPersistUserTags(ctx, projectID, userTags); err != nil { return nil, err } } @@ -123,7 +113,7 @@ func GetUserTags(ctx context.Context, mgr TagManager, projectID string, userTags // with key of the form `tagKeys/{tag_key_id}` and value of the form // `tagValues/{tag_value_id}`. Returns error when fetching a tag fails or when // tag already exists on the project resource. -func validateAndPersistUserTags(ctx context.Context, mgr TagManager, project string, userTags []gcp.UserTag) error { +func (t *TagManager) validateAndPersistUserTags(ctx context.Context, project string, userTags []gcp.UserTag) error { if len(userTags) == 0 { return nil } @@ -135,7 +125,7 @@ func validateAndPersistUserTags(ctx context.Context, mgr TagManager, project str return fmt.Errorf("more than %d user tags is not allowed, configured count: %d", maxUserTagLimit, len(userTags)) } - projectTags, err := mgr.GetProjectTags(ctx, project) + projectTags, err := t.client.GetProjectTags(ctx, project) if err != nil { return err } @@ -148,7 +138,7 @@ func validateAndPersistUserTags(ctx context.Context, mgr TagManager, project str nonexistentTags := make([]string, 0) for _, tag := range userTags { name := fmt.Sprintf("%s/%s/%s", tag.ParentID, tag.Key, tag.Value) - tagValue, err := mgr.GetNamespacedTagValue(ctx, name) + tagValue, err := t.client.GetNamespacedTagValue(ctx, name) if err != nil { // check and return all non-existing tags at once // for user to fix in one go. @@ -187,60 +177,3 @@ func findDuplicateTags(userTags []gcp.UserTag, parentTags sets.Set[string]) []st } return dupTags } - -// getCloudResourceServiceForTags returns the client required for querying resource manager resources. -func (m *tagManager) getCloudResourceServiceForTags(ctx context.Context) (*tags.Service, error) { - svc, err := tags.NewService(ctx, option.WithCredentials(m.client.GetCredentials())) - if err != nil { - return nil, fmt.Errorf("failed to create cloud resource service: %w", err) - } - return svc, nil -} - -// GetProjectTags returns the list of effective tags attached to the provided project resource. -func (m *tagManager) GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) { - const ( - // projectParentPathFmt is the format string for parent path of a project resource. - projectParentPathFmt = "//cloudresourcemanager.googleapis.com/projects/%s" - ) - - ctx, cancel := context.WithTimeout(ctx, defaultTimeout) - defer cancel() - - service, err := m.getCloudResourceServiceForTags(ctx) - if err != nil { - return nil, fmt.Errorf("failed to create cloud resource service: %w", err) - } - - effectiveTags := sets.New[string]() - effectiveTagsService := tags.NewEffectiveTagsService(service) - effectiveTagsRequest := effectiveTagsService.List().Context(ctx).Parent(fmt.Sprintf(projectParentPathFmt, projectID)) - if err := effectiveTagsRequest.Pages(ctx, func(page *tags.ListEffectiveTagsResponse) error { - for _, effectiveTag := range page.EffectiveTags { - effectiveTags.Insert(effectiveTag.NamespacedTagValue) - } - return nil - }); err != nil { - return nil, err - } - - return effectiveTags, nil -} - -// GetNamespacedTagValue returns the Tag Value metadata fetched using the tag's NamespacedName. -func (m *tagManager) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*tags.TagValue, error) { - ctx, cancel := context.WithTimeout(ctx, defaultTimeout) - defer cancel() - - service, err := m.getCloudResourceServiceForTags(ctx) - if err != nil { - return nil, fmt.Errorf("failed to create cloud resource service: %w", err) - } - - tagValuesService := tags.NewTagValuesService(service) - - return tagValuesService.GetNamespaced(). - Context(ctx). - Name(tagNamespacedName). - Do() -} diff --git a/pkg/asset/installconfig/gcp/usertags_test.go b/pkg/asset/installconfig/gcp/usertags_test.go index 9154618417a..bf9e0093581 100644 --- a/pkg/asset/installconfig/gcp/usertags_test.go +++ b/pkg/asset/installconfig/gcp/usertags_test.go @@ -211,16 +211,16 @@ func TestGetUserTags(t *testing.T) { defer mockCtrl.Finish() gcpClient := mock.NewMockAPI(mockCtrl) - tagMgr := mock.NewMockTagManager(mockCtrl) + tagMgr := NewTagManager(gcpClient) // Return fake credentials. gcpClient.EXPECT().GetCredentials().Return(&googleoauth.Credentials{JSON: []byte(fakeCreds)}).AnyTimes() // Return ["openshift/key1/value1", "openshift/key2/value2"] tags for "test-project" project. - tagMgr.EXPECT().GetProjectTags(gomock.Any(), testProject).Return(sets.Insert(sets.New[string](), "openshift/key1/value1", "openshift/key2/value2"), nil).AnyTimes() + gcpClient.EXPECT().GetProjectTags(gomock.Any(), testProject).Return(sets.Insert(sets.New[string](), "openshift/key1/value1", "openshift/key2/value2"), nil).AnyTimes() // Return error for "fail-project" project. - tagMgr.EXPECT().GetProjectTags(gomock.Any(), "fail-project"). + gcpClient.EXPECT().GetProjectTags(gomock.Any(), "fail-project"). Return( nil, fmt.Errorf("failed to fetch tags attached to fail-project: The caller does not have permission")). @@ -228,7 +228,7 @@ func TestGetUserTags(t *testing.T) { // Return ["openshift/key6/value6", "openshift/key7/value7", "openshift/key8/value8", // "openshift/key9/value9", "openshift/key10/value10"] tags for "dup-project" project. - tagMgr.EXPECT().GetProjectTags(gomock.Any(), "dup-project"). + gcpClient.EXPECT().GetProjectTags(gomock.Any(), "dup-project"). Return( sets.Insert(sets.New[string](), "openshift/key6/value6", @@ -239,13 +239,13 @@ func TestGetUserTags(t *testing.T) { ), nil).AnyTimes() // Return Forbidden error "openshift/key51/value51" tag. - tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key51/value51").Return(nil, getTestGetNamespacedTagValueForbiddenError("openshift/key51/value51")).AnyTimes() + gcpClient.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key51/value51").Return(nil, getTestGetNamespacedTagValueForbiddenError("openshift/key51/value51")).AnyTimes() // Return Internal error "openshift/key52/value52" tag. - tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key52/value52").Return(nil, getTestGetNamespacedTagValueInternalError("openshift/key52/value52")).AnyTimes() + gcpClient.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key52/value52").Return(nil, getTestGetNamespacedTagValueInternalError("openshift/key52/value52")).AnyTimes() // Return details for the requested tag. - tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), gomock.Any()). + gcpClient.EXPECT().GetNamespacedTagValue(gomock.Any(), gomock.Any()). DoAndReturn(func(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) { return getTestNamespacedTagValueResp(tagNamespacedName), nil }).AnyTimes() @@ -260,7 +260,7 @@ func TestGetUserTags(t *testing.T) { if tc.projectID != "" { projectID = tc.projectID } - tags, err := GetUserTags(context.Background(), tagMgr, projectID, tc.userTags) + tags, err := tagMgr.GetUserTags(context.Background(), projectID, tc.userTags) if (err != nil || tc.expectedError != "") && err.Error() != tc.expectedError { t.Errorf("GetUserTags(): error: got: %v, want: %v", err, tc.expectedError) diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 41b12eb30af..c035fba33d5 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -641,5 +641,5 @@ func checkArchitecture(imageArch string, icArch types.Architecture, role string) // validateUserTags check for existence and accessibility of user-defined tags and persists // validated tags in-memory. func validateUserTags(client API, projectID string, userTags []gcp.UserTag) error { - return validateAndPersistUserTags(context.Background(), NewTagManager(client), projectID, userTags) + return NewTagManager(client).validateAndPersistUserTags(context.Background(), projectID, userTags) } diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 288e7d6502d..323ba879976 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -12,7 +12,7 @@ import ( logrusTest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" googleoauth "golang.org/x/oauth2/google" - "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/api/cloudresourcemanager/v3" compute "google.golang.org/api/compute/v1" dns "google.golang.org/api/dns/v1" "google.golang.org/api/googleapi" diff --git a/pkg/constants/gcp/gcp.go b/pkg/constants/gcp/gcp.go new file mode 100644 index 00000000000..16e1a712a36 --- /dev/null +++ b/pkg/constants/gcp/gcp.go @@ -0,0 +1,13 @@ +package gcp + +const ( + // ProjectNameFmt is the format string for GCP project resource name. + ProjectNameFmt = "projects/%s" + + // ProjectParentPathFmt is the format string for parent path of a GCP project resource. + ProjectParentPathFmt = "//cloudresourcemanager.googleapis.com/projects/%s" + + // ClusterIDLabelFmt is the format string for the default label + // added to the OpenShift created GCP resources. + ClusterIDLabelFmt = "kubernetes-io-cluster-%s" +) diff --git a/pkg/destroy/gcp/gcp.go b/pkg/destroy/gcp/gcp.go index 60a14ec3e7c..8f6e55658b6 100644 --- a/pkg/destroy/gcp/gcp.go +++ b/pkg/destroy/gcp/gcp.go @@ -10,7 +10,7 @@ import ( "github.com/pborman/uuid" "github.com/pkg/errors" "github.com/sirupsen/logrus" - resourcemanager "google.golang.org/api/cloudresourcemanager/v1" + resourcemanager "google.golang.org/api/cloudresourcemanager/v3" "google.golang.org/api/compute/v1" "google.golang.org/api/dns/v1" "google.golang.org/api/googleapi" @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp" + gcpconsts "github.com/openshift/installer/pkg/constants/gcp" "github.com/openshift/installer/pkg/destroy/providers" "github.com/openshift/installer/pkg/types" gcptypes "github.com/openshift/installer/pkg/types/gcp" @@ -35,10 +36,6 @@ var ( type resourceScope string const ( - // ocpDefaultLabelFmt is the format string for the default label - // added to the OpenShift created GCP resources. - ocpDefaultLabelFmt = "kubernetes-io-cluster-%s" - // capgProviderOwnedLabelFmt is the format string for the label // used for resources created by the Cluster API GCP provider. capgProviderOwnedLabelFmt = "capg-cluster-%s" @@ -255,7 +252,7 @@ func (o *ClusterUninstaller) clusterIDFilter() string { func (o *ClusterUninstaller) clusterLabelFilter() string { return fmt.Sprintf("(labels.%s = \"owned\") OR (labels.%s = \"owned\")", - fmt.Sprintf(ocpDefaultLabelFmt, o.ClusterID), fmt.Sprintf(capgProviderOwnedLabelFmt, o.ClusterID)) + fmt.Sprintf(gcpconsts.ClusterIDLabelFmt, o.ClusterID), fmt.Sprintf(capgProviderOwnedLabelFmt, o.ClusterID)) } func (o *ClusterUninstaller) clusterLabelOrClusterIDFilter() string { diff --git a/pkg/destroy/gcp/policybinding.go b/pkg/destroy/gcp/policybinding.go index f258ca6344e..48408fa183f 100644 --- a/pkg/destroy/gcp/policybinding.go +++ b/pkg/destroy/gcp/policybinding.go @@ -2,20 +2,26 @@ package gcp import ( "context" + "fmt" "strings" "github.com/pkg/errors" "github.com/sirupsen/logrus" - resourcemanager "google.golang.org/api/cloudresourcemanager/v1" + resourcemanager "google.golang.org/api/cloudresourcemanager/v3" "k8s.io/apimachinery/pkg/util/sets" ) +const ( + // projectNameFmt is the format string for project resource name. + projectNameFmt = "projects/%s" +) + func (o *ClusterUninstaller) getProjectIAMPolicy(ctx context.Context) (*resourcemanager.Policy, error) { o.Logger.Debug("Fetching project IAM policy") ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() req := &resourcemanager.GetIamPolicyRequest{} - policy, err := o.rmSvc.Projects.GetIamPolicy(o.ProjectID, req).Context(ctx).Do() + policy, err := o.rmSvc.Projects.GetIamPolicy(fmt.Sprintf(projectNameFmt, o.ProjectID), req).Context(ctx).Do() if err != nil { return nil, errors.Wrapf(err, "failed to fetch project IAM policy") } @@ -27,7 +33,7 @@ func (o *ClusterUninstaller) setProjectIAMPolicy(ctx context.Context, policy *re ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() req := &resourcemanager.SetIamPolicyRequest{Policy: policy} - _, err := o.rmSvc.Projects.SetIamPolicy(o.ProjectID, req).Context(ctx).Do() + _, err := o.rmSvc.Projects.SetIamPolicy(fmt.Sprintf(projectNameFmt, o.ProjectID), req).Context(ctx).Do() if err != nil { return errors.Wrapf(err, "failed to set project IAM policy") }