From d47e4bc37a891e206f42b2deac35330833c81fb4 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Wed, 16 Nov 2022 16:50:53 +0530 Subject: [PATCH 1/3] Adds support for container-overrides Signed-off-by: Dharmit Shah --- go.mod | 3 ++ go.sum | 2 + pkg/devfile/generator/utils.go | 68 ++++++++++++++++++++++++++++- pkg/devfile/generator/utils_test.go | 59 ++++++++++++++++++++++++- 4 files changed, 128 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 46d8672e..49913242 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.15 require ( github.com/devfile/api/v2 v2.2.0 + github.com/devfile/library v1.2.1-0.20220308191614-f0f7e11b17de github.com/devfile/registry-support/registry-library v0.0.0-20221018213054-47b3ffaeadba github.com/fatih/color v1.7.0 github.com/fsnotify/fsnotify v1.4.9 @@ -17,11 +18,13 @@ require ( github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 github.com/openshift/api v0.0.0-20200930075302-db52bc4ef99f github.com/pkg/errors v0.9.1 + github.com/qjebbs/go-jsons v0.0.0-20221021030617-ee43abc0a749 github.com/spf13/afero v1.6.0 github.com/stretchr/testify v1.7.0 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/go.sum b/go.sum index 276feacd..a733ae69 100644 --- a/go.sum +++ b/go.sum @@ -805,6 +805,8 @@ github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O github.com/prometheus/procfs v0.6.0 h1:mxy4L2jP6qMonqmq+aTtOx1ifVWUgG/TAmntgbh3xv4= github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= +github.com/qjebbs/go-jsons v0.0.0-20221021030617-ee43abc0a749 h1:WVxhEUqfIi/euX8eVRfl7CBtewZkv88DiIulPjzJfrU= +github.com/qjebbs/go-jsons v0.0.0-20221021030617-ee43abc0a749/go.mod h1:wNJrtinHyC3YSf6giEh4FJN8+yZV7nXBjvmfjhBIcw4= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index 941321bb..bd6fdc2a 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -16,8 +16,8 @@ package generator import ( + "encoding/json" "fmt" - "github.com/hashicorp/go-multierror" "path/filepath" "reflect" "strings" @@ -25,14 +25,18 @@ import ( v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/v2/pkg/devfile/parser" "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common" + "github.com/hashicorp/go-multierror" buildv1 "github.com/openshift/api/build/v1" routev1 "github.com/openshift/api/route/v1" + jsons "github.com/qjebbs/go-jsons" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" extensionsv1 "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -619,11 +623,71 @@ func getAllContainers(devfileObj parser.DevfileObj, options common.DevfileOption return nil, err } } - containers = append(containers, *container) + modifiedContainer, err := containerOverridesHandler(comp, container) + if err != nil { + return nil, err + } + if modifiedContainer != nil { + containers = append(containers, *modifiedContainer) + } else { + containers = append(containers, *container) + } } return containers, nil } +// containerOverridesHandler modifies the container component to include any container-overrides values in the devfile. +// It does so by merging the devfile JSON with the JSON string representation of container-overrides field +func containerOverridesHandler(comp v1.Component, container *corev1.Container) (*corev1.Container, error) { + var errHandler *error + val := comp.Attributes.Get("container-overrides", errHandler) + if errHandler != nil { + return nil, *errHandler + } + // no need to continue any further if container-overrides wasn't used for the container component + if val == nil { + return nil, nil + } + v, err := json.Marshal(val) + if err != nil { + return nil, err + } + + unstructuredContainer, err := convertResourceToUnstructured(container) + if err != nil { + return nil, err + } + u, err := json.Marshal(unstructuredContainer) + if err != nil { + return nil, err + } + + result, err := jsons.Merge(u, v) + if err != nil { + return nil, err + } + err = json.Unmarshal(result, &unstructuredContainer) + if err != nil { + return nil, err + } + + err = convertUnstructuredToResource(unstructured.Unstructured{Object: unstructuredContainer}, container) + if err != nil { + return nil, err + } + return container, nil +} + +// convertResourceToUnstructured converts a resource to map[string]interface{} which is the element of unstructured.Unstructured +func convertResourceToUnstructured(obj interface{}) (map[string]interface{}, error) { + return runtime.DefaultUnstructuredConverter.ToUnstructured(obj) +} + +// convertUnstructuredToResource converts an unstructured.Unstructured object to type of object passed to it +func convertUnstructuredToResource(u unstructured.Unstructured, obj interface{}) error { + return runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), obj) +} + // 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..a1fc8c0e 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -16,13 +16,16 @@ package generator import ( - "github.com/hashicorp/go-multierror" - "github.com/stretchr/testify/assert" + "fmt" "path/filepath" "reflect" "strings" "testing" + "github.com/hashicorp/go-multierror" + "github.com/stretchr/testify/assert" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/devfile/api/v2/pkg/attributes" "github.com/devfile/library/v2/pkg/devfile/parser" "github.com/devfile/library/v2/pkg/devfile/parser/data" @@ -1720,3 +1723,55 @@ 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"} + containerOverridesString := "container-overrides" + + type args struct { + comp v1.Component + container *corev1.Container + } + tests := []struct { + name string + args args + want *corev1.Container + wantErr error + }{ + { + name: "Case 1: No container-overrides field in the container component", + args: args{ + comp: v1.Component{ + Name: "component1", + Attributes: nil, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), + }, + want: nil, + wantErr: nil, + }, + { + name: "Case 2: Override the image of the container component", + args: args{ + comp: v1.Component{ + Name: "component2", + Attributes: attributes.Attributes{ + containerOverridesString: apiextensionsv1.JSON{Raw: []byte("{\"image\": \"quay.io/other/image\"}")}}, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), + }, + want: getContainer(containerParams{Name: name, Image: "quay.io/other/image", Command: command, Args: argsSlice}), + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := containerOverridesHandler(tt.args.comp, tt.args.container) + assert.Equalf(t, tt.wantErr, err, fmt.Sprintf("Expected %v and %v to be equal", tt.wantErr, err)) + assert.Equalf(t, tt.want, got, "containerOverridesHandler(%v, %v)", tt.args.comp, tt.args.container) + }) + } +} From a5609fa57e41f2fb3289be24c313f0e45911a084 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Thu, 17 Nov 2022 11:01:25 +0530 Subject: [PATCH 2/3] go mod tidy to tidy up go.mod Signed-off-by: Dharmit Shah --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index 49913242..1b5fdda4 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.15 require ( github.com/devfile/api/v2 v2.2.0 - github.com/devfile/library v1.2.1-0.20220308191614-f0f7e11b17de github.com/devfile/registry-support/registry-library v0.0.0-20221018213054-47b3ffaeadba github.com/fatih/color v1.7.0 github.com/fsnotify/fsnotify v1.4.9 From aa6e68dd385ef2d666f76801e9e76101f65bf962 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Fri, 18 Nov 2022 17:01:30 +0530 Subject: [PATCH 3/3] More unit tests, minor change in pointer usage Signed-off-by: Dharmit Shah --- pkg/devfile/generator/utils.go | 10 +++++++--- pkg/devfile/generator/utils_test.go | 26 +++++++++++++++++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index bd6fdc2a..da352fbd 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -17,12 +17,14 @@ package generator import ( "encoding/json" + "errors" "fmt" "path/filepath" "reflect" "strings" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/api/v2/pkg/attributes" "github.com/devfile/library/v2/pkg/devfile/parser" "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common" "github.com/hashicorp/go-multierror" @@ -639,9 +641,11 @@ func getAllContainers(devfileObj parser.DevfileObj, options common.DevfileOption // containerOverridesHandler modifies the container component to include any container-overrides values in the devfile. // It does so by merging the devfile JSON with the JSON string representation of container-overrides field func containerOverridesHandler(comp v1.Component, container *corev1.Container) (*corev1.Container, error) { - var errHandler *error - val := comp.Attributes.Get("container-overrides", errHandler) - if errHandler != nil { + errHandler := new(error) + key := "container-overrides" + + val := comp.Attributes.Get(key, errHandler) + if *errHandler != nil && !errors.Is(*errHandler, &attributes.KeyNotFoundError{Key: key}) { return nil, *errHandler } // no need to continue any further if container-overrides wasn't used for the container component diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index a1fc8c0e..0f3612f6 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -16,7 +16,6 @@ package generator import ( - "fmt" "path/filepath" "reflect" "strings" @@ -1739,7 +1738,7 @@ func Test_containerOverridesHandler(t *testing.T) { name string args args want *corev1.Container - wantErr error + wantErr bool }{ { name: "Case 1: No container-overrides field in the container component", @@ -1751,7 +1750,7 @@ func Test_containerOverridesHandler(t *testing.T) { container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), }, want: nil, - wantErr: nil, + wantErr: false, }, { name: "Case 2: Override the image of the container component", @@ -1764,13 +1763,30 @@ func Test_containerOverridesHandler(t *testing.T) { container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), }, want: getContainer(containerParams{Name: name, Image: "quay.io/other/image", Command: command, Args: argsSlice}), - wantErr: nil, + wantErr: false, + }, + { + name: "Case 3: Invalid JSON for container-overrides", + args: args{ + comp: v1.Component{ + Name: "component3", + Attributes: attributes.Attributes{ + containerOverridesString: apiextensionsv1.JSON{Raw: []byte(`{"image quay.io/other/image"}`)}}, + }, + container: getContainer(containerParams{Name: name, Image: image, Command: command, Args: argsSlice}), + }, + want: nil, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := containerOverridesHandler(tt.args.comp, tt.args.container) - assert.Equalf(t, tt.wantErr, err, fmt.Sprintf("Expected %v and %v to be equal", tt.wantErr, err)) + if tt.wantErr { + assert.NotNil(t, err, tt.name) + } else { + assert.Nil(t, err, tt.name) + } assert.Equalf(t, tt.want, got, "containerOverridesHandler(%v, %v)", tt.args.comp, tt.args.container) }) }