** CAPG machines should also attept to use the default service account as a last resort. This happens when the user has started an install with the default service account.

** Add the missing firewall rules for etcd and health-checks.

Author: barbacbd

pkg/asset/machines/gcp/gcpmachines.go

@@ -2,6 +2,8 @@
 package gcp
 
 import (
+	"context"
+	"encoding/json"
 	"fmt"
 
 	"github.com/sirupsen/logrus"
@@ -14,6 +16,7 @@ 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"
@@ -49,7 +52,10 @@ func GenerateMachines(installConfig *installconfig.InstallConfig, infraID string
 	// Create GCP and CAPI machines for all master replicas in pool
 	for idx := int64(0); idx < total; idx++ {
 		name := fmt.Sprintf("%s-%s-%d", infraID, pool.Name, idx)
-		gcpMachine := createGCPMachine(name, installConfig, infraID, mpool, imageName)
+		gcpMachine, err := createGCPMachine(name, installConfig, infraID, mpool, imageName)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create control plane (%d): %w", idx, err)
+		}
 
 		result = append(result, &asset.RuntimeFile{
 			File:   asset.File{Filename: fmt.Sprintf("10_inframachine_%s.yaml", gcpMachine.Name)},
@@ -83,7 +89,10 @@ func GenerateBootstrapMachines(name string, installConfig *installconfig.Install
 	mpool := pool.Platform.GCP
 
 	// Create one GCP and CAPI machine for bootstrap
-	bootstrapGCPMachine := createGCPMachine(name, installConfig, infraID, mpool, imageName)
+	bootstrapGCPMachine, err := createGCPMachine(name, installConfig, infraID, mpool, imageName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create bootstrap machine: %w", err)
+	}
 
 	// Identify this as a bootstrap machine
 	bootstrapGCPMachine.Labels["install.openshift.io/bootstrap"] = ""
@@ -107,7 +116,7 @@ func GenerateBootstrapMachines(name string, installConfig *installconfig.Install
 }
 
 // Create a CAPG-specific machine.
-func createGCPMachine(name string, installConfig *installconfig.InstallConfig, infraID string, mpool *gcptypes.MachinePool, imageName string) *capg.GCPMachine {
+func createGCPMachine(name string, installConfig *installconfig.InstallConfig, infraID string, mpool *gcptypes.MachinePool, imageName string) (*capg.GCPMachine, error) {
 	// Use the rhcosImage as image name if not defined
 	osImage := imageName
 	if mpool.OSImage != nil {
@@ -165,8 +174,29 @@ func createGCPMachine(name string, installConfig *installconfig.InstallConfig, i
 	// 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 {
-		if mpool.ServiceAccount != "" {
-			serviceAccount.Email = mpool.ServiceAccount
+		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/[email protected]/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
@@ -184,7 +214,7 @@ func createGCPMachine(name string, installConfig *installconfig.InstallConfig, i
 		gcpMachine.Spec.RootDiskEncryptionKey = encryptionKey
 	}
 
-	return gcpMachine
+	return gcpMachine, nil
 }
 
 // Create a CAPI machine based on the CAPG machine.

pkg/asset/manifests/gcp/cluster.go

@@ -1,10 +1,14 @@
 package gcp
 
 import (
+	"context"
 	"fmt"
 	"net"
+	"time"
 
 	"github.com/apparentlymart/go-cidr/cidr"
+	"google.golang.org/api/compute/v1"
+	"google.golang.org/api/option"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -14,6 +18,7 @@ import (
 
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/installconfig"
+	gcpic "github.com/openshift/installer/pkg/asset/installconfig/gcp"
 	"github.com/openshift/installer/pkg/asset/manifests/capiutils"
 	gcpconsts "github.com/openshift/installer/pkg/constants/gcp"
 	"github.com/openshift/installer/pkg/types/gcp"
@@ -33,26 +38,42 @@ func GenerateClusterAssets(installConfig *installconfig.InstallConfig, clusterID
 		networkName = installConfig.Config.GCP.Network
 	}
 
-	masterSubnet := gcp.DefaultSubnetName(clusterID.InfraID, "master")
+	controlPlaneSubnetName := gcp.DefaultSubnetName(clusterID.InfraID, "master")
+	controlPlaneSubnetCidr := ""
 	if installConfig.Config.GCP.ControlPlaneSubnet != "" {
-		masterSubnet = installConfig.Config.GCP.ControlPlaneSubnet
+		controlPlaneSubnetName = installConfig.Config.GCP.ControlPlaneSubnet
+
+		controlPlaneSubnet, err := getSubnet(context.TODO(), installConfig.Config.GCP.NetworkProjectID, installConfig.Config.GCP.Region, controlPlaneSubnetName)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get control plane subnet: %w", err)
+		}
+		// IpCidr is the IPv4 version, the IPv6 version can be accessed as well
+		controlPlaneSubnetCidr = controlPlaneSubnet.IpCidrRange
 	}
 
-	master := capg.SubnetSpec{
-		Name:        masterSubnet,
-		CidrBlock:   "",
+	controlPlane := capg.SubnetSpec{
+		Name:        controlPlaneSubnetName,
+		CidrBlock:   controlPlaneSubnetCidr,
 		Description: ptr.To(description),
 		Region:      installConfig.Config.GCP.Region,
 	}
 
-	workerSubnet := gcp.DefaultSubnetName(clusterID.InfraID, "worker")
+	computeSubnetName := gcp.DefaultSubnetName(clusterID.InfraID, "worker")
+	computeSubnetCidr := ""
 	if installConfig.Config.GCP.ComputeSubnet != "" {
-		workerSubnet = installConfig.Config.GCP.ComputeSubnet
+		computeSubnetName = installConfig.Config.GCP.ComputeSubnet
+
+		computeSubnet, err := getSubnet(context.TODO(), installConfig.Config.GCP.NetworkProjectID, installConfig.Config.GCP.Region, computeSubnetName)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get compute subnet: %w", err)
+		}
+		// IpCidr is the IPv4 version, the IPv6 version can be accessed as well
+		computeSubnetCidr = computeSubnet.IpCidrRange
 	}
 
-	worker := capg.SubnetSpec{
-		Name:        workerSubnet,
-		CidrBlock:   "",
+	compute := capg.SubnetSpec{
+		Name:        computeSubnetName,
+		CidrBlock:   computeSubnetCidr,
 		Description: ptr.To(description),
 		Region:      installConfig.Config.GCP.Region,
 	}
@@ -79,18 +100,18 @@ func GenerateClusterAssets(installConfig *installconfig.InstallConfig, clusterID
 		if err != nil {
 			return nil, fmt.Errorf("failed to create the master subnet %w", err)
 		}
-		master.CidrBlock = masterCIDR.String()
+		controlPlane.CidrBlock = masterCIDR.String()
 	}
 
 	if installConfig.Config.GCP.ComputeSubnet == "" {
-		workerCIDR, err := cidr.Subnet(ipv4Net, 1, 1)
+		computeCIDR, err := cidr.Subnet(ipv4Net, 1, 1)
 		if err != nil {
-			return nil, fmt.Errorf("failed to create the worker subnet %w", err)
+			return nil, fmt.Errorf("failed to create the compute subnet %w", err)
 		}
-		worker.CidrBlock = workerCIDR.String()
+		compute.CidrBlock = computeCIDR.String()
 	}
 
-	subnets := []capg.SubnetSpec{master, worker}
+	subnets := []capg.SubnetSpec{controlPlane, compute}
 	// Subnets should never be auto created, even in shared VPC installs
 	autoCreateSubnets := false
 
@@ -126,6 +147,11 @@ func GenerateClusterAssets(installConfig *installconfig.InstallConfig, clusterID
 	}
 	gcpCluster.SetGroupVersionKind(capg.GroupVersion.WithKind("GCPCluster"))
 
+	// Set the network project during shared vpc installs
+	if installConfig.Config.GCP.NetworkProjectID != "" {
+		gcpCluster.Spec.Network.HostProject = ptr.To(installConfig.Config.GCP.NetworkProjectID)
+	}
+
 	manifests = append(manifests, &asset.RuntimeFile{
 		Object: gcpCluster,
 		File:   asset.File{Filename: "02_gcp-cluster.yaml"},
@@ -182,3 +208,30 @@ func findFailureDomains(installConfig *installconfig.InstallConfig) []string {
 
 	return zones.UnsortedList()
 }
+
+// getSubnet will find a subnet in a project by the name. The matching subnet structure will be returned if
+// one is found.
+func getSubnet(ctx context.Context, project, region, subnetName string) (*compute.Subnetwork, error) {
+	ctx, cancel := context.WithTimeout(ctx, time.Minute*1)
+	defer cancel()
+
+	ssn, err := gcpic.GetSession(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get session: %w", err)
+	}
+
+	computeService, err := compute.NewService(ctx, option.WithCredentials(ssn.Credentials))
+	if err != nil {
+		return nil, fmt.Errorf("failed to create compute service: %w", err)
+	}
+
+	subnetService := compute.NewSubnetworksService(computeService)
+	subnet, err := subnetService.Get(project, region, subnetName).Context(ctx).Do()
+	if err != nil {
+		return nil, fmt.Errorf("failed to find subnet %s: %w", subnetName, err)
+	} else if subnet == nil {
+		return nil, fmt.Errorf("subnet %s is empty", subnetName)
+	}
+
+	return subnet, nil
+}

pkg/infrastructure/gcp/clusterapi/clusterapi.go

@@ -51,7 +51,9 @@ func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionIn
 	// 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.Platform.GCP.ServiceAccount == "" {
+		(in.InstallConfig.Config.ControlPlane != nil &&
+			in.InstallConfig.Config.ControlPlane.Platform.GCP != nil &&
+			in.InstallConfig.Config.ControlPlane.Platform.GCP.ServiceAccount == "") {
 		masterSA, err := CreateServiceAccount(ctx, in.InfraID, projectID, "master")
 		if err != nil {
 			return fmt.Errorf("failed to create master serviceAccount: %w", err)
@@ -230,7 +232,11 @@ func (p Provider) DestroyBootstrap(ctx context.Context, in clusterapi.BootstrapD
 		return fmt.Errorf("failed to destroy storage: %w", err)
 	}
 
-	if err := removeBootstrapFirewallRules(ctx, in.Metadata.InfraID, in.Metadata.GCP.ProjectID); err != nil {
+	projectID := in.Metadata.GCP.ProjectID
+	if in.Metadata.GCP.NetworkProjectID != "" {
+		projectID = in.Metadata.GCP.NetworkProjectID
+	}
+	if err := removeBootstrapFirewallRules(ctx, in.Metadata.InfraID, projectID); err != nil {
 		return fmt.Errorf("failed to remove bootstrap firewall rules: %w", err)
 	}
 

pkg/infrastructure/gcp/clusterapi/firewallrules.go

@@ -11,6 +11,30 @@ import (
 	"github.com/openshift/installer/pkg/types"
 )
 
+func getEtcdPorts() []*compute.FirewallAllowed {
+	return []*compute.FirewallAllowed{
+		{
+			IPProtocol: "tcp",
+			Ports: []string{
+				"2379-2380",
+			},
+		},
+	}
+}
+
+func getHealthChecksPorts() []*compute.FirewallAllowed {
+	return []*compute.FirewallAllowed{
+		{
+			IPProtocol: "tcp",
+			Ports: []string{
+				"6080",
+				"6443",
+				"22624",
+			},
+		},
+	}
+}
+
 func getControlPlanePorts() []*compute.FirewallAllowed {
 	return []*compute.FirewallAllowed{
 		{
@@ -186,6 +210,9 @@ func deleteFirewallRule(ctx context.Context, name, projectID string) error {
 // createFirewallRules creates the rules needed between the worker and master nodes.
 func createFirewallRules(ctx context.Context, in clusterapi.InfraReadyInput, network string) error {
 	projectID := in.InstallConfig.Config.Platform.GCP.ProjectID
+	if in.InstallConfig.Config.Platform.GCP.NetworkProjectID != "" {
+		projectID = in.InstallConfig.Config.Platform.GCP.NetworkProjectID
+	}
 	workerTag := fmt.Sprintf("%s-worker", in.InfraID)
 	masterTag := fmt.Sprintf("%s-control-plane", in.InfraID)
 
@@ -198,6 +225,29 @@ func createFirewallRules(ctx context.Context, in clusterapi.InfraReadyInput, net
 		return err
 	}
 
+	// etcd are needed for master communication for etcd nodes
+	firewallName = fmt.Sprintf("%s-etcd", in.InfraID)
+	srcTags = []string{masterTag}
+	targetTags = []string{masterTag}
+	srcRanges = []string{}
+	if err := addFirewallRule(ctx, firewallName, network, projectID, getEtcdPorts(), srcTags, targetTags, srcRanges); err != nil {
+		return err
+	}
+
+	// Add a single firewall rule to allow the Google Cloud Engine health checks to access all of the services.
+	// This rule enables the ingress load balancers to determine the health status of their instances.
+	firewallName = fmt.Sprintf("%s-health-checks", in.InfraID)
+	srcTags = []string{}
+	targetTags = []string{masterTag}
+	srcRanges = []string{"35.191.0.0/16", "130.211.0.0/22"}
+	if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy {
+		// public installs require additional google ip addresses for health checks
+		srcRanges = append(srcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...)
+	}
+	if err := addFirewallRule(ctx, firewallName, network, projectID, getHealthChecksPorts(), srcTags, targetTags, srcRanges); err != nil {
+		return err
+	}
+
 	// internal-cluster rules are needed for worker<->master communication for k8s nodes
 	firewallName = fmt.Sprintf("%s-internal-cluster", in.InfraID)
 	srcTags = []string{workerTag, masterTag}
@@ -230,6 +280,9 @@ func createFirewallRules(ctx context.Context, in clusterapi.InfraReadyInput, net
 // createBootstrapFirewallRules creates the rules needed for the bootstrap node.
 func createBootstrapFirewallRules(ctx context.Context, in clusterapi.InfraReadyInput, network string) error {
 	projectID := in.InstallConfig.Config.Platform.GCP.ProjectID
+	if in.InstallConfig.Config.Platform.GCP.NetworkProjectID != "" {
+		projectID = in.InstallConfig.Config.Platform.GCP.NetworkProjectID
+	}
 	firewallName := fmt.Sprintf("%s-bootstrap-in-ssh", in.InfraID)
 	srcTags := []string{}
 	bootstrapTag := fmt.Sprintf("%s-control-plane", in.InfraID)

pkg/infrastructure/gcp/clusterapi/iam.go

@@ -148,6 +148,8 @@ func setPolicy(ctx context.Context, crmService *resourcemanager.Service, project
 
 // addMemberToRole adds a member to a role binding.
 func addMemberToRole(policy *resourcemanager.Policy, role, member string) error {
+	var policyBinding *resourcemanager.Binding
+
 	for _, binding := range policy.Bindings {
 		if binding.Role == role {
 			for _, m := range binding.Members {
@@ -156,11 +158,20 @@ func addMemberToRole(policy *resourcemanager.Policy, role, member string) error
 					return nil
 				}
 			}
-			binding.Members = append(binding.Members, member)
-			logrus.Debugf("found %s role, added %s member", role, member)
-			return nil
+			policyBinding = binding
 		}
 	}
 
-	return fmt.Errorf("role %s not found, member %s not added", role, member)
+	if policyBinding == nil {
+		policyBinding = &resourcemanager.Binding{
+			Role:    role,
+			Members: []string{member},
+		}
+		logrus.Debugf("creating new policy binding for %s role and %s member", role, member)
+		policy.Bindings = append(policy.Bindings, policyBinding)
+	}
+
+	policyBinding.Members = append(policyBinding.Members, member)
+	logrus.Debugf("adding %s role, added %s member", role, member)
+	return nil
 }