diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go new file mode 100644 index 0000000..20ef631 --- /dev/null +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -0,0 +1,495 @@ +// SPDX-FileCopyrightText: The Fission Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package newdeploy + +import ( + "context" + "errors" + "fmt" + "maps" + "path/filepath" + + appsv1 "k8s.io/api/apps/v1" + apiv1 "k8s.io/api/core/v1" + k8s_err "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + fv1 "github.com/fission/fission/pkg/apis/core/v1" + "github.com/fission/fission/pkg/executor/util" + "github.com/fission/fission/pkg/svcinfo" + "github.com/fission/fission/pkg/utils" + otelUtils "github.com/fission/fission/pkg/utils/otel" +) + +func (deploy *NewDeploy) createOrGetDeployment(ctx context.Context, fn *fv1.Function, env *fv1.Environment, + deployName string, deployLabels map[string]string, deployAnnotations map[string]string, deployNamespace string) (*appsv1.Deployment, error) { + + specializationTimeout := fn.Spec.InvokeStrategy.ExecutionStrategy.SpecializationTimeout + minScale := int32(fn.Spec.InvokeStrategy.ExecutionStrategy.MinScale) + + // Always scale to at least one pod when createOrGetDeployment + // is called. The idleObjectReaper will scale-in the deployment + // later if no requests to the function. + if minScale <= 0 { + minScale = 1 + } + + deployment, err := deploy.getDeploymentSpec(ctx, fn, env, &minScale, deployName, deployNamespace, deployLabels, deployAnnotations) + if err != nil { + return nil, err + } + + existingDepl, err := deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Get(ctx, deployName, metav1.GetOptions{}) + if err == nil { + // Try to adopt orphan deployment created by the old executor. + if existingDepl.Annotations[fv1.EXECUTOR_INSTANCEID_LABEL] != deploy.instanceID { + existingDepl.Annotations = deployment.Annotations + existingDepl.Labels = deployment.Labels + existingDepl.OwnerReferences = deployment.OwnerReferences + existingDepl.Spec.Template.Spec.Containers = deployment.Spec.Template.Spec.Containers + existingDepl.Spec.Template.Spec.ServiceAccountName = deployment.Spec.Template.Spec.ServiceAccountName + existingDepl.Spec.Template.Spec.TerminationGracePeriodSeconds = deployment.Spec.Template.Spec.TerminationGracePeriodSeconds + + // Update with the latest deployment spec. Kubernetes will trigger + // rolling update if spec is different from the one in the cluster. + existingDepl, err = deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Update(ctx, existingDepl, metav1.UpdateOptions{}) + if err != nil { + deploy.logger.Error(err, "error adopting deploy", "deploy", deployName, "ns", deployNamespace) + return nil, err + } + // In this case, we just return without waiting for it for fast bootstraping. + return existingDepl, nil + } + + if *existingDepl.Spec.Replicas < minScale { + err = util.ScaleDeployment(ctx, deploy.kubernetesClient, deploy.logger, existingDepl.Namespace, existingDepl.Name, minScale) + if err != nil { + deploy.logger.Error(err, "error scaling up function deployment", "function", fn.Name) + return nil, err + } + } + if existingDepl.Status.AvailableReplicas < minScale { + existingDepl, err = util.WaitForDeployment(ctx, deploy.kubernetesClient, deploy.logger, existingDepl, minScale, specializationTimeout) + } + + return existingDepl, err + } else if k8s_err.IsNotFound(err) { + + depl, err := deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Create(ctx, deployment, metav1.CreateOptions{}) + if err != nil { + if k8s_err.IsAlreadyExists(err) { + depl, err = deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Get(ctx, deployName, metav1.GetOptions{}) + } + if err != nil { + deploy.logger.Error(err, "error while creating function deployment", "function", fn.Name, + "deployment_name", deployName, + "deployment_namespace", deployNamespace) + return nil, err + } + } + if minScale > 0 { + depl, err = util.WaitForDeployment(ctx, deploy.kubernetesClient, deploy.logger, depl, minScale, specializationTimeout) + } + return depl, err + } + return nil, err +} + +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 +} + +func (deploy *NewDeploy) deleteDeployment(ctx context.Context, ns string, name string) error { + // DeletePropagationBackground deletes the object immediately and dependent are deleted later + // DeletePropagationForeground not advisable; it marks for deletion and API can still serve those objects + deletePropagation := metav1.DeletePropagationBackground + return deploy.kubernetesClient.AppsV1().Deployments(ns).Delete(ctx, name, metav1.DeleteOptions{ + PropagationPolicy: &deletePropagation, + }) +} + +func (deploy *NewDeploy) getDeploymentSpec(ctx context.Context, fn *fv1.Function, env *fv1.Environment, targetReplicas *int32, + deployName string, deployNamespace string, deployLabels map[string]string, deployAnnotations map[string]string) (*appsv1.Deployment, error) { + + replicas := int32(fn.Spec.InvokeStrategy.ExecutionStrategy.MinScale) + if targetReplicas != nil { + replicas = *targetReplicas + } + + // Effective value: the apiserver fills an absent field with the CRD + // default (90); the helper mirrors that for objects that never crossed + // it. An explicit 0 — expressible from typed clients since the field + // became a pointer — means no drain window. + gracePeriodSeconds := env.Spec.EffectiveTerminationGracePeriod() + + podAnnotations := env.Annotations + if podAnnotations == nil { + podAnnotations = make(map[string]string) + } + + // Here, we don't append deployAnnotations to podAnnotations + // since newdeploy doesn't manager pod lifecycle directly. + + if deploy.useIstio && env.Spec.AllowAccessToExternalNetwork { + podAnnotations["sidecar.istio.io/inject"] = "false" + } + + podLabels := env.Labels + if podLabels == nil { + podLabels = make(map[string]string) + } + + maps.Copy(podLabels, deployLabels) + + resources := deploy.getResources(env, fn) + + // Set maxUnavailable and maxSurge to 20% is because we want + // fission to rollout newer function version gradually without + // affecting any online service. For example, if you set maxSurge + // to 100%, the new ReplicaSet scales up immediately and may + // consume all remaining compute resources which might be an + // issue if a cluster's resource is on a budget. + // TODO: add to ExecutionStrategy so that the user + // can do more fine control over different functions. + maxUnavailable := intstr.FromString("20%") + maxSurge := intstr.FromString("20%") + + // Newdeploy updates the environment variable "LastUpdateTimestamp" of deployment + // whenever a configmap/secret gets an update, but it also leaves multiple ReplicaSets for + // rollback purpose. Since fission always update a deployment instead of performing a + // rollback, set RevisionHistoryLimit to 0 to disable this feature. + revisionHistoryLimit := int32(0) + + rvCount, err := util.ReferencedResourcesRVSum(ctx, deploy.kubernetesClient, fn.Namespace, fn.Spec) + if err != nil { + return nil, err + } + + // Platform-owned env for the user container. Kept as one slice because + // ApplyFunctionEnv re-appends it LAST when the function declares its own + // env — kubelet last-wins is what keeps these names authoritative + // (RFC-0030 §1); admission's reserved-name denylist is only fast feedback. + platformEnv := append([]apiv1.EnvVar{ + { + Name: fv1.ResourceVersionCount, + Value: fmt.Sprintf("%d", rvCount), + }, + // RFC-0023 state API env (nil when functionState is off). The token + // itself arrives via the specialize-on-startup fetcher, which writes + // it to the shared mount (see fetcher.StateTokenFileName). + }, util.StateAPIEnvVars(deploy.fetcherConfig.SharedMountPath())...) + + container, err := util.MergeContainer(&apiv1.Container{ + Name: env.Name, + Image: env.Spec.Runtime.Image, + ImagePullPolicy: deploy.runtimeImagePullPolicy, + TerminationMessagePath: "/dev/termination-log", + // Connection-draining preStop hook; see utils.DrainLifecycle. + Lifecycle: utils.DrainLifecycle(gracePeriodSeconds), + Env: platformEnv, + // https://istio.io/docs/setup/kubernetes/additional-setup/requirements/ + Ports: []apiv1.ContainerPort{ + { + Name: "http-env", + // Now that we have added Port field in spec, should we make this configurable too? + ContainerPort: int32(svcinfo.PortEnvRuntime), + }, + }, + Resources: resources, + }, env.Spec.Runtime.Container) + if err != nil { + return nil, err + } + + // AutomountServiceAccountToken=false stops Kubernetes from injecting + // the fission-fetcher ServiceAccount token into every container in + // the pod. The fetcher container re-mounts the token via the + // projected volume defined below — the user-code container does not. + // See GHSA-85g2-pmrx-r49q. + automountSAToken := false + pod := apiv1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: podLabels, + Annotations: podAnnotations, + }, + Spec: apiv1.PodSpec{ + Containers: []apiv1.Container{*container}, + ServiceAccountName: fv1.FissionFetcherSA, + AutomountServiceAccountToken: &automountSAToken, + TerminationGracePeriodSeconds: &gracePeriodSeconds, + Volumes: []apiv1.Volume{ + util.FetcherSATokenProjectedVolume(), + }, + }, + } + + if deploy.podSpecPatch != nil { + + updatedPodSpec, err := util.MergePodSpec(&pod.Spec, deploy.podSpecPatch) + if err == nil { + pod.Spec = *updatedPodSpec + } else { + deploy.logger.Error(err, "Failed to merge the specs") + } + // Re-clamp after the merge: MergePodSpec propagates a non-nil + // AutomountServiceAccountToken from the patch, which would otherwise + // re-enable the kubelet auto-mount on the user container. See + // GHSA-85g2-pmrx-r49q. + pod.Spec.AutomountServiceAccountToken = new(false) + } + + pod.Spec = *(util.ApplyImagePullSecret(env.Spec.ImagePullSecret, pod.Spec)) + + var ownerReferences []metav1.OwnerReference + if deploy.enableOwnerReferences { + ownerReferences = []metav1.OwnerReference{ + *metav1.NewControllerRef(fn, fv1.SchemeGroupVersion.WithKind("Function")), + } + } + + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deployName, + Labels: deployLabels, + Annotations: deployAnnotations, + OwnerReferences: ownerReferences, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Selector: &metav1.LabelSelector{ + MatchLabels: deployLabels, + }, + Template: pod, + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RollingUpdateDeploymentStrategyType, + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxUnavailable: &maxUnavailable, + MaxSurge: &maxSurge, + }, + }, + RevisionHistoryLimit: &revisionHistoryLimit, + }, + } + + // If custom runtime container name - default env name + mainContainerName, err := deploy.mainContainerName(env) + if err != nil { + return nil, err + } + + // Order of merging is important here - first fetcher, then containers and lastly pod spec + err = deploy.fetcherConfig.AddSpecializingFetcherToPodSpec( + &deployment.Spec.Template.Spec, + mainContainerName, + deployNamespace, + fn, + env, + ) + if err != nil { + return nil, err + } + + if env.Spec.Runtime.PodSpec != nil { + newPodSpec, err := util.MergePodSpec(&deployment.Spec.Template.Spec, env.Spec.Runtime.PodSpec) + if err != nil { + return nil, err + } + deployment.Spec.Template.Spec = *newPodSpec + // Re-clamp after the merge: MergePodSpec propagates a non-nil + // AutomountServiceAccountToken from env.Spec.Runtime.PodSpec, which + // would otherwise re-enable the kubelet auto-mount on the user + // container. See GHSA-85g2-pmrx-r49q. + deployment.Spec.Template.Spec.AutomountServiceAccountToken = new(false) + } + + // Fill-if-nil: applies env.Spec.RuntimeClassName only when the merge + // above (or the base spec) left RuntimeClassName unset, so a + // Runtime.PodSpec override always wins. Unconditional — an env can set + // RuntimeClassName with no Runtime.PodSpec at all. + util.ApplyEnvRuntimeClass(&deployment.Spec.Template.Spec, env) + + // RFC-0030 per-function env. This must run AFTER the + // env.Spec.Runtime.PodSpec merge (and after the container merge above): + // MergePodSpec appends the environment podspec container's Env/EnvFrom, + // so applying earlier would let the environment's entries land last and + // win — inverting the specified precedence (env podspec < function + // EnvFrom < function Env) — and would leave a duplicate env name for + // MergeContainer to reject outright. It also targets mainContainerName, + // not env.Name: a custom runtime container renames the user container. + if err := util.ApplyFunctionEnv(&deployment.Spec.Template.Spec, mainContainerName, deployNamespace, fn, platformEnv); err != nil { + return nil, err + } + + // Re-mount the fission-fetcher SA token at the canonical Kubernetes + // path on the fetcher container only. The pod-level + // AutomountServiceAccountToken=false flag set above suppresses the + // implicit mount on every container, including fetcher, so we have + // to add it back explicitly here. This must run AFTER the + // env.Spec.Runtime.PodSpec merge — MergePodSpec can append additional + // volumeMounts to the fetcher container, including one at this same + // path, and kubelet would reject the pod with a duplicate-mount-path + // error. The helper strips any pre-existing mount at the path before + // adding its own, so running it last guarantees a single mount on the + // fetcher container backed by the projected SA token volume. See + // GHSA-85g2-pmrx-r49q. + if err := util.MountFetcherSATokenOnFetcher(&deployment.Spec.Template.Spec); err != nil { + return nil, err + } + + // RFC-0001 Path B: mount the package image read-only at the fetcher's + // store path on both containers. The fetcher's exists-early-exit then + // skips the pull and proceeds straight to secrets + load — Path B for + // newdeploy is delivery-only; the stock fetcher flow runs unchanged + // (the early-exit makes the fetch a no-op). Applied AFTER every + // MergePodSpec (same convention as the SA-token re-clamps) so a runtime + // pod spec cannot strip or shadow the code mount. + oci, err := deploy.getFunctionOCIArchive(ctx, fn) + if err != nil { + return nil, err + } + if oci != nil { + if err := util.AddImageVolume(&deployment.Spec.Template.Spec, oci, + filepath.Join(deploy.fetcherConfig.SharedMountPath(), deploy.fetcherConfig.TargetFilename(fn, env)), + mainContainerName, util.FetcherContainerName); err != nil { + return nil, err + } + } + + return deployment, nil +} + +// mainContainerName resolves the name of the function's main (user-code) +// container in the deployment's pod spec. It defaults to the environment name +// and switches to the custom runtime container name when the environment +// declares one that is actually present in the runtime PodSpec. It is shared by +// the deployment-spec builder and the HPA call site so the ContainerResource +// metric can never name a container that differs from the one in the pod. +func (deploy *NewDeploy) mainContainerName(env *fv1.Environment) (string, error) { + mainContainerName := env.Name + if env.Spec.Runtime.Container != nil && env.Spec.Runtime.Container.Name != "" && env.Spec.Runtime.PodSpec != nil { + if util.DoesContainerExistInPodSpec(env.Spec.Runtime.Container.Name, env.Spec.Runtime.PodSpec) { + mainContainerName = env.Spec.Runtime.Container.Name + } else { + return "", fmt.Errorf("runtime container %s not found in pod spec", env.Spec.Runtime.Container.Name) + } + } + return mainContainerName, nil +} + +// getResources overrides only the resources which are overridden at function level otherwise +// default to resources specified at environment level +func (deploy *NewDeploy) getResources(env *fv1.Environment, fn *fv1.Function) apiv1.ResourceRequirements { + resources := env.Spec.Resources + if resources.Requests == nil { + resources.Requests = make(map[apiv1.ResourceName]resource.Quantity) + } + if resources.Limits == nil { + resources.Limits = make(map[apiv1.ResourceName]resource.Quantity) + } + // Only override the once specified at function, rest default to values from env. + val, ok := fn.Spec.Resources.Requests[apiv1.ResourceCPU] + if ok && !val.IsZero() { + resources.Requests[apiv1.ResourceCPU] = fn.Spec.Resources.Requests[apiv1.ResourceCPU] + } + + val, ok = fn.Spec.Resources.Requests[apiv1.ResourceMemory] + if ok && !val.IsZero() { + resources.Requests[apiv1.ResourceMemory] = fn.Spec.Resources.Requests[apiv1.ResourceMemory] + } + + val, ok = fn.Spec.Resources.Limits[apiv1.ResourceCPU] + if ok && !val.IsZero() { + resources.Limits[apiv1.ResourceCPU] = fn.Spec.Resources.Limits[apiv1.ResourceCPU] + } + + val, ok = fn.Spec.Resources.Limits[apiv1.ResourceMemory] + if ok && !val.IsZero() { + resources.Limits[apiv1.ResourceMemory] = fn.Spec.Resources.Limits[apiv1.ResourceMemory] + } + + return resources +} + +func (deploy *NewDeploy) createOrGetSvc(ctx context.Context, fn *fv1.Function, deployLabels map[string]string, deployAnnotations map[string]string, svcName string, svcNamespace string) (*apiv1.Service, error) { + logger := otelUtils.LoggerWithTraceID(ctx, deploy.logger) + var ownerReferences []metav1.OwnerReference + if deploy.enableOwnerReferences { + ownerReferences = []metav1.OwnerReference{ + *metav1.NewControllerRef(fn, fv1.SchemeGroupVersion.WithKind("Function")), + } + } + + // The Service carries the managed-by label (RFC-0002) so the EndpointSlice + // controller mirrors it onto the slices and the router's label-filtered + // informer sees them. Labels only — the selector stays deployLabels. + svcLabels := make(map[string]string, len(deployLabels)+1) + maps.Copy(svcLabels, deployLabels) + svcLabels[fv1.MANAGED_BY_LABEL] = fv1.MANAGED_BY_VALUE + + service := &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + Labels: svcLabels, + Annotations: deployAnnotations, + OwnerReferences: ownerReferences, + }, + Spec: apiv1.ServiceSpec{ + Ports: []apiv1.ServicePort{ + { + Name: "http-env", + Port: int32(80), + // Since Function spec now supports Port , should we make this configurable too? + TargetPort: intstr.FromInt(svcinfo.PortEnvRuntime), + }, + }, + Selector: deployLabels, + Type: apiv1.ServiceTypeClusterIP, + }, + } + + svc, created, err := util.CreateOrAdoptService(ctx, deploy.kubernetesClient, logger, deploy.instanceID, svcNamespace, service) + if err != nil { + return nil, err + } + if created { + otelUtils.SpanTrackEvent(ctx, "createdService", otelUtils.GetAttributesForSvc(svc)...) + } + return svc, nil +} + +func (deploy *NewDeploy) deleteSvc(ctx context.Context, ns string, name string) error { + return deploy.kubernetesClient.CoreV1().Services(ns).Delete(ctx, name, metav1.DeleteOptions{}) +} + +// cleanupNewdeploy cleans all kubernetes objects related to function +func (deploy *NewDeploy) cleanupNewdeploy(ctx context.Context, ns string, name string) error { + var result error + + err := deploy.deleteSvc(ctx, ns, name) + if err != nil && !k8s_err.IsNotFound(err) { + deploy.logger.Error(err, "error deleting service for newdeploy function", "function_name", name, + "function_namespace", ns) + result = errors.Join(result, err) + } + + err = deploy.hpaops.DeleteHpa(ctx, ns, name) + if err != nil && !k8s_err.IsNotFound(err) { + deploy.logger.Error(err, "error deleting HPA for newdeploy function", "function_name", name, + "function_namespace", ns) + result = errors.Join(result, err) + } + + err = deploy.deleteDeployment(ctx, ns, name) + if err != nil && !k8s_err.IsNotFound(err) { + deploy.logger.Error(err, "error deleting deployment for newdeploy function", "function_name", name, + "function_namespace", ns) + result = errors.Join(result, err) + } + + return result +} diff --git a/pkg/executor/executortype/newdeploy/newdeploy_test.go b/pkg/executor/executortype/newdeploy/newdeploy_test.go new file mode 100644 index 0000000..42b4b1a --- /dev/null +++ b/pkg/executor/executortype/newdeploy/newdeploy_test.go @@ -0,0 +1,641 @@ +// SPDX-FileCopyrightText: The Fission Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package newdeploy + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + fv1 "github.com/fission/fission/pkg/apis/core/v1" + ferror "github.com/fission/fission/pkg/error" + "github.com/fission/fission/pkg/executor/fscache" + "github.com/fission/fission/pkg/executor/util" + hpautils "github.com/fission/fission/pkg/executor/util/hpa" + fetcherConfig "github.com/fission/fission/pkg/fetcher/config" + fissionfake "github.com/fission/fission/pkg/generated/clientset/versioned/fake" + "github.com/fission/fission/pkg/utils" + "github.com/fission/fission/pkg/utils/loggerfactory" +) + +const envCName = "newdeploy-test-env" + +// newTestNewDeploy returns a minimal NewDeploy wired up for unit tests of +// getDeploymentSpec. +func newTestNewDeploy(t *testing.T) *NewDeploy { + t.Helper() + cfg, err := fetcherConfig.MakeFetcherConfig("/userfunc") + require.NoError(t, err) + return &NewDeploy{ + logger: loggerfactory.GetLogger(), + kubernetesClient: fake.NewSimpleClientset(), + fetcherConfig: cfg, + } +} + +func newTestNewDeployEnv() *fv1.Environment { + return &fv1.Environment{ + Name: envCName, + Namespace: "default", + Spec: fv1.EnvironmentSpec{ + Version: 1, + Runtime: fv1.Runtime{ + Image: "fission/test-env:latest", + }, + }, + } +} + +func newTestNewDeployFunction() *fv1.Function { + return &fv1.Function{ + Name: "newdeploy-test-fn", + Namespace: "default", + Spec: fv1.FunctionSpec{ + Environment: fv1.EnvironmentReference{ + Name: envCName, + Namespace: "default", + }, + Package: fv1.FunctionPackageRef{ + PackageRef: fv1.PackageRef{ + Namespace: "default", + Name: "pkg-1", + ResourceVersion: "1", + }, + }, + InvokeStrategy: fv1.InvokeStrategy{ + ExecutionStrategy: fv1.ExecutionStrategy{ + ExecutorType: fv1.ExecutorTypeNewdeploy, + }, + }, + }, + } +} + +// TestMainContainerName covers mainContainerName, the helper shared by the +// deployment-spec builder and the HPA call site. The updateFunction path relies +// on its error branch to refuse rewriting the ContainerResource metric to a +// container that does not exist in the pod spec. +func TestMainContainerName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mutate func(env *fv1.Environment) + want string + wantErr bool + }{ + { + name: "defaults to env name when no custom runtime container", + mutate: func(*fv1.Environment) {}, + want: envCName, + }, + { + name: "uses custom runtime container name present in pod spec", + mutate: func(env *fv1.Environment) { + env.Spec.Runtime.Container = &apiv1.Container{Name: "custom-runtime"} + env.Spec.Runtime.PodSpec = &apiv1.PodSpec{ + Containers: []apiv1.Container{{Name: "custom-runtime"}}, + } + }, + want: "custom-runtime", + }, + { + name: "errors when custom runtime container absent from pod spec", + mutate: func(env *fv1.Environment) { + env.Spec.Runtime.Container = &apiv1.Container{Name: "custom-runtime"} + env.Spec.Runtime.PodSpec = &apiv1.PodSpec{ + Containers: []apiv1.Container{{Name: "some-other-container"}}, + } + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + tc.mutate(env) + + got, err := deploy.mainContainerName(env) + if tc.wantErr { + require.Error(t, err) + assert.Empty(t, got) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestNewDeployFnDeleteCacheMiss verifies that deleting a function whose fsvc +// is absent from the in-memory cache (never specialized, evicted, or executor +// restarted) does not error out — it must fall back to the deterministic +// computed object name and clean up the backing objects instead of leaking +// them. +func TestNewDeployFnDeleteCacheMiss(t *testing.T) { + logger := loggerfactory.GetLogger() + deploy := &NewDeploy{ + logger: logger, + kubernetesClient: fake.NewSimpleClientset(), + fsCache: fscache.MakeFunctionServiceCache(logger), + nsResolver: utils.DefaultNSResolver(), + hpaops: hpautils.NewHpaOperations(logger, fake.NewSimpleClientset(), "test-instance"), + } + + fn := newTestNewDeployFunction() + fn.UID = "abcdef01-2345-6789-abcd-ef0123456789" + + // fsCache is intentionally left empty so ListByFunctionUID finds nothing. + require.NoError(t, deploy.fnDelete(t.Context(), fn), + "fnDelete must tolerate a cache miss and clean up by computed name") +} + +// TestNewDeployFnCreateDeletionGuard verifies the authoritative re-read at the +// top of fnCreate refuses to create backing objects for a Function that the +// router presented but which is gone or being deleted in the cluster. Without +// this guard an in-flight create can race the delete teardown and re-create the +// Deployment/Service/HPA after teardown removed them, leaking objects whose +// owning Function CR is already gone. +func TestNewDeployFnCreateDeletionGuard(t *testing.T) { + t.Parallel() + + const liveUID = "abcdef01-2345-6789-abcd-ef0123456789" + + tests := []struct { + name string + // stored is the Function in the authoritative store (fake + // fissionClient); nil means the function is absent. + stored *fv1.Function + }{ + { + name: "function absent from fissionClient", + stored: nil, + }, + { + name: "function present but being deleted", + stored: func() *fv1.Function { + fn := newTestNewDeployFunction() + fn.UID = liveUID + fn.DeletionTimestamp = &metav1.Time{Time: time.Now()} + fn.Finalizers = []string{"fission.io/test"} + return fn + }(), + }, + { + name: "function present but with a different UID", + stored: func() *fv1.Function { + fn := newTestNewDeployFunction() + fn.UID = "00000000-0000-0000-0000-000000000000" + return fn + }(), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + logger := loggerfactory.GetLogger() + kubeClient := fake.NewSimpleClientset() + fissionClient := fissionfake.NewSimpleClientset() + if tc.stored != nil { + fissionClient = fissionfake.NewSimpleClientset(tc.stored) + } + deploy := &NewDeploy{ + logger: logger, + kubernetesClient: kubeClient, + fissionClient: fissionClient, + fsCache: fscache.MakeFunctionServiceCache(logger), + nsResolver: utils.DefaultNSResolver(), + hpaops: hpautils.NewHpaOperations(logger, kubeClient, "test-instance"), + } + + // The Function the router presents looks alive (no DeletionTimestamp). + fn := newTestNewDeployFunction() + fn.UID = liveUID + + _, err := deploy.fnCreate(t.Context(), fn) + require.Error(t, err, "fnCreate must refuse a gone/deleting function") + assert.True(t, ferror.IsNotFound(err), + "guard must surface a ferror NotFound, got: %v", err) + + deploys, listErr := kubeClient.AppsV1().Deployments(metav1.NamespaceAll).List(t.Context(), metav1.ListOptions{}) + require.NoError(t, listErr) + assert.Empty(t, deploys.Items, + "no Deployment must be created when the function is gone/deleting") + }) + } +} + +// TestNewDeployFnCreateGuardPass verifies the guard lets a live, matching +// Function proceed past the re-read. We do not stand up a full happy-path +// (Environment, package, pods); proving the guard passed is enough: fnCreate +// fails later on the missing Environment with a non-NotFound error (a k8s +// NotFound from the apiserver lookup, which is not a ferror NotFound). +func TestNewDeployFnCreateGuardPass(t *testing.T) { + t.Parallel() + + live := newTestNewDeployFunction() + live.UID = "abcdef01-2345-6789-abcd-ef0123456789" + + logger := loggerfactory.GetLogger() + kubeClient := fake.NewSimpleClientset() + deploy := &NewDeploy{ + logger: logger, + kubernetesClient: kubeClient, + fissionClient: fissionfake.NewSimpleClientset(live), + fsCache: fscache.MakeFunctionServiceCache(logger), + nsResolver: utils.DefaultNSResolver(), + hpaops: hpautils.NewHpaOperations(logger, kubeClient, "test-instance"), + } + + fn := newTestNewDeployFunction() + fn.UID = live.UID + + _, err := deploy.fnCreate(t.Context(), fn) + require.Error(t, err, "expected fnCreate to fail past the guard on the missing Environment") + assert.False(t, ferror.IsNotFound(err), + "guard must have passed; the error should come from the missing Environment lookup, not the guard. got: %v", err) +} + +// TestNewDeployPodSpecDoesNotAutomountTokenInUserContainer asserts the +// security-advisory-5 invariant for the new-deploy backend: the +// fission-fetcher SA token is only mounted inside the fetcher container, +// never in the user-code container. +func TestNewDeployPodSpecDoesNotAutomountTokenInUserContainer(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + pod := deployment.Spec.Template + + require.NotNil(t, pod.Spec.AutomountServiceAccountToken, + "pod-level AutomountServiceAccountToken must be explicitly set, not nil") + assert.False(t, *pod.Spec.AutomountServiceAccountToken, + "pod-level AutomountServiceAccountToken must be false to keep the user container clean") + assert.Equal(t, fv1.FissionFetcherSA, pod.Spec.ServiceAccountName) + + var projected *apiv1.Volume + for i := range pod.Spec.Volumes { + if pod.Spec.Volumes[i].Name == util.FetcherSATokenVolumeName { + projected = &pod.Spec.Volumes[i] + break + } + } + require.NotNil(t, projected, "projected SA token volume %q must exist", util.FetcherSATokenVolumeName) + require.NotNil(t, projected.Projected, "%q must be a projected volume", util.FetcherSATokenVolumeName) + var hasToken, hasCA, hasNS bool + for _, src := range projected.Projected.Sources { + switch { + case src.ServiceAccountToken != nil: + hasToken = true + assert.Equal(t, "token", src.ServiceAccountToken.Path) + case src.ConfigMap != nil: + hasCA = src.ConfigMap.Name == "kube-root-ca.crt" + case src.DownwardAPI != nil: + hasNS = true + } + } + assert.True(t, hasToken, "projected volume must include a ServiceAccountToken source") + assert.True(t, hasCA, "projected volume must include the kube-root-ca.crt ConfigMap") + assert.True(t, hasNS, "projected volume must include a namespace DownwardAPI source") + + var fetcher, user *apiv1.Container + for i := range pod.Spec.Containers { + switch pod.Spec.Containers[i].Name { + case util.FetcherContainerName: + fetcher = &pod.Spec.Containers[i] + case envCName: + user = &pod.Spec.Containers[i] + } + } + require.NotNil(t, fetcher, "fetcher container must be present") + require.NotNil(t, user, "user (env) container must be present") + + hasProjectedTokenMount := false + for _, vm := range fetcher.VolumeMounts { + if vm.MountPath == util.FetcherSATokenMountPath { + hasProjectedTokenMount = true + assert.Equal(t, util.FetcherSATokenVolumeName, vm.Name, + "fetcher SA token mount must be backed by the projected volume") + assert.True(t, vm.ReadOnly, "fetcher SA token mount must be read-only") + } + } + assert.True(t, hasProjectedTokenMount, + "fetcher must mount its own SA token via projected volume") + + for _, vm := range user.VolumeMounts { + assert.NotEqual(t, util.FetcherSATokenMountPath, vm.MountPath, + "user container must not have any volume mount at the SA token path") + } +} + +// TestNewDeployPodSpecRuntimePodSpecCannotReEnableAutomount asserts that an +// environment whose Spec.Runtime.PodSpec sets AutomountServiceAccountToken=true +// cannot override the security-advisory-5 invariant on the new-deploy +// backend. See GHSA-85g2-pmrx-r49q. +func TestNewDeployPodSpecRuntimePodSpecCannotReEnableAutomount(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + env.Spec.Runtime.PodSpec = &apiv1.PodSpec{ + AutomountServiceAccountToken: new(true), + } + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + pod := deployment.Spec.Template + + require.NotNil(t, pod.Spec.AutomountServiceAccountToken, + "pod-level AutomountServiceAccountToken must be explicitly set, not nil") + assert.False(t, *pod.Spec.AutomountServiceAccountToken, + "env.Spec.Runtime.PodSpec must not be able to re-enable auto-mount of the SA token") +} + +// TestNewDeployPodSpecRuntimePodSpecCannotIntroduceDuplicateSATokenMount +// pins the Copilot-flagged invariant from PR #3366: an env author who supplies +// env.Spec.Runtime.PodSpec.Containers = [{name: "fetcher", volumeMounts: +// [{mountPath: }]}] must not cause the final fetcher container +// to end up with two mounts at the SA token path, which kubelet would reject +// as a duplicate. The mount-enforcement helper must run AFTER the merge so +// any user-supplied mount at that path is stripped before the projected +// volume is added back. See GHSA-85g2-pmrx-r49q. +func TestNewDeployPodSpecRuntimePodSpecCannotIntroduceDuplicateSATokenMount(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + env.Spec.Runtime.PodSpec = &apiv1.PodSpec{ + Containers: []apiv1.Container{ + { + Name: util.FetcherContainerName, + VolumeMounts: []apiv1.VolumeMount{ + { + Name: "evil-sa-mount", + MountPath: util.FetcherSATokenMountPath, + }, + }, + }, + }, + } + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + pod := deployment.Spec.Template + + var fetcher *apiv1.Container + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == util.FetcherContainerName { + fetcher = &pod.Spec.Containers[i] + break + } + } + require.NotNil(t, fetcher, "fetcher container must be present") + + mountsAtSAPath := 0 + var mountVolumeName string + for _, vm := range fetcher.VolumeMounts { + if vm.MountPath == util.FetcherSATokenMountPath { + mountsAtSAPath++ + mountVolumeName = vm.Name + } + } + assert.Equal(t, 1, mountsAtSAPath, + "fetcher container must have exactly one mount at the SA token path; user-supplied mount at the same path must be stripped by MountFetcherSATokenOnFetcher") + assert.Equal(t, util.FetcherSATokenVolumeName, mountVolumeName, + "the surviving mount must be backed by the projected SA token volume, not the user-supplied one") +} + +// TestGetDeploymentSpecRuntimeClassFieldOnly asserts that +// env.Spec.RuntimeClassName reaches the pod template's RuntimeClassName when +// NO env.Spec.Runtime.PodSpec is set at all — the common case, and the one +// that would silently pass if ApplyEnvRuntimeClass were wired inside the +// "if env.Spec.Runtime.PodSpec != nil" block instead of unconditionally. +func TestGetDeploymentSpecRuntimeClassFieldOnly(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + env.Spec.RuntimeClassName = new("gvisor") + require.Nil(t, env.Spec.Runtime.PodSpec, "this test must exercise the no-PodSpec path") + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + + require.NotNil(t, deployment.Spec.Template.Spec.RuntimeClassName) + assert.Equal(t, "gvisor", *deployment.Spec.Template.Spec.RuntimeClassName) +} + +// TestGetDeploymentSpecRuntimeClassNilWhenUnset asserts that an env with no +// RuntimeClassName leaves the pod template's RuntimeClassName nil. +func TestGetDeploymentSpecRuntimeClassNilWhenUnset(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + + assert.Nil(t, deployment.Spec.Template.Spec.RuntimeClassName) +} + +// TestGetDeploymentSpecRuntimeClassPodSpecOverridesField pins the precedence +// rule: when both env.Spec.RuntimeClassName and env.Spec.Runtime.PodSpec. +// RuntimeClassName are set to different values, the PodSpec's value wins. +func TestGetDeploymentSpecRuntimeClassPodSpecOverridesField(t *testing.T) { + deploy := newTestNewDeploy(t) + env := newTestNewDeployEnv() + env.Spec.RuntimeClassName = new("gvisor") + env.Spec.Runtime.PodSpec = &apiv1.PodSpec{ + RuntimeClassName: new("kata"), + } + fn := newTestNewDeployFunction() + ctx := t.Context() + + replicas := int32(1) + deployment, err := deploy.getDeploymentSpec( + ctx, fn, env, &replicas, + "newdeploy-test-fn", + "default", + map[string]string{"app": "newdeploy-test"}, + map[string]string{}, + ) + require.NoError(t, err) + + require.NotNil(t, deployment.Spec.Template.Spec.RuntimeClassName) + assert.Equal(t, "kata", *deployment.Spec.Template.Spec.RuntimeClassName, + "env.Spec.Runtime.PodSpec.RuntimeClassName must win over env.Spec.RuntimeClassName") +} + +func ociTestPackage(deployment fv1.Archive) *fv1.Package { + return &fv1.Package{ + Name: "pkg-1", Namespace: "default", + Spec: fv1.PackageSpec{Deployment: deployment}, + } +} + +// newTestOCINewDeploy wires a NewDeploy whose fissionClient holds pkg and +// whose image-volume gate is set as given. +func newTestOCINewDeploy(t *testing.T, pkg *fv1.Package, imageVolumeOK bool) *NewDeploy { + t.Helper() + deploy := newTestNewDeploy(t) + deploy.fissionClient = fissionfake.NewSimpleClientset(pkg) + deploy.imageVolumeOK = imageVolumeOK + return deploy +} + +// TestGetDeploymentSpecOCIImageVolume asserts the newdeploy Path B pod shape: +// the fetcher container STAYS (it materializes secrets and drives the load), +// and the package image mounts read-only at the fetcher's store path on both +// containers, so the fetcher's exists-early-exit skips the pull. +func TestGetDeploymentSpecOCIImageVolume(t *testing.T) { + t.Parallel() + oci := &fv1.OCIArchive{ + Image: "registry.example.com/code/hello:v1", + ImagePullSecrets: []apiv1.LocalObjectReference{{Name: "regcred"}}, + } + deploy := newTestOCINewDeploy(t, ociTestPackage(fv1.Archive{Type: fv1.ArchiveTypeOCI, OCI: oci}), true) + env := newTestNewDeployEnv() + env.Spec.Version = 2 + fn := newTestNewDeployFunction() + + dep, err := deploy.getDeploymentSpec(t.Context(), fn, env, nil, + "nd-oci", "default", map[string]string{"a": "b"}, map[string]string{}) + require.NoError(t, err) + pod := dep.Spec.Template + + // Both containers present: env runtime + fetcher. + var fetcher, user *apiv1.Container + for i := range pod.Spec.Containers { + switch pod.Spec.Containers[i].Name { + case util.FetcherContainerName: + fetcher = &pod.Spec.Containers[i] + case envCName: + user = &pod.Spec.Containers[i] + } + } + require.NotNil(t, fetcher, "newdeploy Path B keeps the fetcher container") + require.NotNil(t, user) + + // Image volume present. + var imgVol *apiv1.Volume + for i := range pod.Spec.Volumes { + if pod.Spec.Volumes[i].Image != nil { + imgVol = &pod.Spec.Volumes[i] + } + } + require.NotNil(t, imgVol, "an image volume must be present") + assert.Equal(t, oci.Image, imgVol.Image.Reference) + assert.Equal(t, apiv1.PullIfNotPresent, imgVol.Image.PullPolicy) + + // Mounted read-only at the fetcher's store path on BOTH containers. + wantPath := deploy.fetcherConfig.SharedMountPath() + "/" + fetcherConfig.TargetFilenameDeployArchive + for _, c := range []*apiv1.Container{fetcher, user} { + var mount *apiv1.VolumeMount + for i := range c.VolumeMounts { + if c.VolumeMounts[i].Name == imgVol.Name { + mount = &c.VolumeMounts[i] + } + } + require.NotNilf(t, mount, "container %s must mount the image volume", c.Name) + assert.Equal(t, wantPath, mount.MountPath) + assert.True(t, mount.ReadOnly) + } + + // Pull secrets reach the pod; fetcher SA token mount intact; automount off. + assert.Contains(t, pod.Spec.ImagePullSecrets, apiv1.LocalObjectReference{Name: "regcred"}) + hasSAToken := false + for _, vm := range fetcher.VolumeMounts { + if vm.MountPath == util.FetcherSATokenMountPath { + hasSAToken = true + } + } + assert.True(t, hasSAToken, "fetcher keeps its projected SA token mount") + require.NotNil(t, pod.Spec.AutomountServiceAccountToken) + assert.False(t, *pod.Spec.AutomountServiceAccountToken) +} + +// TestGetDeploymentSpecOCIGateOff asserts Path A behavior when the gate is +// off: no image volume; the fetcher pulls instead. +func TestGetDeploymentSpecOCIGateOff(t *testing.T) { + t.Parallel() + oci := &fv1.OCIArchive{Image: "registry.example.com/code/hello:v1"} + deploy := newTestOCINewDeploy(t, ociTestPackage(fv1.Archive{Type: fv1.ArchiveTypeOCI, OCI: oci}), false) + env := newTestNewDeployEnv() + env.Spec.Version = 2 + fn := newTestNewDeployFunction() + + dep, err := deploy.getDeploymentSpec(t.Context(), fn, env, nil, + "nd-oci-off", "default", map[string]string{"a": "b"}, map[string]string{}) + require.NoError(t, err) + for _, v := range dep.Spec.Template.Spec.Volumes { + assert.Nil(t, v.Image, "gate off must not produce an image volume") + } +} + +// TestGetDeploymentSpecNonOCIUnchanged is the parity guard for tarball +// packages with the gate ON: nothing about their pods changes. +func TestGetDeploymentSpecNonOCIUnchanged(t *testing.T) { + t.Parallel() + deploy := newTestOCINewDeploy(t, ociTestPackage(fv1.Archive{Type: fv1.ArchiveTypeUrl, URL: "http://example.com/a.zip"}), true) + env := newTestNewDeployEnv() + env.Spec.Version = 2 + fn := newTestNewDeployFunction() + + dep, err := deploy.getDeploymentSpec(t.Context(), fn, env, nil, + "nd-tarball", "default", map[string]string{"a": "b"}, map[string]string{}) + require.NoError(t, err) + for _, v := range dep.Spec.Template.Spec.Volumes { + assert.Nil(t, v.Image, "tarball packages must not grow an image volume") + } + assert.Empty(t, dep.Spec.Template.Spec.ImagePullSecrets) +}