Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
72 changes: 70 additions & 2 deletions pkg/devfile/generator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,29 @@
package generator

import (
"encoding/json"
"errors"
"fmt"
"github.com/hashicorp/go-multierror"
"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"
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"
)

Expand Down Expand Up @@ -619,11 +625,73 @@ 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) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bad idea to pass a pointer and return a pointer. But I'm struggling to do otherwise because of the way runtime.DefaultUnstructuredConverter.ToUnstructured works.

I'm open to ideas/suggestions.

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
if val == nil {
return nil, nil
}
Comment on lines +648 to +654
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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
if val == nil {
return nil, nil
}
if *errHandler != nil {
if errors.Is(*errHandler, &attributes.KeyNotFoundError{Key: key}){
// no need to continue any further if container-overrides wasn't used for the container component
return nil, nil
}
return nil, *errHandler
}

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)
}
Comment on lines +691 to +693
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
To keep this function consistent with convertResourceToUnstructured, I think the receiving argument for this function should be the same as return value of the other one.

Suggested change
func convertUnstructuredToResource(u unstructured.Unstructured, obj interface{}) error {
return runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), obj)
}
func convertUnstructuredToResource(content map[string]interface{}, obj interface{}) error {
return runtime.DefaultUnstructuredConverter.FromUnstructured(content, 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{
Expand Down
75 changes: 73 additions & 2 deletions pkg/devfile/generator/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
package generator

import (
"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/assert"
"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"
Expand Down Expand Up @@ -1720,3 +1722,72 @@ 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 bool
}{
{
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: false,
},
{
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: 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)
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)
})
}
}