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
  • Loading branch information
rm3l committed May 10, 2023
1 parent cbe8a84 commit 164ec3d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
7 changes: 6 additions & 1 deletion pkg/devfile/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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 @@ -137,6 +138,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
28 changes: 25 additions & 3 deletions pkg/devfile/parser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package parser
import (
"context"
"fmt"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"io/ioutil"
"net"
"net/http"
Expand All @@ -29,6 +28,8 @@ import (
"strings"
"testing"

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

"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 @@ -3329,6 +3330,7 @@ commands:
tests := []struct {
name string
parserArgs ParserArgs
wantErr bool
wantBoolPropsSet []setFields
}{
{
Expand Down Expand Up @@ -3371,12 +3373,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 164ec3d

Please sign in to comment.