Merge pull request #8750 from bfournie/precreated-sas

CORS-3568: Support GCP pre-created Service Accounts for CAPG

data/data/install.openshift.io_installconfigs.yaml

@@ -582,11 +582,10 @@ spec:
                           type: string
                         serviceAccount:
                           description: ServiceAccount is the email of a gcp service
-                            account to be used for shared vpc installations. The provided
-                            service account will be attached to control-plane nodes
-                            in order to provide the permissions required by the cloud
-                            provider in the host project. This field is only supported
-                            in the control-plane machinepool.
+                            account to be used during installations. The provided
+                            service account can be attached to both control-plane
+                            nodes and worker nodes in order to provide the permissions
+                            required by the cloud provider.
                           type: string
                         tags:
                           description: Tags defines a set of network tags which will
@@ -1490,11 +1489,10 @@ spec:
                         type: string
                       serviceAccount:
                         description: ServiceAccount is the email of a gcp service
-                          account to be used for shared vpc installations. The provided
-                          service account will be attached to control-plane nodes
-                          in order to provide the permissions required by the cloud
-                          provider in the host project. This field is only supported
-                          in the control-plane machinepool.
+                          account to be used during installations. The provided service
+                          account can be attached to both control-plane nodes and
+                          worker nodes in order to provide the permissions required
+                          by the cloud provider.
                         type: string
                       tags:
                         description: Tags defines a set of network tags which will
@@ -3122,11 +3120,10 @@ spec:
                         type: string
                       serviceAccount:
                         description: ServiceAccount is the email of a gcp service
-                          account to be used for shared vpc installations. The provided
-                          service account will be attached to control-plane nodes
-                          in order to provide the permissions required by the cloud
-                          provider in the host project. This field is only supported
-                          in the control-plane machinepool.
+                          account to be used during installations. The provided service
+                          account can be attached to both control-plane nodes and
+                          worker nodes in order to provide the permissions required
+                          by the cloud provider.
                         type: string
                       tags:
                         description: Tags defines a set of network tags which will

pkg/asset/machines/gcp/gcpmachines.go

@@ -2,8 +2,6 @@
 package gcp
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
 
 	"github.com/sirupsen/logrus"
