Make OwnerRefs handle cleanup of deployer's RBAC resources (#460)

* Make OwnerRefs handle cleanup of deployer's RBAC resources * Fix a typo * Change 'deployer_name' var to 'deployer_service_account_name' * Add comment to clean_iam_resources to point to set_ownership
GoogleCloudPlatform · Jan 7, 2020 · 6860bf7 · 6860bf7
1 parent f0bd3d5
commit 6860bf7
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 29 deletions.
diff --git a/marketplace/deployer_util/clean_iam_resources.sh b/marketplace/deployer_util/clean_iam_resources.sh
@@ -19,12 +19,13 @@ set -eox pipefail
 [[ -z "$NAME" ]] && echo "NAME must be set" && exit 1
 [[ -z "$NAMESPACE" ]] && echo "NAMESPACE must be set" && exit 1
 
-# Delete the service account and RBAC objects by labels.
+# Delete the service account (which owns its RBAC objects) by label.
+# Update _DEPLOYER_OWNED_KINDS in set_ownership.py to delete additional
+# resource types besides Role and RoleBinding.
 # Note that only resources of a one-shot deployer have this label.
 # Resources of a KALM-managed deployer have
 # app.kubernetes.io/component=kalm.marketplace.cloud.google.com label.
-# TODO(#347): Only delete the ServiceAccount once it owns the Role{,Binding}
 kubectl delete --namespace="$NAMESPACE" \
-  ServiceAccount,Role,RoleBinding \
+  ServiceAccount \
   -l 'app.kubernetes.io/component'='deployer.marketplace.cloud.google.com','app.kubernetes.io/name'="$NAME" \
   --ignore-not-found
diff --git a/marketplace/deployer_util/magic_schema.sh b/marketplace/deployer_util/magic_schema.sh
@@ -39,7 +39,7 @@ namespace_key="$(/bin/extract_schema_key.py --type NAMESPACE)"
 # Provisioned manifests are inserted into the original schema
 # under top-level __manifests__ field.
 printf '{%s: "m4g1cn8m3", %s: "m4g1cn8m32p4c3"}' "${name_key}" "${namespace_key}" \
-  | /bin/provision.py --values_mode=stdin --deployer_image="${deployer}" \
+  | /bin/provision.py --values_mode=stdin --deployer_image="${deployer}" --deployer_service_account_name='m4g1cn8m3-deployer-sa' \
   | /bin/set_app_labels.py --manifests=- --dest=- --name="m4g1cn8m3" --namespace="m4g1cn8m32p4c3" \
   | /bin/yaml2json \
   | jq -s . \

diff --git a/marketplace/deployer_util/make_dns1123_name.py b/marketplace/deployer_util/make_dns1123_name.py
@@ -56,3 +56,7 @@ def limit_name(name, length=127):
     h4sh = m.hexdigest()[:4]
     result = '{}-{}'.format(result, h4sh)
   return result
+
+
+if __name__ == "__main__":
+  main()
diff --git a/marketplace/deployer_util/provision.py b/marketplace/deployer_util/provision.py
@@ -37,6 +37,7 @@ def main():
   schema_values_common.add_to_argument_parser(parser)
   parser.add_argument('--deployer_image', required=True)
   parser.add_argument('--deployer_entrypoint', default=None)
+  parser.add_argument('--deployer_service_account_name', required=True)
   parser.add_argument('--version_repo', default=None)
   parser.add_argument('--image_pull_secret', default=None)
   args = parser.parse_args()
@@ -49,12 +50,13 @@ def main():
       deployer_image=args.deployer_image,
       deployer_entrypoint=args.deployer_entrypoint,
       version_repo=args.version_repo,
-      image_pull_secret=args.image_pull_secret)
+      image_pull_secret=args.image_pull_secret,
+      deployer_service_account_name=args.deployer_service_account_name)
   print(yaml.safe_dump_all(manifests, default_flow_style=False, indent=2))
 
 
 def process(schema, values, deployer_image, deployer_entrypoint, version_repo,
-            image_pull_secret):
+            image_pull_secret, deployer_service_account_name):
   props = {}
   manifests = []
   app_name = get_name(schema, values)
@@ -126,7 +128,8 @@ def process(schema, values, deployer_image, deployer_entrypoint, version_repo,
         namespace=namespace,
         deployer_image=deployer_image,
         image_pull_secret=image_pull_secret,
-        app_params=app_params)
+        app_params=app_params,
+        deployer_service_account_name=deployer_service_account_name)
   else:
     manifests += provision_deployer(
         schema,
@@ -135,7 +138,8 @@ def process(schema, values, deployer_image, deployer_entrypoint, version_repo,
         deployer_image=deployer_image,
         deployer_entrypoint=deployer_entrypoint,
         image_pull_secret=image_pull_secret,
-        app_params=app_params)
+        app_params=app_params,
+        deployer_service_account_name=deployer_service_account_name)
   return manifests
 
 
@@ -164,13 +168,12 @@ def provision_from_storage(key, value, app_name, namespace):
 
 
 def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
-                   app_params, image_pull_secret):
+                   app_params, deployer_service_account_name,
+                   image_pull_secret):
   """Provisions KALM resource for installing the application."""
   if not version_repo:
     raise Exception('A valid --version_repo must be specified')
 
-  sa_name = dns1123_name('{}-deployer-sa'.format(app_name))
-
   labels = {
       'app.kubernetes.io/component': 'kalm.marketplace.cloud.google.com',
   }
@@ -209,7 +212,7 @@ def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
           'applicationRef': {
               'name': app_name,
           },
-          'serviceAccountName': sa_name,
+          'serviceAccountName': deployer_service_account_name,
           'valuesSecretRef': {
               'name': secret['metadata']['name']
           }
@@ -220,7 +223,7 @@ def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
       'apiVersion': 'v1',
       'kind': 'ServiceAccount',
       'metadata': {
-          'name': sa_name,
+          'name': deployer_service_account_name,
           'namespace': namespace,
           'labels': labels,
       },
@@ -231,8 +234,10 @@ def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
     }]
 
   role_binding = {
-      'apiVersion': 'rbac.authorization.k8s.io/v1',
-      'kind': 'RoleBinding',
+      'apiVersion':
+          'rbac.authorization.k8s.io/v1',
+      'kind':
+          'RoleBinding',
       'metadata': {
           'name': '{}-deployer-rb'.format(app_name),
           'namespace': namespace,
@@ -245,7 +250,7 @@ def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
       },
       'subjects': [{
           'kind': 'ServiceAccount',
-          'name': sa_name,
+          'name': deployer_service_account_name,
       },]
   }
 
@@ -259,9 +264,9 @@ def provision_kalm(schema, version_repo, app_name, namespace, deployer_image,
 
 
 def provision_deployer(schema, app_name, namespace, deployer_image,
-                       deployer_entrypoint, app_params, image_pull_secret):
+                       deployer_entrypoint, app_params,
+                       deployer_service_account_name, image_pull_secret):
   """Provisions resources to run the deployer."""
-  sa_name = dns1123_name('{}-deployer-sa'.format(app_name))
   labels = {
       'app.kubernetes.io/component': 'deployer.marketplace.cloud.google.com',
       'marketplace.cloud.google.com/deployer': 'Dependent',
@@ -276,7 +281,7 @@ def provision_deployer(schema, app_name, namespace, deployer_image,
                             app_params)
     pod_spec = {
         'serviceAccountName':
-            sa_name,
+            deployer_service_account_name,
         'containers': [{
             'name':
                 'deployer',
@@ -304,7 +309,7 @@ def provision_deployer(schema, app_name, namespace, deployer_image,
     config = make_v1_config(schema, namespace, app_name, labels, app_params)
     pod_spec = {
         'serviceAccountName':
-            sa_name,
+            deployer_service_account_name,
         'containers': [{
             'name':
                 'deployer',
@@ -334,7 +339,7 @@ def provision_deployer(schema, app_name, namespace, deployer_image,
       'apiVersion': 'v1',
       'kind': 'ServiceAccount',
       'metadata': {
-          'name': sa_name,
+          'name': deployer_service_account_name,
           'namespace': namespace,
           'labels': labels,
       },
@@ -369,7 +374,7 @@ def provision_deployer(schema, app_name, namespace, deployer_image,
       },
   ]
   manifests += make_deployer_rolebindings(schema, namespace, app_name, labels,
-                                          sa_name)
+                                          deployer_service_account_name)
   return manifests
 
 

diff --git a/marketplace/deployer_util/set_ownership.py b/marketplace/deployer_util/set_ownership.py
@@ -21,7 +21,7 @@
 import log_util as log
 
 from argparse import ArgumentParser
-from resources import set_app_resource_ownership
+from resources import set_app_resource_ownership, set_service_account_resource_ownership
 from yaml_util import load_resources_yaml
 from yaml_util import parse_resources_yaml
 
@@ -54,6 +54,7 @@
     "StorageClass",
     "VolumeAttachment",
 ]
+_DEPLOYER_OWNED_KINDS = ["Role", "RoleBinding"]
 
 
 def main():
@@ -66,6 +67,16 @@ def main():
       "--app_api_version",
       help="The apiVersion of the Application CRD",
       required=True)
+  parser.add_argument(
+      "--deployer_name",
+      help="The name of the deployer service account instance. "
+      "If deployer_uid is also set, the deployer service account is set "
+      "as the owner of namespaced deployer components.")
+  parser.add_argument(
+      "--deployer_uid",
+      help="The uid of the deployer service account instance. "
+      "If deployer_name is also set, the deployer service account is set "
+      "as the owner of namespaced deployer components.")
   parser.add_argument(
       "--manifests",
       help="The folder containing the manifest templates, "
@@ -118,7 +129,9 @@ def main():
         included_kinds,
         app_name=args.app_name,
         app_uid=args.app_uid,
-        app_api_version=args.app_api_version)
+        app_api_version=args.app_api_version,
+        deployer_name=args.deployer_name,
+        deployer_uid=args.deployer_uid)
     sys.stdout.flush()
   else:
     with open(args.dest, "w") as outfile:
@@ -128,11 +141,13 @@ def main():
           included_kinds,
           app_name=args.app_name,
           app_uid=args.app_uid,
-          app_api_version=args.app_api_version)
+          app_api_version=args.app_api_version,
+          deployer_name=args.deployer_name,
+          deployer_uid=args.deployer_uid)
 
 
-def dump(outfile, resources, included_kinds, app_name, app_uid,
-         app_api_version):
+def dump(outfile, resources, included_kinds, app_name, app_uid, app_api_version,
+         deployer_name, deployer_uid):
 
   def maybe_assign_ownership(resource):
     if resource["kind"] in _CLUSTER_SCOPED_KINDS:
@@ -151,11 +166,29 @@ def maybe_assign_ownership(resource):
           app_api_version=app_api_version,
           resource=resource)
 
+    if deployer_name and deployer_uid and should_be_deployer_owned(resource):
+      log.info("ServiceAccount '{:s}' owns '{:s}/{:s}'", deployer_name,
+               resource["kind"], resource["metadata"]["name"])
+      resource = copy.deepcopy(resource)
+      set_service_account_resource_ownership(
+          account_uid=deployer_uid,
+          account_name=deployer_name,
+          resource=resource)
+
     return resource
 
   to_be_dumped = [maybe_assign_ownership(resource) for resource in resources]
   yaml.safe_dump_all(to_be_dumped, outfile, default_flow_style=False, indent=2)
 
 
+def should_be_deployer_owned(resource):
+  if not resource["kind"] in _DEPLOYER_OWNED_KINDS:
+    return False
+  if resource.get("metadata", {}).get("labels", {}).get(
+      "app.kubernetes.io/component") != "deployer.marketplace.cloud.google.com":
+    return False
+  return True
+
+
 if __name__ == "__main__":
   main()
diff --git a/scripts/install b/scripts/install
@@ -114,14 +114,30 @@ app_uid=$(kubectl get "applications.app.k8s.io/$name" \
   --namespace="$namespace" \
   --output=jsonpath='{.metadata.uid}')
 
+# Create ServiceAccount instance.
+deployer_service_account_name="$(make_dns1123_name.py --name="${name}")-deployer-sa"
+kubectl apply --namespace="$namespace" --filename=- <<EOF
+apiVersion: "v1"
+kind: ServiceAccount
+metadata:
+  name: "${deployer_service_account_name}"
+  namespace: "${namespace}"
+EOF
+
+deployer_uid=$(kubectl get sa ${deployer_service_account_name} \
+  --namespace="$namespace" \
+  --output=jsonpath='{.metadata.uid}')
+
 # Provisions external resource dependencies and the deployer resources.
-# We set the application as the owner for all of the namespaced resources.
+# We set the application as the owner for all of the namespaced resources,
+# and the deployer as the owner of its dependent RBAC resources.
 manifest_dir="/tmp/${namespace}_${name}_$(date +'%Y-%m-%d_%H-%M-%S')"
 echo "${parameters}" \
   | provision.py \
     --values_mode=stdin \
     --deployer_image="${deployer}" \
     --deployer_entrypoint="${entrypoint}" \
+    --deployer_service_account_name="${deployer_service_account_name}" \
     --version_repo="${version_repo}" \
     --image_pull_secret="${image_pull_secret}" \
   | set_app_labels.py \
@@ -135,7 +151,9 @@ echo "${parameters}" \
     --noapp \
     --app_name="${name}" \
     --app_uid="${app_uid}" \
-    --app_api_version="${app_version}"
+    --app_api_version="${app_version}" \
+    --deployer_name="${deployer_service_account_name}" \
+    --deployer_uid="${deployer_uid}"
 
 echo "INFO: Applying the following manifests:"
 cat "${manifest_dir}"