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
kubernetes · Apr 5, 2020 · daf16a2 · daf16a2
1 parent b3d65ff
commit daf16a2
Show file tree

Hide file tree

Showing 14 changed files with 99 additions and 101 deletions.
diff --git a/cmd/kops/create_cluster.go b/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"
 		}
 	}
 

diff --git a/cmd/kops/create_cluster_integration_test.go b/cmd/kops/create_cluster_integration_test.go
@@ -67,6 +67,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) {
 	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha_shared_zones", "v1alpha2")

diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/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:

diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go
diff --git a/pkg/model/gcemodel/autoscalinggroup.go b/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)

diff --git a/tests/integration/create_cluster/gce_byo_sa/expected-v1alpha2.yaml b/tests/integration/create_cluster/gce_byo_sa/expected-v1alpha2.yaml
@@ -2,37 +2,30 @@ apiVersion: kops.k8s.io/v1alpha2
 kind: Cluster
 metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
-  name: ha-gce.example.com
+  name: gce.example.com
 spec:
   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
+  gceServiceAccount: test-account@testproject.iam.gserviceaccount.com
   iam:
     allowContainerRegistry: true
     legacy: false
@@ -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
diff --git a/tests/integration/create_cluster/gce_byo_sa/options.yaml b/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:

diff --git a/tests/integration/create_cluster/ha_gce/expected-v1alpha2.yaml b/tests/integration/create_cluster/ha_gce/expected-v1alpha2.yaml
@@ -4,7 +4,6 @@ metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   name: ha-gce.example.com
 spec:
-  GCEServiceAccount: default
   api:
     dns: {}
   authorization:
@@ -34,6 +33,7 @@ spec:
       name: c
     memoryRequest: 100Mi
     name: events
+  gceServiceAccount: default
   iam:
     allowContainerRegistry: true
     legacy: false

diff --git a/tests/integration/update_cluster/ha_gce/in-v1alpha2.yaml b/tests/integration/update_cluster/ha_gce/in-v1alpha2.yaml
@@ -28,6 +28,7 @@ spec:
     - instanceGroup: master-us-test1-c
       name: "3"
     name: events
+  gceServiceAccount: default
   iam:
     legacy: false
   kubelet:

diff --git a/tests/integration/update_cluster/ha_gce/kubernetes.tf b/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"

diff --git a/tests/integration/update_cluster/minimal_gce/in-v1alpha2.yaml b/tests/integration/update_cluster/minimal_gce/in-v1alpha2.yaml
@@ -20,6 +20,7 @@ spec:
     - instanceGroup: master-us-test1-a
       name: "1"
     name: events
+  gceServiceAccount: default
   iam:
     legacy: false
   kubelet:

diff --git a/tests/integration/update_cluster/minimal_gce/kubernetes.tf b/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"]
   }