@@ -16,7 +14,6 @@ import (
 
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/installconfig"
-	gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
 	gcpconsts "github.com/openshift/installer/pkg/constants/gcp"
 	"github.com/openshift/installer/pkg/types"
 	gcptypes "github.com/openshift/installer/pkg/types/gcp"
@@ -160,45 +157,17 @@ func createGCPMachine(name string, installConfig *installconfig.InstallConfig, i
 		gcpMachine.Spec.ShieldedInstanceConfig = ptr.To(shieldedInstanceConfig)
 	}
 
+	serviceAccountEmail := gcptypes.GetConfiguredServiceAccount(installConfig.Config.Platform.GCP, mpool)
+	if serviceAccountEmail == "" {
+		serviceAccountEmail = gcptypes.GetDefaultServiceAccount(installConfig.Config.Platform.GCP, infraID, masterRole[0:1])
+	}
 	serviceAccount := &capg.ServiceAccount{
+		Email: serviceAccountEmail,
 		// Set scopes to value defined at
 		// https://cloud.google.com/compute/docs/access/service-accounts#scopes_best_practice
 		Scopes: []string{compute.CloudPlatformScope},
 	}
 
-	projectID := installConfig.Config.Platform.GCP.ProjectID
-	serviceAccount.Email = fmt.Sprintf("%s-%s@%s.iam.gserviceaccount.com", infraID, masterRole[0:1], projectID)
-	// The installer will create a service account for compute nodes with the above naming convention.
-	// The same service account will be used for control plane nodes during a vanilla installation. During a
-	// xpn installation, the installer will attempt to use an existing service account from a user supplied
-	// value in install-config.
-	// Note - the derivation of the ServiceAccount from credentials will no longer be supported.
-	if len(installConfig.Config.Platform.GCP.NetworkProjectID) > 0 {
-		serviceAccount.Email = mpool.ServiceAccount
-		if serviceAccount.Email == "" {
-			sess, err := gcpconfig.GetSession(context.TODO())
-			if err != nil {
-				return nil, fmt.Errorf("gcp machine creation failed to get session: %w", err)
-			}
-
-			// The JSON can be `nil` if auth is provided from env
-			// https://pkg.go.dev/golang.org/x/oauth2@v0.17.0/google#Credentials
-			if len(sess.Credentials.JSON) == 0 {
-				return nil, fmt.Errorf("could not extract service account from loaded credentials. Please specify a service account to be used for shared vpc installations in the install-config.yaml")
-			}
-
-			var found bool
-			sa := make(map[string]interface{})
-			err = json.Unmarshal(sess.Credentials.JSON, &sa)
-			if err != nil {
-				return nil, fmt.Errorf("failed to unmarshal gcp session: %w", err)
-			}
-			serviceAccount.Email, found = sa["client_email"].(string)
-			if !found {
-				return nil, fmt.Errorf("could not find google service account")
-			}
-		}
-	}
 	gcpMachine.Spec.ServiceAccount = serviceAccount
 
 	if mpool.OSDisk.EncryptionKey != nil {

pkg/asset/machines/gcp/gcpmachines_test.go

@@ -55,6 +55,16 @@ func Test_GenerateMachines(t *testing.T) {
 			installConfig:     getICWithSecureBoot(),
 			expectedGCPConfig: getGCPMachineWithSecureBoot(),
 		},
+		{
+			name:              "serviceaccount",
+			installConfig:     getICWithServiceAccount(),
+			expectedGCPConfig: getGCPMachineWithServiceAccount(),
+		},
+		{
+			name:              "serviceaccount-controlplane-machine",
+			installConfig:     getICWithServiceAccountControlPlaneMachine(),
+			expectedGCPConfig: getGCPMachineWithServiceAccountControlPlaneMachine(),
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -149,6 +159,26 @@ func getICWithSecureBoot() *installconfig.InstallConfig {
 	return ic
 }
 
+func getICWithServiceAccount() *installconfig.InstallConfig {
+	ic := getBaseInstallConfig()
+	ic.Config.Platform.GCP.DefaultMachinePlatform = &gcptypes.MachinePool{ServiceAccount: "user-service-account@some-project.iam.gserviceaccount.com"}
+	return ic
+}
+
+func getICWithServiceAccountControlPlaneMachine() *installconfig.InstallConfig {
+	ic := getBaseInstallConfig()
+	ic.Config.Platform.GCP.DefaultMachinePlatform = &gcptypes.MachinePool{ServiceAccount: "user-service-account@some-project.iam.gserviceaccount.com"}
+	ic.Config.ControlPlane = &types.MachinePool{
+		Name:     "master",
+		Replicas: ptr.To(int64(numReplicas)),
+		Platform: types.MachinePoolPlatform{
+			GCP: &gcptypes.MachinePool{
+				ServiceAccount: "other-service-account@some-project.iam.gserviceaccount.com"},
+		},
+	}
+	return ic
+}
+
 func getBaseGCPMachine() *capg.GCPMachine {
 	subnet := "012345678-master-subnet"
 	image := "rhcos-415-92-202311241643-0-gcp-x86-64"
@@ -209,6 +239,24 @@ func getGCPMachineWithSecureBoot() *capg.GCPMachine {
 	return gcpMachine
 }
 
+func getGCPMachineWithServiceAccount() *capg.GCPMachine {
+	gcpMachine := getBaseGCPMachine()
+	gcpMachine.Spec.ServiceAccount = &capg.ServiceAccount{
+		Email:  "user-service-account@some-project.iam.gserviceaccount.com",
+		Scopes: []string{compute.CloudPlatformScope},
+	}
+	return gcpMachine
+}
+
+func getGCPMachineWithServiceAccountControlPlaneMachine() *capg.GCPMachine {
+	gcpMachine := getBaseGCPMachine()
+	gcpMachine.Spec.ServiceAccount = &capg.ServiceAccount{
+		Email:  "other-service-account@some-project.iam.gserviceaccount.com",
+		Scopes: []string{compute.CloudPlatformScope},
+	}
+	return gcpMachine
+}
+
 func getBaseCapiMachine() *capi.Machine {
 	dataSecret := fmt.Sprintf("%s-master", "012345678")
 

pkg/asset/machines/gcp/machines.go

@@ -2,8 +2,6 @@
 package gcp
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
 	"sort"
 
@@ -15,7 +13,6 @@ import (
 	v1 "github.com/openshift/api/config/v1"
 	machinev1 "github.com/openshift/api/machine/v1"
 	machineapi "github.com/openshift/api/machine/v1beta1"
-	gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/gcp"
 )
@@ -154,37 +151,9 @@ func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool,
 		}
 	}
 
-	instanceServiceAccount := fmt.Sprintf("%s-%s@%s.iam.gserviceaccount.com", clusterID, role[0:1], platform.ProjectID)
-	// The installer will create a service account for compute nodes with the above naming convention.
-	// The same service account will be used for control plane nodes during a vanilla installation. During a
-	// xpn installation, the installer will attempt to use an existing service account either through the
-	// credentials or through a user supplied value from the install-config.
-	if role == "master" && len(platform.NetworkProjectID) > 0 {
-		instanceServiceAccount = mpool.ServiceAccount
-
-		if instanceServiceAccount == "" {
-			sess, err := gcpconfig.GetSession(context.TODO())
-			if err != nil {
-				return nil, err
-			}
-
-			// The JSON can be `nil` if auth is provided from env
-			// https://pkg.go.dev/golang.org/x/oauth2@v0.17.0/google#Credentials
-			if len(sess.Credentials.JSON) == 0 {
-				return nil, fmt.Errorf("could not extract service account from loaded credentials. Please specify a service account to be used for shared vpc installations in the install-config.yaml")
-			}
-
-			var found bool
-			serviceAccount := make(map[string]interface{})
-			err = json.Unmarshal(sess.Credentials.JSON, &serviceAccount)
-			if err != nil {
-				return nil, err
-			}
-			instanceServiceAccount, found = serviceAccount["client_email"].(string)
-			if !found {
-				return nil, errors.New("could not find google service account")
-			}
-		}
+	serviceAccountEmail := gcp.GetConfiguredServiceAccount(platform, mpool)
+	if serviceAccountEmail == "" {
+		serviceAccountEmail = gcp.GetDefaultServiceAccount(platform, clusterID, role[0:1])
 	}
 
 	shieldedInstanceConfig := machineapi.GCPShieldedInstanceConfig{}
@@ -225,7 +194,7 @@ func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool,
 			Subnetwork: subnetwork,
 		}},
 		ServiceAccounts: []machineapi.GCPServiceAccount{{
-			Email:  instanceServiceAccount,
+			Email:  serviceAccountEmail,
 			Scopes: []string{"https://www.googleapis.com/auth/cloud-platform"},
 		}},
 		Tags:                   append(mpool.Tags, []string{fmt.Sprintf("%s-%s", clusterID, role)}...),

pkg/infrastructure/gcp/clusterapi/clusterapi.go

@@ -46,14 +46,21 @@ func (Provider) BootstrapHasPublicIP() bool { return true }
 // created here using the gcp sdk.
 func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {
 	// Create ServiceAccounts which will be used for machines
-	projectID := in.InstallConfig.Config.Platform.GCP.ProjectID
-
-	// ServiceAccount for masters
-	// Only create ServiceAccount for masters if a shared VPC install is not being done
-	if len(in.InstallConfig.Config.Platform.GCP.NetworkProjectID) == 0 ||
-		(in.InstallConfig.Config.ControlPlane != nil &&
-			in.InstallConfig.Config.ControlPlane.Platform.GCP != nil &&
-			in.InstallConfig.Config.ControlPlane.Platform.GCP.ServiceAccount == "") {
+	platform := in.InstallConfig.Config.Platform.GCP
+	projectID := platform.ProjectID
+
+	// Only create ServiceAccounts for machines if a pre-created Service Account is not defined
+	controlPlaneMpool := &gcptypes.MachinePool{}
+	controlPlaneMpool.Set(in.InstallConfig.Config.GCP.DefaultMachinePlatform)
+	if in.InstallConfig.Config.ControlPlane != nil {
+		controlPlaneMpool.Set(in.InstallConfig.Config.ControlPlane.Platform.GCP)
+	}
+
+	if controlPlaneMpool.ServiceAccount != "" {
+		logrus.Debugf("Using pre-created ServiceAccount for control plane nodes")
+	} else {
+		// Create ServiceAccount for control plane nodes
+		logrus.Debugf("Creating ServiceAccount for control plane nodes")
 		masterSA, err := CreateServiceAccount(ctx, in.InfraID, projectID, "master")
 		if err != nil {
 			return fmt.Errorf("failed to create master serviceAccount: %w", err)
@@ -63,13 +70,24 @@ func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionIn
 		}
 	}
 
-	// ServiceAccount for workers
-	workerSA, err := CreateServiceAccount(ctx, in.InfraID, projectID, "worker")
-	if err != nil {
-		return fmt.Errorf("failed to create worker serviceAccount: %w", err)
+	createSA := false
+	for _, compute := range in.InstallConfig.Config.Compute {
+		computeMpool := compute.Platform.GCP
+		if gcptypes.GetConfiguredServiceAccount(platform, computeMpool) == "" {
+			// If any compute nodes aren't using defined service account then create the service account
+			createSA = true
+		}
 	}
-	if err = AddServiceAccountRoles(ctx, projectID, workerSA, GetWorkerRoles()); err != nil {
-		return fmt.Errorf("failed to add worker roles: %w", err)
+	if createSA {
+		// Create ServiceAccount for workers
+		logrus.Debugf("Creating ServiceAccount for compute nodes")
+		workerSA, err := CreateServiceAccount(ctx, in.InfraID, projectID, "worker")
+		if err != nil {
+			return fmt.Errorf("failed to create worker serviceAccount: %w", err)
+		}
+		if err = AddServiceAccountRoles(ctx, projectID, workerSA, GetWorkerRoles()); err != nil {
+			return fmt.Errorf("failed to add worker roles: %w", err)
+		}
 	}
 
 	return nil

pkg/types/gcp/machinepools.go

@@ -74,10 +74,9 @@ type MachinePool struct {
 	// +optional
 	ConfidentialCompute string `json:"confidentialCompute,omitempty"`
 
-	// ServiceAccount is the email of a gcp service account to be used for shared
-	// vpc installations. The provided service account will be attached to control-plane nodes
-	// in order to provide the permissions required by the cloud provider in the host project.
-	// This field is only supported in the control-plane machinepool.
+	// ServiceAccount is the email of a gcp service account to be used during installations.
+	// The provided service account can be attached to both control-plane nodes
+	// and worker nodes in order to provide the permissions required by the cloud provider.
 	//
 	// +optional
 	ServiceAccount string `json:"serviceAccount,omitempty"`

pkg/types/gcp/platform.go

@@ -114,3 +114,21 @@ type UserTag struct {
 func DefaultSubnetName(infraID, role string) string {
 	return fmt.Sprintf("%s-%s-subnet", infraID, role)
 }
+
+// GetConfiguredServiceAccount returns the service account email from a configured service account for
+// a control plane or compute node. Returns empty string if not configured.
+func GetConfiguredServiceAccount(platform *Platform, mpool *MachinePool) string {
+	if mpool != nil && mpool.ServiceAccount != "" {
+		return mpool.ServiceAccount
+	} else if platform.DefaultMachinePlatform != nil {
+		return platform.DefaultMachinePlatform.ServiceAccount
+	}
+
+	return ""
+}
+
+// GetDefaultServiceAccount returns the default service account email to use based on role.
+// The default should be used when an existing service account is not configured.
+func GetDefaultServiceAccount(platform *Platform, clusterID string, role string) string {
+	return fmt.Sprintf("%s-%s@%s.iam.gserviceaccount.com", clusterID, role[0:1], platform.ProjectID)
+}

pkg/types/gcp/validation/machinepool.go

@@ -54,20 +54,10 @@ func ValidateMachinePool(platform *gcp.Platform, p *gcp.MachinePool, fldPath *fi
 	return allErrs
 }
 
-// ValidateServiceAccount checks that the service account is only supplied for control plane nodes and during
-// a shared vpn installation.
+// ValidateServiceAccount does not do any checks on the service account since it can be set for all nodes and
+// in non-shared vpn installations.
 func ValidateServiceAccount(platform *gcp.Platform, p *types.MachinePool, fldPath *field.Path) field.ErrorList {
-	allErrs := field.ErrorList{}
-
-	if p.Platform.GCP.ServiceAccount != "" {
-		if p.Name != "master" {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceAccount"), p.Platform.GCP.ServiceAccount, fmt.Sprintf("service accounts only valid for master nodes, provided for %s nodes", p.Name)))
-		}
-		if platform.NetworkProjectID == "" {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceAccount"), p.Platform.GCP.ServiceAccount, "service accounts only valid for xpn installs"))
-		}
-	}
-	return allErrs
+	return field.ErrorList{}
 }
 
 // ValidateMasterDiskType checks that the specified disk type is valid for control plane.

pkg/types/validation/machinepools_test.go

@@ -204,7 +204,7 @@ func TestValidateMachinePool(t *testing.T) {
 				}
 				return p
 			}(),
-			valid: false,
+			valid: true,
 		},
 		{
 			name:     "invalid GCP service account non xpn install",
@@ -218,7 +218,7 @@ func TestValidateMachinePool(t *testing.T) {
 				}
 				return p
 			}(),
-			valid: false,
+			valid: true,
 		},
 	}
 	for _, tc := range cases {