Disallow vague escalated permissions on all service accounts (#546)

GoogleCloudPlatform · Feb 12, 2021 · 5bdaac8 · 5bdaac8
1 parent d9d3a6f
commit 5bdaac8
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/marketplace/deployer_util/config_helper.py b/marketplace/deployer_util/config_helper.py
@@ -168,14 +168,6 @@ def validate(self):
               'x-google-marketplace.images')
       if self._x_google_marketplace._deployer_service_account:
         self._x_google_marketplace._deployer_service_account.validate()
-        # Move to validate() once enforced on SERVICE_ACCOUNT properties as well.
-        if (self._x_google_marketplace._deployer_service_account
-            .has_discouraged_cluster_scoped_permissions()):
-          raise InvalidSchema(
-              'Disallowed deployerServiceAccount role(s): '
-              'For `ClusterRole` roles, only the "view" predefined role is '
-              'allowed. Instead, use a "CUSTOM" role with specific '
-              '"apiGroups" and/or "resources".')
 
     for _, p in self._properties.items():
       if p.xtype == XTYPE_SERVICE_ACCOUNT:
@@ -1040,6 +1032,12 @@ def validate(self):
           'explaining purpose and permission requirements. See docs: '
           'https://github.com/GoogleCloudPlatform/marketplace-k8s-app-tools/blob/master/docs/schema.md#type-service_account'
       )
+    if self.has_discouraged_cluster_scoped_permissions():
+      raise InvalidSchema(
+          'Disallowed service account role(s): '
+          'For `ClusterRole` roles, only the "view" predefined role is '
+          'allowed. Instead, use a "CUSTOM" role with specific '
+          '"apiGroups" and/or "resources".')
 
   def has_discouraged_cluster_scoped_permissions(self):
     """Returns true if the service account has discouraged permissions."""

diff --git a/marketplace/deployer_util/config_helper_test.py b/marketplace/deployer_util/config_helper_test.py
@@ -716,7 +716,7 @@ def test_deployer_service_account_cluster_scoped_write_predefined_role_enforced_
             type: string
         """)
     with self.assertRaisesRegex(config_helper.InvalidSchema,
-                                'Disallowed deployerServiceAccount role'):
+                                'Disallowed service account role'):
       schema.validate()
 
   def test_deployer_service_account_cluster_scoped_mock_cluster_admin_role_enforced_validate(
@@ -749,7 +749,7 @@ def test_deployer_service_account_cluster_scoped_mock_cluster_admin_role_enforce
             type: string
         """)
     with self.assertRaisesRegex(config_helper.InvalidSchema,
-                                'Disallowed deployerServiceAccount role'):
+                                'Disallowed service account role'):
       schema.validate()
 
   def test_deployer_service_account_no_escalated_permissions_allowed_validate(
@@ -938,6 +938,30 @@ def test_service_account_custom_empty_verbs(self):
                       verbs: ['']
           """)
 
+  def test_service_account_cluster_scoped_disallowed_permissions_enforced_validate(
+      self):
+    schema = config_helper.Schema.load_yaml("""
+        applicationApiVersion: v1beta1
+        properties:
+          sa:
+            type: string
+            x-google-marketplace:
+              type: SERVICE_ACCOUNT
+              serviceAccount:
+                description: >
+                  Asks for vague cluster-scoped permissions which is disallowed
+                roles:
+                - type: ClusterRole
+                  rulesType: CUSTOM
+                  rules:
+                  - apiGroups: ['*']
+                    resources: ['*']
+                    verbs: ['*']
+        """)
+    with self.assertRaisesRegex(config_helper.InvalidSchema,
+                                'Disallowed service account role'):
+      schema.validate()
+
   def test_storage_class(self):
     schema = config_helper.Schema.load_yaml("""
         properties: