diff --git a/go.mod b/go.mod index 46d8672e..92530e38 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/xeipuuv/gojsonschema v1.2.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.21.3 + k8s.io/apiextensions-apiserver v0.21.3 k8s.io/apimachinery v0.21.3 k8s.io/client-go v0.21.3 k8s.io/klog v1.0.0 diff --git a/pkg/devfile/generator/generators_test.go b/pkg/devfile/generator/generators_test.go index 3a650174..fac1c503 100644 --- a/pkg/devfile/generator/generators_test.go +++ b/pkg/devfile/generator/generators_test.go @@ -48,6 +48,12 @@ func TestGetContainers(t *testing.T) { containerNames := []string{"testcontainer1", "testcontainer2", "testcontainer3"} containerImages := []string{"image1", "image2", "image3"} + defaultPullPolicy := corev1.PullAlways + defaultEnv := []corev1.EnvVar{ + {Name: "PROJECTS_ROOT", Value: "/projects"}, + {Name: "PROJECT_SOURCE", Value: "/projects/test-project"}, + } + trueMountSources := true falseMountSources := false @@ -94,16 +100,17 @@ func TestGetContainers(t *testing.T) { } tests := []struct { - name string - eventCommands EventCommands - containerComponents []v1.Component - filteredComponents []v1.Component - filterOptions common.DevfileOptions - wantContainerName string - wantContainerImage string - wantContainerEnv []corev1.EnvVar - wantContainerVolMount []corev1.VolumeMount - wantErr *string + name string + eventCommands EventCommands + containerComponents []v1.Component + filteredComponents []v1.Component + filterOptions common.DevfileOptions + wantContainerName string + wantContainerImage string + wantContainerEnv []corev1.EnvVar + wantContainerVolMount []corev1.VolumeMount + wantContainerOverrideData *corev1.Container + wantErr *string }{ { name: "Container with default project root", @@ -297,6 +304,36 @@ func TestGetContainers(t *testing.T) { name: "Simulating error case, check if error matches", wantErr: &errMatches, }, + { + name: "container with container-overrides", + containerComponents: []v1.Component{ + { + Name: containerNames[0], + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: containerImages[0], + }, + }, + }, + Attributes: attributes.Attributes{}.FromMap(map[string]interface{}{ + "container-overrides": map[string]interface{}{"securityContext": map[string]int64{"runAsGroup": 3000}}, + }, nil), + }, + }, + wantContainerName: containerNames[0], + wantContainerImage: containerImages[0], + wantContainerEnv: defaultEnv, + wantContainerOverrideData: &corev1.Container{ + Name: containerNames[0], + Image: containerImages[0], + Env: defaultEnv, + ImagePullPolicy: defaultPullPolicy, + SecurityContext: &corev1.SecurityContext{ + RunAsGroup: pointer.Int64(3000), + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -354,6 +391,9 @@ func TestGetContainers(t *testing.T) { if len(container.VolumeMounts) > 0 && !reflect.DeepEqual(container.VolumeMounts, tt.wantContainerVolMount) { t.Errorf("TestGetContainers() error: Vol Mount mismatch - got: %+v, wanted: %+v", container.VolumeMounts, tt.wantContainerVolMount) } + if tt.wantContainerOverrideData != nil && !reflect.DeepEqual(container, *tt.wantContainerOverrideData) { + t.Errorf("TestGetContainers() error: Container override mismatch - got: %+v, wanted: %+v", container, *tt.wantContainerOverrideData) + } } } else { assert.Regexp(t, *tt.wantErr, err.Error(), "TestGetContainers(): Error message does not match") diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index 941321bb..b6ca034c 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -16,6 +16,7 @@ package generator import ( + "encoding/json" "fmt" "github.com/hashicorp/go-multierror" "path/filepath" @@ -34,8 +35,11 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/strategicpatch" ) +const ContainerOverridesAttribute = "container-overrides" + // convertEnvs converts environment variables from the devfile structure to kubernetes structure func convertEnvs(vars []v1.EnvVar) []corev1.EnvVar { kVars := []corev1.EnvVar{} @@ -422,7 +426,7 @@ func getNetworkingV1IngressSpec(ingressSpecParams IngressSpecParams) *networking }, }, }, - //Field is required to be set based on attempt to create the ingress + // Field is required to be set based on attempt to create the ingress PathType: &pathTypeImplementationSpecific, }, }, @@ -619,11 +623,87 @@ func getAllContainers(devfileObj parser.DevfileObj, options common.DevfileOption return nil, err } } - containers = append(containers, *container) + // Check if there is an override attribute + if comp.Attributes.Exists(ContainerOverridesAttribute) { + patched, err := containerOverridesHandler(comp, container) + if err != nil { + return nil, err + } + containers = append(containers, *patched) + } else { + containers = append(containers, *container) + } } return containers, nil } +// containerOverridesHandler overrides the attributes of a container component as defined inside ContainerOverridesAttribute by a strategic merge patch. +func containerOverridesHandler(comp v1.Component, container *corev1.Container) (*corev1.Container, error) { + // Apply the override + override := &corev1.Container{} + if err := comp.Attributes.GetInto(ContainerOverridesAttribute, override); err != nil { + return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", ContainerOverridesAttribute, comp.Name, err) + } + + restrictContainerOverride := func(override *corev1.Container) error { + var invalidFields []string + if override.Name != "" { + invalidFields = append(invalidFields, "name") + } + if override.Image != "" { + invalidFields = append(invalidFields, "image") + } + if override.Command != nil { + invalidFields = append(invalidFields, "command") + + } + if override.Args != nil { + invalidFields = append(invalidFields, "args") + + } + if override.Ports != nil { + invalidFields = append(invalidFields, "ports") + + } + if override.VolumeMounts != nil { + invalidFields = append(invalidFields, "volumeMounts") + + } + if override.Env != nil { + invalidFields = append(invalidFields, "env") + } + if len(invalidFields) != 0 { + return fmt.Errorf("cannot use %s to override container %s", ContainerOverridesAttribute, strings.Join(invalidFields, ", ")) + } + return nil + } + // check if the override key is allowed + if err := restrictContainerOverride(override); err != nil { + return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", ContainerOverridesAttribute, comp.Name, err) + } + + // get the container-overrides data + overrideJSON := comp.Attributes[ContainerOverridesAttribute] + + originalBytes, err := json.Marshal(container) + if err != nil { + return nil, fmt.Errorf("failed to marshal container to yaml: %w", err) + } + patchedBytes, err := strategicpatch.StrategicMergePatch(originalBytes, overrideJSON.Raw, &corev1.Container{}) + if err != nil { + return nil, fmt.Errorf("failed to apply container overrides: %w", err) + } + patched := &corev1.Container{} + if err := json.Unmarshal(patchedBytes, patched); err != nil { + return nil, fmt.Errorf("error applying container overrides: %w", err) + } + // Applying the patch will overwrite the container's name and image as corev1.Container.Name + // does not have the omitempty json tag. + patched.Name = container.Name + patched.Image = container.Image + return patched, nil +} + // getContainerAnnotations iterates through container components and returns all annotations func getContainerAnnotations(devfileObj parser.DevfileObj, options common.DevfileOptions) (v1.Annotation, error) { options.ComponentOptions = common.ComponentOptions{ diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index 8899731c..3806debf 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -18,6 +18,8 @@ package generator import ( "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" + "k8s.io/utils/pointer" + "path/filepath" "reflect" "strings" @@ -34,6 +36,7 @@ import ( v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -1720,3 +1723,128 @@ func TestMergeMaps(t *testing.T) { }) } } + +func Test_containerOverridesHandler(t *testing.T) { + name := "testcontainer" + image := "quay.io/some/image" + command := []string{"tail"} + argsSlice := []string{"-f", "/dev/null"} + + actualResourcesReqs, _ := testingutil.FakeResourceRequirements("5Mi", "300Mi") + + type args struct { + comp v1.Component + container *corev1.Container + } + tests := []struct { + name string + args args + want *corev1.Container + wantErr bool + errString string + }{ + { + name: "Override the resource requirements of the container component", + args: args{ + comp: v1.Component{ + Name: "component2", + Attributes: attributes.Attributes{ + ContainerOverridesAttribute: apiextensionsv1.JSON{Raw: []byte("{\"resources\": {\"limits\": {\"nvidia.com/gpu\": \"1\"}}, \"requests\": {\"nvidia.com/gpu\": \"1\"}}")}, + }, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice, ResourceReqs: actualResourcesReqs}), + }, + want: func() *corev1.Container { + wantResourcesReqs := actualResourcesReqs + qty, _ := resource.ParseQuantity("1") + wantResourcesReqs.Limits["nvidia.com/gpu"] = qty + wantResourcesReqs.Requests["nvidia.com/gpu"] = qty + return getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice, ResourceReqs: wantResourcesReqs}) + }(), + wantErr: false, + }, + { + name: "Override securityContext of the container component with replace patchDirective", + args: args{ + comp: v1.Component{ + Attributes: attributes.Attributes{ + ContainerOverridesAttribute: apiextensionsv1.JSON{Raw: []byte("{\"securityContext\": {\"runAsUser\": 1001, \"$patch\": \"replace\"}}")}, + }, + }, + container: func() *corev1.Container { + container := getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}) + container.SecurityContext = &corev1.SecurityContext{ + RunAsUser: pointer.Int64(1000), + RunAsGroup: pointer.Int64(2000), + } + return container + }(), + }, + want: func() *corev1.Container { + container := getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}) + container.SecurityContext = &corev1.SecurityContext{ + RunAsUser: pointer.Int64(1001), + } + return container + }(), + wantErr: false, + }, + { + name: "Override securityContext of the container with delete patchDirective", + args: args{ + comp: v1.Component{ + Attributes: attributes.Attributes{ + ContainerOverridesAttribute: apiextensionsv1.JSON{Raw: []byte("{\"securityContext\": {\"$patch\": \"delete\"}}")}, + }, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice, IsPrivileged: true}), + }, + want: func() *corev1.Container { + container := getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}) + container.SecurityContext = &corev1.SecurityContext{} + return container + }(), + wantErr: false, + }, + { + name: "Should not override restricted fields of the container component", + args: args{ + comp: v1.Component{ + Name: "component2", + Attributes: attributes.Attributes{ + ContainerOverridesAttribute: apiextensionsv1.JSON{Raw: []byte("{\"name\": \"othername\",\"image\": \"quay.io/other/image\", \"command\": [\"echo\"], \"args\": [\"hello world\"], \"ports\": [{\"containerPort\":9090}], \"env\": [{\"name\":\"somename\", \"value\":\"somevalue\"}], \"volumeMounts\": [{\"name\":\"volume1\",\"mountPath\":\"/var/www\"}]}")}}, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), + }, + want: nil, + wantErr: true, + errString: "cannot use container-overrides to override container name, image, command, args, ports, volumeMounts, env", + }, + { + name: "Invalid JSON for container-overrides", + args: args{ + comp: v1.Component{ + Name: "component3", + Attributes: attributes.Attributes{ + ContainerOverridesAttribute: apiextensionsv1.JSON{Raw: []byte(`{"image quay.io/other/image"}`)}}, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), + }, + want: nil, + wantErr: true, + errString: "failed to parse container-overrides attribute on component component3", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := containerOverridesHandler(tt.args.comp, tt.args.container) + if tt.wantErr { + assert.NotNil(t, err, tt.name) + assert.Contains(t, err.Error(), tt.errString, "containerOverridesHandler() error does not match") + } else { + assert.Nil(t, err, tt.name) + } + assert.Equalf(t, tt.want, got, "containerOverridesHandler(%v, %v)", tt.args.comp, tt.args.container) + }) + } +}