From 353453e9a763582fb7244b7bc52d9c3cd8e19e36 Mon Sep 17 00:00:00 2001 From: Sanket Sudake Date: Tue, 18 Jan 2022 09:18:53 +0530 Subject: [PATCH] Rbac resources per release for multiple installation on same cluster (#2302) * Change RBAC resource names work for multiple Fission release * Fetch secret configmap and package cluster role based on the release name * Remove default namespace hardcoding from helm chart Signed-off-by: Sanket Sudake --- .../templates/buildermgr/deployment.yaml | 2 ++ .../templates/common/clusterrole.yaml | 2 +- .../templates/common/clusterrolebinding.yaml | 4 ++-- .../templates/executor/deployment.yaml | 2 ++ .../templates/misc-functions/clusterrole.yaml | 4 ++-- .../templates/misc-functions/role.yaml | 10 +++++----- .../templates/misc-functions/rolebinding.yaml | 16 ++++++++-------- charts/fission-all/values.yaml | 4 ++++ pkg/apis/core/v1/const.go | 2 -- pkg/buildermgr/pkgwatcher.go | 2 +- .../executortype/newdeploy/newdeploy.go | 4 ++-- .../executortype/poolmgr/funchandlers.go | 4 ++-- .../executortype/poolmgr/packagehandlers.go | 4 ++-- pkg/utils/rbacutils.go | 17 +++++++++++++++++ 14 files changed, 50 insertions(+), 27 deletions(-) diff --git a/charts/fission-all/templates/buildermgr/deployment.yaml b/charts/fission-all/templates/buildermgr/deployment.yaml index f7fb573f..43aa2406 100644 --- a/charts/fission-all/templates/buildermgr/deployment.yaml +++ b/charts/fission-all/templates/buildermgr/deployment.yaml @@ -46,6 +46,8 @@ spec: value: {{ .Values.debugEnv | quote }} - name: PPROF_ENABLED value: {{ .Values.pprof.enabled | quote }} + - name: HELM_RELEASE_NAME + value: {{ .Release.Name | quote }} {{- include "opentracing.envs" . | indent 8 }} {{- include "opentelemtry.envs" . | indent 8 }} {{- if .Values.terminationMessagePath }} diff --git a/charts/fission-all/templates/common/clusterrole.yaml b/charts/fission-all/templates/common/clusterrole.yaml index 50665467..290243fa 100644 --- a/charts/fission-all/templates/common/clusterrole.yaml +++ b/charts/fission-all/templates/common/clusterrole.yaml @@ -1,7 +1,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: fission-cr-admin + name: {{ .Release.Name }}-fission-cr-admin rules: - apiGroups: - "" diff --git a/charts/fission-all/templates/common/clusterrolebinding.yaml b/charts/fission-all/templates/common/clusterrolebinding.yaml index 178f6b8a..2801a4ad 100644 --- a/charts/fission-all/templates/common/clusterrolebinding.yaml +++ b/charts/fission-all/templates/common/clusterrolebinding.yaml @@ -1,12 +1,12 @@ kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: - name: fission-cr-admin + name: {{ .Release.Name }}-fission-cr-admin subjects: - kind: ServiceAccount name: fission-svc namespace: {{ .Release.Namespace }} roleRef: kind: ClusterRole - name: fission-cr-admin + name: {{ .Release.Name }}-fission-cr-admin apiGroup: rbac.authorization.k8s.io diff --git a/charts/fission-all/templates/executor/deployment.yaml b/charts/fission-all/templates/executor/deployment.yaml index 86e8289b..b095d769 100644 --- a/charts/fission-all/templates/executor/deployment.yaml +++ b/charts/fission-all/templates/executor/deployment.yaml @@ -54,6 +54,8 @@ spec: value: {{ .Values.debugEnv | quote }} - name: PPROF_ENABLED value: {{ .Values.pprof.enabled | quote }} + - name: HELM_RELEASE_NAME + value: {{ .Release.Name | quote }} {{- include "opentracing.envs" . | indent 8 }} {{- include "opentelemtry.envs" . | indent 8 }} readinessProbe: diff --git a/charts/fission-all/templates/misc-functions/clusterrole.yaml b/charts/fission-all/templates/misc-functions/clusterrole.yaml index 5bb4ec5d..e5a5e7e0 100644 --- a/charts/fission-all/templates/misc-functions/clusterrole.yaml +++ b/charts/fission-all/templates/misc-functions/clusterrole.yaml @@ -1,7 +1,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: secret-configmap-getter + name: {{ .Release.Name }}-secret-configmap-getter rules: - apiGroups: - "*" @@ -17,7 +17,7 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: package-getter + name: {{ .Release.Name }}-package-getter rules: - apiGroups: - "*" diff --git a/charts/fission-all/templates/misc-functions/role.yaml b/charts/fission-all/templates/misc-functions/role.yaml index 639a1055..a0c5efbc 100644 --- a/charts/fission-all/templates/misc-functions/role.yaml +++ b/charts/fission-all/templates/misc-functions/role.yaml @@ -1,8 +1,8 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - name: fission-fetcher - namespace: default + name: {{ .Release.Name }}-fission-fetcher + namespace: {{ .Values.defaultNamespace }} rules: - apiGroups: - "" @@ -38,8 +38,8 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - name: fission-builder - namespace: default + name: {{ .Release.Name }}-fission-builder + namespace: {{ .Values.defaultNamespace }} rules: - apiGroups: - fission.io @@ -60,7 +60,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: namespace: {{ .Values.functionNamespace }} - name: event-fetcher + name: {{ .Release.Name }}-event-fetcher rules: - apiGroups: [""] # "" indicates the core API group resources: ["pods"] diff --git a/charts/fission-all/templates/misc-functions/rolebinding.yaml b/charts/fission-all/templates/misc-functions/rolebinding.yaml index 0675bfc4..c80af74c 100644 --- a/charts/fission-all/templates/misc-functions/rolebinding.yaml +++ b/charts/fission-all/templates/misc-functions/rolebinding.yaml @@ -1,12 +1,12 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: fission-fetcher - namespace: default + name: {{ .Release.Name }}-fission-fetcher + namespace: {{ .Values.defaultNamespace }} roleRef: apiGroup: rbac.authorization.k8s.io kind: Role - name: fission-fetcher + name: {{ .Release.Name }}-fission-fetcher subjects: - kind: ServiceAccount name: fission-fetcher @@ -16,12 +16,12 @@ subjects: apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: fission-builder - namespace: default + name: {{ .Release.Name }}-fission-builder + namespace: {{ .Values.defaultNamespace }} roleRef: apiGroup: rbac.authorization.k8s.io kind: Role - name: fission-builder + name: {{ .Release.Name }}-fission-builder subjects: - kind: ServiceAccount name: fission-builder @@ -31,12 +31,12 @@ subjects: apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: fission-fetcher-pod-reader + name: {{ .Release.Name }}-fission-fetcher-pod-reader namespace: {{ .Values.functionNamespace }} roleRef: apiGroup: rbac.authorization.k8s.io kind: Role - name: event-fetcher + name: {{ .Release.Name }}-event-fetcher subjects: - kind: ServiceAccount name: fission-fetcher diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index ce152572..95bca869 100644 --- a/charts/fission-all/values.yaml +++ b/charts/fission-all/values.yaml @@ -68,6 +68,10 @@ functionNamespace: fission-function ## builderNamespace: fission-builder +## defaultNamespace represents the default namespace in Kubernetes. +## +defaultNamespace: default + ## createNamespace decides to create namespaces by the chart. ## If set to true, functionNamespace and builderNamespace namespaces mentioned above will be created by the chart. ## Set to false if you want to create the namespaces manually. diff --git a/pkg/apis/core/v1/const.go b/pkg/apis/core/v1/const.go index 8acdded7..3b5d5263 100644 --- a/pkg/apis/core/v1/const.go +++ b/pkg/apis/core/v1/const.go @@ -140,10 +140,8 @@ const ( FissionBuilderSA = "fission-builder" FissionFetcherSA = "fission-fetcher" - SecretConfigMapGetterCR = "secret-configmap-getter" SecretConfigMapGetterRB = "secret-configmap-getter-binding" - PackageGetterCR = "package-getter" PackageGetterRB = "package-getter-binding" ClusterRole = "ClusterRole" diff --git a/pkg/buildermgr/pkgwatcher.go b/pkg/buildermgr/pkgwatcher.go index c509d892..cd15038c 100644 --- a/pkg/buildermgr/pkgwatcher.go +++ b/pkg/buildermgr/pkgwatcher.go @@ -167,7 +167,7 @@ func (pkgw *packageWatcher) build(ctx context.Context, srcpkg *fv1.Package) { // Add the package getter rolebinding to builder sa // we continue here if role binding was not setup successfully. this is because without this, the fetcher won't be able to fetch the source pkg into the container and // the build will fail eventually - err := utils.SetupRoleBinding(ctx, pkgw.logger, pkgw.k8sClient, fv1.PackageGetterRB, pkg.ObjectMeta.Namespace, fv1.PackageGetterCR, fv1.ClusterRole, fv1.FissionBuilderSA, builderNs) + err := utils.SetupRoleBinding(ctx, pkgw.logger, pkgw.k8sClient, fv1.PackageGetterRB, pkg.ObjectMeta.Namespace, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionBuilderSA, builderNs) if err != nil { pkgw.logger.Error("error setting up role binding for package", zap.Error(err), diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go index 81d67144..4ef32a33 100644 --- a/pkg/executor/executortype/newdeploy/newdeploy.go +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -140,7 +140,7 @@ func (deploy *NewDeploy) setupRBACObjs(ctx context.Context, deployNamespace stri } // create a cluster role binding for the fetcher SA, if not already created, granting access to do a get on packages in any ns - err = utils.SetupRoleBinding(ctx, deploy.logger, deploy.kubernetesClient, fv1.PackageGetterRB, fn.Spec.Package.PackageRef.Namespace, fv1.PackageGetterCR, fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) + err = utils.SetupRoleBinding(ctx, deploy.logger, deploy.kubernetesClient, fv1.PackageGetterRB, fn.Spec.Package.PackageRef.Namespace, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) if err != nil { deploy.logger.Error("error creating role binding for function", zap.Error(err), @@ -151,7 +151,7 @@ func (deploy *NewDeploy) setupRBACObjs(ctx context.Context, deployNamespace stri } // create rolebinding in function namespace for fetcherSA.envNamespace to be able to get secrets and configmaps - err = utils.SetupRoleBinding(ctx, deploy.logger, deploy.kubernetesClient, fv1.SecretConfigMapGetterRB, fn.ObjectMeta.Namespace, fv1.SecretConfigMapGetterCR, fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) + err = utils.SetupRoleBinding(ctx, deploy.logger, deploy.kubernetesClient, fv1.SecretConfigMapGetterRB, fn.ObjectMeta.Namespace, utils.GetSecretConfigMapGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) if err != nil { deploy.logger.Error("error creating role binding for function", zap.Error(err), diff --git a/pkg/executor/executortype/poolmgr/funchandlers.go b/pkg/executor/executortype/poolmgr/funchandlers.go index 442d1d97..3aa41708 100644 --- a/pkg/executor/executortype/poolmgr/funchandlers.go +++ b/pkg/executor/executortype/poolmgr/funchandlers.go @@ -70,7 +70,7 @@ func FunctionEventHandlers(logger *zap.Logger, kubernetesClient *kubernetes.Clie // setup rolebinding is tried, if it fails, we don't return. we just log an error and move on, because : // 1. not all functions have secrets and/or configmaps, so things will work without this rolebinding in that case. // 2. on the contrary, when the route is tried, the env fetcher logs will show a 403 forbidden message and same will be relayed to executor. - err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.SecretConfigMapGetterRB, fn.ObjectMeta.Namespace, fv1.SecretConfigMapGetterCR, fv1.ClusterRole, fv1.FissionFetcherSA, envNs) + err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.SecretConfigMapGetterRB, fn.ObjectMeta.Namespace, utils.GetSecretConfigMapGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, envNs) if err != nil { logger.Error("error creating rolebinding", zap.Error(err), zap.String("role_binding", fv1.SecretConfigMapGetterRB)) } else { @@ -185,7 +185,7 @@ func FunctionEventHandlers(logger *zap.Logger, kubernetesClient *kubernetes.Clie } ctx := context.Background() err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.SecretConfigMapGetterRB, - newFunc.ObjectMeta.Namespace, fv1.SecretConfigMapGetterCR, fv1.ClusterRole, + newFunc.ObjectMeta.Namespace, utils.GetSecretConfigMapGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, envNs) if err != nil { diff --git a/pkg/executor/executortype/poolmgr/packagehandlers.go b/pkg/executor/executortype/poolmgr/packagehandlers.go index bfa29ba3..10c36e73 100644 --- a/pkg/executor/executortype/poolmgr/packagehandlers.go +++ b/pkg/executor/executortype/poolmgr/packagehandlers.go @@ -47,7 +47,7 @@ func PackageEventHandlers(logger *zap.Logger, kubernetesClient *kubernetes.Clien ctx := context.Background() // here, we return if we hit an error during rolebinding setup. this is because this rolebinding is mandatory for // every function's package to be loaded into its env. without that, there's no point to move forward. - err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.PackageGetterRB, pkg.ObjectMeta.Namespace, fv1.PackageGetterCR, fv1.ClusterRole, fv1.FissionFetcherSA, envNs) + err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.PackageGetterRB, pkg.ObjectMeta.Namespace, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, envNs) if err != nil { logger.Error("error creating rolebinding for package", zap.Error(err), @@ -83,7 +83,7 @@ func PackageEventHandlers(logger *zap.Logger, kubernetesClient *kubernetes.Clien ctx := context.Background() err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.PackageGetterRB, - newPkg.ObjectMeta.Namespace, fv1.PackageGetterCR, fv1.ClusterRole, + newPkg.ObjectMeta.Namespace, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, envNs) if err != nil { logger.Error("error updating rolebinding for package", diff --git a/pkg/utils/rbacutils.go b/pkg/utils/rbacutils.go index 08e68f25..8cc88af5 100644 --- a/pkg/utils/rbacutils.go +++ b/pkg/utils/rbacutils.go @@ -19,6 +19,7 @@ package utils import ( "context" "fmt" + "os" "go.uber.org/zap" @@ -295,3 +296,19 @@ func DeleteRoleBinding(ctx context.Context, k8sClient *kubernetes.Clientset, rol func MakeSAMapKey(saName, saNamespace string) string { return fmt.Sprintf("%s-%s", saName, saNamespace) } + +func GetSecretConfigMapGetterCR() string { + releaseName := os.Getenv("HELM_RELEASE_NAME") + if len(releaseName) > 0 { + return fmt.Sprintf("%s-secret-configmap-getter", releaseName) + } + return "secret-configmap-getter" +} + +func GetPackageGetterCR() string { + releaseName := os.Getenv("HELM_RELEASE_NAME") + if len(releaseName) > 0 { + return fmt.Sprintf("%s-package-getter", releaseName) + } + return "package-getter" +}