From aafc518062d0ca82262b5a98fa59043be7027fa7 Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Thu, 24 Mar 2022 21:14:55 +0100 Subject: [PATCH] fix: do not assume container 0 is the function container Add a utility method for correctly finding the index of the function Container inside a Deployment spec. Signed-off-by: Lucas Roesler --- pkg/handlers/update.go | 17 ++++++++++------ pkg/k8s/function_status.go | 15 ++++++++++++-- pkg/k8s/securityContext.go | 40 +++++++++++++++++++++++++++++--------- 3 files changed, 55 insertions(+), 17 deletions(-) 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 29b080b7d..0185869a4 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,7 +20,7 @@ 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{ @@ -69,7 +70,8 @@ func nodeSelectorToConstraint(deploy appsv1.Deployment) []string { } func hasReadOnlyRootFilesystem(function appsv1.Deployment) bool { - securityContext := function.Spec.Template.Spec.Containers[0].SecurityContext + _, c := FunctionContainer(function) + securityContext := c.SecurityContext if securityContext == nil { return false } @@ -80,3 +82,12 @@ func hasReadOnlyRootFilesystem(function appsv1.Deployment) bool { return *securityContext.ReadOnlyRootFilesystem } + +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/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",