Adds some initial tests. Fixes some logic

Need to fix service account implementation first

Fixing tests and iterating on the serviceaccount logic

Run the gce_byo_sa test

cmd/kops/create_cluster.go

@@ -994,12 +994,16 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 			}
 			cluster.Spec.Project = project
 		}
-		if c.GCEServiceAccount != "" {
-			klog.Infof("using GCE service account: %v", c.GCEServiceAccount)
-			cluster.Spec.GCEServiceAccount = fi.String(c.GCEServiceAccount)
-		} else {
-			klog.Warning("using GCE default service account")
-			cluster.Spec.GCEServiceAccount = fi.String("default")
+	}
+
+	if c.GCEServiceAccount != "" {
+		klog.Infof("VMs will be configured to use specified Service Account: %v", c.GCEServiceAccount)
+		cluster.Spec.GCEServiceAccount = c.GCEServiceAccount
+	} else {
+		if api.CloudProviderID(cluster.Spec.CloudProvider) == api.CloudProviderGCE {
+			klog.Warning("VMs will be configured to use the GCE default compute Service Account! This is an anti-pattern")
+			klog.Warning("Use a pre-create Service Account with the flag: --gce-service-account=account@projectname.iam.gserviceaccount.com")
+			cluster.Spec.GCEServiceAccount = "default"
 		}
 	}
 

cmd/kops/create_cluster_integration_test.go

@@ -70,6 +70,11 @@ func TestCreateClusterHAGCE(t *testing.T) {
 	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha_gce", "v1alpha2")
 }
 
+// TestCreateClusterGCE runs kops create cluster gce.example.com --cloud gce --zones us-test1-a --gce-service-account=test-account@testproject.iam.gserviceaccounts.com
+func TestCreateClusterGCE(t *testing.T) {
+	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/gce_byo_sa", "v1alpha2")
+}
+
 // TestCreateClusterHASharedZones tests kops create cluster when the master count is bigger than the number of zones
 func TestCreateClusterHASharedZones(t *testing.T) {
 	// Cannot be expressed in v1alpha1 API:	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha_shared_zones", "v1alpha1")

k8s/crds/kops.k8s.io_clusters.yaml

@@ -739,7 +739,7 @@ spec:
                 type: object
               type: array
             gceServiceAccount:
-              description: gceServiceAccount specifies the service account with which
+              description: GCEServiceAccount specifies the service account with which
                 the GCE VM runs
               type: string
             gossipConfig:

pkg/model/gcemodel/autoscalinggroup.go

@@ -139,7 +139,17 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
 				t.CanIPForward = fi.Bool(true)
 			}
 
-			t.ServiceAccount = b.Cluster.Spec.GCEServiceAccount
+			if b.Cluster.Spec.GCEServiceAccount != "" {
+				klog.Infof("VMs using Service Account: %v", b.Cluster.Spec.GCEServiceAccount)
+				// b.Cluster.Spec.GCEServiceAccount = c.GCEServiceAccount
+			} else {
+				klog.Warning("VMs will be configured to use the GCE default compute Service Account! This is an anti-pattern")
+				klog.Warning("Use a pre-created Service Account with the flag: --gce-service-account=account@projectname.iam.gserviceaccount.com")
+				b.Cluster.Spec.GCEServiceAccount = "default"
+			}
+
+			klog.Infof("gsa: %v", b.Cluster.Spec.GCEServiceAccount)
+			t.ServiceAccounts = []string{b.Cluster.Spec.GCEServiceAccount}
 			//labels, err := b.CloudTagsForInstanceGroup(ig)
 			//if err != nil {
 			//	return fmt.Errorf("error building cloud tags: %v", err)

tests/integration/create_cluster/gce_byo_sa/expected-v1alpha2.yaml

@@ -2,35 +2,28 @@ apiVersion: kops.k8s.io/v1alpha2
 kind: Cluster
 metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
-  name: ha-gce.example.com
+  name: gce.example.com
 spec:
+  GCEServiceAccount: test-account@testproject.iam.gserviceaccount.com
   api:
     dns: {}
   authorization:
     rbac: {}
   channel: stable
   cloudProvider: gce
-  configBase: memfs://tests/ha-gce.example.com
+  configBase: memfs://tests/gce.example.com
   containerRuntime: docker
   etcdClusters:
   - cpuRequest: 200m
     etcdMembers:
     - instanceGroup: master-us-test1-a
       name: a
-    - instanceGroup: master-us-test1-b
-      name: b
-    - instanceGroup: master-us-test1-c
-      name: c
     memoryRequest: 100Mi
     name: main
   - cpuRequest: 100m
     etcdMembers:
     - instanceGroup: master-us-test1-a
       name: a
-    - instanceGroup: master-us-test1-b
-      name: b
-    - instanceGroup: master-us-test1-c
-      name: c
     memoryRequest: 100Mi
     name: events
   iam:
@@ -41,7 +34,7 @@ spec:
   kubernetesApiAccess:
   - 0.0.0.0/0
   kubernetesVersion: v1.15.6-beta.1
-  masterPublicName: api.ha-gce.example.com
+  masterPublicName: api.gce.example.com
   networking:
     kubenet: {}
   nonMasqueradeCIDR: 100.64.0.0/10
@@ -65,7 +58,7 @@ kind: InstanceGroup
 metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   labels:
-    kops.k8s.io/cluster: ha-gce.example.com
+    kops.k8s.io/cluster: gce.example.com
   name: master-us-test1-a
 spec:
   image: cos-cloud/cos-stable-65-10323-99-0
@@ -88,53 +81,7 @@ kind: InstanceGroup
 metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   labels:
-    kops.k8s.io/cluster: ha-gce.example.com
-  name: master-us-test1-b
-spec:
-  image: cos-cloud/cos-stable-65-10323-99-0
-  machineType: n1-standard-1
-  maxSize: 1
-  minSize: 1
-  nodeLabels:
-    cloud.google.com/metadata-proxy-ready: "true"
-    kops.k8s.io/instancegroup: master-us-test1-b
-  role: Master
-  subnets:
-  - us-test1
-  zones:
-  - us-test1-b
-
----
-
-apiVersion: kops.k8s.io/v1alpha2
-kind: InstanceGroup
-metadata:
-  creationTimestamp: "2017-01-01T00:00:00Z"
-  labels:
-    kops.k8s.io/cluster: ha-gce.example.com
-  name: master-us-test1-c
-spec:
-  image: cos-cloud/cos-stable-65-10323-99-0
-  machineType: n1-standard-1
-  maxSize: 1
-  minSize: 1
-  nodeLabels:
-    cloud.google.com/metadata-proxy-ready: "true"
-    kops.k8s.io/instancegroup: master-us-test1-c
-  role: Master
-  subnets:
-  - us-test1
-  zones:
-  - us-test1-c
-
----
-
-apiVersion: kops.k8s.io/v1alpha2
-kind: InstanceGroup
-metadata:
-  creationTimestamp: "2017-01-01T00:00:00Z"
-  labels:
-    kops.k8s.io/cluster: ha-gce.example.com
+    kops.k8s.io/cluster: gce.example.com
   name: nodes
 spec:
   image: cos-cloud/cos-stable-65-10323-99-0
@@ -149,5 +96,3 @@ spec:
   - us-test1
   zones:
   - us-test1-a
-  - us-test1-b
-  - us-test1-c

tests/integration/create_cluster/gce_byo_sa/options.yaml

@@ -1,4 +1,4 @@
-ClusterName: ha-gce.example.com
+ClusterName: gce.example.com
 Zones:
 - us-test1-a
 MasterZones:

tests/integration/update_cluster/ha_gce/kubernetes.tf

@@ -397,6 +397,11 @@ resource "google_compute_instance_template" "master-us-test1-a-ha-gce-example-co
   can_ip_forward = true
   machine_type   = "n1-standard-1"
 
+  service_account = {
+    email  = "default"
+    scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_write", "https://www.googleapis.com/auth/ndev.clouddns.readwrite"]
+  }
+
   scheduling = {
     automatic_restart   = true
     on_host_maintenance = "MIGRATE"
@@ -434,6 +439,11 @@ resource "google_compute_instance_template" "master-us-test1-b-ha-gce-example-co
   can_ip_forward = true
   machine_type   = "n1-standard-1"
 
+  service_account = {
+    email  = "default"
+    scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_write", "https://www.googleapis.com/auth/ndev.clouddns.readwrite"]
+  }
+
   scheduling = {
     automatic_restart   = true
     on_host_maintenance = "MIGRATE"
@@ -471,6 +481,11 @@ resource "google_compute_instance_template" "master-us-test1-c-ha-gce-example-co
   can_ip_forward = true
   machine_type   = "n1-standard-1"
 
+  service_account = {
+    email  = "default"
+    scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_write", "https://www.googleapis.com/auth/ndev.clouddns.readwrite"]
+  }
+
   scheduling = {
     automatic_restart   = true
     on_host_maintenance = "MIGRATE"
@@ -508,6 +523,11 @@ resource "google_compute_instance_template" "nodes-ha-gce-example-com" {
   can_ip_forward = true
   machine_type   = "n1-standard-2"
 
+  service_account = {
+    email  = "default"
+    scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_only"]
+  }
+
   scheduling = {
     automatic_restart   = true
     on_host_maintenance = "MIGRATE"

tests/integration/update_cluster/minimal_gce/kubernetes.tf

@@ -298,6 +298,7 @@ resource "google_compute_instance_template" "master-us-test1-a-minimal-gce-examp
   machine_type   = "n1-standard-1"
 
   service_account = {
+    email  = "default"
     scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_write", "https://www.googleapis.com/auth/ndev.clouddns.readwrite"]
   }
 
@@ -339,6 +340,7 @@ resource "google_compute_instance_template" "nodes-minimal-gce-example-com" {
   machine_type   = "n1-standard-2"
 
   service_account = {
+    email  = "default"
     scopes = ["https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/monitoring", "https://www.googleapis.com/auth/logging.write", "https://www.googleapis.com/auth/devstorage.read_only"]
   }
 

upup/pkg/fi/cloudup/gcetasks/instancetemplate.go

@@ -58,8 +58,8 @@ type InstanceTemplate struct {
 	Subnet        *Subnet
 	AliasIPRanges map[string]string
 
-	Scopes         []string
-	ServiceAccount *string
+	Scopes          []string
+	ServiceAccounts []string
 
 	Metadata    map[string]*fi.ResourceHolder
 	MachineType *string
@@ -270,20 +270,32 @@ func (e *InstanceTemplate) mapToGCE(project string, region string) (*compute.Ins
 	}
 	networkInterfaces = append(networkInterfaces, ni)
 
-	var serviceAccounts []*compute.ServiceAccount
-	if e.ServiceAccount != nil {
-		scopes := make([]string, 0)
-		if e.Scopes != nil {
-			for _, s := range e.Scopes {
-				s = scopeToLongForm(s)
-				scopes = append(scopes, s)
-			}
+	scopes := make([]string, 0)
+	if e.Scopes != nil {
+		for _, s := range e.Scopes {
+			s = scopeToLongForm(s)
+			scopes = append(scopes, s)
 		}
-		serviceAccounts = append(serviceAccounts, &compute.ServiceAccount{
-			Email:  fi.StringValue(e.ServiceAccount),
+	}
+	serviceAccounts := []*compute.ServiceAccount{
+		{
+			Email:  e.ServiceAccounts[0],
 			Scopes: scopes,
-		})
+		},
 	}
+	// if e.ServiceAccounts != nil {
+	// 	for _, s := range e.ServiceAccounts {
+	// 		serviceAccounts = append(serviceAccounts, &compute.ServiceAccount{
+	// 			Email:  s,
+	// 			Scopes: scopes,
+	// 		})
+	// 	}
+	// } else {
+	// 	serviceAccounts = append(serviceAccounts, &compute.ServiceAccount{
+	// 		Email:  "default",
+	// 		Scopes: scopes,
+	// 	})
+	// }
 
 	var metadataItems []*compute.MetadataItems
 	for key, r := range e.Metadata {
@@ -425,6 +437,7 @@ type terraformInstanceCommon struct {
 }
 
 type terraformServiceAccount struct {
+	Email  string   `json:"email"`
 	Scopes []string `json:"scopes"`
 }
 
@@ -516,16 +529,23 @@ func (t *terraformInstanceCommon) AddMetadata(target *terraform.TerraformTarget,
 }
 
 func (t *terraformInstanceCommon) AddServiceAccounts(serviceAccounts []*compute.ServiceAccount) {
-	for _, g := range serviceAccounts {
-		for _, scope := range g.Scopes {
-			if t.ServiceAccount == nil {
-				t.ServiceAccount = &terraformServiceAccount{}
-			}
-			t.ServiceAccount.Scopes = append(t.ServiceAccount.Scopes, scope)
+	// there's an inconsistency here- GCP only lets you have one service account per VM
+	// terraform gets it right, but the golang api doesn't. womp womp :(
+	if len(serviceAccounts) != 1 {
+		klog.Fatal("Instances can only have 1 service account assigned.")
+	} else {
+		klog.Infof("adding csa: %v", serviceAccounts[0].Email)
+		csa := serviceAccounts[0]
+		tsa := &terraformServiceAccount{
+			Email:  csa.Email,
+			Scopes: csa.Scopes,
 		}
+		// for _, scope := range csa.Scopes {
+		// 	tsa.Scopes = append(tsa.Scopes, scope)
+		// }
+		t.ServiceAccount = tsa
 	}
 }
-
 func (_ *InstanceTemplate) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *InstanceTemplate) error {
 	project := t.Project