Skip to content

Commit

Permalink
Make ParserArgs.ImageNamesAsSelector.Registry mandatory
Browse files Browse the repository at this point in the history
It does not make sense to set a non-nil ParserArgs.ImageNamesAsSelector with no Registry in it

Signed-off-by: Armel Soro <[email protected]>
  • Loading branch information
rm3l committed May 10, 2023
1 parent 2013a9d commit d32bcd6
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
7 changes: 6 additions & 1 deletion pkg/devfile/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ type ParserArgs struct {
// all container and Kubernetes/OpenShift components matching a relative image name (say "my-image-name") of an Image component
// will be replaced in the resulting Devfile by: "<local-registry>/<user-org>/<devfile-name>-my-image-name:some-dynamic-unique-tag".
type ImageSelectorArgs struct {
// Registry is the registry base path under which images matching selectors will be built and pushed to.
// Registry is the registry base path under which images matching selectors will be built and pushed to. Required.
//
// Example: <local-registry>/<user-org>
Registry string
// Tag represents a tag identifier under which images matching selectors will be built and pushed to.
Expand All @@ -192,6 +193,10 @@ type ImageSelectorArgs struct {
// ParseDevfile func populates the devfile data, parses and validates the devfile integrity.
// Creates devfile context and runtime objects
func ParseDevfile(args ParserArgs) (d DevfileObj, err error) {
if args.ImageNamesAsSelector != nil && strings.TrimSpace(args.ImageNamesAsSelector.Registry) == "" {
return DevfileObj{}, errors.New("registry is mandatory when setting ImageNamesAsSelector in the parser args")
}

if args.Data != nil {
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(args.Data)
if err != nil {
Expand Down
30 changes: 26 additions & 4 deletions pkg/devfile/parser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
"bytes"
"context"
"fmt"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/git"
"io/ioutil"
"net"
"net/http"
Expand All @@ -32,6 +30,9 @@ import (
"strings"
"testing"

v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/git"

"github.com/devfile/api/v2/pkg/attributes"
devfilepkg "github.com/devfile/api/v2/pkg/devfile"
devfileCtx "github.com/devfile/library/v2/pkg/devfile/parser/context"
Expand Down Expand Up @@ -3338,6 +3339,7 @@ commands:
tests := []struct {
name string
parserArgs ParserArgs
wantErr bool
wantBoolPropsSet []setFields
}{
{
Expand Down Expand Up @@ -3380,12 +3382,32 @@ commands:
{compProp: compBooleanProp{prop: "Secure", name: "kubernetes-deploy", value: nil, compType: v1.KubernetesComponentType}}, //unset property should be nil
},
},
{
name: "error if ImageNamesAsSelector is non-nil but ImageNamesAsSelector.Registry is empty",
parserArgs: ParserArgs{
Data: []byte(mainDevfile),
ConvertKubernetesContentInUri: &isFalse, //this is a workaround for a known parsing issue that requires support for downloading the deploy.yaml https://github.com/devfile/api/issues/1073
ImageNamesAsSelector: &ImageSelectorArgs{},
},
wantErr: true,
},
{
name: "error if ImageNamesAsSelector is non-nil but ImageNamesAsSelector.Registry is blank",
parserArgs: ParserArgs{
Data: []byte(mainDevfile),
ConvertKubernetesContentInUri: &isFalse, //this is a workaround for a known parsing issue that requires support for downloading the deploy.yaml https://github.com/devfile/api/issues/1073
ImageNamesAsSelector: &ImageSelectorArgs{
Registry: " \t \n",
},
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d, err := ParseDevfile(tt.parserArgs)
if err != nil {
t.Errorf("Problems parsing devfile")
if (err != nil) != tt.wantErr {
t.Errorf("unexpected err when parsing devfile: error=%v, wantErr =%v", err, tt.wantErr)
}
for i, _ := range tt.wantBoolPropsSet {
wantProps := tt.wantBoolPropsSet[i]
Expand Down

0 comments on commit d32bcd6

Please sign in to comment.