From 8df4fd0e7cdc9973744d5742e7cb83d50e9c053a Mon Sep 17 00:00:00 2001 From: Sanket Sudake Date: Thu, 15 Dec 2022 15:04:10 +0530 Subject: [PATCH] Allow service account check to run only once at start of executor (#2673) Signed-off-by: Sanket Sudake Signed-off-by: Sanket Sudake --- charts/fission-all/values.yaml | 5 +++-- pkg/utils/serviceaccount.go | 33 ++++++++++++++-------------- pkg/utils/serviceaccount_test.go | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 pkg/utils/serviceaccount_test.go diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index f68ef75c..a55368b9 100644 --- a/charts/fission-all/values.yaml +++ b/charts/fission-all/values.yaml @@ -192,8 +192,9 @@ executor: enabled: true ## indicates the time interval in minutes, after that fission will create service account, roles and rolebinding for builder and fetcher. ## interval will be applicable only if enable value is set to true. - ## default timing will be 30 minutes. - interval: 30 + ## default timing will be 0 minutes. That means check will run only once. + ## if you want to run check every 30 minutes then set interval to 30. + interval: 0 ## router is responsible for routing function calls to the appropriate function. ## router: diff --git a/pkg/utils/serviceaccount.go b/pkg/utils/serviceaccount.go index 98314994..19216309 100644 --- a/pkg/utils/serviceaccount.go +++ b/pkg/utils/serviceaccount.go @@ -97,7 +97,12 @@ func CreateMissingPermissionForSA(ctx context.Context, kubernetesClient kubernet interval := getSAInterval() logger.Debug("interval value", zap.Any("interval", interval)) sa := getSAObj(ctx, kubernetesClient, logger) - go sa.doSACheck(ctx, interval) + logger.Info("Starting service account check", zap.Any("interval", interval)) + if interval > 0 { + go wait.UntilWithContext(ctx, sa.runSACheck, interval) + } else { + sa.runSACheck(ctx) + } } } @@ -112,10 +117,6 @@ func getSAObj(ctx context.Context, kubernetesClient kubernetes.Interface, logger return saObj } -func (sa *ServiceAccount) doSACheck(ctx context.Context, interval time.Duration) { - wait.UntilWithContext(ctx, sa.runSACheck, interval) -} - func (sa *ServiceAccount) runSACheck(ctx context.Context) { for _, ns := range sa.nsResolver.FissionResourceNS { for _, permission := range sa.permissions { @@ -133,7 +134,7 @@ func setupSAAndRoleBindings(ctx context.Context, client kubernetes.Interface, lo SAObj, err := createGetSA(ctx, client, ps.saName, namespace) if err != nil { logger.Error("error while creating or getting service account", - zap.String("SA_name", ps.saName), + zap.String("sa_name", ps.saName), zap.String("namespace", namespace), zap.Error(err)) return @@ -179,7 +180,7 @@ func setupSAAndRoleBindings(ctx context.Context, client kubernetes.Interface, lo func setupRoles(ctx context.Context, client kubernetes.Interface, logger *zap.Logger, sa *v1.ServiceAccount, rules []rbac.PolicyRule, suffix string) (*rbac.Role, error) { logger.Debug("creating role", zap.String("role_name", fmt.Sprintf("%s-role-%s", sa.Name, suffix)), - zap.String("SA_Name", sa.Name), + zap.String("sa_name", sa.Name), zap.String("namespace", sa.Namespace)) roleObj := &rbac.Role{ @@ -193,17 +194,17 @@ func setupRoles(ctx context.Context, client kubernetes.Interface, logger *zap.Lo if err != nil { return nil, fmt.Errorf("error while creating role for sa %s in namespace %s error: %s", sa.Name, sa.Namespace, err.Error()) } - logger.Debug("role created successfully", + logger.Info("role created successfully", zap.String("role_name", role.Name), zap.String("namespace", role.Namespace), - zap.String("SA_Name", sa.Name)) + zap.String("sa_name", sa.Name)) return role, nil } func setupRoleBinding(ctx context.Context, client kubernetes.Interface, logger *zap.Logger, sa *v1.ServiceAccount, role *rbac.Role, suffix string) (*rbac.RoleBinding, error) { logger.Debug("creating role binding", zap.String("rolebinding_name", fmt.Sprintf("%s-rolebinding-%s", sa.Name, suffix)), - zap.String("SA_Name", sa.Name), + zap.String("sa_name", sa.Name), zap.String("namespace", sa.Namespace)) roleBindingObj := &rbac.RoleBinding{ @@ -227,16 +228,19 @@ func setupRoleBinding(ctx context.Context, client kubernetes.Interface, logger * if err != nil { return nil, fmt.Errorf("error while creating rolebinding for sa %s in namespace %s error: %s", sa.Name, sa.Namespace, err.Error()) } - logger.Debug("role binding created successfully", + logger.Info("role binding created successfully", zap.String("rolebinding_name", roleBinding.Name), zap.String("namespace", roleBinding.Namespace), - zap.String("SA_Name", sa.Name)) + zap.String("sa_name", sa.Name)) return roleBinding, nil } func checkPermission(ctx context.Context, client kubernetes.Interface, sa *v1.ServiceAccount, gvr *schema.GroupVersionResource, verb string) (bool, error) { user := fmt.Sprintf("system:serviceaccount:%s:%s", sa.Namespace, sa.Name) sar := authorizationv1.LocalSubjectAccessReview{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: sa.Namespace, + }, Spec: authorizationv1.SubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: sa.Namespace, @@ -298,9 +302,6 @@ func createServiceAccount() bool { } func getSAInterval() time.Duration { - SAInterval, err := GetUIntValueFromEnv(ENV_SA_INTERVAL) - if err != nil { - return time.Duration(30) * time.Minute - } + SAInterval, _ := GetUIntValueFromEnv(ENV_SA_INTERVAL) return time.Duration(SAInterval) * time.Minute } diff --git a/pkg/utils/serviceaccount_test.go b/pkg/utils/serviceaccount_test.go new file mode 100644 index 00000000..60f5282b --- /dev/null +++ b/pkg/utils/serviceaccount_test.go @@ -0,0 +1,37 @@ +package utils + +import ( + "context" + "os" + "regexp" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + "github.com/fission/fission/pkg/utils/loggerfactory" +) + +func TestServiceAccountCheck(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + kubernetesClient := fake.NewSimpleClientset() + logger := loggerfactory.GetLogger() + os.Setenv(ENV_CREATE_SA, "true") + CreateMissingPermissionForSA(ctx, kubernetesClient, logger) + + // Get rolebinding for a service account + rolebindings, err := kubernetesClient.RbacV1().RoleBindings("default").List(ctx, metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + if len(rolebindings.Items) != 2 { + t.Fatal("Rolebinding not created", len(rolebindings.Items)) + } + regexp := regexp.MustCompile(`fission\-(fetcher|builder)\-rolebinding\-[a-z0-9]{6}`) + for _, rolebinding := range rolebindings.Items { + if !regexp.Match([]byte(rolebinding.Name)) { + t.Fatal("Rolebinding not created for fission-builder or fission-fetcher", rolebinding.Name) + } + } +}