From c6020678209d65194ad9212de24615af785f2153 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 25 Feb 2021 12:06:03 -0500 Subject: [PATCH 1/7] add unit tests Signed-off-by: Stephanie --- devfile.yaml | 3 +- go.mod | 2 +- go.sum | 2 + pkg/devfile/parser/parse.go | 21 +- pkg/devfile/parser/parse_test.go | 541 +++++++++++++++++++++++++++++-- 5 files changed, 537 insertions(+), 32 deletions(-) diff --git a/devfile.yaml b/devfile.yaml index 0508b153..62725cc8 100644 --- a/devfile.yaml +++ b/devfile.yaml @@ -5,7 +5,8 @@ metadata: attributes: alpha.build-dockerfile: /relative/path/to/Dockerfile parent: - uri: https://raw.githubusercontent.com/odo-devfiles/registry/master/devfiles/nodejs/devfile.yaml + # uri: https://raw.githubusercontent.com/odo-devfiles/registry/master/devfiles/nodejs/devfile.yaml + id: nodejs commands: - id: install exec: diff --git a/go.mod b/go.mod index 7e886bf3..8f010c86 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/devfile/library go 1.13 require ( - github.com/devfile/api/v2 v2.0.0-20210219185433-585f5fe35835 + github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987 github.com/fatih/color v1.7.0 github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 github.com/gobwas/glob v0.2.3 diff --git a/go.sum b/go.sum index 0fae7b99..97ff4109 100644 --- a/go.sum +++ b/go.sum @@ -46,6 +46,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/devfile/api/v2 v2.0.0-20210219185433-585f5fe35835 h1:PalHtpqhvX/yu5DKFqwTWa1h7UJBd0H7+veJStur/wg= github.com/devfile/api/v2 v2.0.0-20210219185433-585f5fe35835/go.mod h1:Cot4snybn3qhIh48oIFi9McocnIx7zY5fFbjfrIpPvg= +github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987 h1:3wclWpWL/+IP6oAMY1M+RECQYa/4ZBkff7jDX1RyLxg= +github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987/go.mod h1:Cot4snybn3qhIh48oIFi9McocnIx7zY5fFbjfrIpPvg= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/docker/docker v0.7.3-0.20190327010347-be7ac8be2ae0/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 087b3f3f..651e6cc8 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -3,6 +3,7 @@ package parser import ( "encoding/json" "fmt" + "github.com/devfile/library/pkg/util" "net/url" "path" "strings" @@ -20,6 +21,8 @@ import ( "github.com/pkg/errors" ) +const defaultRegistry = "preview-devfile-registry-stage.apps.app-sre-stage-0.k3s7.p1.openshiftapps.com" + // ParseDevfile func validates the devfile integrity. // Creates devfile context and runtime objects func parseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { @@ -123,8 +126,24 @@ func parseParentAndPlugin(d DevfileObj) (err error) { if err != nil { return err } + } else if parent.Id != "" { + registry := defaultRegistry + if parent.RegistryUrl != "" { + registry = parent.RegistryUrl + } + param := util.HTTPRequestParams{ + URL: fmt.Sprintf("http://%s/devfiles/%s", registry, parent.Id), + } + returnedVar, err := util.HTTPGetRequest(param, 0) + if err != nil { + return err + } + parentDevfileObj, err = ParseFromData(returnedVar) + if err != nil { + return err + } } else { - return fmt.Errorf("parent URI undefined, currently only URI is suppported") + return fmt.Errorf("parent URI or registry id undefined, currently only URI and Id are suppported") } parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspace() diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index b1df4bd5..b92a5684 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -22,7 +22,7 @@ import ( const schemaV200 = "2.0.0" -func Test_parseParentAndPlugin(t *testing.T) { +func Test_parseParentAndPluginFromURI(t *testing.T) { type args struct { devFileObj DevfileObj @@ -38,7 +38,7 @@ func Test_parseParentAndPlugin(t *testing.T) { testRecursiveReference bool }{ { - name: "case 1: it should override the requested parent's data and add the local devfile's data", + name: "it should override the requested parent's data and add the local devfile's data", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -253,7 +253,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 2: handle a parent'data without any local override and add the local devfile's data", + name: "handle a parent'data without any local override and add the local devfile's data", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -436,7 +436,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 3: it should error out when the override is invalid", + name: "it should error out when the override is invalid", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -502,7 +502,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 4: error out if the same parent command is defined again in the local devfile", + name: "error out if the same parent command is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -555,7 +555,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 5: error out if the same parent component is defined again in the local devfile", + name: "error out if the same parent component is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -612,7 +612,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 6: should not have error if the same event is defined again in the local devfile", + name: "should not have error if the same event is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -669,7 +669,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 7: error out if the parent project is defined again in the local devfile", + name: "error out if the parent project is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -714,7 +714,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 8: it should merge the plugin's uri data and add the local devfile's data", + name: "it should merge the plugin's uri data and add the local devfile's data", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -897,7 +897,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 9: it should override the plugin's data with local overrides and add the local devfile's data", + name: "it should override the plugin's data with local overrides and add the local devfile's data", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1107,7 +1107,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 10: it should error out when the plugin devfile is invalid", + name: "it should error out when the plugin devfile is invalid", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1152,7 +1152,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 11: error out if the same plugin command is defined again in the local devfile", + name: "error out if the same plugin command is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1205,7 +1205,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 12: error out if the same plugin component is defined again in the local devfile", + name: "error out if the same plugin component is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1262,7 +1262,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 13: error out if the plugin project is defined again in the local devfile", + name: "error out if the plugin project is defined again in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1307,7 +1307,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 14: error out if the same project is defined in the both plugin devfile and parent", + name: "error out if the same project is defined in the both plugin devfile and parent", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1371,7 +1371,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 15: error out if the same command is defined in both plugin devfile and parent devfile", + name: "error out if the same command is defined in both plugin devfile and parent devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1447,7 +1447,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 16: error out if the same component is defined in both plugin devfile and parent devfile", + name: "error out if the same component is defined in both plugin devfile and parent devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1529,7 +1529,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 17: it should override the requested parent's data and plugin's data, and add the local devfile's data", + name: "it should override the requested parent's data and plugin's data, and add the local devfile's data", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1784,7 +1784,7 @@ func Test_parseParentAndPlugin(t *testing.T) { }, }, { - name: "case 18: error out if the plugin component is defined with a different component type in the local devfile", + name: "error out if the plugin component is defined with a different component type in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1841,7 +1841,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 19: it should override with no errors if the plugin component is defined with a different component type in the plugin override", + name: "it should override with no errors if the plugin component is defined with a different component type in the plugin override", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1914,7 +1914,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: false, }, { - name: "case 20: error out if the parent component is defined with a different component type in the local devfile", + name: "error out if the parent component is defined with a different component type in the local devfile", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -1971,7 +1971,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: true, }, { - name: "case 21: it should override with no errors if the parent component is defined with a different component type in the parent override", + name: "it should override with no errors if the parent component is defined with a different component type in the parent override", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -2049,7 +2049,7 @@ func Test_parseParentAndPlugin(t *testing.T) { wantErr: false, }, { - name: "case 22: error out if the URI is recursively referenced", + name: "error out if the URI is recursively referenced", args: args{ devFileObj: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), @@ -2399,6 +2399,489 @@ func Test_parseParentAndPlugin_RecursivelyReference_withMultipleURI(t *testing.T }) } +func Test_parseParentFromRegistry(t *testing.T) { + const validRegistry = "127.0.0.1:8080" + const invalidRegistry = "invalid-registry.io" + mountSource := true + parentDevfile := DevfileObj{ + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevfileHeader: devfilepkg.DevfileHeader{ + SchemaVersion: schemaV200, + }, + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Components: []v1.Component{ + { + Name: "parent-runtime", + ComponentUnion: v1.ComponentUnion{ + Volume: &v1.VolumeComponent{ + Volume: v1.Volume{ + Size: "500Mi", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + var data []byte + var err error + if strings.Contains(r.URL.Path, "/devfiles/nodejs") { + data, err = yaml.Marshal(parentDevfile.Data) + } else { + w.WriteHeader(http.StatusNotFound) + return + } + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + _, err = w.Write(data) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + })) + // create a listener with the desired port. + l, err := net.Listen("tcp", validRegistry) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + // NewUnstartedServer creates a listener. Close that listener and replace + // with the one we created. + testServer.Listener.Close() + testServer.Listener = l + + testServer.Start() + defer testServer.Close() + + tests := []struct { + name string + mainDevfile DevfileObj + registryURI string + wantDevFile DevfileObj + wantErr bool + testRecursiveReference bool + }{ + { + name: "it should override the requested parent's data from provided registry and add the local devfile's data", + mainDevfile: DevfileObj{ + Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + Parent: &v1.Parent{ + ImportReference: v1.ImportReference{ + RegistryUrl: validRegistry, + ImportReferenceUnion: v1.ImportReferenceUnion{ + Id: "nodejs", + }, + }, + ParentOverrides: v1.ParentOverrides{ + Components: []v1.ComponentParentOverride{ + { + Name: "parent-runtime", + ComponentUnionParentOverride: v1.ComponentUnionParentOverride{ + Container: &v1.ContainerComponentParentOverride{ + ContainerParentOverride: v1.ContainerParentOverride{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + }, + }, + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStop: []string{"post-stop"}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + }, + }, + }, + }, + }, + wantDevFile: DevfileObj{ + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "parent-runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStart: []string{}, + PostStop: []string{"post-stop"}, + PreStop: []string{}, + PreStart: []string{}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "it should override the requested parent's data from default registry and add the local devfile's data", + mainDevfile: DevfileObj{ + Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + Parent: &v1.Parent{ + ImportReference: v1.ImportReference{ + ImportReferenceUnion: v1.ImportReferenceUnion{ + Id: "nodejs", + }, + }, + ParentOverrides: v1.ParentOverrides{ + Components: []v1.ComponentParentOverride{ + { + Name: "runtime", + ComponentUnionParentOverride: v1.ComponentUnionParentOverride{ + Container: &v1.ContainerComponentParentOverride{ + ContainerParentOverride: v1.ContainerParentOverride{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + }, + }, + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStop: []string{"post-stop"}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + }, + }, + }, + }, + }, + wantDevFile: DevfileObj{ + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "install", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + Component: "runtime", + WorkingDir: "/project", + CommandLine: "npm install", + LabeledCommand: v1.LabeledCommand{ + BaseCommand: v1.BaseCommand{ + Group: &v1.CommandGroup{ + Kind: v1.BuildCommandGroupKind, + IsDefault: true, + }, + }, + }, + }, + }, + }, + { + Id: "run", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + Component: "runtime", + WorkingDir: "/project", + CommandLine: "npm start", + LabeledCommand: v1.LabeledCommand{ + BaseCommand: v1.BaseCommand{ + Group: &v1.CommandGroup{ + Kind: v1.RunCommandGroupKind, + IsDefault: true, + }, + }, + }, + }, + }, + }, + { + Id: "debug", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + Component: "runtime", + WorkingDir: "/project", + CommandLine: "npm run debug", + LabeledCommand: v1.LabeledCommand{ + BaseCommand: v1.BaseCommand{ + Group: &v1.CommandGroup{ + Kind: v1.DebugCommandGroupKind, + IsDefault: true, + }, + }, + }, + }, + }, + }, + { + Id: "test", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + Component: "runtime", + WorkingDir: "/project", + CommandLine: "npm test", + LabeledCommand: v1.LabeledCommand{ + BaseCommand: v1.BaseCommand{ + Group: &v1.CommandGroup{ + Kind: v1.TestCommandGroupKind, + IsDefault: true, + }, + }, + }, + }, + }, + }, + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Endpoints: []v1.Endpoint{ + { + Name: "http-3000", + TargetPort: 3000, + }, + }, + Container: v1.Container{ + Image: "quay.io/nodejs-12", + MemoryLimit: "1024Mi", + MountSources: &mountSource, + SourceMapping: "/project", + }, + }, + }, + }, + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStart: []string{}, + PostStop: []string{"post-stop"}, + PreStop: []string{}, + PreStart: []string{}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + StarterProjects: []v1.StarterProject{ + { + Name: "nodejs-starter", + ProjectSource: v1.ProjectSource{ + Git: &v1.GitProjectSource{ + GitLikeProjectSource: v1.GitLikeProjectSource{ + Remotes: map[string]string{ + "origin": "https://github.com/odo-devfiles/nodejs-ex.git", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "it should error out with invalid registry provided", + mainDevfile: DevfileObj{ + Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + Parent: &v1.Parent{ + ImportReference: v1.ImportReference{ + ImportReferenceUnion: v1.ImportReferenceUnion{ + Id: "nodejs", + }, + RegistryUrl: invalidRegistry, + }, + }, + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{}, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "it should error out with non-exist registry id provided", + mainDevfile: DevfileObj{ + Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + Parent: &v1.Parent{ + ImportReference: v1.ImportReference{ + ImportReferenceUnion: v1.ImportReferenceUnion{ + Id: "not-exist", + }, + }, + }, + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{}, + }, + }, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + err := parseParentAndPlugin(tt.mainDevfile) + + // Unexpected error + if (err != nil) != tt.wantErr { + t.Errorf("parseParentAndPlugin() error = %v, wantErr %v", err, tt.wantErr) + return + } + + // Expected error and got an err + if tt.wantErr && err != nil { + return + } + + if !reflect.DeepEqual(tt.mainDevfile.Data, tt.wantDevFile.Data) { + t.Errorf("wanted: %v, got: %v, difference at %v", tt.wantDevFile.Data, tt.mainDevfile.Data, pretty.Compare(tt.mainDevfile.Data, tt.wantDevFile.Data)) + } + + }) + } +} + func Test_parseFromURI(t *testing.T) { const uri1 = "127.0.0.1:8080" const httpPrefix = "http://" @@ -2556,36 +3039,36 @@ func Test_parseFromURI(t *testing.T) { wantErr bool }{ { - name: "case 1: should be able to parse from relative uri on local disk", + name: "should be able to parse from relative uri on local disk", curDevfileCtx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), wantDevFile: localDevfile, uri: localRelativeURI, }, { - name: "case 2: should be able to parse relative uri from URL", + name: "should be able to parse relative uri from URL", curDevfileCtx: parentDevfile.Ctx, wantDevFile: relativeParentDevfile, uri: localRelativeURI, }, { - name: "case 3: should fail if no path or url has been set for devfile ctx", + name: "should fail if no path or url has been set for devfile ctx", curDevfileCtx: devfileCtx.DevfileCtx{}, wantErr: true, }, { - name: "case 4: should fail if file not exist", + name: "should fail if file not exist", curDevfileCtx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), uri: notExistURI, wantErr: true, }, { - name: "case 5: should fail if url not exist", + name: "should fail if url not exist", curDevfileCtx: devfileCtx.NewURLDevfileCtx(httpPrefix + uri1), uri: notExistURI, wantErr: true, }, { - name: "case 6: should fail if with invalid URI format", + name: "should fail if with invalid URI format", curDevfileCtx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), uri: invalidURL, wantErr: true, From 211c6c4d12e1f69cc79539366dc00d437885762f Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 25 Feb 2021 12:10:43 -0500 Subject: [PATCH 2/7] run go mod Signed-off-by: Stephanie --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 97ff4109..252c1324 100644 --- a/go.sum +++ b/go.sum @@ -44,8 +44,6 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/devfile/api/v2 v2.0.0-20210219185433-585f5fe35835 h1:PalHtpqhvX/yu5DKFqwTWa1h7UJBd0H7+veJStur/wg= -github.com/devfile/api/v2 v2.0.0-20210219185433-585f5fe35835/go.mod h1:Cot4snybn3qhIh48oIFi9McocnIx7zY5fFbjfrIpPvg= github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987 h1:3wclWpWL/+IP6oAMY1M+RECQYa/4ZBkff7jDX1RyLxg= github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987/go.mod h1:Cot4snybn3qhIh48oIFi9McocnIx7zY5fFbjfrIpPvg= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= From cd17c8fd8b1f8b32db0cdb7ae5439c9cee028859 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 9 Mar 2021 10:57:51 -0500 Subject: [PATCH 3/7] restructure parse functions Signed-off-by: Stephanie --- pkg/devfile/parse.go | 22 ++ pkg/devfile/parser/context/context.go | 13 + pkg/devfile/parser/parse.go | 149 ++++++--- pkg/devfile/parser/parse_test.go | 448 +++++++------------------- 4 files changed, 255 insertions(+), 377 deletions(-) diff --git a/pkg/devfile/parse.go b/pkg/devfile/parse.go index cebc9fab..92a858ed 100644 --- a/pkg/devfile/parse.go +++ b/pkg/devfile/parse.go @@ -9,6 +9,7 @@ import ( // and validates the devfile integrity with the schema // and validates the devfile data. // Creates devfile context and runtime objects. +// Deprecated, use ParseDevfileAndValidate() instead func ParseFromURLAndValidate(url string) (d parser.DevfileObj, err error) { // read and parse devfile from the given URL @@ -30,6 +31,7 @@ func ParseFromURLAndValidate(url string) (d parser.DevfileObj, err error) { // and validates the devfile integrity with the schema // and validates the devfile data. // Creates devfile context and runtime objects. +// Deprecated, use ParseDevfileAndValidate() instead func ParseFromDataAndValidate(data []byte) (d parser.DevfileObj, err error) { // read and parse devfile from the given bytes d, err = parser.ParseFromData(data) @@ -49,6 +51,7 @@ func ParseFromDataAndValidate(data []byte) (d parser.DevfileObj, err error) { // and validates the devfile integrity with the schema // and validates the devfile data. // Creates devfile context and runtime objects. +// Deprecated, use ParseDevfileAndValidate() instead func ParseAndValidate(path string) (d parser.DevfileObj, err error) { // read and parse devfile from given path @@ -65,3 +68,22 @@ func ParseAndValidate(path string) (d parser.DevfileObj, err error) { return d, err } + +// ParseDevfileAndValidate func parses the devfile data +// and validates the devfile integrity with the schema +// and validates the devfile data. +// Creates devfile context and runtime objects. +func ParseDevfileAndValidate(args parser.ParserArgs) (d parser.DevfileObj, err error) { + d, err = parser.ParseDevfile(args) + if err != nil { + return d, err + } + + // generic validation on devfile content + err = validate.ValidateDevfileData(d.Data) + if err != nil { + return d, err + } + + return d, err +} diff --git a/pkg/devfile/parser/context/context.go b/pkg/devfile/parser/context/context.go index fa51fa7e..6dd99de5 100644 --- a/pkg/devfile/parser/context/context.go +++ b/pkg/devfile/parser/context/context.go @@ -35,6 +35,9 @@ type DevfileCtx struct { // trace of all url referenced uriMap map[string]bool + + // registry URLs list + registryURLs []string } // NewDevfileCtx returns a new DevfileCtx type object @@ -149,3 +152,13 @@ func (d *DevfileCtx) GetURIMap() map[string]bool { func (d *DevfileCtx) SetURIMap(uriMap map[string]bool) { d.uriMap = uriMap } + +// GetRegistryURLs func returns current devfile registry URLs +func (d *DevfileCtx) GetRegistryURLs() []string { + return d.registryURLs +} + +// SetRegistryURLs set registry URLs in the devfile ctx +func (d *DevfileCtx) SetRegistryURLs(registryURLs []string) { + d.registryURLs = registryURLs +} diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 76b77216..ca9caedc 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -21,8 +21,6 @@ import ( "github.com/pkg/errors" ) -const defaultRegistry = "preview-devfile-registry-stage.apps.app-sre-stage-0.k3s7.p1.openshiftapps.com" - // ParseDevfile func validates the devfile integrity. // Creates devfile context and runtime objects func parseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { @@ -56,60 +54,99 @@ func parseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { return d, nil } -// Parse func populates the flattened devfile data, parses and validates the devfile integrity. +type ParserArgs struct { + path string + url string + data []byte + flattenedDevfile *bool // default to true + registryURLs []string +} + +// ParseDevfile func populates the devfile data, parses and validates the devfile integrity. // Creates devfile context and runtime objects -func Parse(path string) (d DevfileObj, err error) { +func ParseDevfile(args ParserArgs) (d DevfileObj, err error) { + if args.data != nil { + d.Ctx = devfileCtx.DevfileCtx{} + err = d.Ctx.SetDevfileContentFromBytes(args.data) + if err != nil { + return d, errors.Wrap(err, "failed to set devfile content from bytes") + } + } else if args.path != "" { + d.Ctx = devfileCtx.NewDevfileCtx(args.path) + } else if args.url != "" { + d.Ctx = devfileCtx.NewURLDevfileCtx(args.url) + } else { + return d, errors.Wrap(err, "the devfile source is not provided") + } - // NewDevfileCtx - d.Ctx = devfileCtx.NewDevfileCtx(path) + if args.registryURLs != nil { + d.Ctx.SetRegistryURLs(args.registryURLs) + } + + flattenedDevfile := true + if args.flattenedDevfile != nil { + flattenedDevfile = *args.flattenedDevfile + } + + return populateAndParseDevfile(d, flattenedDevfile) +} + +func populateAndParseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { + var err error // Fill the fields of DevfileCtx struct - err = d.Ctx.Populate() + if d.Ctx.GetURL() != "" { + err = d.Ctx.PopulateFromURL() + } else if d.Ctx.GetDevfileContent() != nil { + err = d.Ctx.PopulateFromRaw() + } else { + err = d.Ctx.Populate() + } if err != nil { return d, err } - return parseDevfile(d, true) + + return parseDevfile(d, flattenedDevfile) +} + +// Parse func populates the flattened devfile data, parses and validates the devfile integrity. +// Creates devfile context and runtime objects +// Deprecated, use ParseDevfile() instead +func Parse(path string) (d DevfileObj, err error) { + + // NewDevfileCtx + d.Ctx = devfileCtx.NewDevfileCtx(path) + + return populateAndParseDevfile(d, true) } // ParseRawDevfile populates the raw devfile data without overriding and merging +// Deprecated, use ParseDevfile() instead func ParseRawDevfile(path string) (d DevfileObj, err error) { // NewDevfileCtx d.Ctx = devfileCtx.NewDevfileCtx(path) - // Fill the fields of DevfileCtx struct - err = d.Ctx.Populate() - if err != nil { - return d, err - } - return parseDevfile(d, false) + return populateAndParseDevfile(d, false) } // ParseFromURL func parses and validates the devfile integrity. // Creates devfile context and runtime objects +// Deprecated, use ParseDevfile() instead func ParseFromURL(url string) (d DevfileObj, err error) { d.Ctx = devfileCtx.NewURLDevfileCtx(url) - // Fill the fields of DevfileCtx struct - err = d.Ctx.PopulateFromURL() - if err != nil { - return d, err - } - return parseDevfile(d, true) + return populateAndParseDevfile(d, true) } // ParseFromData func parses and validates the devfile integrity. // Creates devfile context and runtime objects +// Deprecated, use ParseDevfile() instead func ParseFromData(data []byte) (d DevfileObj, err error) { d.Ctx = devfileCtx.DevfileCtx{} err = d.Ctx.SetDevfileContentFromBytes(data) if err != nil { return d, errors.Wrap(err, "failed to set devfile content from bytes") } - err = d.Ctx.PopulateFromRaw() - if err != nil { - return d, err - } - - return parseDevfile(d, true) + return populateAndParseDevfile(d, true) } func parseParentAndPlugin(d DevfileObj) (err error) { @@ -125,18 +162,7 @@ func parseParentAndPlugin(d DevfileObj) (err error) { return err } } else if parent.Id != "" { - registry := defaultRegistry - if parent.RegistryUrl != "" { - registry = parent.RegistryUrl - } - param := util.HTTPRequestParams{ - URL: fmt.Sprintf("http://%s/devfiles/%s", registry, parent.Id), - } - returnedVar, err := util.HTTPGetRequest(param, 0) - if err != nil { - return err - } - parentDevfileObj, err = ParseFromData(returnedVar) + parentDevfileObj, err = parseFromRegistry(parent.Id, parent.RegistryUrl, d.Ctx) if err != nil { return err } @@ -157,6 +183,7 @@ func parseParentAndPlugin(d DevfileObj) (err error) { klog.V(4).Infof("adding data of devfile with URI: %v", parent.Uri) } } + flattenedPlugins := []*v1.DevWorkspaceTemplateSpecContent{} components, err := d.Data.GetComponents(common.DevfileOptions{}) if err != nil { @@ -185,6 +212,7 @@ func parseParentAndPlugin(d DevfileObj) (err error) { flattenedPlugins = append(flattenedPlugins, flattenedPlugin) } } + mergedContent, err := apiOverride.MergeDevWorkspaceTemplateSpec(d.Data.GetDevfileWorkspace(), flattenedParent, flattenedPlugins...) if err != nil { return err @@ -209,20 +237,11 @@ func parseFromURI(uri string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, // relative path on disk if !absoluteURL && curDevfileCtx.GetAbsPath() != "" { d.Ctx = devfileCtx.NewDevfileCtx(path.Join(path.Dir(curDevfileCtx.GetAbsPath()), uri)) - d.Ctx.SetURIMap(curDevfileCtx.GetURIMap()) - - // Fill the fields of DevfileCtx struct - err = d.Ctx.Populate() - if err != nil { - return DevfileObj{}, err - } - return parseDevfile(d, true) - } - - // absolute URL address - if absoluteURL { + } else if absoluteURL { + // absolute URL address d.Ctx = devfileCtx.NewURLDevfileCtx(uri) } else if curDevfileCtx.GetURL() != "" { + // relative path to a URL u, err := url.Parse(curDevfileCtx.GetURL()) if err != nil { return DevfileObj{}, err @@ -231,11 +250,35 @@ func parseFromURI(uri string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, d.Ctx = devfileCtx.NewURLDevfileCtx(u.String()) } d.Ctx.SetURIMap(curDevfileCtx.GetURIMap()) - // Fill the fields of DevfileCtx struct - err = d.Ctx.PopulateFromURL() + return populateAndParseDevfile(d, true) +} + +func parseFromRegistry(registryId, registryURL string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, error) { + var err error + var devfileContent []byte + if registryURL != "" { + devfileContent, err = getDevfileFromRegistry(registryId, registryURL) + } else if curDevfileCtx.GetRegistryURLs() != nil { + for _, registry := range curDevfileCtx.GetRegistryURLs() { + devfileContent, err = getDevfileFromRegistry(registryId, registry) + } + } else { + return DevfileObj{}, fmt.Errorf("failed to parse from registry, registry URL is not provided") + } + if err != nil { return DevfileObj{}, err } - return parseDevfile(d, true) + return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) +} + +func getDevfileFromRegistry(registryId, registryURL string) ([]byte, error) { + if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") { + registryURL = fmt.Sprintf("http://%s", registryURL) + } + param := util.HTTPRequestParams{ + URL: fmt.Sprintf("%s/devfiles/%s", registryURL, registryId), + } + return util.HTTPGetRequest(param, 0) } diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index a236296f..daa75f24 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -2403,7 +2403,6 @@ func Test_parseParentAndPlugin_RecursivelyReference_withMultipleURI(t *testing.T func Test_parseParentFromRegistry(t *testing.T) { const validRegistry = "127.0.0.1:8080" const invalidRegistry = "invalid-registry.io" - mountSource := true parentDevfile := DevfileObj{ Data: &v2.DevfileV2{ Devfile: v1.Devfile{ @@ -2463,6 +2462,123 @@ func Test_parseParentFromRegistry(t *testing.T) { testServer.Start() defer testServer.Close() + mainDevfileContent := v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + Parent: &v1.Parent{ + ImportReference: v1.ImportReference{ + RegistryUrl: validRegistry, + ImportReferenceUnion: v1.ImportReferenceUnion{ + Id: "nodejs", + }, + }, + ParentOverrides: v1.ParentOverrides{ + Components: []v1.ComponentParentOverride{ + { + Name: "parent-runtime", + ComponentUnionParentOverride: v1.ComponentUnionParentOverride{ + Container: &v1.ContainerComponentParentOverride{ + ContainerParentOverride: v1.ContainerParentOverride{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + }, + }, + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStop: []string{"post-stop"}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + }, + }, + } + wantDevfileContent := v1.Devfile{ + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "parent-runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + }, + }, + }, + }, + Events: &v1.Events{ + WorkspaceEvents: v1.WorkspaceEvents{ + PostStart: []string{}, + PostStop: []string{"post-stop"}, + PreStop: []string{}, + PreStart: []string{}, + }, + }, + Projects: []v1.Project{ + { + ClonePath: "/projects", + Name: "nodejs-starter-build", + }, + }, + }, + }, + } + + ctxWithRegistry := devfileCtx.NewDevfileCtx(OutputDevfileYamlPath) + ctxWithRegistry.SetRegistryURLs([]string{validRegistry}) + tests := []struct { name string mainDevfile DevfileObj @@ -2472,346 +2588,30 @@ func Test_parseParentFromRegistry(t *testing.T) { testRecursiveReference bool }{ { - name: "it should override the requested parent's data from provided registry and add the local devfile's data", + name: "it should override the requested parent's data from provided registryURL and add the local devfile's data", mainDevfile: DevfileObj{ Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ - Parent: &v1.Parent{ - ImportReference: v1.ImportReference{ - RegistryUrl: validRegistry, - ImportReferenceUnion: v1.ImportReferenceUnion{ - Id: "nodejs", - }, - }, - ParentOverrides: v1.ParentOverrides{ - Components: []v1.ComponentParentOverride{ - { - Name: "parent-runtime", - ComponentUnionParentOverride: v1.ComponentUnionParentOverride{ - Container: &v1.ContainerComponentParentOverride{ - ContainerParentOverride: v1.ContainerParentOverride{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - }, - }, - DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ - Commands: []v1.Command{ - { - Id: "devbuild", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - WorkingDir: "/projects/nodejs-starter", - }, - }, - }, - }, - Components: []v1.Component{ - { - Name: "runtime2", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Container: v1.Container{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - Events: &v1.Events{ - WorkspaceEvents: v1.WorkspaceEvents{ - PostStop: []string{"post-stop"}, - }, - }, - Projects: []v1.Project{ - { - ClonePath: "/projects", - Name: "nodejs-starter-build", - }, - }, - }, - }, - }, + Devfile: mainDevfileContent, }, }, wantDevFile: DevfileObj{ Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ - DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ - Commands: []v1.Command{ - { - Id: "devbuild", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - WorkingDir: "/projects/nodejs-starter", - }, - }, - }, - }, - Components: []v1.Component{ - { - Name: "parent-runtime", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Container: v1.Container{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - { - Name: "runtime2", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Container: v1.Container{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - Events: &v1.Events{ - WorkspaceEvents: v1.WorkspaceEvents{ - PostStart: []string{}, - PostStop: []string{"post-stop"}, - PreStop: []string{}, - PreStart: []string{}, - }, - }, - Projects: []v1.Project{ - { - ClonePath: "/projects", - Name: "nodejs-starter-build", - }, - }, - }, - }, - }, + Devfile: wantDevfileContent, }, }, }, { - name: "it should override the requested parent's data from default registry and add the local devfile's data", + name: "it should override the requested parent's data from registryURLs set in context and add the local devfile's data", mainDevfile: DevfileObj{ - Ctx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + Ctx: ctxWithRegistry, Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ - Parent: &v1.Parent{ - ImportReference: v1.ImportReference{ - ImportReferenceUnion: v1.ImportReferenceUnion{ - Id: "nodejs", - }, - }, - ParentOverrides: v1.ParentOverrides{ - Components: []v1.ComponentParentOverride{ - { - Name: "runtime", - ComponentUnionParentOverride: v1.ComponentUnionParentOverride{ - Container: &v1.ContainerComponentParentOverride{ - ContainerParentOverride: v1.ContainerParentOverride{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - }, - }, - DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ - Commands: []v1.Command{ - { - Id: "devbuild", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - WorkingDir: "/projects/nodejs-starter", - }, - }, - }, - }, - Components: []v1.Component{ - { - Name: "runtime2", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Container: v1.Container{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - Events: &v1.Events{ - WorkspaceEvents: v1.WorkspaceEvents{ - PostStop: []string{"post-stop"}, - }, - }, - Projects: []v1.Project{ - { - ClonePath: "/projects", - Name: "nodejs-starter-build", - }, - }, - }, - }, - }, + Devfile: mainDevfileContent, }, }, wantDevFile: DevfileObj{ Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ - DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ - Commands: []v1.Command{ - { - Id: "install", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - Component: "runtime", - WorkingDir: "/project", - CommandLine: "npm install", - LabeledCommand: v1.LabeledCommand{ - BaseCommand: v1.BaseCommand{ - Group: &v1.CommandGroup{ - Kind: v1.BuildCommandGroupKind, - IsDefault: true, - }, - }, - }, - }, - }, - }, - { - Id: "run", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - Component: "runtime", - WorkingDir: "/project", - CommandLine: "npm start", - LabeledCommand: v1.LabeledCommand{ - BaseCommand: v1.BaseCommand{ - Group: &v1.CommandGroup{ - Kind: v1.RunCommandGroupKind, - IsDefault: true, - }, - }, - }, - }, - }, - }, - { - Id: "debug", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - Component: "runtime", - WorkingDir: "/project", - CommandLine: "npm run debug", - LabeledCommand: v1.LabeledCommand{ - BaseCommand: v1.BaseCommand{ - Group: &v1.CommandGroup{ - Kind: v1.DebugCommandGroupKind, - IsDefault: true, - }, - }, - }, - }, - }, - }, - { - Id: "test", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - Component: "runtime", - WorkingDir: "/project", - CommandLine: "npm test", - LabeledCommand: v1.LabeledCommand{ - BaseCommand: v1.BaseCommand{ - Group: &v1.CommandGroup{ - Kind: v1.TestCommandGroupKind, - IsDefault: true, - }, - }, - }, - }, - }, - }, - { - Id: "devbuild", - CommandUnion: v1.CommandUnion{ - Exec: &v1.ExecCommand{ - WorkingDir: "/projects/nodejs-starter", - }, - }, - }, - }, - Components: []v1.Component{ - { - Name: "runtime", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Endpoints: []v1.Endpoint{ - { - Name: "http-3000", - TargetPort: 3000, - }, - }, - Container: v1.Container{ - Image: "quay.io/nodejs-12", - MemoryLimit: "1024Mi", - MountSources: &mountSource, - SourceMapping: "/project", - }, - }, - }, - }, - { - Name: "runtime2", - ComponentUnion: v1.ComponentUnion{ - Container: &v1.ContainerComponent{ - Container: v1.Container{ - Image: "quay.io/nodejs-12", - }, - }, - }, - }, - }, - Events: &v1.Events{ - WorkspaceEvents: v1.WorkspaceEvents{ - PostStart: []string{}, - PostStop: []string{"post-stop"}, - PreStop: []string{}, - PreStart: []string{}, - }, - }, - Projects: []v1.Project{ - { - ClonePath: "/projects", - Name: "nodejs-starter-build", - }, - }, - StarterProjects: []v1.StarterProject{ - { - Name: "nodejs-starter", - ProjectSource: v1.ProjectSource{ - Git: &v1.GitProjectSource{ - GitLikeProjectSource: v1.GitLikeProjectSource{ - Remotes: map[string]string{ - "origin": "https://github.com/odo-devfiles/nodejs-ex.git", - }, - }, - }, - }, - }, - }, - }, - }, - }, + Devfile: wantDevfileContent, }, }, }, From 441f6838ad2c4cd455c14d31ae2ff5e2ef6a014f Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 9 Mar 2021 16:07:49 -0500 Subject: [PATCH 4/7] add error message Signed-off-by: Stephanie --- pkg/devfile/parser/parse.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index ca9caedc..01f94ebd 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -254,23 +254,24 @@ func parseFromURI(uri string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, } func parseFromRegistry(registryId, registryURL string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, error) { - var err error - var devfileContent []byte if registryURL != "" { - devfileContent, err = getDevfileFromRegistry(registryId, registryURL) + devfileContent, err := getDevfileFromRegistry(registryId, registryURL) + if err != nil { + return DevfileObj{}, err + } + return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) } else if curDevfileCtx.GetRegistryURLs() != nil { for _, registry := range curDevfileCtx.GetRegistryURLs() { - devfileContent, err = getDevfileFromRegistry(registryId, registry) + devfileContent, err := getDevfileFromRegistry(registryId, registry) + if devfileContent != nil && err == nil { + return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) + } } } else { return DevfileObj{}, fmt.Errorf("failed to parse from registry, registry URL is not provided") } - if err != nil { - return DevfileObj{}, err - } - - return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) + return DevfileObj{}, fmt.Errorf("failed to get registry id: %s from registry URLs provided", registryId) } func getDevfileFromRegistry(registryId, registryURL string) ([]byte, error) { From 818dfab744aec0321cdc7d3298fc9e576cabe8d1 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 11 Mar 2021 16:38:11 -0500 Subject: [PATCH 5/7] add unit test for parseFromRegistry Signed-off-by: Stephanie --- devfile.yaml | 1 + pkg/devfile/parser/parse_test.go | 135 +++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/devfile.yaml b/devfile.yaml index 62725cc8..db769a31 100644 --- a/devfile.yaml +++ b/devfile.yaml @@ -7,6 +7,7 @@ metadata: parent: # uri: https://raw.githubusercontent.com/odo-devfiles/registry/master/devfiles/nodejs/devfile.yaml id: nodejs + registryUrl: "https://registry.devfile.io" commands: - id: install exec: diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index daa75f24..9e72c7f5 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -2897,3 +2897,138 @@ func Test_parseFromURI(t *testing.T) { }) } } + +func Test_parseFromRegistry(t *testing.T) { + const ( + registry = "127.0.0.1:8080" + httpPrefix = "http://" + notExistId = "notexist" + invalidRegistry = "http//invalid.com" + registryId = "nodejs" + ) + + ctxWithRegistry := devfileCtx.NewDevfileCtx(OutputDevfileYamlPath) + ctxWithRegistry.SetRegistryURLs([]string{registry}) + + parentDevfile := DevfileObj{ + Data: &v2.DevfileV2{ + Devfile: v1.Devfile{ + DevfileHeader: devfilepkg.DevfileHeader{ + SchemaVersion: schemaV200, + }, + DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{ + Components: []v1.Component{ + { + Name: "runtime2", + ComponentUnion: v1.ComponentUnion{ + Volume: &v1.VolumeComponent{ + Volume: v1.Volume{ + Size: "500Mi", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var data []byte + var err error + if strings.Contains(r.URL.Path, "/devfiles/"+registryId) { + data, err = yaml.Marshal(parentDevfile.Data) + } else { + w.WriteHeader(http.StatusNotFound) + return + } + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + _, err = w.Write(data) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + })) + // create a listener with the desired port. + l, err := net.Listen("tcp", registry) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + // NewUnstartedServer creates a listener. Close that listener and replace + // with the one we created. + testServer.Listener.Close() + testServer.Listener = l + + testServer.Start() + defer testServer.Close() + + tests := []struct { + name string + curDevfileCtx devfileCtx.DevfileCtx + registryUrl string + registryId string + wantDevFile DevfileObj + wantErr bool + }{ + { + name: "should be able to parse from provided registryUrl without prefix", + curDevfileCtx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + wantDevFile: parentDevfile, + registryUrl: registry, + registryId: registryId, + }, + { + name: "should be able to parse from provided registryUrl with prefix", + curDevfileCtx: devfileCtx.NewDevfileCtx(OutputDevfileYamlPath), + wantDevFile: parentDevfile, + registryUrl: httpPrefix + registry, + registryId: registryId, + }, + { + name: "should be able to parse from registry URL defined in ctx", + curDevfileCtx: ctxWithRegistry, + wantDevFile: parentDevfile, + registryId: registryId, + }, + { + name: "should fail if registryId does not exist", + curDevfileCtx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), + registryUrl: registry, + registryId: notExistId, + wantErr: true, + }, + { + name: "should fail if registryUrl is not provided, and no registry URLs has been set in ctx", + curDevfileCtx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), + registryId: registryId, + wantErr: true, + }, + { + name: "should fail if registryUrl is invalid", + curDevfileCtx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), + registryUrl: invalidRegistry, + registryId: registryId, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseFromRegistry(tt.registryId, tt.registryUrl, tt.curDevfileCtx) + if tt.wantErr == (err == nil) { + t.Errorf("Test_parseFromRegistry() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(got.Data, tt.wantDevFile.Data) { + t.Errorf("wanted: %v, got: %v, difference at %v", tt.wantDevFile, got, pretty.Compare(tt.wantDevFile, got)) + } + }) + } +} From c289f59f5155f8896347e2c1f8708b81afe2b108 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Fri, 12 Mar 2021 14:29:56 -0500 Subject: [PATCH 6/7] resolve some review comments Signed-off-by: Stephanie --- main.go | 25 ++++++++--------- pkg/devfile/parser/parse.go | 55 +++++++++++++++++++++---------------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/main.go b/main.go index 60cedcee..42d8817d 100644 --- a/main.go +++ b/main.go @@ -20,28 +20,27 @@ func main() { } } -//ParseDevfile to parse devfile from library -func ParseDevfile(devfileLocation string) (parser.DevfileObj, error) { - - devfile, err := devfilepkg.ParseAndValidate(devfileLocation) - return devfile, err -} - func parserTest() { - var devfile parser.DevfileObj - var err error + var args parser.ParserArgs if len(os.Args) > 1 { if strings.HasPrefix(os.Args[1], "http") { - devfile, err = devfilepkg.ParseFromURLAndValidate(os.Args[1]) + args = parser.ParserArgs{ + URL: os.Args[1], + } } else { - devfile, err = ParseDevfile(os.Args[1]) + args = parser.ParserArgs{ + Path: os.Args[1], + } } fmt.Println("parsing devfile from " + os.Args[1]) } else { - devfile, err = ParseDevfile("devfile.yaml") - fmt.Println("parsing devfile from " + devfile.Ctx.GetAbsPath()) + args = parser.ParserArgs{ + Path: "devfile.yaml", + } + fmt.Println("parsing devfile from ./devfile.yaml") } + devfile, err := devfilepkg.ParseDevfileAndValidate(args) if err != nil { fmt.Println(err) } else { diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 01f94ebd..a60b867f 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -54,38 +54,45 @@ func parseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { return d, nil } +// ParserArgs is the struct to pass into parser functions which contains required info for parsing devfile. +// It accepts devfile path, devfile URL or devfile content in []byte format. +// Path is a relative or absolute devfile path. +// URL is the URL address of the specific devfile. +// Data is the devfile content in []byte format. +// FlattenedDevfile defines if the returned devfileObj is flattened content (true) or raw content (false), the value is default to be true. +// RegistryURLs is a list of registry hosts which parser should pull parent devfile from. If registryUrl is defined in devfile, this list will be ignored. type ParserArgs struct { - path string - url string - data []byte - flattenedDevfile *bool // default to true - registryURLs []string + Path string + URL string + Data []byte + FlattenedDevfile *bool // default to true + RegistryURLs []string } // 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.data != nil { + if args.Data != nil { d.Ctx = devfileCtx.DevfileCtx{} - err = d.Ctx.SetDevfileContentFromBytes(args.data) + err = d.Ctx.SetDevfileContentFromBytes(args.Data) if err != nil { return d, errors.Wrap(err, "failed to set devfile content from bytes") } - } else if args.path != "" { - d.Ctx = devfileCtx.NewDevfileCtx(args.path) - } else if args.url != "" { - d.Ctx = devfileCtx.NewURLDevfileCtx(args.url) + } else if args.Path != "" { + d.Ctx = devfileCtx.NewDevfileCtx(args.Path) + } else if args.URL != "" { + d.Ctx = devfileCtx.NewURLDevfileCtx(args.URL) } else { return d, errors.Wrap(err, "the devfile source is not provided") } - if args.registryURLs != nil { - d.Ctx.SetRegistryURLs(args.registryURLs) + if args.RegistryURLs != nil { + d.Ctx.SetRegistryURLs(args.RegistryURLs) } flattenedDevfile := true - if args.flattenedDevfile != nil { - flattenedDevfile = *args.flattenedDevfile + if args.FlattenedDevfile != nil { + flattenedDevfile = *args.FlattenedDevfile } return populateAndParseDevfile(d, flattenedDevfile) @@ -167,7 +174,7 @@ func parseParentAndPlugin(d DevfileObj) (err error) { return err } } else { - return fmt.Errorf("parent URI or registry id undefined, currently only URI and Id are suppported") + return fmt.Errorf("parent URI or parent Id undefined, currently only URI and Id are suppported") } parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspace() @@ -253,33 +260,33 @@ func parseFromURI(uri string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, return populateAndParseDevfile(d, true) } -func parseFromRegistry(registryId, registryURL string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, error) { +func parseFromRegistry(parentId, registryURL string, curDevfileCtx devfileCtx.DevfileCtx) (DevfileObj, error) { if registryURL != "" { - devfileContent, err := getDevfileFromRegistry(registryId, registryURL) + devfileContent, err := getDevfileFromRegistry(parentId, registryURL) if err != nil { return DevfileObj{}, err } - return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) + return ParseDevfile(ParserArgs{Data: devfileContent, RegistryURLs: curDevfileCtx.GetRegistryURLs()}) } else if curDevfileCtx.GetRegistryURLs() != nil { for _, registry := range curDevfileCtx.GetRegistryURLs() { - devfileContent, err := getDevfileFromRegistry(registryId, registry) + devfileContent, err := getDevfileFromRegistry(parentId, registry) if devfileContent != nil && err == nil { - return ParseDevfile(ParserArgs{data: devfileContent, registryURLs: curDevfileCtx.GetRegistryURLs()}) + return ParseDevfile(ParserArgs{Data: devfileContent, RegistryURLs: curDevfileCtx.GetRegistryURLs()}) } } } else { return DevfileObj{}, fmt.Errorf("failed to parse from registry, registry URL is not provided") } - return DevfileObj{}, fmt.Errorf("failed to get registry id: %s from registry URLs provided", registryId) + return DevfileObj{}, fmt.Errorf("failed to get parent Id: %s from registry URLs provided", parentId) } -func getDevfileFromRegistry(registryId, registryURL string) ([]byte, error) { +func getDevfileFromRegistry(parentId, registryURL string) ([]byte, error) { if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") { registryURL = fmt.Sprintf("http://%s", registryURL) } param := util.HTTPRequestParams{ - URL: fmt.Sprintf("%s/devfiles/%s", registryURL, registryId), + URL: fmt.Sprintf("%s/devfiles/%s", registryURL, parentId), } return util.HTTPGetRequest(param, 0) } From d8423c386e9da084ee48cb2f556c8efd4c99d969 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Fri, 12 Mar 2021 16:06:46 -0500 Subject: [PATCH 7/7] update the comment and error message Signed-off-by: Stephanie --- pkg/devfile/parser/parse.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index a60b867f..38398f68 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -56,17 +56,19 @@ func parseDevfile(d DevfileObj, flattenedDevfile bool) (DevfileObj, error) { // ParserArgs is the struct to pass into parser functions which contains required info for parsing devfile. // It accepts devfile path, devfile URL or devfile content in []byte format. -// Path is a relative or absolute devfile path. -// URL is the URL address of the specific devfile. -// Data is the devfile content in []byte format. -// FlattenedDevfile defines if the returned devfileObj is flattened content (true) or raw content (false), the value is default to be true. -// RegistryURLs is a list of registry hosts which parser should pull parent devfile from. If registryUrl is defined in devfile, this list will be ignored. type ParserArgs struct { - Path string - URL string - Data []byte - FlattenedDevfile *bool // default to true - RegistryURLs []string + // Path is a relative or absolute devfile path. + Path string + // URL is the URL address of the specific devfile. + URL string + // Data is the devfile content in []byte format. + Data []byte + // FlattenedDevfile defines if the returned devfileObj is flattened content (true) or raw content (false). + // The value is default to be true. + FlattenedDevfile *bool + // RegistryURLs is a list of registry hosts which parser should pull parent devfile from. + // If registryUrl is defined in devfile, this list will be ignored. + RegistryURLs []string } // ParseDevfile func populates the devfile data, parses and validates the devfile integrity. @@ -275,7 +277,7 @@ func parseFromRegistry(parentId, registryURL string, curDevfileCtx devfileCtx.De } } } else { - return DevfileObj{}, fmt.Errorf("failed to parse from registry, registry URL is not provided") + return DevfileObj{}, fmt.Errorf("failed to fetch from registry, registry URL is not provided") } return DevfileObj{}, fmt.Errorf("failed to get parent Id: %s from registry URLs provided", parentId)