Consume podspec patch directly on executor/builder mounts (#2661)

Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>

Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>

charts/fission-all/templates/buildermgr/configmap.yaml

@@ -4,6 +4,6 @@ kind: ConfigMap
 metadata:
   name: builder-podspec-patch
 data: 
-  spec: |
+  builder-podspec-patch.yaml: |
     {{- toYaml .Values.builderPodSpec.podSpec | nindent 4 }}
 {{- end -}}

charts/fission-all/templates/buildermgr/deployment.yaml

@@ -44,7 +44,7 @@ spec:
         - name: FISSION_FUNCTION_NAMESPACE
           value: "{{ .Values.functionNamespace }}"
         - name: FISSION_DEFAULT_NAMESPACE
-          value: "{{ .Values.defaultNamespace }}"      
+          value: "{{ .Values.defaultNamespace }}"
         - name: ENABLE_ISTIO
           value: "{{ .Values.enableIstio }}"
         - name: FETCHER_MINCPU
@@ -61,8 +61,15 @@ spec:
           value: {{ .Values.pprof.enabled | quote }}
         - name: HELM_RELEASE_NAME
           value: {{ .Release.Name | quote }}
-        {{- include "fission-resource-namespace.envs" . | indent 8 }}  
+        {{- include "fission-resource-namespace.envs" . | indent 8 }}
         {{- include "opentelemtry.envs" . | indent 8 }}
+        {{- if .Values.builderPodSpec.enabled }}
+        volumeMounts:
+        - name: builder-podspec-patch-volume
+          mountPath: /etc/fission/builder-podspec-patch.yaml
+          subPath: builder-podspec-patch.yaml
+          readOnly: true
+        {{- end }}
         ports:
           - containerPort: 8080
             name: metrics
@@ -75,11 +82,17 @@ spec:
         terminationMessagePolicy: {{ .Values.terminationMessagePolicy }}
         {{- end }}
       serviceAccountName: fission-buildermgr
+      {{- if .Values.builderPodSpec.enabled }}
+      volumes:
+      - name: builder-podspec-patch-volume
+        configMap:
+          name: builder-podspec-patch
+      {{- end }}
 {{- if .Values.priorityClassName }}
       priorityClassName: {{ .Values.priorityClassName }}
 {{- end }}
     {{- with .Values.imagePullSecrets }}
-      imagePullSecrets: 
+      imagePullSecrets:
         {{- toYaml . | nindent 8 }}
     {{- end }}
 {{- if .Values.extraCoreComponentPodConfig }}

charts/fission-all/templates/executor/configmap.yaml

@@ -4,6 +4,6 @@ kind: ConfigMap
 metadata:
   name: runtime-podspec-patch
 data: 
-  spec: |
+  runtime-podspec-patch.yaml: |
     {{- toYaml .Values.runtimePodSpec.podSpec | nindent 4 }}
 {{- end -}}

charts/fission-all/templates/executor/deployment.yaml

@@ -42,7 +42,7 @@ spec:
         - name: FISSION_FUNCTION_NAMESPACE
           value: "{{ .Values.functionNamespace }}"
         - name: FISSION_DEFAULT_NAMESPACE
-          value: "{{ .Values.defaultNamespace }}"  
+          value: "{{ .Values.defaultNamespace }}"
         - name: RUNTIME_IMAGE_PULL_POLICY
           value: "{{ .Values.pullPolicy }}"
         - name: ADOPT_EXISTING_RESOURCES
@@ -96,6 +96,13 @@ spec:
             port: 8888
           initialDelaySeconds: 35
           periodSeconds: 5
+        {{- if .Values.runtimePodSpec.enabled }}
+        volumeMounts:
+        - name: runtime-podspec-patch-volume
+          mountPath: /etc/fission/runtime-podspec-patch.yaml
+          subPath: runtime-podspec-patch.yaml
+          readOnly: true
+        {{- end }}
         ports:
         - containerPort: 8080
           name: metrics
@@ -116,13 +123,19 @@ spec:
         terminationMessagePolicy: {{ .Values.terminationMessagePolicy }}
         {{- end }}
       serviceAccountName: fission-executor
+      {{- if .Values.runtimePodSpec.enabled }}
+      volumes:
+      - name: runtime-podspec-patch-volume
+        configMap:
+          name: runtime-podspec-patch
+      {{- end }}
 {{- if .Values.executor.priorityClassName }}
       priorityClassName: {{ .Values.executor.priorityClassName }}
 {{- else if .Values.priorityClassName }}
       priorityClassName: {{ .Values.priorityClassName }}
 {{- end }}
     {{- with .Values.imagePullSecrets }}
-      imagePullSecrets: 
+      imagePullSecrets:
         {{- toYaml . | nindent 8 }}
     {{- end }}
 {{- if .Values.extraCoreComponentPodConfig }}

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/go-git/go-git/v5 v5.4.2
 	github.com/go-openapi/spec v0.20.7
 	github.com/golang-jwt/jwt/v4 v4.4.2
+	github.com/google/go-cmp v0.5.9
 	github.com/gorilla/mux v1.8.0
 	github.com/graymeta/stow v0.2.8
 	github.com/hashicorp/go-multierror v1.1.1
@@ -108,7 +109,6 @@ require (
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/gotestyourself/gotestyourself v2.2.0+incompatible // indirect

pkg/apis/core/v1/const.go

@@ -67,8 +67,8 @@ const (
 )
 
 const (
-	RuntimePodSpecConfigmap = "runtime-podspec-patch"
+	RuntimePodSpecPath = "/etc/fission/runtime-podspec-patch.yaml"
-	BuilderPodSpecConfigmap = "builder-podspec-patch"
+	BuilderPodSpecPath = "/etc/fission/builder-podspec-patch.yaml"
 )
 
 const (

pkg/buildermgr/buildermgr.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
-	apiv1 "k8s.io/api/core/v1"
 
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
 	"github.com/fission/fission/pkg/crd"
@@ -50,15 +49,9 @@ func Start(ctx context.Context, logger *zap.Logger, storageSvcUrl string) error
 		return errors.Wrap(err, "error making fetcher config")
 	}
 
-	var podSpecPatch *apiv1.PodSpec
+	podSpecPatch, err := util.GetSpecFromConfigMap(fv1.BuilderPodSpecPath)
-	namespace, err := utils.GetCurrentNamespace()
 	if err != nil {
-		logger.Warn("Current namespace not found %v", zap.Error(err))
+		logger.Warn("error reading data for pod spec patch", zap.String("path", fv1.BuilderPodSpecPath), zap.Error(err))
-	} else {
-		podSpecPatch, err = util.GetSpecFromConfigMap(ctx, kubernetesClient, fv1.BuilderPodSpecConfigmap, namespace)
-		if err != nil {
-			logger.Warn("Either configmap is not found or error reading data %v", zap.Error(err))
-		}
 	}
 
 	envWatcher := makeEnvironmentWatcher(ctx, bmLogger, fissionClient, kubernetesClient, fetcherConfig, podSpecPatch)

pkg/executor/executor.go

@@ -28,7 +28,6 @@ import (
 	"github.com/dchest/uniuri"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
-	apiv1 "k8s.io/api/core/v1"
 	k8sCache "k8s.io/client-go/tools/cache"
 
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
@@ -271,15 +270,9 @@ func StartExecutor(ctx context.Context, logger *zap.Logger, port int) error {
 
 	executorInstanceID := strings.ToLower(uniuri.NewLen(8))
 
-	var podSpecPatch *apiv1.PodSpec
+	podSpecPatch, err := util.GetSpecFromConfigMap(fv1.RuntimePodSpecPath)
-	namespace, err := utils.GetCurrentNamespace()
 	if err != nil {
-		logger.Warn("Current namespace not found %s", zap.Error(err))
+		logger.Warn("error reading data for pod spec patch", zap.String("path", fv1.RuntimePodSpecPath), zap.Error(err))
-	} else {
-		podSpecPatch, err = util.GetSpecFromConfigMap(ctx, kubernetesClient, fv1.RuntimePodSpecConfigmap, namespace)
-		if err != nil {
-			logger.Warn("Either configmap is not found or error reading data %v", zap.Error(err))
-		}
 	}
 
 	logger.Info("Starting executor", zap.String("instanceID", executorInstanceID))

pkg/executor/executortype/newdeploy/newdeploymgr_test.go

@@ -18,7 +18,6 @@ import (
 	k8sCache "k8s.io/client-go/tools/cache"
 
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
-	"github.com/fission/fission/pkg/executor/util"
 	fetcherConfig "github.com/fission/fission/pkg/fetcher/config"
 	fClient "github.com/fission/fission/pkg/generated/clientset/versioned/fake"
 	genInformer "github.com/fission/fission/pkg/generated/informers/externalversions"
@@ -65,23 +64,13 @@ func TestRefreshFuncPods(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	err = BuildConfigMap(ctx, kubernetesClient, functionNamespace, fv1.RuntimePodSpecConfigmap, map[string]string{})
-	if err != nil {
-		t.Fatalf("Error building configmap: %s", err)
-	}
-
-	podSpecPatch, err := util.GetSpecFromConfigMap(ctx, kubernetesClient, fv1.RuntimePodSpecConfigmap, functionNamespace)
-	if err != nil {
-		t.Fatalf("Error creating pod spec: %s", err)
-	}
-
 	fetcherConfig, err := fetcherConfig.MakeFetcherConfig("/userfunc")
 	if err != nil {
 		t.Fatalf("Error creating fetcher config: %s", err)
 	}
 
 	executor, err := MakeNewDeploy(ctx, logger, fissionClient, kubernetesClient, fetcherConfig, "test",
-		funcInformer, envInformer, ndmInformerFactory, podSpecPatch)
+		funcInformer, envInformer, ndmInformerFactory, nil)
 	if err != nil {
 		t.Fatalf("new deploy manager creation failed: %s", err)
 	}

pkg/executor/util/util.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"strings"
 	"sync"
 	"time"
@@ -119,18 +120,14 @@ func ConvertConfigSecrets(ctx context.Context, fn *fv1.Function, kc kubernetes.I
 	return envFromSources, nil
 }
 
-func GetSpecFromConfigMap(ctx context.Context, kubeClient kubernetes.Interface, cm string, cmns string) (*apiv1.PodSpec, error) {
+func GetSpecFromConfigMap(filePath string) (*apiv1.PodSpec, error) {
-
+	content, err := os.ReadFile(filePath)
-	podSpecPatch, err := kubeClient.CoreV1().ConfigMaps(cmns).Get(ctx, cm, metav1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error reading YAML file %s: %w", filePath, err)
 	}
-
+	additionalSpec := &apiv1.PodSpec{}
-	var additionalSpec apiv1.PodSpec
+	err = yaml.UnmarshalStrict(content, &additionalSpec)
-
+	return additionalSpec, err
-	err = yaml.Unmarshal([]byte(podSpecPatch.Data["spec"]), &additionalSpec)
-
-	return &additionalSpec, err
 }
 
 func GetObjectReaperInterval(logger *zap.Logger, executorType fv1.ExecutorType, defaultReaperInterval uint) uint {

pkg/executor/util/util_test.go

@@ -17,57 +17,45 @@ limitations under the License.
 package util
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"reflect"
 	"testing"
 
-	apiv1 "k8s.io/api/core/v1"
+	"github.com/google/go-cmp/cmp"
-	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"
+	apiv1 "k8s.io/api/core/v1"
 )
 
 func TestGetSpecFromConfigMap(t *testing.T) {
-
+	runtimePodSpecPath := "runtime-podspec-patch.yaml"
-	kubeClient := fake.NewSimpleClientset()
+	tempDir := t.TempDir()
-
-	var permissionNum int64 = 10001
-	var runAsNonRoot bool = true
-
-	configMapData := make(map[string]string, 0)
 	specPatch := `
 securityContext:
   fsGroup: 10001
   runAsGroup: 10001
   runAsNonRoot: true
   runAsUser: 10001`
-
+	err := os.WriteFile(tempDir+"/"+runtimePodSpecPath, []byte(specPatch), 0644)
-	configMapData["spec"] = specPatch
-
-	testConfigMap := apiv1.ConfigMap{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ConfigMap",
-			APIVersion: "v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-config-map",
-			Namespace: "fission",
-		},
-		Data: configMapData,
-	}
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	configmap, err := kubeClient.CoreV1().ConfigMaps("fission").Create(ctx, &testConfigMap, metav1.CreateOptions{})
 	if err != nil {
-		t.Errorf("Error creating configmap %v", err)
+		t.Errorf("Error writing to file %s", err)
+	}
+	specPatch2 := `
+securityContext2:
+securityContext:
+  fsGroup: "invalida_input"
+  runAsGroup: 10001
+  runAsNonRoot: true
+  runAsUser: 10001`
+	err = os.WriteFile(tempDir+"/"+runtimePodSpecPath+"2", []byte(specPatch2), 0644)
+	if err != nil {
+		t.Errorf("Error writing to file %s", err)
 	}
 
-	t.Logf("Configmap: %v", configmap.Data)
+	var permissionNum int64 = 10001
+	var runAsNonRoot bool = true
 
 	testSpecPatch := apiv1.PodSpec{
 		SecurityContext: &apiv1.PodSecurityContext{
@@ -79,42 +67,41 @@ securityContext:
 	}
 	tests := []struct {
 		name    string
-		cm      string
+		path    string
-		cmns    string
 		want    *apiv1.PodSpec
 		wantErr bool
 	}{
 		{
-			name:    "Configmap exists",
+			name:    "File exists with valid data",
-			cm:      "test-config-map",
+			path:    tempDir + "/" + runtimePodSpecPath,
-			cmns:    "fission",
 			want:    &testSpecPatch,
 			wantErr: false,
 		},
 		{
-			name:    "Configmap does not exists",
+			name:    "File with invalid data",
-			cm:      "wrongname",
+			path:    tempDir + "/" + runtimePodSpecPath + "2",
-			cmns:    "fission",
 			want:    nil,
 			wantErr: true,
 		},
 		{
-			name:    "Wrong namespace",
+			name:    "File does not exist",
-			cm:      "test-config-map",
+			path:    tempDir + "/" + "notexist",
-			cmns:    "fissio",
 			want:    nil,
 			wantErr: true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := GetSpecFromConfigMap(ctx, kubeClient, tt.cm, tt.cmns)
+			got, err := GetSpecFromConfigMap(tt.path)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("GetSpecFromConfigMap() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			if err != nil {
+				return
+			}
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("GetSpecFromConfigMap() got = %v, want %v", got, tt.want)
+				t.Errorf("GetSpecFromConfigMap() diff = %s", cmp.Diff(tt.want, got))
 			}
 		})
 	}

pkg/utils/utils.go

@@ -216,17 +216,6 @@ func IsZip(filename string) (bool, error) {
 	return archiver.DefaultZip.Match(f)
 }
 
-// GetCurrentNamespace returns Kubernetes namespace of current Pod
-func GetCurrentNamespace() (string, error) {
-
-	// This file contains the namespace and can be found in each container.
-	body, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace")
-	if err != nil {
-		return "", err
-	}
-	return string(body), nil
-}
-
 func GetStringValueFromEnv(envVar string) (string, error) {
 	v := os.Getenv(envVar)
 	if v == "" {