diff --git a/pkg/handlers/update.go b/pkg/handlers/update.go index 7068a3b4a..b40b05732 100644 --- a/pkg/handlers/update.go +++ b/pkg/handlers/update.go @@ -91,14 +91,19 @@ func updateDeploymentSpec( } if len(deployment.Spec.Template.Spec.Containers) > 0 { - deployment.Spec.Template.Spec.Containers[0].Image = request.Image + idx, _ := k8s.FunctionContainer(*deployment) + if idx < 0 { + return fmt.Errorf("deployment is not a valid function spec"), http.StatusBadRequest + } + + deployment.Spec.Template.Spec.Containers[idx].Image = request.Image // Disabling update support to prevent unexpected mutations of deployed functions, // since imagePullPolicy is now configurable. This could be reconsidered later depending // on desired behavior, but will need to be updated to take config. - //deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy = v1.PullAlways + //deployment.Spec.Template.Spec.Containers[idx].ImagePullPolicy = v1.PullAlways - deployment.Spec.Template.Spec.Containers[0].Env = buildEnvVars(&request) + deployment.Spec.Template.Spec.Containers[idx].Env = buildEnvVars(&request) factory.ConfigureReadOnlyRootFilesystem(request, deployment) factory.ConfigureContainerUserID(deployment) @@ -135,7 +140,7 @@ func updateDeploymentSpec( return resourceErr, http.StatusBadRequest } - deployment.Spec.Template.Spec.Containers[0].Resources = *resources + deployment.Spec.Template.Spec.Containers[idx].Resources = *resources secrets := k8s.NewSecretsClient(factory.Client) existingSecrets, err := secrets.GetSecrets(functionNamespace, request.Secrets) @@ -154,8 +159,8 @@ func updateDeploymentSpec( return err, http.StatusBadRequest } - deployment.Spec.Template.Spec.Containers[0].LivenessProbe = probes.Liveness - deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = probes.Readiness + deployment.Spec.Template.Spec.Containers[idx].LivenessProbe = probes.Liveness + deployment.Spec.Template.Spec.Containers[idx].ReadinessProbe = probes.Readiness // compare the annotations from args to the cache copy of the deployment annotations // at this point we have already updated the annotations to the new value, if we diff --git a/pkg/k8s/function_status.go b/pkg/k8s/function_status.go index dd5ff2e08..8daca558f 100644 --- a/pkg/k8s/function_status.go +++ b/pkg/k8s/function_status.go @@ -6,6 +6,7 @@ package k8s import ( types "github.com/openfaas/faas-provider/types" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" ) // EnvProcessName is the name of the env variable containing the function process @@ -19,20 +20,23 @@ func AsFunctionStatus(item appsv1.Deployment) *types.FunctionStatus { replicas = uint64(*item.Spec.Replicas) } - functionContainer := item.Spec.Template.Spec.Containers[0] + _, functionContainer := FunctionContainer(item) labels := item.Spec.Template.Labels function := types.FunctionStatus{ - Name: item.Name, - Replicas: replicas, - Image: functionContainer.Image, - AvailableReplicas: uint64(item.Status.AvailableReplicas), - InvocationCount: 0, - Labels: &labels, - Annotations: &item.Spec.Template.Annotations, - Namespace: item.Namespace, - Secrets: ReadFunctionSecretsSpec(item), - CreatedAt: item.CreationTimestamp.Time, + Name: item.Name, + Replicas: replicas, + Image: functionContainer.Image, + AvailableReplicas: uint64(item.Status.AvailableReplicas), + InvocationCount: 0, + Labels: &labels, + Annotations: &item.Spec.Template.Annotations, + Namespace: item.Namespace, + Secrets: ReadFunctionSecretsSpec(item), + CreatedAt: item.CreationTimestamp.Time, + Constraints: nodeSelectorToConstraint(item), + ReadOnlyRootFilesystem: hasReadOnlyRootFilesystem(item), + EnvVars: map[string]string{}, } req := &types.FunctionResources{Memory: functionContainer.Resources.Requests.Memory().String(), CPU: functionContainer.Resources.Requests.Cpu().String()} @@ -48,8 +52,48 @@ func AsFunctionStatus(item appsv1.Deployment) *types.FunctionStatus { for _, v := range functionContainer.Env { if EnvProcessName == v.Name { function.EnvProcess = v.Value + continue } + function.EnvVars[v.Name] = v.Value } return &function } + +func nodeSelectorToConstraint(deploy appsv1.Deployment) []string { + nodeSelector := deploy.Spec.Template.Spec.NodeSelector + constraints := []string{} + for k, v := range nodeSelector { + constraints = append(constraints, k+"="+v) + } + return constraints +} + +func hasReadOnlyRootFilesystem(function appsv1.Deployment) bool { + _, c := FunctionContainer(function) + securityContext := c.SecurityContext + if securityContext == nil { + return false + } + + if securityContext.ReadOnlyRootFilesystem == nil { + return false + } + + return *securityContext.ReadOnlyRootFilesystem +} + +// FunctionContainer returns the index and spec of the OpenFaaS function container +// in the deployment. Use this method to safely retrieve the container spec, it protects +// the controller from potential changes in the deployment spec by other Operators and +// Admission webhooks in the cluster. +// +// idx will be -1 if the function container is not found. +func FunctionContainer(function appsv1.Deployment) (idx int, c corev1.Container) { + for idx, container := range function.Spec.Template.Spec.Containers { + if container.Name == function.Name { + return idx, container + } + } + return -1, c +} diff --git a/pkg/k8s/function_status_test.go b/pkg/k8s/function_status_test.go index fda4b14c8..4eb674b9a 100644 --- a/pkg/k8s/function_status_test.go +++ b/pkg/k8s/function_status_test.go @@ -10,6 +10,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -23,7 +24,10 @@ func Test_AsFunctionStatus(t *testing.T) { annotations := map[string]string{"data": "datavalue"} namespace := "func-namespace" envProcess := "process string here" + envVars := map[string]string{"env1": "env1value", "env2": "env2value"} secrets := []string{"0-imagepullsecret", "1-genericsecret", "2-genericsecret"} + readOnlyRootFilesystem := false + constraints := []string{"node-label=node-value"} deploy := appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -39,6 +43,9 @@ func Test_AsFunctionStatus(t *testing.T) { Labels: labels, }, Spec: corev1.PodSpec{ + NodeSelector: map[string]string{ + "node-label": "node-value", + }, ImagePullSecrets: []corev1.LocalObjectReference{ {Name: "0-imagepullsecret"}, }, @@ -48,7 +55,8 @@ func Test_AsFunctionStatus(t *testing.T) { Image: image, Env: []corev1.EnvVar{ {Name: "fprocess", Value: envProcess}, - {Name: "customEnv", Value: "customValue"}, + {Name: "env1", Value: "env1value"}, + {Name: "env2", Value: "env2value"}, }, }, }, @@ -122,4 +130,73 @@ func Test_AsFunctionStatus(t *testing.T) { t.Errorf("incorrect Secrets: expected %v, got : %v", secrets, status.Secrets) } + if !reflect.DeepEqual(status.EnvVars, envVars) { + t.Errorf("incorrect EnvVars: expected %+v, got %+v", envVars, status.EnvVars) + } + + if !reflect.DeepEqual(status.Constraints, constraints) { + t.Errorf("incorrect Constraints: expected %+v, got %+v", constraints, status.Constraints) + } + + if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem { + t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem) + } + + t.Run("can parse readonly root filesystem when nil", func(t *testing.T) { + readOnlyRootFilesystem := false + deployment := deploy + deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{} + + status := AsFunctionStatus(deployment) + if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem { + t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem) + } + }) + + t.Run("can parse readonly root filesystem enabled", func(t *testing.T) { + readOnlyRootFilesystem := true + deployment := deploy + deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + ReadOnlyRootFilesystem: &readOnlyRootFilesystem, + } + + status := AsFunctionStatus(deployment) + if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem { + t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem) + } + }) + + t.Run("returns non-empty resource requests", func(t *testing.T) { + deployment := deploy + deployment.Spec.Template.Spec.Containers[0].Resources.Requests = corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + } + + status := AsFunctionStatus(deployment) + if status.Requests.CPU != "100m" { + t.Errorf("incorrect Requests.CPU: expected %s, got %s", "100m", status.Requests.CPU) + } + + if status.Requests.Memory != "100Mi" { + t.Errorf("incorrect Requests.Memory: expected %s, got %s", "100Mi", status.Requests.Memory) + } + }) + + t.Run("returns non-empty resource limits", func(t *testing.T) { + deployment := deploy + deployment.Spec.Template.Spec.Containers[0].Resources.Limits = corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + } + + status := AsFunctionStatus(deployment) + if status.Limits.CPU != "100m" { + t.Errorf("incorrect Limits.CPU: expected %s, got %s", "100m", status.Limits.CPU) + } + + if status.Limits.Memory != "100Mi" { + t.Errorf("incorrect Limits.Memory: expected %s, got %s", "100Mi", status.Limits.Memory) + } + }) } diff --git a/pkg/k8s/securityContext.go b/pkg/k8s/securityContext.go index 01a940965..65b70cc82 100644 --- a/pkg/k8s/securityContext.go +++ b/pkg/k8s/securityContext.go @@ -23,11 +23,22 @@ func (f *FunctionFactory) ConfigureContainerUserID(deployment *appsv1.Deployment functionUser = &userID } - if deployment.Spec.Template.Spec.Containers[0].SecurityContext == nil { - deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{} + if deployment == nil { + return } - deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser = functionUser + idx, container := FunctionContainer(*deployment) + if idx < 0 { + // function container not found + // and there is nothing we can do at this point + return + } + + if container.SecurityContext == nil { + deployment.Spec.Template.Spec.Containers[idx].SecurityContext = &corev1.SecurityContext{} + } + + deployment.Spec.Template.Spec.Containers[idx].SecurityContext.RunAsUser = functionUser } // ConfigureReadOnlyRootFilesystem will create or update the required settings and mounts to ensure @@ -39,10 +50,21 @@ func (f *FunctionFactory) ConfigureContainerUserID(deployment *appsv1.Deployment // // This method is safe for both create and update operations. func (f *FunctionFactory) ConfigureReadOnlyRootFilesystem(request types.FunctionDeployment, deployment *appsv1.Deployment) { - if deployment.Spec.Template.Spec.Containers[0].SecurityContext != nil { - deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem = &request.ReadOnlyRootFilesystem + if deployment == nil { + return + } + + idx, container := FunctionContainer(*deployment) + if idx < 0 { + // function container not found + // and there is nothing we can do at this point + return + } + + if container.SecurityContext != nil { + deployment.Spec.Template.Spec.Containers[idx].SecurityContext.ReadOnlyRootFilesystem = &request.ReadOnlyRootFilesystem } else { - deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + deployment.Spec.Template.Spec.Containers[idx].SecurityContext = &corev1.SecurityContext{ ReadOnlyRootFilesystem: &request.ReadOnlyRootFilesystem, } } @@ -50,8 +72,8 @@ func (f *FunctionFactory) ConfigureReadOnlyRootFilesystem(request types.Function existingVolumes := removeVolume("temp", deployment.Spec.Template.Spec.Volumes) deployment.Spec.Template.Spec.Volumes = existingVolumes - existingMounts := removeVolumeMount("temp", deployment.Spec.Template.Spec.Containers[0].VolumeMounts) - deployment.Spec.Template.Spec.Containers[0].VolumeMounts = existingMounts + existingMounts := removeVolumeMount("temp", container.VolumeMounts) + deployment.Spec.Template.Spec.Containers[idx].VolumeMounts = existingMounts if request.ReadOnlyRootFilesystem { deployment.Spec.Template.Spec.Volumes = append( @@ -64,7 +86,7 @@ func (f *FunctionFactory) ConfigureReadOnlyRootFilesystem(request types.Function }, ) - deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append( + deployment.Spec.Template.Spec.Containers[idx].VolumeMounts = append( existingMounts, corev1.VolumeMount{ Name: "temp", diff --git a/pkg/k8s/securityContext_test.go b/pkg/k8s/securityContext_test.go index 8fa29cfbd..575a94a53 100644 --- a/pkg/k8s/securityContext_test.go +++ b/pkg/k8s/securityContext_test.go @@ -10,6 +10,7 @@ import ( appsv1 "k8s.io/api/apps/v1" apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func readOnlyRootDisabled(t *testing.T, deployment *appsv1.Deployment) { @@ -68,6 +69,9 @@ func readOnlyRootEnabled(t *testing.T, deployment *appsv1.Deployment) { func Test_configureReadOnlyRootFilesystem_Disabled_To_Disabled(t *testing.T) { f := mockFactory() deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testfunc", + }, Spec: appsv1.DeploymentSpec{ Template: apiv1.PodTemplateSpec{ Spec: apiv1.PodSpec{ @@ -91,6 +95,9 @@ func Test_configureReadOnlyRootFilesystem_Disabled_To_Disabled(t *testing.T) { func Test_configureReadOnlyRootFilesystem_Disabled_To_Enabled(t *testing.T) { f := mockFactory() deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testfunc", + }, Spec: appsv1.DeploymentSpec{ Template: apiv1.PodTemplateSpec{ Spec: apiv1.PodSpec{ @@ -115,6 +122,9 @@ func Test_configureReadOnlyRootFilesystem_Enabled_To_Disabled(t *testing.T) { f := mockFactory() trueValue := true deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testfunc", + }, Spec: appsv1.DeploymentSpec{ Template: apiv1.PodTemplateSpec{ Spec: apiv1.PodSpec{ @@ -155,6 +165,9 @@ func Test_configureReadOnlyRootFilesystem_Enabled_To_Enabled(t *testing.T) { f := mockFactory() trueValue := true deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testfunc", + }, Spec: appsv1.DeploymentSpec{ Template: apiv1.PodTemplateSpec{ Spec: apiv1.PodSpec{