From ff262212fd3e41f184f1c9ebcd90e11f5b594659 Mon Sep 17 00:00:00 2001 From: Michelle Nguyen Date: Tue, 27 Sep 2022 11:11:06 -0700 Subject: [PATCH] Tighten Vizier clusterroles/roles Summary: Some of our clusterroles were overly permissive. This diff updates the clusterroles and creates some new namespace-scoped roles for the various Vizier service accounts. Test Plan: - Deploy fresh Vizier - Update existing Vizier - Update existing non-operator Vizier Reviewers: vihang, zasgar Reviewed By: vihang Signed-off-by: Michelle Nguyen Differential Revision: https://phab.corp.pixielabs.ai/D12291 GitOrigin-RevId: c0b3b187190121b94e29a23c7872b1f26fb52eca --- k8s/operator/helm/templates/deleter_role.yaml | 41 +++++++++++---- k8s/vizier/base/metadata_role.yaml | 28 +++++++++- .../bootstrap/cloud_connector_role.yaml | 52 +++++++++++++++---- k8s/vizier/bootstrap/updater_role.yaml | 52 +++++++++++++------ .../vizier_yamls/vizier_yamls.go | 18 +++++-- 5 files changed, 152 insertions(+), 39 deletions(-) diff --git a/k8s/operator/helm/templates/deleter_role.yaml b/k8s/operator/helm/templates/deleter_role.yaml index b51227347e5..73e5ec7e481 100644 --- a/k8s/operator/helm/templates/deleter_role.yaml +++ b/k8s/operator/helm/templates/deleter_role.yaml @@ -7,7 +7,7 @@ metadata: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: pl-deleter-binding + name: pl-deleter-cluster-binding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole @@ -20,21 +20,34 @@ subjects: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: pl-deleter-role + name: pl-deleter-cluster-role rules: # Allow actions on Kubernetes objects +- apiGroups: + - rbac.authorization.k8s.io + - etcd.database.coreos.com + - nats.io + resources: + - clusterroles + - clusterrolebindings + - persistentvolumes + - etcdclusters + - natsclusters + verbs: ["*"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: pl-deleter-role +rules: - apiGroups: - "" - apps - rbac.authorization.k8s.io - extensions - - etcd.database.coreos.com - batch - - nats.io - policy resources: - - clusterroles - - clusterrolebindings - configmaps - secrets - pods @@ -42,13 +55,23 @@ rules: - deployments - daemonsets - persistentvolumes - - persistentvolumeclaims - roles - rolebindings - serviceaccounts - - etcdclusters - statefulsets - cronjobs - jobs - - natsclusters verbs: ["*"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: pl-deleter-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: pl-deleter-role +subjects: +- kind: ServiceAccount + name: pl-deleter-service-account + namespace: "{{ .Release.Namespace }}" diff --git a/k8s/vizier/base/metadata_role.yaml b/k8s/vizier/base/metadata_role.yaml index 803d042668e..f8cf4848f97 100644 --- a/k8s/vizier/base/metadata_role.yaml +++ b/k8s/vizier/base/metadata_role.yaml @@ -20,7 +20,9 @@ rules: - replicasets - deployments verbs: - - "*" + - "watch" + - "get" + - "list" --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -60,3 +62,27 @@ subjects: - kind: ServiceAccount name: metadata-service-account namespace: pl +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: pl-vizier-metadata-role +rules: +- apiGroups: + - "" + resources: + - endpoints + verbs: ["*"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: pl-vizier-metadata-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: pl-vizier-metadata-role +subjects: +- kind: ServiceAccount + name: metadata-service-account + namespace: pl diff --git a/k8s/vizier/bootstrap/cloud_connector_role.yaml b/k8s/vizier/bootstrap/cloud_connector_role.yaml index a500c12ace3..6ba80e88b9e 100644 --- a/k8s/vizier/bootstrap/cloud_connector_role.yaml +++ b/k8s/vizier/bootstrap/cloud_connector_role.yaml @@ -6,6 +6,40 @@ metadata: --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole +metadata: + name: pl-cloud-connector-cluster-role +rules: +- apiGroups: + - "" + resources: + - nodes + verbs: + - "get" + - "watch" + - "list" +- apiGroups: + - "" + resources: + - namespaces + verbs: + - "get" + resourceNames: + - "kube-system" +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: pl-cloud-connector-cluster-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: pl-cloud-connector-cluster-role +subjects: +- kind: ServiceAccount + name: cloud-conn-service-account +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role metadata: name: pl-cloud-connector-role rules: @@ -13,17 +47,14 @@ rules: - "" - px.dev resources: - - pods - - nodes - services - - endpoints - - namespaces - - jobs - events - pods/log - viziers verbs: - - "*" + - "get" + - "watch" + - "list" - apiGroups: - batch resources: @@ -34,17 +65,20 @@ rules: - "" resources: - secrets + - endpoints + - pods verbs: - "*" --- apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding +kind: RoleBinding metadata: - name: pl-cloud-connector-cluster-binding + name: pl-cloud-connector-binding roleRef: apiGroup: rbac.authorization.k8s.io - kind: ClusterRole + kind: Role name: pl-cloud-connector-role subjects: - kind: ServiceAccount name: cloud-conn-service-account + namespace: pl diff --git a/k8s/vizier/bootstrap/updater_role.yaml b/k8s/vizier/bootstrap/updater_role.yaml index cf52b4df6b0..d4db4660829 100644 --- a/k8s/vizier/bootstrap/updater_role.yaml +++ b/k8s/vizier/bootstrap/updater_role.yaml @@ -2,44 +2,52 @@ apiVersion: v1 kind: ServiceAccount metadata: - name: updater-service-account - labels: - vizier-updater-dep: "true" + name: pl-updater-service-account --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: pl-updater-binding - labels: - vizier-updater-dep: "true" + name: pl-updater-cluster-binding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole name: pl-updater-role subjects: - kind: ServiceAccount - name: updater-service-account + name: pl-updater-service-account + namespace: "{{ .Release.Namespace }}" --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: pl-updater-role - labels: - vizier-updater-dep: "true" + name: pl-updater-cluster-role rules: # Allow actions on Kubernetes objects +- apiGroups: + - rbac.authorization.k8s.io + - etcd.database.coreos.com + - nats.io + resources: + - clusterroles + - clusterrolebindings + - persistentvolumes + - etcdclusters + - natsclusters + verbs: ["*"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: pl-updater-role +rules: - apiGroups: - "" - apps - rbac.authorization.k8s.io - extensions - - etcd.database.coreos.com - batch - - nats.io - policy resources: - - clusterroles - - clusterrolebindings - configmaps - secrets - pods @@ -47,13 +55,23 @@ rules: - deployments - daemonsets - persistentvolumes - - persistentvolumeclaims - roles - rolebindings - serviceaccounts - - etcdclusters - statefulsets - cronjobs - jobs - - natsclusters verbs: ["*"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: pl-updater-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: pl-updater-role +subjects: +- kind: ServiceAccount + name: pl-updater-service-account + namespace: "{{ .Release.Namespace }}" diff --git a/src/utils/template_generator/vizier_yamls/vizier_yamls.go b/src/utils/template_generator/vizier_yamls/vizier_yamls.go index 404a511d01a..9dbd39d030c 100644 --- a/src/utils/template_generator/vizier_yamls/vizier_yamls.go +++ b/src/utils/template_generator/vizier_yamls/vizier_yamls.go @@ -362,14 +362,14 @@ func generateVzYAMLs(clientset *kubernetes.Clientset, yamlMap map[string]string) TemplateValue: fmt.Sprintf(".%s.svc", nsTmpl), }, { - TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-psp-binding"), + TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-updater-binding"), Patch: `{ "subjects": [{ "name": "updater-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, Placeholder: "__PX_SUBJECT_NAMESPACE__", TemplateValue: nsTmpl, }, { - TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-updater-binding"), - Patch: `{ "subjects": [{ "name": "updater-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, + TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-updater-cluster-binding"), + Patch: `{ "subjects": [{ "name": "updater-service-account", "namespace": "__PXqgq_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, Placeholder: "__PX_SUBJECT_NAMESPACE__", TemplateValue: nsTmpl, }, @@ -379,12 +379,24 @@ func generateVzYAMLs(clientset *kubernetes.Clientset, yamlMap map[string]string) Placeholder: "__PX_SUBJECT_NAMESPACE__", TemplateValue: nsTmpl, }, + { + TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-cloud-connector-binding"), + Patch: `{ "subjects": [{ "name": "cloud-conn-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, + Placeholder: "__PX_SUBJECT_NAMESPACE__", + TemplateValue: nsTmpl, + }, { TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-vizier-metadata-cluster-binding"), Patch: `{ "subjects": [{ "name": "metadata-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, Placeholder: "__PX_SUBJECT_NAMESPACE__", TemplateValue: nsTmpl, }, + { + TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-vizier-metadata-binding"), + Patch: `{ "subjects": [{ "name": "metadata-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`, + Placeholder: "__PX_SUBJECT_NAMESPACE__", + TemplateValue: nsTmpl, + }, { TemplateMatcher: yamls.GenerateResourceNameMatcherFn("pl-vizier-crd-metadata-binding"), Patch: `{ "subjects": [{ "name": "metadata-service-account", "namespace": "__PX_SUBJECT_NAMESPACE__", "kind": "ServiceAccount" }] }`,