From d3bd3ee2364d85bb8509b2f697c99f940419213c Mon Sep 17 00:00:00 2001
From: Matt Cary <34742400+mattcary@users.noreply.github.com>
Date: Tue, 3 Mar 2020 16:12:08 -0800
Subject: [PATCH] fix: Add dependency on service enablement. (#387)

* Add dependency on service enablement.

Adds a dependency on project services to the output project id and
number.  This prevents a race for using a robot account based on the
project id or number, as the robot accounts can be created only when
the service enabling is finished.

* Improve formatting

* Add binding dependency test to minimal

* Ran make generate_docs

Co-authored-by: Morgante Pell <morgante.pell@morgante.net>
---
 modules/core_project_factory/outputs.tf      |  4 ++-
 test/fixtures/minimal/README.md              |  1 +
 test/fixtures/minimal/main.tf                |  8 +++++
 test/fixtures/minimal/outputs.tf             |  4 +++
 test/integration/minimal/controls/minimal.rb | 31 ++++++++++++++++----
 test/integration/minimal/inspec.yml          |  3 ++
 6 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/modules/core_project_factory/outputs.tf b/modules/core_project_factory/outputs.tf
index 8595d5a8..8943e22e 100644
--- a/modules/core_project_factory/outputs.tf
+++ b/modules/core_project_factory/outputs.tf
@@ -26,10 +26,12 @@ output "project_id" {
     ),
     0,
   )
+  depends_on = [module.project_services]
 }
 
 output "project_number" {
-  value = google_project.main.number
+  value      = google_project.main.number
+  depends_on = [module.project_services]
 }
 
 output "service_account_id" {
diff --git a/test/fixtures/minimal/README.md b/test/fixtures/minimal/README.md
index e013f3f9..b5cf1e1f 100644
--- a/test/fixtures/minimal/README.md
+++ b/test/fixtures/minimal/README.md
@@ -15,6 +15,7 @@
 | Name | Description |
 |------|-------------|
 | compute\_service\_account\_email |  |
+| container\_service\_account\_email |  |
 | group\_email |  |
 | project\_id |  |
 | project\_name |  |
diff --git a/test/fixtures/minimal/main.tf b/test/fixtures/minimal/main.tf
index 5e86484a..0c0bd8e5 100644
--- a/test/fixtures/minimal/main.tf
+++ b/test/fixtures/minimal/main.tf
@@ -47,3 +47,11 @@ module "project-factory" {
   default_service_account     = "disable"
   disable_services_on_destroy = "false"
 }
+
+// Add a binding to the container service robot account to test that the
+// dependency on that service is correctly sequenced.
+resource "google_project_iam_member" "iam-binding" {
+  project = module.project-factory.project_id
+  role    = "roles/container.developer"
+  member  = "serviceAccount:service-${module.project-factory.project_number}@container-engine-robot.iam.gserviceaccount.com"
+}
diff --git a/test/fixtures/minimal/outputs.tf b/test/fixtures/minimal/outputs.tf
index 267e94d5..3b6f090b 100644
--- a/test/fixtures/minimal/outputs.tf
+++ b/test/fixtures/minimal/outputs.tf
@@ -34,6 +34,10 @@ output "compute_service_account_email" {
   value = "${module.project-factory.project_number}-compute@developer.gserviceaccount.com"
 }
 
+output "container_service_account_email" {
+  value = "service-${module.project-factory.project_number}@container-engine-robot.iam.gserviceaccount.com"
+}
+
 output "group_email" {
   value = module.project-factory.group_email
 }
diff --git a/test/integration/minimal/controls/minimal.rb b/test/integration/minimal/controls/minimal.rb
index c16483fc..954b244b 100644
--- a/test/integration/minimal/controls/minimal.rb
+++ b/test/integration/minimal/controls/minimal.rb
@@ -12,11 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-project_id                    = attribute('project_id')
-service_account_email         = attribute('service_account_email')
-compute_service_account_email = attribute('compute_service_account_email')
-group_email                   = attribute('group_email')
-group_name                    = attribute('group_name')
+project_id                      = attribute('project_id')
+service_account_email           = attribute('service_account_email')
+compute_service_account_email   = attribute('compute_service_account_email')
+container_service_account_email = attribute('container_service_account_email')
+group_email                     = attribute('group_email')
+group_name                      = attribute('group_name')
 
 control 'project-factory-minimal' do
   title 'Project Factory minimal configuration'
@@ -86,4 +87,24 @@
       expect(group_email).to be_empty
     end
   end
+
+  describe command("gcloud projects get-iam-policy #{project_id} --format=json") do
+    its('exit_status') { should eq 0 }
+    its('stderr') { should eq '' }
+
+    let(:bindings) do
+      if subject.exit_status == 0
+        JSON.parse(subject.stdout, symbolize_names: true)[:bindings]
+      else
+        []
+      end
+    end
+
+    it "container.developer role has been given to #{container_service_account_email}" do
+      expect(bindings).to include(
+        members: including("serviceAccount:#{container_service_account_email}"),
+        role: "roles/container.developer",
+      )
+    end
+  end
 end
diff --git a/test/integration/minimal/inspec.yml b/test/integration/minimal/inspec.yml
index 7026fd0f..d9fc6161 100644
--- a/test/integration/minimal/inspec.yml
+++ b/test/integration/minimal/inspec.yml
@@ -12,6 +12,9 @@ attributes:
   - name: compute_service_account_email
     required: true
 
+  - name: container_service_account_email
+    required: true
+
   - name: group_email
     required: true