Merge pull request spotify#235 from spotify/lynn/fix-verify-roles

[cli] Log gcloud cmds to add required GCP roles to service accts

cli/src/klio_cli/commands/job/verify.py

@@ -24,15 +24,38 @@
 from googleapiclient import discovery
 from googleapiclient import errors as google_errors
 
+from klio_core import variables as var
+
 from klio_cli.utils import stackdriver_utils as sd_utils
 
 
 GCP_ROLE_METRIC_WRITER = "roles/monitoring.metricWriter"
+GCP_ROLE_PUBLISHER = "roles/pubsub.publisher"
+GCP_ROLE_SUBSCRIBER = "roles/pubsub.subscriber"
+GCP_ROLE_STORAGE_CREATOR = "roles/storage.objectCreator"
+GCP_ROLE_STORAGE_VIEWER = "roles/storage.objectViewer"
+GCP_ROLE_LOG_WRITER = "roles/logging.logWriter"
+GCP_ROLE_SERV_ACCT_USER = "roles/iam.serviceAccountUser"
 #
 # Add any extra roles we need to check on the default service account
 # to this set:
 #
-ROLES_TO_CHECK = {GCP_ROLE_METRIC_WRITER}
+BASE_ROLES_TO_CHECK = {
+    GCP_ROLE_METRIC_WRITER,
+    GCP_ROLE_STORAGE_CREATOR,
+    GCP_ROLE_STORAGE_VIEWER,
+}
+STREAMING_ROLES_TO_CHECK = {
+    *BASE_ROLES_TO_CHECK,
+    GCP_ROLE_PUBLISHER,
+    GCP_ROLE_SUBSCRIBER,
+}
+DIRECT_GKE_ROLES_TO_CHECK = {
+    *STREAMING_ROLES_TO_CHECK,
+    GCP_ROLE_LOG_WRITER,
+    GCP_ROLE_SERV_ACCT_USER,
+}
+
 
 logging.getLogger("googleapiclient.discovery").setLevel(logging.ERROR)
 
@@ -427,22 +450,42 @@ def _verify_iam_roles(self):
                 "unsafe project editor or owner permissions."
             )
 
-        if ROLES_TO_CHECK.issubset(svc_account_roles):
+        runner = self.klio_config.pipeline_options.runner
+        if runner == var.KlioRunner.DIRECT_GKE_RUNNER:
+            roles_to_check = DIRECT_GKE_ROLES_TO_CHECK
+        if self.klio_config.pipeline_options.streaming:
+            roles_to_check = STREAMING_ROLES_TO_CHECK
+        else:
+            roles_to_check = BASE_ROLES_TO_CHECK
+
+        if roles_to_check.issubset(svc_account_roles):
             logging.info(
                 "Verified that the service account has the required roles"
             )
             return True
 
-        logging.error(
-            "The default compute service account is missing the following IAM "
-            "roles: {}".format(ROLES_TO_CHECK - svc_account_roles)
+        missing_roles = roles_to_check - svc_account_roles
+        logging.warning(
+            "The configured compute service account is missing the following "
+            f"IAM role(s): {', '.join(missing_roles)}"
         )
         if self.create_resources:
-            logging.error(
-                "--create-resources is not able to add"
-                " these roles to the service "
-                "account at this time. Please add them manually via the Google "
-                "Cloud console."
+            gcloud_cmd_base = (
+                f"\tgcloud projects add-iam-policy-binding {self.project} \\\n"
+                f"\t\t--member={svc_account_string} \\\n"
+                "\t\t--role="
+            )
+            gcloud_cmds = []
+            for role in missing_roles:
+                cmd = gcloud_cmd_base + role
+                gcloud_cmds.append(cmd)
+
+            gcloud_cmds = "\n".join(gcloud_cmds)
+            logging.warning(
+                "Klio is unable to add the required role(s) to the service "
+                "account at this time. Add them with the following gcloud "
+                "command(s):\n"
+                f"{gcloud_cmds}"
             )
         return False
 

cli/tests/commands/job/test_verify.py

@@ -127,7 +127,23 @@ def mock_create_sd_group(mocker):
                 {
                     "role": "roles/monitoring.metricWriter",
                     "members": ["serviceAccount:the-default-svc-account"],
-                }
+                },
+                {
+                    "role": "roles/pubsub.publisher",
+                    "members": ["serviceAccount:the-default-svc-account"],
+                },
+                {
+                    "role": "roles/pubsub.subscriber",
+                    "members": ["serviceAccount:the-default-svc-account"],
+                },
+                {
+                    "role": "roles/storage.objectCreator",
+                    "members": ["serviceAccount:the-default-svc-account"],
+                },
+                {
+                    "role": "roles/storage.objectViewer",
+                    "members": ["serviceAccount:the-default-svc-account"],
+                },
             ],
             False,
         ),
@@ -185,7 +201,7 @@ def test_verify_iam_roles(
     result = job._verify_iam_roles()
     if create_resources:
         assert (
-            "--create-resources is not able to add these roles"
+            "Klio is unable to add the required role(s)"
             in caplog.records[-1].msg
         )
 
@@ -204,6 +220,22 @@ def test_verify_iam_roles_editor(caplog, klio_config, mock_discovery_client):
             "role": "roles/monitoring.metricWriter",
             "members": ["serviceAccount:the-default-svc-account"],
         },
+        {
+            "role": "roles/pubsub.publisher",
+            "members": ["serviceAccount:the-default-svc-account"],
+        },
+        {
+            "role": "roles/pubsub.subscriber",
+            "members": ["serviceAccount:the-default-svc-account"],
+        },
+        {
+            "role": "roles/storage.objectCreator",
+            "members": ["serviceAccount:the-default-svc-account"],
+        },
+        {
+            "role": "roles/storage.objectViewer",
+            "members": ["serviceAccount:the-default-svc-account"],
+        },
         {
             "role": "roles/editor",
             "members": ["serviceAccount:the-default-svc-account"],
@@ -248,6 +280,22 @@ def test_verify_iam_roles_with_svc_account(klio_config, mock_discovery_client):
             "role": "roles/monitoring.metricWriter",
             "members": ["serviceAccount:[email protected]"],
         },
+        {
+            "role": "roles/pubsub.publisher",
+            "members": ["serviceAccount:[email protected]"],
+        },
+        {
+            "role": "roles/pubsub.subscriber",
+            "members": ["serviceAccount:[email protected]"],
+        },
+        {
+            "role": "roles/storage.objectCreator",
+            "members": ["serviceAccount:[email protected]"],
+        },
+        {
+            "role": "roles/storage.objectViewer",
+            "members": ["serviceAccount:[email protected]"],
+        },
         {
             "role": "roles/editor",
             "members": ["serviceAccount:the-default-svc-account"],

docs/src/reference/cli/changelog.rst

@@ -8,10 +8,16 @@ CLI Changelog
 
 .. start-21.10.0
 
+Added
+*****
+
+* Include more GCP roles when verifying a job's service account in ``klio job verify``.
+
 Fixed
 *****
 
 * Correctly validate existence of Dataflow-related Klio config when running on Dataflow (and not just "not --direct-runner").
+* Print out ``gcloud`` commands to add necessary GCP roles when running ``klio job verify --create-resources``.
 
 Changed
 *******