Merge branch 'master' into add_skip_provisioners_variable_to_skip_local-exec

aaron-lane · web-flow · commit eb09369765e2 · 2019-10-18T21:12:35.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ Extending the adopted spec, each change should have a link to its corresponding
 ### Added
 
 * Added [private](https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/tree/master/modules/private-cluster-update-variant) and [beta private](https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/tree/master/modules/beta-private-cluster-update-variant) variants which allow node pools to be created before being destroyed. [#256]
+* Add a parameter `registry_project_id` to allow connecting to registries in other projects. [#273]
 
 ## [v5.0.0] - 2019-09-25
 v5.0.0 is a backwards-incompatible release. Please see the [upgrading guide](./docs/upgrading_to_v5.0.md).
@@ -204,6 +205,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 [v0.3.0]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/compare/v0.2.0...v0.3.0
 [v0.2.0]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/compare/v0.1.0...v0.2.0
 
+[#273]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/273
 [#247]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/247
 [#256]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/256
 [#248]: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/248
diff --git a/README.md b/README.md
@@ -167,6 +167,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | project\_id | The project ID to host the cluster in (required) | string | n/a | yes |
 | region | The region to host the cluster in (optional if zonal cluster / required if regional) | string | `"null"` | no |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
+| registry\_project\_id | Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project. | string | `""` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
 | service\_account | The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created. | string | `""` | no |
 | skip\_provisioners | Flag to skip all local-exec provisioners. It breaks down `stub_domains` and `upstream_nameservers` variables functionality. | bool | `"false"` | no |
@@ -229,6 +230,9 @@ following project roles:
 - roles/iam.serviceAccountUser
 - roles/resourcemanager.projectIamAdmin (only required if `service_account` is set to `create`)
 
+Additionally, if `service_account` is set to `create` and `grant_registry_access` is requested, the service account requires the following role on the `registry_project_id` project:
+- roles/resourcemanager.projectIamAdmin
+
 ### Enable APIs
 In order to operate with the Service Account you must activate the following APIs on the project where the Service Account was created:
 
diff --git a/autogen/README.md b/autogen/README.md
@@ -269,6 +269,9 @@ following project roles:
 - roles/iam.serviceAccountUser
 - roles/resourcemanager.projectIamAdmin (only required if `service_account` is set to `create`)
 
+Additionally, if `service_account` is set to `create` and `grant_registry_access` is requested, the service account requires the following role on the `registry_project_id` project:
+- roles/resourcemanager.projectIamAdmin
+
 ### Enable APIs
 In order to operate with the Service Account you must activate the following APIs on the project where the Service Account was created:
 
diff --git a/autogen/sa.tf b/autogen/sa.tf
@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"
 
 resource "google_project_iam_member" "cluster_service_account-gcr" {
   count   = var.create_service_account && var.grant_registry_access ? 1 : 0
-  project = var.project_id
+  project = var.registry_project_id == "" ? var.project_id : var.registry_project_id
   role    = "roles/storage.objectViewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
diff --git a/autogen/variables.tf b/autogen/variables.tf
@@ -270,6 +270,12 @@ variable "grant_registry_access" {
   default     = false
 }
 
+variable "registry_project_id" {
+  type        = string
+  description = "Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project."
+  default     = ""
+}
+
 variable "service_account" {
   type        = string
   description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."
diff --git a/examples/workload_metadata_config/main.tf b/examples/workload_metadata_config/main.tf
@@ -40,8 +40,9 @@ module "gke" {
   subnetwork              = var.subnetwork
   ip_range_pods           = var.ip_range_pods
   ip_range_services       = var.ip_range_services
-  create_service_account  = false
-  service_account         = var.compute_engine_service_account
+  create_service_account  = true
+  grant_registry_access   = true
+  registry_project_id     = var.registry_project_id
   enable_private_endpoint = true
   enable_private_nodes    = true
   master_ipv4_cidr_block  = "172.16.0.0/28"
diff --git a/examples/workload_metadata_config/variables.tf b/examples/workload_metadata_config/variables.tf
@@ -48,7 +48,6 @@ variable "ip_range_services" {
   description = "The secondary ip range to use for pods"
 }
 
-variable "compute_engine_service_account" {
-  description = "Service account to associate to the nodes in the cluster"
+variable "registry_project_id" {
+  description = "Project name for the GCR registry"
 }
-
diff --git a/modules/beta-private-cluster/README.md b/modules/beta-private-cluster/README.md
@@ -190,6 +190,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | project\_id | The project ID to host the cluster in (required) | string | n/a | yes |
 | region | The region to host the cluster in (optional if zonal cluster / required if regional) | string | `"null"` | no |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
+| registry\_project\_id | Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project. | string | `""` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
 | resource\_usage\_export\_dataset\_id | The dataset id for which network egress metering for this cluster will be enabled. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | string | `""` | no |
 | sandbox\_enabled | (Beta) Enable GKE Sandbox (Do not forget to set `image_type` = `COS_CONTAINERD` and `node_version` = `1.12.7-gke.17` or later to use it). | bool | `"false"` | no |
@@ -259,6 +260,9 @@ following project roles:
 - roles/iam.serviceAccountUser
 - roles/resourcemanager.projectIamAdmin (only required if `service_account` is set to `create`)
 
+Additionally, if `service_account` is set to `create` and `grant_registry_access` is requested, the service account requires the following role on the `registry_project_id` project:
+- roles/resourcemanager.projectIamAdmin
+
 ### Enable APIs
 In order to operate with the Service Account you must activate the following APIs on the project where the Service Account was created:
 
diff --git a/modules/beta-private-cluster/sa.tf b/modules/beta-private-cluster/sa.tf
@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"
 
 resource "google_project_iam_member" "cluster_service_account-gcr" {
   count   = var.create_service_account && var.grant_registry_access ? 1 : 0
-  project = var.project_id
+  project = var.registry_project_id == "" ? var.project_id : var.registry_project_id
   role    = "roles/storage.objectViewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
diff --git a/modules/beta-private-cluster/variables.tf b/modules/beta-private-cluster/variables.tf
@@ -268,6 +268,12 @@ variable "grant_registry_access" {
   default     = false
 }
 
+variable "registry_project_id" {
+  type        = string
+  description = "Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project."
+  default     = ""
+}
+
 variable "service_account" {
   type        = string
   description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."
diff --git a/modules/beta-public-cluster/README.md b/modules/beta-public-cluster/README.md
@@ -181,6 +181,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | project\_id | The project ID to host the cluster in (required) | string | n/a | yes |
 | region | The region to host the cluster in (optional if zonal cluster / required if regional) | string | `"null"` | no |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
+| registry\_project\_id | Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project. | string | `""` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
 | resource\_usage\_export\_dataset\_id | The dataset id for which network egress metering for this cluster will be enabled. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | string | `""` | no |
 | sandbox\_enabled | (Beta) Enable GKE Sandbox (Do not forget to set `image_type` = `COS_CONTAINERD` and `node_version` = `1.12.7-gke.17` or later to use it). | bool | `"false"` | no |
@@ -250,6 +251,9 @@ following project roles:
 - roles/iam.serviceAccountUser
 - roles/resourcemanager.projectIamAdmin (only required if `service_account` is set to `create`)
 
+Additionally, if `service_account` is set to `create` and `grant_registry_access` is requested, the service account requires the following role on the `registry_project_id` project:
+- roles/resourcemanager.projectIamAdmin
+
 ### Enable APIs
 In order to operate with the Service Account you must activate the following APIs on the project where the Service Account was created:
 
diff --git a/modules/beta-public-cluster/sa.tf b/modules/beta-public-cluster/sa.tf
@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"
 
 resource "google_project_iam_member" "cluster_service_account-gcr" {
   count   = var.create_service_account && var.grant_registry_access ? 1 : 0
-  project = var.project_id
+  project = var.registry_project_id == "" ? var.project_id : var.registry_project_id
   role    = "roles/storage.objectViewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
diff --git a/modules/beta-public-cluster/variables.tf b/modules/beta-public-cluster/variables.tf
@@ -268,6 +268,12 @@ variable "grant_registry_access" {
   default     = false
 }
 
+variable "registry_project_id" {
+  type        = string
+  description = "Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project."
+  default     = ""
+}
+
 variable "service_account" {
   type        = string
   description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."
diff --git a/modules/private-cluster/README.md b/modules/private-cluster/README.md
@@ -176,6 +176,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | project\_id | The project ID to host the cluster in (required) | string | n/a | yes |
 | region | The region to host the cluster in (optional if zonal cluster / required if regional) | string | `"null"` | no |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
+| registry\_project\_id | Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project. | string | `""` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
 | service\_account | The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created. | string | `""` | no |
 | skip\_provisioners | Flag to skip all local-exec provisioners. It breaks down `stub_domains` and `upstream_nameservers` variables functionality. | bool | `"false"` | no |
@@ -238,6 +239,9 @@ following project roles:
 - roles/iam.serviceAccountUser
 - roles/resourcemanager.projectIamAdmin (only required if `service_account` is set to `create`)
 
+Additionally, if `service_account` is set to `create` and `grant_registry_access` is requested, the service account requires the following role on the `registry_project_id` project:
+- roles/resourcemanager.projectIamAdmin
+
 ### Enable APIs
 In order to operate with the Service Account you must activate the following APIs on the project where the Service Account was created:
 
diff --git a/modules/private-cluster/sa.tf b/modules/private-cluster/sa.tf
@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"
 
 resource "google_project_iam_member" "cluster_service_account-gcr" {
   count   = var.create_service_account && var.grant_registry_access ? 1 : 0
-  project = var.project_id
+  project = var.registry_project_id == "" ? var.project_id : var.registry_project_id
   role    = "roles/storage.objectViewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
diff --git a/modules/private-cluster/variables.tf b/modules/private-cluster/variables.tf
@@ -258,6 +258,12 @@ variable "grant_registry_access" {
   default     = false
 }
 
+variable "registry_project_id" {
+  type        = string
+  description = "Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project."
+  default     = ""
+}
+
 variable "service_account" {
   type        = string
   description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."
diff --git a/sa.tf b/sa.tf
@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"
 
 resource "google_project_iam_member" "cluster_service_account-gcr" {
   count   = var.create_service_account && var.grant_registry_access ? 1 : 0
-  project = var.project_id
+  project = var.registry_project_id == "" ? var.project_id : var.registry_project_id
   role    = "roles/storage.objectViewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
diff --git a/test/fixtures/shared/outputs.tf b/test/fixtures/shared/outputs.tf
@@ -79,3 +79,6 @@ output "service_account" {
   value       = module.example.service_account
 }
 
+output "registry_project_id" {
+  value = var.registry_project_id
+}
diff --git a/test/fixtures/shared/variables.tf b/test/fixtures/shared/variables.tf
@@ -33,3 +33,7 @@ variable "compute_engine_service_account" {
   description = "The email address of the service account to associate with the GKE cluster"
 }
 
+variable "registry_project_id" {
+  description = "Project to use for granting access to the GCR registry, if requested"
+}
+
diff --git a/test/fixtures/workload_metadata_config/example.tf b/test/fixtures/workload_metadata_config/example.tf
@@ -17,13 +17,13 @@
 module "example" {
   source = "../../../examples/workload_metadata_config"
 
-  project_id                     = var.project_id
-  cluster_name_suffix            = "-${random_string.suffix.result}"
-  region                         = var.region
-  zones                          = slice(var.zones, 0, 1)
-  network                        = google_compute_network.main.name
-  subnetwork                     = google_compute_subnetwork.main.name
-  ip_range_pods                  = google_compute_subnetwork.main.secondary_ip_range[0].range_name
-  ip_range_services              = google_compute_subnetwork.main.secondary_ip_range[1].range_name
-  compute_engine_service_account = var.compute_engine_service_account
+  project_id          = var.project_id
+  cluster_name_suffix = "-${random_string.suffix.result}"
+  region              = var.region
+  zones               = slice(var.zones, 0, 1)
+  network             = google_compute_network.main.name
+  subnetwork          = google_compute_subnetwork.main.name
+  ip_range_pods       = google_compute_subnetwork.main.secondary_ip_range[0].range_name
+  ip_range_services   = google_compute_subnetwork.main.secondary_ip_range[1].range_name
+  registry_project_id = var.registry_project_id
 }
diff --git a/test/integration/workload_metadata_config/controls/gcloud.rb b/test/integration/workload_metadata_config/controls/gcloud.rb
@@ -13,8 +13,10 @@
 # limitations under the License.
 
 project_id = attribute('project_id')
+registry_project_id = attribute('registry_project_id')
 location = attribute('location')
 cluster_name = attribute('cluster_name')
+service_account = attribute('service_account')
 
 control "gcloud" do
   title "Google Compute Engine GKE configuration"
@@ -55,4 +57,20 @@
       end
     end
   end
+
+  describe command("gcloud projects get-iam-policy #{registry_project_id} --format=json") do
+    its(:exit_status) { should eq 0 }
+    its(:stderr) { should eq '' }
+
+    let!(:iam) do
+      if subject.exit_status == 0
+        JSON.parse(subject.stdout)
+      else
+        {}
+      end
+    end
+    it "has expected registry roles" do
+      expect(iam['bindings']).to include("members" => ["serviceAccount:#{service_account}"], "role" => "roles/storage.objectViewer")
+    end
+  end
 end
diff --git a/test/integration/workload_metadata_config/inspec.yml b/test/integration/workload_metadata_config/inspec.yml
@@ -9,3 +9,9 @@ attributes:
   - name: project_id
     required: true
     type: string
+  - name: service_account
+    required: true
+    type: string
+  - name: registry_project_id
+    required: false
+    type: string
diff --git a/test/setup/make_source.sh b/test/setup/make_source.sh
@@ -19,6 +19,9 @@ echo "#!/usr/bin/env bash" > ../source.sh
 project_id=$(terraform output project_id)
 echo "export TF_VAR_project_id='$project_id'" >> ../source.sh
 
+# We use the same project for registry project in the tests.
+echo "export TF_VAR_registry_project_id='$project_id'" >> ../source.sh
+
 sa_json=$(terraform output sa_key)
 # shellcheck disable=SC2086
 echo "export SERVICE_ACCOUNT_JSON='$(echo $sa_json | base64 --decode)'" >> ../source.sh
diff --git a/variables.tf b/variables.tf
@@ -258,6 +258,12 @@ variable "grant_registry_access" {
   default     = false
 }
 
+variable "registry_project_id" {
+  type        = string
+  description = "Project holding the Google Container Registry. If empty, we use the cluster project. If grant_registry_access is true, storage.objectViewer role is assigned on this project."
+  default     = ""
+}
+
 variable "service_account" {
   type        = string
   description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ resource "google_project_iam_member" "cluster_service_account-monitoring_viewer"`
`64`	`64`
`65`	`65`	`resource "google_project_iam_member" "cluster_service_account-gcr" {`
`66`	`66`	`count = var.create_service_account && var.grant_registry_access ? 1 : 0`
`67`		`- project = var.project_id`
	`67`	`+ project = var.registry_project_id == "" ? var.project_id : var.registry_project_id`
`68`	`68`	`role = "roles/storage.objectViewer"`
`69`	`69`	`member = "serviceAccount:${google_service_account.cluster_service_account[0].email}"`
`70`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,6 @@ variable "ip_range_services" {`
`48`	`48`	`description = "The secondary ip range to use for pods"`
`49`	`49`	`}`
`50`	`50`
`51`		`-variable "compute_engine_service_account" {`
`52`		`- description = "Service account to associate to the nodes in the cluster"`
	`51`	`+variable "registry_project_id" {`
	`52`	`+ description = "Project name for the GCR registry"`
`53`	`53`	`}`
`54`		`-`