From 03d46a5bc79b530ca2b80b70d46e07ff210937cd Mon Sep 17 00:00:00 2001 From: Brent Barbachem Date: Tue, 10 Sep 2024 12:30:03 -0400 Subject: [PATCH] OCPBUGS-41538: Remove bindings for XPN installs ** During XPN installs bindings are created in the host project to the service account used to perform actions in the host project. These bindings are left over during the destruction process if they are not explicitly found and removed in the host project rather than the default (service) project. --- pkg/destroy/gcp/policybinding.go | 13 ++++++------ pkg/destroy/gcp/serviceaccount.go | 35 ++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/pkg/destroy/gcp/policybinding.go b/pkg/destroy/gcp/policybinding.go index 48408fa183f..b1f260474f8 100644 --- a/pkg/destroy/gcp/policybinding.go +++ b/pkg/destroy/gcp/policybinding.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/pkg/errors" "github.com/sirupsen/logrus" resourcemanager "google.golang.org/api/cloudresourcemanager/v3" "k8s.io/apimachinery/pkg/util/sets" @@ -16,26 +15,26 @@ const ( projectNameFmt = "projects/%s" ) -func (o *ClusterUninstaller) getProjectIAMPolicy(ctx context.Context) (*resourcemanager.Policy, error) { +func (o *ClusterUninstaller) getProjectIAMPolicy(ctx context.Context, projectID string) (*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(fmt.Sprintf(projectNameFmt, o.ProjectID), req).Context(ctx).Do() + policy, err := o.rmSvc.Projects.GetIamPolicy(fmt.Sprintf(projectNameFmt, projectID), req).Context(ctx).Do() if err != nil { - return nil, errors.Wrapf(err, "failed to fetch project IAM policy") + return nil, fmt.Errorf("failed to fetch project IAM policy in project %s: %w", projectID, err) } return policy, nil } -func (o *ClusterUninstaller) setProjectIAMPolicy(ctx context.Context, policy *resourcemanager.Policy) error { +func (o *ClusterUninstaller) setProjectIAMPolicy(ctx context.Context, projectID string, policy *resourcemanager.Policy) error { o.Logger.Debug("Setting project IAM policy") ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() req := &resourcemanager.SetIamPolicyRequest{Policy: policy} - _, err := o.rmSvc.Projects.SetIamPolicy(fmt.Sprintf(projectNameFmt, o.ProjectID), req).Context(ctx).Do() + _, err := o.rmSvc.Projects.SetIamPolicy(fmt.Sprintf(projectNameFmt, projectID), req).Context(ctx).Do() if err != nil { - return errors.Wrapf(err, "failed to set project IAM policy") + return fmt.Errorf("failed to set project IAM policy in project %s: %w", projectID, err) } return nil } diff --git a/pkg/destroy/gcp/serviceaccount.go b/pkg/destroy/gcp/serviceaccount.go index 67c27dbc803..611941373aa 100644 --- a/pkg/destroy/gcp/serviceaccount.go +++ b/pkg/destroy/gcp/serviceaccount.go @@ -27,6 +27,7 @@ func (o *ClusterUninstaller) listServiceAccounts(ctx context.Context) ([]cloudRe key: item.Name, name: item.Name, url: item.Email, + project: item.ProjectId, typeName: "serviceaccount", quota: []gcp.QuotaUsage{{ Metric: &gcp.Metric{ @@ -44,7 +45,7 @@ func (o *ClusterUninstaller) listClusterServiceAccount(ctx context.Context) ([]* ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() result := []*iam.ServiceAccount{} - req := o.iamSvc.Projects.ServiceAccounts.List(fmt.Sprintf("projects/%s", o.ProjectID)).Fields("accounts(name,displayName,email),nextPageToken") + req := o.iamSvc.Projects.ServiceAccounts.List(fmt.Sprintf("projects/%s", o.ProjectID)).Fields("accounts(name,displayName,email,projectId),nextPageToken") err := req.Pages(ctx, func(list *iam.ListServiceAccountsResponse) error { for idx, item := range list.Accounts { if o.isClusterResource(item.Email) || o.isClusterResource(item.DisplayName) { @@ -54,7 +55,7 @@ func (o *ClusterUninstaller) listClusterServiceAccount(ctx context.Context) ([]* return nil }) if err != nil { - return nil, errors.Wrapf(err, "failed to fetch service accounts") + return nil, fmt.Errorf("failed to fetch service accounts: %w", err) } return result, nil } @@ -84,22 +85,32 @@ func (o *ClusterUninstaller) destroyServiceAccounts(ctx context.Context) error { return nil } - // Remove service accounts from project policy - policy, err := o.getProjectIAMPolicy(ctx) - if err != nil { - return err - } emails := sets.NewString() for _, item := range items { emails.Insert(item.url) } - if o.clearIAMPolicyBindings(policy, emails, o.Logger) { - err = o.setProjectIAMPolicy(ctx, policy) + + projects := []string{o.ProjectID} + if o.NetworkProjectID != "" { + projects = append(projects, o.NetworkProjectID) + } + + // During XPN installs, permissions bindings occur in the host project. Those + // should be deleted here. + for _, project := range projects { + // Remove service accounts from project policy + policy, err := o.getProjectIAMPolicy(ctx, project) if err != nil { - o.errorTracker.suppressWarning("iampolicy", err, o.Logger) - return errors.Errorf("%d items pending", len(items)) + return err + } + if o.clearIAMPolicyBindings(policy, emails, o.Logger) { + err = o.setProjectIAMPolicy(ctx, project, policy) + if err != nil { + o.errorTracker.suppressWarning("iampolicy", err, o.Logger) + return errors.Errorf("%d items pending", len(items)) + } + o.Logger.Infof("Deleted IAM project role bindings") } - o.Logger.Infof("Deleted IAM project role bindings") } for _, item := range items {