From af253c97022068c8c0500988b45e1ef0f727c7fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wiktor=20Niesiob=C4=99dzki?= <wiktorn@google.com>
Date: Sun, 12 May 2024 21:02:04 +0200
Subject: [PATCH] Fix 0-bootstrap iam_by_principals not taking into account all
 principals (#2267)

* Fix 0-bootstrap iam_by_principals not taking into account all principals
* Add test-case for iam_by_principals for 0-bootstrap stage

---------

Co-authored-by: Ludovico Magnocavallo <ludomagno@google.com>
---
 fast/stages/0-bootstrap/organization.tf       | 10 +++++++--
 tests/collectors.py                           |  2 +-
 .../s0_bootstrap/iam_by_principals.tfvars     | 20 +++++++++++++++++
 .../s0_bootstrap/iam_by_principals.yaml       | 22 +++++++++++++++++++
 tests/fast/stages/s0_bootstrap/tftest.yaml    |  2 ++
 5 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 tests/fast/stages/s0_bootstrap/iam_by_principals.tfvars
 create mode 100644 tests/fast/stages/s0_bootstrap/iam_by_principals.yaml

diff --git a/fast/stages/0-bootstrap/organization.tf b/fast/stages/0-bootstrap/organization.tf
index f91b4e8c..fdd08937 100644
--- a/fast/stages/0-bootstrap/organization.tf
+++ b/fast/stages/0-bootstrap/organization.tf
@@ -138,8 +138,14 @@ module "organization" {
   organization_id = module.organization-logging.id
   # human (groups) IAM bindings
   iam_by_principals = {
-    for k, v in local.iam_principals :
-    k => distinct(concat(v, lookup(var.iam_by_principals, k, [])))
+    for key in distinct(concat(
+      keys(local.iam_principals),
+      keys(var.iam_by_principals),
+    )) :
+    key => distinct(concat(
+      lookup(local.iam_principals, key, []),
+      lookup(var.iam_by_principals, key, []),
+    ))
   }
   # machine (service accounts) IAM bindings
   iam = merge(
diff --git a/tests/collectors.py b/tests/collectors.py
index 310b8151..9e26c067 100644
--- a/tests/collectors.py
+++ b/tests/collectors.py
@@ -92,7 +92,7 @@ class FabricTestItem(pytest.Item):
                              self.tf_var_files, self.extra_files)
     except AssertionError:
       def full_paths(x):
-        return [(self.parent.path.parent / x ) for x in x]
+        return [str(self.parent.path.parent / x ) for x in x]
       print(f'Error in inventory file: {" ".join(full_paths(self.inventory))}')
       print(f'To regenerate inventory run: python tools/plan_summary.py {self.module} {" ".join(full_paths(self.tf_var_files))}')
       raise
diff --git a/tests/fast/stages/s0_bootstrap/iam_by_principals.tfvars b/tests/fast/stages/s0_bootstrap/iam_by_principals.tfvars
new file mode 100644
index 00000000..4ceaffb6
--- /dev/null
+++ b/tests/fast/stages/s0_bootstrap/iam_by_principals.tfvars
@@ -0,0 +1,20 @@
+organization = {
+  domain      = "fast.example.com"
+  id          = 123456789012
+  customer_id = "C00000000"
+}
+billing_account = {
+  id = "000000-111111-222222"
+}
+essential_contacts = "gcp-organization-admins@fast.example.com"
+iam_by_principals = {
+  "user:other@fast.example.com" = ["roles/browser"]
+}
+prefix = "fast"
+org_policies_config = {
+  import_defaults = false
+}
+outputs_location = "/fast-config"
+groups = {
+  gcp-support = "group:gcp-support@example.com"
+}
diff --git a/tests/fast/stages/s0_bootstrap/iam_by_principals.yaml b/tests/fast/stages/s0_bootstrap/iam_by_principals.yaml
new file mode 100644
index 00000000..83b965d3
--- /dev/null
+++ b/tests/fast/stages/s0_bootstrap/iam_by_principals.yaml
@@ -0,0 +1,22 @@
+# Copyright 2024 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+values:
+  module.organization.google_organization_iam_binding.authoritative["roles/browser"]:
+    condition: []
+    members:
+    - domain:fast.example.com
+    - user:other@fast.example.com
+    org_id: '123456789012'
+    role: roles/browser
diff --git a/tests/fast/stages/s0_bootstrap/tftest.yaml b/tests/fast/stages/s0_bootstrap/tftest.yaml
index 8f415a21..2643714e 100644
--- a/tests/fast/stages/s0_bootstrap/tftest.yaml
+++ b/tests/fast/stages/s0_bootstrap/tftest.yaml
@@ -25,3 +25,5 @@ tests:
       - simple.yaml
       - simple_projects.yaml
       - simple_sas.yaml
+  
+  iam_by_principals: