Manage organization roles

hack/verify-yamllint.sh

@@ -18,9 +18,10 @@ set -o nounset
 set -o pipefail
 
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd )"
 
 pip=pip3
-pip_requirements="${SCRIPT_DIR}/requirements.txt"
+pip_requirements="${REPO_ROOT}/requirements.txt"
 yamllint_config="${SCRIPT_DIR}/.yamllint.conf"
 yamllint_version=$(<"${pip_requirements}" grep yamllint | sed -e 's/.*==//')
 
@@ -35,17 +36,16 @@ if [ $# != 0 ]; then
     exit 1
 fi
 
-if ! which yamllint >/dev/null 2>&1; then
-  echo >&2 "ERROR: yamllint not found - please install with: ${pip} install -r ${pip_requirements}"
+if ! command -v yamllint >/dev/null 2>&1; then
+  echo >/dev/stderr "ERROR: yamllint not found - please install with: ${pip} install -r ${pip_requirements}"
   exit 1
 fi
 
 version=$(yamllint --version | awk '{ print $2 }')
 if [[ "${version}" != "${yamllint_version}" ]]; then
-  echo >&2 "ERROR: incorrect yamllint version '${version}' - please install with: ${pip} install ${pip_requirements}"
+  echo >/dev/stderr "ERROR: incorrect yamllint version '${version}' - please install with: ${pip} install ${pip_requirements}"
   exit 1
 fi
 
-cd ${SCRIPT_DIR}/..
-
+cd "${REPO_ROOT}"
 yamllint -c "${yamllint_config}" .

infra/gcp/ensure-organization.sh

@@ -34,13 +34,21 @@ if [ $# != 0 ]; then
     exit 1
 fi
 
-# TODO: setup custom role StorageBucketLister, I don't see that defined in code
-# TODO: setup custom role CustomRole ("Billing Viewer"), I don't see that defined in code
+org_roles=(
+    prow.viewer
+    audit.viewer
+    secretmanager.secretLister
+    organization.admin
+    CustomRole
+    StorageBucketLister
+)
 
-## setup custom role for prow troubleshooting
-color 6 "Ensuring custom org role prow.viewer role exists"
+color 6 "Ensuring organization custom roles exist"
 (
-    ensure_custom_iam_role_from_file "org" "prow.viewer" "${SCRIPT_DIR}/roles/prow.viewer.yaml"
+    for role in "${org_roles[@]}"; do
+      color 6 "Ensuring organization custom role ${role}"
+      ensure_custom_iam_role_from_file "org" "${role}" "${SCRIPT_DIR}/roles/${role}.yaml"
+    done
 ) 2>&1 | indent
 
 color 6 "Ensuring org-level IAM bindings exist"
@@ -53,27 +61,46 @@ color 6 "Ensuring org-level IAM bindings exist"
     ensure_org_role_binding "group:gke-security-groups@kubernetes.io" "roles/browser"
 
     # k8s-infra-gcp-accounting@
-    # TODO: CustomRole is a brittle name, we should create a better named role,
-    #       or is there a reason we're not using predefined roles/billing.viewer?
     ensure_org_role_binding "group:k8s-infra-gcp-accounting@kubernetes.io" "$(custom_org_role_name "CustomRole")"
 
     # k8s-infra-gcp-auditors@
-    # TODO: this is what already exists, but it might be better to collapse this 
-    #       into a custom role, or use browser+viewer
-    audit_roles=(
-        $(custom_org_role_name "StorageBucketLister")
+    ensure_org_role_binding "group:k8s-infra-gcp-auditors@kubernetes.io" "$(custom_org_role_name "audit.viewer")"
+    # TODO(https://github.com/kubernetes/k8s.io/issues/1659): obviated by audit.viewer, remove when bindings gone
+    old_audit_roles=(
+        "$(custom_org_role_name "StorageBucketLister")"
         roles/compute.viewer
         roles/dns.reader
         roles/iam.securityReviewer
         roles/resourcemanager.organizationViewer
         roles/serviceusage.serviceUsageConsumer
     )
-    for role in "${audit_roles[@]}"; do
-        ensure_org_role_binding "group:k8s-infra-gcp-auditors@kubernetes.io" "${role}"
+    for role in "${old_audit_roles[@]}"; do
+        ensure_removed_org_role_binding "group:k8s-infra-gcp-auditors@kubernetes.io" "${role}"
     done
 
+    echo "exiting early to confirm audit.viewer role migration has worked"
+    exit 0
+
     # k8s-infra-org-admins@
-    # TODO: there are more granular roles also bound, they seem redundant given
-    #       this role
+    # roles/owner has too many permissions to aggregate into a custom role,
+    # and some services (e.g. storage) add bindings based on membership in it
     ensure_org_role_binding "group:k8s-infra-gcp-org-admins@kubernetes.io" "roles/owner"
+    # everything org admins need beyond roles/owner to manage the org
+    ensure_org_role_binding "group:k8s-infra-gcp-org-admins@kubernetes.io" "$(custom_org_role_name "organization.admin")"
+    # TODO(https://github.com/kubernetes/k8s.io/issues/1659): obviated by organization.admin, remove when bindings gone
+    old_org_admin_roles=(
+        roles/billing.user
+        roles/iam.organizationRoleAdmin
+        roles/resourcemanager.organizationAdmin
+        roles/resourcemanager.projectCreator
+        roles/resourcemanager.projectDeleter
+        roles/servicemanagement.quotaAdmin
+    )
+    for role in "${old_audit_roles[@]}"; do
+        # TODO(spiffxp): remove the extra super duper paranoia once we verify
+        #                I haven't locked myself out via group membership
+        ensure_org_role_binding "user:thockin@google.com" "${role}"
+        ensure_org_role_binding "user:davanum@gmail.com" "${role}"
+        ensure_removed_org_role_binding "group:k8s-infra-gcp-org-admins@kubernetes.io" "${role}"
+    done
 ) 2>&1 | indent

infra/gcp/lib_iam.sh

@@ -76,26 +76,39 @@ function ensure_custom_iam_role_from_file() {
     local scope="${1}"
     local name="${2}"
     local file="${3}"
+    local full_name="${name}"
 
     scope_flag=""
     if [[ "${scope}" == "org" ]]; then
         scope_flag="--organization ${GCP_ORG}" 
+        full_name="organizations/${GCP_ORG}/roles/${name}"
     elif [[ "${scope}" =~ "^project:" ]]; then
-        scope_flag="--project $(echo ${scope} | cut -d: -f2-)"
+        project=$(echo "${scope}" | cut -d: -f2-)
+        scope_flag="--project ${project}"
+        full_name="projects/${project}/roles/${name}"
     else
         echo "ensure_custom_iam_role_from_file(scope, name, file) scope must be one of 'org' or 'project:project-id'" >&2
         return 1
     fi
 
-    if ! gcloud iam roles describe ${scope_flag} "${name}" \
-        >/dev/null 2>&1
-    then
-        # be noisy when creating a role
-        gcloud iam roles create ${scope_flag} "${name}" --file "${file}"
-    else
-        # be quiet when updating, only output name of role
-        gcloud iam roles update ${scope_flag} "${name}" --file "${file}" | grep ^name:
+    tmp_dir=$(mktemp -d "/tmp/ensure-role-${name}-XXXXX")
+    trap 'rm -rf "${tmp_dir}"' EXIT
+    before="${tmp_dir}/before.${role}.yaml"
+    ready="${tmp_dir}/ready.${role}.yaml"
+    after="${tmp_dir}/after.${role}.yaml"
+
+    # detect if we should create or update and dump role; silently ignore error
+    verb="update"
+    if ! (gcloud iam roles describe ${scope_flag} "${name}" >"${before}") >/dev/null 2>&1; then
+      verb="create"
     fi
+
+    # name is foo.bar, but gcloud wants scope/id/role/foo.bar in the file
+    <"${file}" sed -e "s|^name: ${name}|name: ${full_name}|" >"${ready}"
+    gcloud iam roles "${verb}" ${scope_flag} "${name}" --file "${ready}" > "${after}"
+
+    # if they differ, ignore the error
+    diff "${before}" "${after}" || true
 }
 
 # Return the full name of a custom IAM role defined at the org level
@@ -153,7 +166,7 @@ function ensure_project_role_binding() {
         --role "${role}"
 }
 
-# Ensure that IAM binding has been removed at project level
+# Ensure that IAM binding has been removed from project
 # Arguments:
 #   $1:  The project id (e.g. "k8s-infra-foo")
 #   $2:  The principal (e.g. "group:k8s-infra-foo@kubernetes.io")
@@ -163,13 +176,31 @@ function ensure_removed_project_role_binding() {
         echo "ensure_removed_project_role_binding(project, principal, role) requires 3 arguments" >&2
         return 1
     fi
+
     local project="${1}"
     local principal="${2}"
     local role="${3}"
 
     _ensure_removed_resource_role_binding "projects" "${project}" "${principal}" "${role}"
 }
 
+# Ensure that IAM binding has been removed from organization
+# Arguments:
+#   $1:  The principal (e.g. "group:k8s-infra-foo@kubernetes.io")
+#   $2:  The role name (e.g. "roles/foo.bar")
+function ensure_removed_org_role_binding() {
+    if [ ! $# -eq 2 -o -z "$1" -o -z "$2" ]; then
+        echo "ensure_removed_org_role_binding(principal, role) requires 2 arguments" >&2
+        return 1
+    fi
+
+    local organization="${GCP_ORG}"
+    local principal="${1}"
+    local role="${2}"
+
+    _ensure_removed_resource_role_binding "organizations" "${organization}" "${principal}" "${role}"
+}
+
 # Ensure that IAM binding has been removed at resource level
 # Arguments:
 #   $1:  The resource type (e.g. "projects", "organizations", "secrets" )

infra/gcp/roles/CustomRole.yaml

@@ -0,0 +1,48 @@
+#### generated by generate-role-yaml.sh from the following spec:
+#
+# # This was retroactively put together to match a role that is already in
+# # production. It's not entirely clear why it was built this way, for example
+# # why all of the permission exclusions from roles/billing.viewer
+# #
+# # TODO: CustomRole is a brittle name, we should migrate to a better named role
+# title: Billing Viewer
+# description: View access to billing info
+# name: CustomRole
+# include:
+#   roles:
+#   - roles/billing.viewer
+#   - roles/browser
+#   permissions:
+#   # not sure what role this permission comes from
+#   - billing.resourceCosts.get
+# exclude:
+#   permissionRegexes:
+#   # unclear why the exclusion of billing.account.* except getSpendingInformation
+#   - ^billing.accounts.get$
+#   - ^billing.accounts.getPaymentInfo
+#   - ^billing.accounts.getUsageExportSpec
+#   - ^billing.accounts.list
+#   # unclear if these were intentionally excluded
+#   - ^consumerprocurement.
+#   - ^dataprocessing.
+#   - ^recommender.
+#   # we may want to allow these two for org hierarchy navigation
+#   - ^resourcemanager.folders.
+#   - ^resourcemanager.organizations.
+#   - getIamPolicy$
+#
+description: View access to billing info
+includedPermissions:
+  - billing.accounts.getSpendingInformation
+  - billing.budgets.get
+  - billing.budgets.list
+  - billing.credits.list
+  - billing.resourceAssociations.list
+  - billing.resourceCosts.get
+  - billing.subscriptions.get
+  - billing.subscriptions.list
+  - resourcemanager.projects.get
+  - resourcemanager.projects.list
+name: CustomRole
+stage: GA
+title: Billing Viewer

infra/gcp/roles/StorageBucketLister.yaml

@@ -0,0 +1,26 @@
+#### generated by generate-role-yaml.sh from the following spec:
+#
+# # allow listing of buckets
+# # TODO(https://github.com/kubernetes/k8s.io/issues/1659): remove once auditor.viewer is used instead
+# title: Storage Bucket Lister
+# description: Can list storage buckets
+# name: StorageBucketLister
+# include:
+#   roles:
+#   - roles/storage.admin
+#   permissionRegexes:
+#   - ^storage.buckets.
+# exclude:
+#   permissionRegexes:
+#   - create$
+#   - update$
+#   - delete$
+#   - IamPolicy$
+#
+description: Can list storage buckets
+includedPermissions:
+  - storage.buckets.get
+  - storage.buckets.list
+name: StorageBucketLister
+stage: GA
+title: Storage Bucket Lister