diff --git a/charts/fission-all/templates/_fission-kubernetes-roles.tpl b/charts/fission-all/templates/_fission-kubernetes-roles.tpl index d5cc39f6..1c943cc6 100644 --- a/charts/fission-all/templates/_fission-kubernetes-roles.tpl +++ b/charts/fission-all/templates/_fission-kubernetes-roles.tpl @@ -3,10 +3,8 @@ rules: - apiGroups: - "" resources: - - configmaps - pods - services - - serviceaccounts verbs: - create - delete @@ -30,24 +28,6 @@ rules: - get - list - watch -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind {{- end }} {{- define "canaryconfig-kuberules" }} rules: @@ -57,20 +37,6 @@ rules: - services verbs: - list -- apiGroups: - - "" - resources: - - configmaps - - secrets - verbs: - - get -- apiGroups: - - "" - resources: - - namespaces - verbs: - - get - - create - apiGroups: - "" resources: @@ -109,7 +75,6 @@ rules: - namespaces verbs: - get - - create - apiGroups: - "" resources: @@ -132,11 +97,8 @@ rules: - apiGroups: - "" resources: - - configmaps - pods - - secrets - services - - serviceaccounts - replicationcontrollers - events verbs: @@ -146,6 +108,15 @@ rules: - list - watch - patch +- apiGroups: + - "" + resources: + - configmaps + - secrets + verbs: + - get + - list + - watch - apiGroups: - apps resources: @@ -180,24 +151,6 @@ rules: - update - patch - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind - apiGroups: - metrics.k8s.io resources: @@ -226,16 +179,12 @@ rules: - pods - secrets - services - - serviceaccounts - replicationcontrollers - events verbs: - - create - - delete - get - list - watch - - patch - apiGroups: - batch resources: @@ -252,24 +201,6 @@ rules: - get - list - watch -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind {{- end }} {{- define "kafka-kuberules" }} rules: @@ -280,7 +211,6 @@ rules: - pods - secrets - services - - serviceaccounts - replicationcontrollers - events verbs: @@ -290,6 +220,13 @@ rules: - list - watch - patch +- apiGroups: + - "" + resources: + - configmaps + - secrets + verbs: + - get - apiGroups: - apps resources: @@ -312,35 +249,14 @@ rules: - get - list - watch -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind {{- end }} {{- define "keda-kuberules" }} rules: - apiGroups: - "" resources: - - configmaps - pods - - secrets - services - - serviceaccounts - replicationcontrollers - events verbs: @@ -350,6 +266,13 @@ rules: - list - watch - patch +- apiGroups: + - "" + resources: + - configmaps + - secrets + verbs: + - get - apiGroups: - apps resources: @@ -372,24 +295,6 @@ rules: - get - list - watch -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind - apiGroups: - keda.sh resources: @@ -440,23 +345,6 @@ rules: [] {{- end }} {{- define "router-kuberules" }} rules: -- apiGroups: - - "" - resources: - - configmaps - - pods - - secrets - - services - - serviceaccounts - - replicationcontrollers - - events - verbs: - - create - - delete - - get - - list - - watch - - patch - apiGroups: - networking.k8s.io resources: @@ -477,40 +365,7 @@ rules: - get - list - watch -- apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterroles - verbs: - - bind {{- end }} {{- define "timer-kuberules" }} -rules: -- apiGroups: - - fission.io - resources: - - environments - - functions - - packages - - timetriggers - verbs: - - create - - get - - list - - watch - - update - - patch - - delete +rules: [] {{- end }} diff --git a/charts/fission-all/templates/_function-access-role.tpl b/charts/fission-all/templates/_function-access-role.tpl index a5235a61..9c431604 100644 --- a/charts/fission-all/templates/_function-access-role.tpl +++ b/charts/fission-all/templates/_function-access-role.tpl @@ -24,7 +24,7 @@ rules: resources: - packages verbs: - - "get" + - get --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -37,7 +37,7 @@ rules: resources: - packages verbs: - - "get" + - get - apiGroups: - "" resources: diff --git a/charts/fission-all/templates/misc-functions/clusterrole.yaml b/charts/fission-all/templates/misc-functions/clusterrole.yaml deleted file mode 100644 index 6620675c..00000000 --- a/charts/fission-all/templates/misc-functions/clusterrole.yaml +++ /dev/null @@ -1,33 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: {{ .Release.Name }}-secret-configmap-getter -rules: - - apiGroups: - - "" - resources: - - secrets - - configmaps - verbs: - - get - - watch - - list - ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: {{ .Release.Name }}-package-getter -rules: - - apiGroups: - - fission.io - resources: - - packages - verbs: - - "*" - - apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - verbs: - - "*" diff --git a/pkg/apis/core/v1/const.go b/pkg/apis/core/v1/const.go index b74a47c4..83f07371 100644 --- a/pkg/apis/core/v1/const.go +++ b/pkg/apis/core/v1/const.go @@ -146,12 +146,6 @@ const ( const ( FissionBuilderSA = "fission-builder" FissionFetcherSA = "fission-fetcher" - - SecretConfigMapGetterRB = "secret-configmap-getter-binding" - - PackageGetterRB = "package-getter-binding" - - ClusterRole = "ClusterRole" ) const ( diff --git a/pkg/buildermgr/envwatcher.go b/pkg/buildermgr/envwatcher.go index 1e82c288..a9796eb7 100644 --- a/pkg/buildermgr/envwatcher.go +++ b/pkg/buildermgr/envwatcher.go @@ -268,12 +268,6 @@ func (envw *environmentWatcher) createBuilder(ctx context.Context, env *fv1.Envi } // there should be only one deploy in deployList if len(deployList) == 0 { - // create builder SA in this ns, if not already created - _, err := utils.SetupSA(ctx, envw.kubernetesClient, fv1.FissionBuilderSA, ns) - if err != nil { - return nil, errors.Wrapf(err, "error creating %q in ns: %s", fv1.FissionBuilderSA, ns) - } - deploy, err = envw.createBuilderDeployment(ctx, env, ns) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("error creating builder deployment for environment in namespace %s %s", env.ObjectMeta.Name, ns)) diff --git a/pkg/buildermgr/pkgwatcher.go b/pkg/buildermgr/pkgwatcher.go index 02833f9a..b5dc1231 100644 --- a/pkg/buildermgr/pkgwatcher.go +++ b/pkg/buildermgr/pkgwatcher.go @@ -169,23 +169,6 @@ func (pkgw *packageWatcher) build(ctx context.Context, srcpkg *fv1.Package) { break } - // 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, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionBuilderSA, builderNs) - if err != nil { - pkgw.logger.Error("error setting up role binding for package", - zap.Error(err), - zap.String("role_binding", fv1.PackageGetterRB), - zap.String("package_name", pkg.ObjectMeta.Name), - zap.String("package_namespace", pkg.ObjectMeta.Namespace)) - continue - } else { - pkgw.logger.Info("setup rolebinding for sa package", - zap.String("sa", fmt.Sprintf("%s.%s", fv1.FissionBuilderSA, builderNs)), - zap.String("package", fmt.Sprintf("%s.%s", pkg.ObjectMeta.Name, pkg.ObjectMeta.Namespace))) - } - uploadResp, buildLogs, err := buildPackage(ctx, pkgw.logger, pkgw.fissionClient, builderNs, pkgw.storageSvcUrl, pkg) if err != nil { pkgw.logger.Error("error building package", zap.Error(err), zap.String("package_name", pkg.ObjectMeta.Name)) diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go index 25729881..9531e211 100644 --- a/pkg/executor/executor.go +++ b/pkg/executor/executor.go @@ -38,7 +38,6 @@ import ( "github.com/fission/fission/pkg/executor/executortype/newdeploy" "github.com/fission/fission/pkg/executor/executortype/poolmgr" "github.com/fission/fission/pkg/executor/fscache" - "github.com/fission/fission/pkg/executor/reaper" "github.com/fission/fission/pkg/executor/util" fetcherConfig "github.com/fission/fission/pkg/fetcher/config" "github.com/fission/fission/pkg/generated/clientset/versioned" @@ -279,13 +278,11 @@ func StartExecutor(ctx context.Context, logger *zap.Logger, port int) error { funcInformer := make(map[string]finformerv1.FunctionInformer, 0) envInformer := make(map[string]finformerv1.EnvironmentInformer, 0) - pkgInformer := make(map[string]finformerv1.PackageInformer, 0) for _, ns := range utils.DefaultNSResolver().FissionResourceNS { factory := genInformer.NewFilteredSharedInformerFactory(fissionClient, time.Minute*30, ns, nil) funcInformer[ns] = factory.Core().V1().Functions() envInformer[ns] = factory.Core().V1().Environments() - pkgInformer[ns] = factory.Core().V1().Packages() } executorLabel, err := utils.GetInformerLabelByExecutor(fv1.ExecutorTypePoolmgr) @@ -297,7 +294,7 @@ func StartExecutor(ctx context.Context, logger *zap.Logger, port int) error { logger, fissionClient, kubernetesClient, metricsClient, fetcherConfig, executorInstanceID, - funcInformer, pkgInformer, envInformer, + funcInformer, envInformer, gpmInformerFactory, podSpecPatch) if err != nil { return errors.Wrap(err, "pool manager creation failed") @@ -365,9 +362,6 @@ func StartExecutor(ctx context.Context, logger *zap.Logger, port int) error { for _, informer := range envInformer { fissionInformers = append(fissionInformers, informer.Informer()) } - for _, informer := range pkgInformer { - fissionInformers = append(fissionInformers, informer.Informer()) - } for _, informer := range configMapInformer { fissionInformers = append(fissionInformers, informer) } @@ -394,7 +388,6 @@ func StartExecutor(ctx context.Context, logger *zap.Logger, port int) error { return err } - go reaper.CleanupRoleBindings(ctx, logger, kubernetesClient, fissionClient, time.Minute*30) go metrics.ServeMetrics(ctx, logger) go api.Serve(ctx, port) diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go index c4f0f9e6..d83f29bb 100644 --- a/pkg/executor/executortype/newdeploy/newdeploy.go +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -34,7 +34,6 @@ import ( fv1 "github.com/fission/fission/pkg/apis/core/v1" "github.com/fission/fission/pkg/executor/util" - "github.com/fission/fission/pkg/utils" otelUtils "github.com/fission/fission/pkg/utils/otel" ) @@ -91,10 +90,6 @@ func (deploy *NewDeploy) createOrGetDeployment(ctx context.Context, fn *fv1.Func return existingDepl, err } else if k8s_err.IsNotFound(err) { - err := deploy.setupRBACObjs(ctx, deployNamespace, fn) - if err != nil { - return nil, err - } depl, err := deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Create(ctx, deployment, metav1.CreateOptions{}) if err != nil { @@ -118,47 +113,6 @@ func (deploy *NewDeploy) createOrGetDeployment(ctx context.Context, fn *fv1.Func return nil, err } -func (deploy *NewDeploy) setupRBACObjs(ctx context.Context, deployNamespace string, fn *fv1.Function) error { - // create fetcher SA in this ns, if not already created - err := deploy.fetcherConfig.SetupServiceAccount(ctx, deploy.kubernetesClient, deployNamespace, fn.ObjectMeta) - if err != nil { - deploy.logger.Error("error creating fission fetcher service account for function", - zap.Error(err), - zap.String("service_account_name", fv1.FissionFetcherSA), - zap.String("service_account_namespace", deployNamespace), - zap.String("function_name", fn.ObjectMeta.Name), - zap.String("function_namespace", fn.ObjectMeta.Namespace)) - return err - } - - // 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, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) - if err != nil { - deploy.logger.Error("error creating role binding for function", - zap.Error(err), - zap.String("role_binding", fv1.PackageGetterRB), - zap.String("function_name", fn.ObjectMeta.Name), - zap.String("function_namespace", fn.ObjectMeta.Namespace)) - return err - } - - // 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, utils.GetSecretConfigMapGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, deployNamespace) - if err != nil { - deploy.logger.Error("error creating role binding for function", - zap.Error(err), - zap.String("role_binding", fv1.SecretConfigMapGetterRB), - zap.String("function_name", fn.ObjectMeta.Name), - zap.String("function_namespace", fn.ObjectMeta.Namespace)) - return err - } - - deploy.logger.Info("set up all RBAC objects for function", - zap.String("function_name", fn.ObjectMeta.Name), - zap.String("function_namespace", fn.ObjectMeta.Namespace)) - return nil -} - func (deploy *NewDeploy) updateDeployment(ctx context.Context, deployment *appsv1.Deployment, ns string) error { _, err := deploy.kubernetesClient.AppsV1().Deployments(ns).Update(ctx, deployment, metav1.UpdateOptions{}) return err diff --git a/pkg/executor/executortype/poolmgr/funchandlers.go b/pkg/executor/executortype/poolmgr/funchandlers.go index 1eb2e2b4..53c314f6 100644 --- a/pkg/executor/executortype/poolmgr/funchandlers.go +++ b/pkg/executor/executortype/poolmgr/funchandlers.go @@ -65,21 +65,6 @@ func FunctionEventHandlers(ctx context.Context, logger *zap.Logger, kubernetesCl envNs = fn.Spec.Environment.Namespace } - // TODO : Just bring to your attention during review : - // 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, 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 { - logger.Debug("successfully set up rolebinding for fetcher service account for function", - zap.String("service_account", fv1.FissionFetcherSA), - zap.String("service_account_namepsace", envNs), - zap.String("function_name", fn.ObjectMeta.Name), - zap.String("function_namespace", fn.ObjectMeta.Namespace)) - } - if istioEnabled { // create a same name service for function // since istio only allows the traffic to service @@ -120,7 +105,7 @@ func FunctionEventHandlers(ctx context.Context, logger *zap.Logger, kubernetesCl } // create function istio service if it does not exist - _, err = kubernetesClient.CoreV1().Services(envNs).Create(ctx, &svc, metav1.CreateOptions{}) + _, err := kubernetesClient.CoreV1().Services(envNs).Create(ctx, &svc, metav1.CreateOptions{}) if err != nil && !kerrors.IsAlreadyExists(err) { logger.Error("error creating istio service for function", zap.Error(err), @@ -157,45 +142,6 @@ func FunctionEventHandlers(ctx context.Context, logger *zap.Logger, kubernetesCl } } }, - - UpdateFunc: func(oldObj, newObj interface{}) { - oldFunc := oldObj.(*fv1.Function) - newFunc := newObj.(*fv1.Function) - - if oldFunc.ObjectMeta.ResourceVersion == newFunc.ObjectMeta.ResourceVersion { - return - } - - envChanged := (oldFunc.Spec.Environment.Namespace != newFunc.Spec.Environment.Namespace) - - executorTypeChangedToPM := (oldFunc.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fv1.ExecutorTypePoolmgr && - newFunc.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fv1.ExecutorTypePoolmgr) - - // if a func's env reference gets updated and the newly referenced env is in a different ns, - // we need to create a rolebinding in func's ns so that the fetcher-sa in env ns has access - // to fetch secrets and config maps from the func's ns. - // similarly if executorType changed to Pool Manager, we now need a rolebinding in the func ns for fetcher sa - // present in env ns because for newdeploy, the fetcher sa is in function namespace - if envChanged || executorTypeChangedToPM { - envNs := fissionfnNamespace - if newFunc.Spec.Environment.Namespace != metav1.NamespaceDefault { - envNs = newFunc.Spec.Environment.Namespace - } - err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.SecretConfigMapGetterRB, - newFunc.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 { - logger.Debug("successfully set up rolebinding for fetcher service account for function", - zap.String("service_account", fv1.FissionFetcherSA), - zap.String("service_account_namepsace", envNs), - zap.String("function_name", newFunc.ObjectMeta.Name), - zap.String("function_namespace", newFunc.ObjectMeta.Namespace)) - } - } - }, } } diff --git a/pkg/executor/executortype/poolmgr/gp.go b/pkg/executor/executortype/poolmgr/gp.go index a6a18288..56307819 100644 --- a/pkg/executor/executortype/poolmgr/gp.go +++ b/pkg/executor/executortype/poolmgr/gp.go @@ -138,14 +138,8 @@ func MakeGenericPool( } func (gp *GenericPool) setup(ctx context.Context) error { - // create fetcher SA in this ns, if not already created - err := gp.fetcherConfig.SetupServiceAccount(ctx, gp.kubernetesClient, gp.namespace, nil) - if err != nil { - return errors.Wrapf(err, "error creating fetcher service account in namespace %q", gp.namespace) - } - // create the pool - err = gp.createPoolDeployment(ctx, gp.env) + err := gp.createPoolDeployment(ctx, gp.env) if err != nil { return err } diff --git a/pkg/executor/executortype/poolmgr/gpm.go b/pkg/executor/executortype/poolmgr/gpm.go index 1267ed0b..92079e0d 100644 --- a/pkg/executor/executortype/poolmgr/gpm.go +++ b/pkg/executor/executortype/poolmgr/gpm.go @@ -118,7 +118,6 @@ func MakeGenericPoolManager(ctx context.Context, fetcherConfig *fetcherConfig.Config, instanceID string, funcInformer map[string]finformerv1.FunctionInformer, - pkgInformer map[string]finformerv1.PackageInformer, envInformer map[string]finformerv1.EnvironmentInformer, gpmInformerFactory map[string]k8sInformers.SharedInformerFactory, podSpecPatch *apiv1.PodSpec, @@ -136,7 +135,7 @@ func MakeGenericPoolManager(ctx context.Context, } poolPodC := NewPoolPodController(ctx, gpmLogger, kubernetesClient, - enableIstio, funcInformer, pkgInformer, envInformer, gpmInformerFactory) + enableIstio, funcInformer, envInformer, gpmInformerFactory) gpm := &GenericPoolManager{ logger: gpmLogger, diff --git a/pkg/executor/executortype/poolmgr/packagehandlers.go b/pkg/executor/executortype/poolmgr/packagehandlers.go index c09fce5e..d9e2a052 100644 --- a/pkg/executor/executortype/poolmgr/packagehandlers.go +++ b/pkg/executor/executortype/poolmgr/packagehandlers.go @@ -15,89 +15,3 @@ limitations under the License. */ package poolmgr - -import ( - "context" - - "go.uber.org/zap" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - k8sCache "k8s.io/client-go/tools/cache" - - fv1 "github.com/fission/fission/pkg/apis/core/v1" - "github.com/fission/fission/pkg/utils" -) - -// PackageEventHandlers provides handlers for package events. -// Based on package create/update event, we create role binding -// for the package which is used by fetcher component. -func PackageEventHandlers(ctx context.Context, logger *zap.Logger, kubernetesClient kubernetes.Interface, fissionfnNamespace string) k8sCache.ResourceEventHandlerFuncs { - return k8sCache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - pkg := obj.(*fv1.Package) - logger.Debug("list watch for package reported a new package addition", - zap.String("package_name", pkg.ObjectMeta.Name), - zap.String("package_namespace", pkg.ObjectMeta.Namespace)) - - // create or update role-binding for fetcher sa in env ns to be able to get the pkg contents from pkg namespace - envNs := fissionfnNamespace - if pkg.Spec.Environment.Namespace != metav1.NamespaceDefault { - envNs = pkg.Spec.Environment.Namespace - } - // 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, utils.GetPackageGetterCR(), fv1.ClusterRole, fv1.FissionFetcherSA, envNs) - if err != nil { - logger.Error("error creating rolebinding for package", - zap.Error(err), - zap.String("role_binding", fv1.PackageGetterRB), - zap.String("package_name", pkg.ObjectMeta.Name), - zap.String("package_namespace", pkg.ObjectMeta.Namespace)) - return - } - - logger.Debug("successfully set up rolebinding for fetcher service account", - zap.String("service_account", fv1.FissionFetcherSA), - zap.String("service_account_namespace", envNs), - zap.String("package_name", pkg.ObjectMeta.Name), - zap.String("package_namespace", pkg.ObjectMeta.Namespace)) - }, - - UpdateFunc: func(oldObj, newObj interface{}) { - oldPkg := oldObj.(*fv1.Package) - newPkg := newObj.(*fv1.Package) - - if oldPkg.ObjectMeta.ResourceVersion == newPkg.ObjectMeta.ResourceVersion { - return - } - - // if a pkg's env reference gets updated and the newly referenced env is in a different ns, - // we need to update the role-binding in pkg ns to grant permissions to the fetcher-sa in env ns - // to do a get on pkg - if oldPkg.Spec.Environment.Namespace != newPkg.Spec.Environment.Namespace { - envNs := fissionfnNamespace - if newPkg.Spec.Environment.Namespace != metav1.NamespaceDefault { - envNs = newPkg.Spec.Environment.Namespace - } - - err := utils.SetupRoleBinding(ctx, logger, kubernetesClient, fv1.PackageGetterRB, - newPkg.ObjectMeta.Namespace, utils.GetPackageGetterCR(), fv1.ClusterRole, - fv1.FissionFetcherSA, envNs) - if err != nil { - logger.Error("error updating rolebinding for package", - zap.Error(err), - zap.String("role_binding", fv1.PackageGetterRB), - zap.String("package_name", newPkg.ObjectMeta.Name), - zap.String("package_namespace", newPkg.ObjectMeta.Namespace)) - return - } - - logger.Debug("successfully updated rolebinding for fetcher service account", - zap.String("service_account", fv1.FissionFetcherSA), - zap.String("service_account_namespace", envNs), - zap.String("package_name", newPkg.ObjectMeta.Name), - zap.String("package_namespace", newPkg.ObjectMeta.Namespace)) - } - }, - } -} diff --git a/pkg/executor/executortype/poolmgr/poolpodcontroller.go b/pkg/executor/executortype/poolmgr/poolpodcontroller.go index e4282f81..064a6b58 100644 --- a/pkg/executor/executortype/poolmgr/poolpodcontroller.go +++ b/pkg/executor/executortype/poolmgr/poolpodcontroller.go @@ -71,7 +71,6 @@ func NewPoolPodController(ctx context.Context, logger *zap.Logger, kubernetesClient kubernetes.Interface, enableIstio bool, funcInformer map[string]finformerv1.FunctionInformer, - pkgInformer map[string]finformerv1.PackageInformer, envInformer map[string]finformerv1.EnvironmentInformer, gpmInformerFactory map[string]k8sInformers.SharedInformerFactory) *PoolPodController { logger = logger.Named("pool_pod_controller") @@ -91,9 +90,6 @@ func NewPoolPodController(ctx context.Context, logger *zap.Logger, for _, informer := range funcInformer { informer.Informer().AddEventHandler(FunctionEventHandlers(ctx, p.logger, p.kubernetesClient, p.nsResolver.ResolveNamespace(p.nsResolver.FunctionNamespace), p.enableIstio)) } - for _, informer := range pkgInformer { - informer.Informer().AddEventHandler(PackageEventHandlers(ctx, p.logger, p.kubernetesClient, p.nsResolver.ResolveNamespace(p.nsResolver.FunctionNamespace))) - } for ns, informer := range envInformer { informer.Informer().AddEventHandler(k8sCache.ResourceEventHandlerFuncs{ AddFunc: p.enqueueEnvAdd, diff --git a/pkg/executor/executortype/poolmgr/poolpodcontroller_test.go b/pkg/executor/executortype/poolmgr/poolpodcontroller_test.go index 22a27135..ad9dd33c 100644 --- a/pkg/executor/executortype/poolmgr/poolpodcontroller_test.go +++ b/pkg/executor/executortype/poolmgr/poolpodcontroller_test.go @@ -69,7 +69,6 @@ func TestPoolPodControllerPodCleanup(t *testing.T) { ppc := NewPoolPodController(ctx, logger, kubernetesClient, false, funcInformer, - pkgInformer, envInformer, gpmInformerFactory) @@ -83,7 +82,7 @@ func TestPoolPodControllerPodCleanup(t *testing.T) { logger, fissionClient, kubernetesClient, metricsClient, fetcherConfig, executorInstanceID, - funcInformer, pkgInformer, envInformer, + funcInformer, envInformer, gpmInformerFactory, nil) if err != nil { t.Fatalf("Error creating generic pool manager: %v", err) diff --git a/pkg/executor/reaper/reaper.go b/pkg/executor/reaper/reaper.go index 564a92f4..8c0f1c4b 100644 --- a/pkg/executor/reaper/reaper.go +++ b/pkg/executor/reaper/reaper.go @@ -19,7 +19,6 @@ package reaper import ( "context" "strings" - "time" "go.uber.org/zap" apiv1 "k8s.io/api/core/v1" @@ -27,7 +26,6 @@ import ( "k8s.io/client-go/kubernetes" fv1 "github.com/fission/fission/pkg/apis/core/v1" - "github.com/fission/fission/pkg/generated/clientset/versioned" "github.com/fission/fission/pkg/utils" ) @@ -217,158 +215,6 @@ func CleanupHpa(ctx context.Context, logger *zap.Logger, client kubernetes.Inter return nil } -// CleanupRoleBindings periodically lists rolebindings across all namespaces and removes Service Accounts from them or -// deletes the rolebindings completely if there are no Service Accounts in a rolebinding object. -func CleanupRoleBindings(ctx context.Context, logger *zap.Logger, client kubernetes.Interface, fissionClient versioned.Interface, cleanupRoleBindingInterval time.Duration) { - nsResolver := utils.DefaultNSResolver() - for { - // some sleep before the next reaper iteration - time.Sleep(cleanupRoleBindingInterval) - - logger.Debug("starting cleanupRoleBindings cycle") - - cleanupRoleBindings := func(namespace string) error { - // get all rolebindings ( just to be efficient, one call to kubernetes ) - rbList, err := client.RbacV1().RoleBindings(namespace).List(ctx, metav1.ListOptions{}) - if err != nil { - // something wrong, but next iteration hopefully succeeds - logger.Error("error listing role bindings in all namespaces", zap.Error(err)) - return err - } - - // go through each role-binding object and do the cleanup necessary - for _, roleBinding := range rbList.Items { - // ignore role-bindings in kube-system namespace - if roleBinding.Namespace == "kube-system" { - continue - } - - // ignore role-bindings not created by fission - if roleBinding.Name != fv1.PackageGetterRB && roleBinding.Name != fv1.SecretConfigMapGetterRB { - continue - } - - // in order to find out if there are any functions that need this role-binding in role-binding namespace, - // we can list the functions once per role-binding. - funcList, err := fissionClient.CoreV1().Functions(roleBinding.Namespace).List(ctx, metav1.ListOptions{}) - if err != nil { - logger.Error("error fetching function list in namespace", zap.Error(err), zap.String("namespace", roleBinding.Namespace)) - continue - } - - // final map of service accounts that can be removed from this roleBinding object - // using a map here instead of a list so the code in RemoveSAFromRoleBindingWithRetries is efficient. - saToRemove := make(map[string]bool) - - // the following flags are needed to decide if any of the service accounts can be removed from role-bindings depending on the functions that need them. - // ndmFunc denotes if there's at least one function that has executor type New deploy Manager - // funcEnvReference denotes if there's at least one function that has reference to an environment in the SA Namespace for the SA in question - var ndmFunc, funcEnvReference bool - - // iterate through each subject in the role-binding and check if there are any references to them - for _, subj := range roleBinding.Subjects { - ndmFunc = false - funcEnvReference = false - - // this is the reverse of what we're doing in setting up of role-bindings. if objects are created in default ns, - // the SA namespace will have the value of "fission-function"/"fission-builder" depending on the SA. - // so now we need to look for the objects in default namespace. - saNs := subj.Namespace - isInReservedNS := false - if subj.Namespace == nsResolver.FunctionNamespace || - subj.Namespace == nsResolver.BuiderNamespace { - saNs = metav1.NamespaceDefault - isInReservedNS = true - } - - // go through each function and find out if there's either at least one function with env reference in the same namespace as the Service Account in this iteration - // or at least one function using ndm executor in the role-binding namespace and set the corresponding flags - for _, fn := range funcList.Items { - if fn.Spec.Environment.Namespace == saNs || - // For the case that the environment is created in the reserved namespace. - (isInReservedNS && (fn.Spec.Environment.Namespace == nsResolver.FunctionNamespace || - fn.Spec.Environment.Namespace == nsResolver.BuiderNamespace)) { - funcEnvReference = true - break - } - - if fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fv1.ExecutorTypeNewdeploy { - ndmFunc = true - break - } - } - - // if its a package-getter-rb, we have 2 kinds of SAs and each of them is handled differently - // else if its a secret-configmap-rb, we have only one SA which is fission-fetcher - if roleBinding.Name == fv1.PackageGetterRB { - // check if there is an env obj in saNs - envList, err := fissionClient.CoreV1().Environments(saNs).List(ctx, metav1.ListOptions{}) - if err != nil { - logger.Error("error fetching environment list in service account namespace", zap.Error(err), zap.String("namespace", saNs)) - continue - } - - // if the SA in this iteration is fission-builder, then we need to only check - // if either there's at least one env object in the SA's namespace, or, - // if there's at least one function in the role-binding namespace with env reference - // to the SA's namespace. - // if neither, then we can remove this SA from this role-binding - if subj.Name == fv1.FissionBuilderSA { - if len(envList.Items) == 0 && !funcEnvReference { - saToRemove[utils.MakeSAMapKey(subj.Name, subj.Namespace)] = true - } - } - - // if the SA in this iteration is fission-fetcher, then in addition to above checks, - // we also need to check if there's at least one function with executor type New deploy - // in the rolebinding's namespace. - // if none of them are true, then remove this SA from this role-binding - if subj.Name == fv1.FissionFetcherSA { - if len(envList.Items) == 0 && !ndmFunc && !funcEnvReference { - // remove SA from rolebinding - saToRemove[utils.MakeSAMapKey(subj.Name, subj.Namespace)] = true - } - } - } else if roleBinding.Name == fv1.SecretConfigMapGetterRB { - // if there's not even one function in the role-binding's namespace and there's not even - // one function with env reference to the SA's namespace, then remove that SA - // from this role-binding - if !ndmFunc && !funcEnvReference { - saToRemove[utils.MakeSAMapKey(subj.Name, subj.Namespace)] = true - } - } - } - - // finally, make a call to RemoveSAFromRoleBindingWithRetries for all the service accounts that need to be removed - // for the role-binding in this iteration - if len(saToRemove) != 0 { - logger.Debug("removing service accounts from role binding", - zap.Any("service_accounts", saToRemove), - zap.String("role_binding_name", roleBinding.Name), - zap.String("role_binding_namespace", roleBinding.Namespace)) - - // call this once in the end for each role-binding - err = utils.RemoveSAFromRoleBindingWithRetries(ctx, logger, client, roleBinding.Name, roleBinding.Namespace, saToRemove) - if err != nil { - // if there's an error, we just log it and proceed with the next role-binding, hoping that this role-binding - // will be processed in next iteration. - logger.Debug("error removing service account from role binding", - zap.Error(err), - zap.Any("service_accounts", saToRemove), - zap.String("role_binding_name", roleBinding.Name), - zap.String("role_binding_namespace", roleBinding.Namespace)) - } - } - } - return nil - } - for _, namespace := range GetReaperNamespace() { - //ignore error - cleanupRoleBindings(namespace) //nolint errcheck - } - } -} - func GetReaperNamespace() map[string]string { ns := utils.DefaultNSResolver() //to support backward compatibility we need to cleanup deployment and rolebinding created in function, buidler and default namespace as well diff --git a/pkg/fetcher/config/config.go b/pkg/fetcher/config/config.go index 06179797..0562e56d 100644 --- a/pkg/fetcher/config/config.go +++ b/pkg/fetcher/config/config.go @@ -1,10 +1,8 @@ package container import ( - "context" "encoding/json" "fmt" - "log" "os" "path/filepath" @@ -14,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" fv1 "github.com/fission/fission/pkg/apis/core/v1" "github.com/fission/fission/pkg/fetcher" @@ -91,16 +88,6 @@ func MakeFetcherConfig(sharedMountPath string) (*Config, error) { }, nil } -func (cfg *Config) SetupServiceAccount(ctx context.Context, kubernetesClient kubernetes.Interface, namespace string, context interface{}) error { - _, err := utils.SetupSA(ctx, kubernetesClient, fv1.FissionFetcherSA, namespace) - if err != nil { - log.Printf("Error : %v creating %s in ns : %s for: %#v", err, fv1.FissionFetcherSA, namespace, context) - return err - } - - return nil -} - func (cfg *Config) SharedMountPath() string { return cfg.sharedMountPath } diff --git a/pkg/utils/rbacutils.go b/pkg/utils/rbacutils.go deleted file mode 100644 index 5a061903..00000000 --- a/pkg/utils/rbacutils.go +++ /dev/null @@ -1,324 +0,0 @@ -/* -Copyright 2016 The Fission Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package utils - -import ( - "context" - "fmt" - "os" - - "go.uber.org/zap" - - "github.com/pkg/errors" - apiv1 "k8s.io/api/core/v1" - rbac "k8s.io/api/rbac/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/json" - "k8s.io/client-go/kubernetes" -) - -// This file has util functions needed for setting up and cleaning up RBAC objects. - -const ( - maxRetries = 10 -) - -// MakeSAObj returns a ServiceAccount object with the given SA name and namespace -func MakeSAObj(sa, ns string) *apiv1.ServiceAccount { - return &apiv1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Name: sa, - }, - } -} - -// SetupSA checks if a service account is present in the namespace, if not creates it. -func SetupSA(ctx context.Context, k8sClient kubernetes.Interface, sa, ns string) (*apiv1.ServiceAccount, error) { - saObj, err := k8sClient.CoreV1().ServiceAccounts(ns).Get(ctx, sa, metav1.GetOptions{}) - if err == nil { - return saObj, nil - } - - if k8serrors.IsNotFound(err) { - saObj = MakeSAObj(sa, ns) - saObj, err = k8sClient.CoreV1().ServiceAccounts(ns).Create(ctx, saObj, metav1.CreateOptions{}) - } - - return saObj, err -} - -// makeRoleBindingObj is a helper function called from other functions in this file only. -// given a rolebinging name and namespace, it makes a rolebinding object mapping the role to the SA of the namespace. -func makeRoleBindingObj(roleBinding, roleBindingNs, role, roleKind, sa, saNamespace string) *rbac.RoleBinding { - return &rbac.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: roleBinding, - Namespace: roleBindingNs, - }, - Subjects: []rbac.Subject{ - { - Kind: "ServiceAccount", - Name: sa, - Namespace: saNamespace, - }, - }, - RoleRef: rbac.RoleRef{ - Kind: roleKind, - Name: role, - }, - } -} - -// isSAInRoleBinding checks if a service account is present in the rolebinding object -func isSAInRoleBinding(rbObj *rbac.RoleBinding, sa, ns string) bool { - for _, subject := range rbObj.Subjects { - if subject.Name == sa && subject.Namespace == ns { - return true - } - } - - return false -} - -// PatchSpec is a standard struct needed for JSON merge. -type PatchSpec struct { - Op string `json:"op"` - Path string `json:"path"` - Value rbac.Subject `json:"value"` -} - -// AddSaToRoleBindingWithRetries adds a service account to a rolebinding object. IT retries on already exists and conflict errors. -func AddSaToRoleBindingWithRetries(ctx context.Context, logger *zap.Logger, k8sClient kubernetes.Interface, roleBinding, roleBindingNs, sa, saNamespace, role, roleKind string) (err error) { - patch := PatchSpec{} - patch.Op = "add" - patch.Path = "/subjects/-" - patch.Value = rbac.Subject{ - Kind: "ServiceAccount", - Name: sa, - Namespace: saNamespace, - } - - patchJson, err := json.Marshal([]PatchSpec{patch}) - if err != nil { - logger.Error("error marshalling patch into json", zap.Error(err)) - return err - } - - for i := 0; i < maxRetries; i++ { - _, err = k8sClient.RbacV1().RoleBindings(roleBindingNs).Patch(ctx, roleBinding, types.JSONPatchType, patchJson, metav1.PatchOptions{}) - if err == nil { - logger.Debug("patched rolebinding", - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return err - } - - if k8serrors.IsNotFound(err) { - logger.Error("rolebinding not found - will try to create it", - zap.Error(err), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - // someone may have deleted the object between us checking if the object is present and deciding to patch - // so just create the object again - rbObj := makeRoleBindingObj(roleBinding, roleBindingNs, role, roleKind, sa, saNamespace) - _, err = k8sClient.RbacV1().RoleBindings(roleBindingNs).Create(ctx, rbObj, metav1.CreateOptions{}) - if err == nil { - logger.Debug("created rolebinding", - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return err - } - - if k8serrors.IsAlreadyExists(err) { - logger.Error("rolebinding object already exists, retrying patch", - zap.Error(err), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - continue - } - - return errors.Wrap(err, "error returned by rolebinding create") - } - - if k8serrors.IsConflict(err) { - // TODO : Need to test this out, not able to simulate conflicts yet. - // Initially, my understanding was that patch can never error on conflict because Api server will handle the conflicts for us. - // but one CI run did show patch errored out on conflict : https://api.travis-ci.org/v3/job/373161490/log.txt, look for : - // Error returned by rolebinding patch : there is a meaningful conflict (firstResourceVersion: "35482724", currentResourceVersion: "35482849") - // so, m guessing retrying patch should help. will watch out for any such conflicts and fix the issue if any - logger.Error("conflict reported on patch of rolebinding - retrying patch operation", - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - continue - } - - return errors.Wrap(err, "error returned by rolebinding patch") - } - - return errors.Wrapf(err, "exceeded max retries (%d) adding SA: %s.%s to rolebinding: %s.%s, giving up", maxRetries, sa, saNamespace, roleBinding, roleBindingNs) -} - -// RemoveSAFromRoleBindingWithRetries removes an SA from the rolebinding passed as parameter. If this is the only SA in -// the rolebinding, then it deletes the rolebinding object. -func RemoveSAFromRoleBindingWithRetries(ctx context.Context, logger *zap.Logger, k8sClient kubernetes.Interface, roleBinding, roleBindingNs string, saToRemove map[string]bool) (err error) { - for i := 0; i < maxRetries; i++ { - rbObj, err := k8sClient.RbacV1().RoleBindings(roleBindingNs).Get( - ctx, - roleBinding, metav1.GetOptions{}) - if err != nil { - // silently ignoring the error. there's no need for us to remove sa anymore. - logger.Debug("rolebinding not found, but ignoring the error since we're cleaning up", - zap.Error(err), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return nil - } - - subjects := rbObj.Subjects - newSubjects := make([]rbac.Subject, 0) - - // TODO : optimize it. - for _, item := range subjects { - if _, ok := saToRemove[MakeSAMapKey(item.Name, item.Namespace)]; ok { - continue - } - - newSubjects = append(newSubjects, rbac.Subject{ - Kind: "ServiceAccount", - Name: item.Name, - Namespace: item.Namespace, - }) - } - if len(newSubjects) == 0 { - return DeleteRoleBinding(ctx, k8sClient, roleBinding, roleBindingNs) - } - - rbObj.Subjects = newSubjects - - // can't use patch for deletes, the results become in-deterministic, so using update. - _, err = k8sClient.RbacV1().RoleBindings(rbObj.Namespace).Update(ctx, rbObj, metav1.UpdateOptions{}) - switch { - case err == nil: - logger.Debug("removed service accounts from rolebinding", - zap.Any("service_accounts", saToRemove), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return nil - case k8serrors.IsConflict(err): - logger.Error("conflict in update of rolebinding - retrying", - zap.Error(err), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - continue - default: - return errors.Wrap(err, "rolebinding update errored out") - } - } - - return errors.Wrapf(err, "max retries: %d exceeded for removing SA's: %v from rolebinding %s.%s, giving up", maxRetries, saToRemove, roleBinding, roleBindingNs) -} - -// SetupRoleBinding adds a role to a service account if the rolebinding object is already present in the namespace. -// if not, it creates a rolebinding object granting the role to the SA in the namespace. -func SetupRoleBinding(ctx context.Context, logger *zap.Logger, k8sClient kubernetes.Interface, roleBinding, roleBindingNs, role, roleKind, sa, saNamespace string) error { - // get the role binding object - rbObj, err := k8sClient.RbacV1().RoleBindings(roleBindingNs).Get( - ctx, - roleBinding, metav1.GetOptions{}) - - if err == nil { - if !isSAInRoleBinding(rbObj, sa, saNamespace) { - logger.Debug("service account is not present in the rolebinding - will add", - zap.String("service_account_name", sa), - zap.String("service_account_namespace", saNamespace), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return AddSaToRoleBindingWithRetries(ctx, logger, k8sClient, roleBinding, roleBindingNs, sa, saNamespace, role, roleKind) - } - if rbObj.RoleRef.Name != role || rbObj.RoleRef.Kind != roleKind { - logger.Error("rolebinding with different role references exists", - zap.String("role_binding", rbObj.Name), - zap.String("role_binding_namespace", roleBindingNs), - zap.String("roleref", role), - zap.String("roleref_kind", roleKind), - zap.String("old_roleref", rbObj.RoleRef.Name), - zap.String("old_roleref_kind", rbObj.RoleRef.Kind)) - return fmt.Errorf("rolebinding %s in namespace %s exists with different roleref, retry by deleting existing rolebinding", rbObj.Name, roleBindingNs) - } - logger.Debug("service account already present in rolebinding so nothing to add", - zap.String("service_account_name", sa), - zap.String("service_account_namespace", saNamespace), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - return nil - } - - // if role binding is missing, create it. also add this sa to the binding. - if k8serrors.IsNotFound(err) { - logger.Debug("rolebinding does NOT exist in namespace - creating it", - zap.Error(err), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - rbObj = makeRoleBindingObj(roleBinding, roleBindingNs, role, roleKind, sa, saNamespace) - _, err = k8sClient.RbacV1().RoleBindings(roleBindingNs).Create(ctx, rbObj, metav1.CreateOptions{}) - if k8serrors.IsAlreadyExists(err) { - logger.Debug("rolebinding already exists in namespace - adding service account to rolebinding", - zap.String("service_account_name", sa), - zap.String("service_account_namespace", saNamespace), - zap.String("role_binding", roleBinding), - zap.String("role_binding_namespace", roleBindingNs)) - err = AddSaToRoleBindingWithRetries(ctx, logger, k8sClient, roleBinding, roleBindingNs, sa, saNamespace, role, roleKind) - } - } - - return err -} - -// DeleteRoleBinding deletes a rolebinding object. if k8s throws an error that the rolebinding is not there, it just -// returns silently. -func DeleteRoleBinding(ctx context.Context, k8sClient kubernetes.Interface, roleBinding, roleBindingNs string) error { - // if deleteRoleBinding is invoked by 2 fission services at the same time for the same rolebinding, - // the first call will succeed while the 2nd will fail with isNotFound. but we dont want to error out then. - err := k8sClient.RbacV1().RoleBindings(roleBindingNs).Delete(ctx, roleBinding, metav1.DeleteOptions{}) - if err == nil || k8serrors.IsNotFound(err) { - return nil - } - return err -} - -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" -} diff --git a/pkg/utils/rbacutils_test.go b/pkg/utils/rbacutils_test.go deleted file mode 100644 index 26b3434e..00000000 --- a/pkg/utils/rbacutils_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package utils - -import ( - "context" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" - - fv1 "github.com/fission/fission/pkg/apis/core/v1" - "github.com/fission/fission/pkg/utils/loggerfactory" -) - -const ( - namespace string = "testns" - serviceAccount string = "testSA" - clusterRole string = "testClusterRole" - rolebinding string = "testRolebinding" -) - -func TestSetupRoleBinding(t *testing.T) { - ctx := context.Background() - logger := loggerfactory.GetLogger() - kubernetesClient := fake.NewSimpleClientset() - - //case 1 => when role binding doesn't exists - _, err := createServiceAccount(ctx, kubernetesClient) - if err != nil { - t.Fatalf("Error creating service account: %s", err.Error()) - } - _, err = createClusterRole(ctx, clusterRole, kubernetesClient) - if err != nil { - t.Fatalf("Error creating cluster role: %s", err.Error()) - } - err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace) - assert.Nil(t, err, "error should be nil and new role binding will get created") - - //case 2 => rolebinding exists but service account doesn't exists - err = kubernetesClient.CoreV1().ServiceAccounts(namespace).Delete(ctx, serviceAccount, metav1.DeleteOptions{}) - if err != nil { - t.Fatalf("Error deleting service account: %s", err.Error()) - } - err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace) - assert.Nil(t, err, "error should be nil and service account should add in rolebinding") - - //case 3 => rolebinding, cluster role and service account, all exists - err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace) - assert.Nil(t, err, "error should be nil and nothing to add") - - //case 4 => This must fail, if there is change in cluster-role-name - err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, "invalid-cluster-name", fv1.ClusterRole, serviceAccount, namespace) - assert.NotNil(t, err) - assert.Equal(t, err.Error(), fmt.Sprintf("rolebinding %s in namespace %s exists with different roleref, retry by deleting existing rolebinding", rolebinding, namespace)) -} - -func createClusterRole(ctx context.Context, clusterRole string, kubernetesClient *fake.Clientset) (*v1.ClusterRole, error) { - objRole := MakeClusterRoleObj(clusterRole) - var err error - objRole, err = kubernetesClient.RbacV1().ClusterRoles().Create(ctx, objRole, metav1.CreateOptions{}) - return objRole, err -} - -func createServiceAccount(ctx context.Context, kubernetesClient *fake.Clientset) (*corev1.ServiceAccount, error) { - objSA := MakeSAObj(serviceAccount, namespace) - var err error - objSA, err = kubernetesClient.CoreV1().ServiceAccounts(namespace).Create(ctx, objSA, metav1.CreateOptions{}) - return objSA, err -} - -// MakeClusterRoleObj returns a ClusterRole object -func MakeClusterRoleObj(clusterRoleName string) *v1.ClusterRole { - return &v1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterRoleName, - }, - } -}