Skip to content

Commit aa85a4a

Browse files
yangcao77yangcao77
committed
address review comments
Signed-off-by: Stephanie <[email protected]>
1 parent ce73444 commit aa85a4a

File tree

7 files changed

+63
-48
lines changed

7 files changed

+63
-48
lines changed

‎pkg/devfile/parser/context/context.go

+9
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ func NewURLDevfileCtx(url string) DevfileCtx {
5252
}
5353
}
5454

55+
// NewByteContentDevfileCtx set devfile content from byte data and returns a new DevfileCtx type object and error
56+
func NewByteContentDevfileCtx(data []byte) (d DevfileCtx, err error) {
57+
err = d.SetDevfileContentFromBytes(data)
58+
if err != nil {
59+
return DevfileCtx{}, err
60+
}
61+
return d, nil
62+
}
63+
5564
// populateDevfile checks the API version is supported and returns the JSON schema for the given devfile API Version
5665
func (d *DevfileCtx) populateDevfile() (err error) {
5766

‎pkg/devfile/parser/data/helper_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func TestNewDevfileData(t *testing.T) {
1414
t.Run("valid devfile apiVersion", func(t *testing.T) {
1515

1616
var (
17-
version = APIVersion200
17+
version = APISchemaVersion200
1818
want = reflect.TypeOf(&v2.DevfileV2{})
1919
obj, err = NewDevfileData(string(version))
2020
got = reflect.TypeOf(obj)
@@ -50,7 +50,7 @@ func TestGetDevfileJSONSchema(t *testing.T) {
5050
t.Run("valid devfile apiVersion", func(t *testing.T) {
5151

5252
var (
53-
version = APIVersion200
53+
version = APISchemaVersion200
5454
want = v200.JsonSchema200
5555
got, err = GetDevfileJSONSchema(string(version))
5656
)
@@ -82,7 +82,7 @@ func TestIsApiVersionSupported(t *testing.T) {
8282
t.Run("valid devfile apiVersion", func(t *testing.T) {
8383

8484
var (
85-
version = APIVersion200
85+
version = APISchemaVersion200
8686
want = true
8787
got = IsApiVersionSupported(string(version))
8888
)

‎pkg/devfile/parser/data/interface.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ type DevfileData interface {
5252
GetVolumeMountPaths(mountName, containerName string) ([]string, error)
5353

5454
// workspace related methods
55-
GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent
56-
SetDevfileWorkspace(content v1.DevWorkspaceTemplateSpecContent)
55+
GetDevfileWorkspaceSpecContent() *v1.DevWorkspaceTemplateSpecContent
56+
SetDevfileWorkspaceSpecContent(content v1.DevWorkspaceTemplateSpecContent)
57+
SetDevfileWorkspaceSpec(spec v1.DevWorkspaceTemplateSpec)
5758

5859
// utils
5960
GetDevfileContainerComponents(common.DevfileOptions) ([]v1.Component, error)

‎pkg/devfile/parser/data/v2/workspace.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import (
44
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
55
)
66

7-
// GetDevfileWorkspace returns the workspace content for the devfile
8-
func (d *DevfileV2) GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent {
7+
// GetDevfileWorkspaceSpecContent returns the workspace spec content for the devfile
8+
func (d *DevfileV2) GetDevfileWorkspaceSpecContent() *v1.DevWorkspaceTemplateSpecContent {
99

1010
return &d.DevWorkspaceTemplateSpecContent
1111
}
1212

13-
// SetDevfileWorkspace sets the workspace content
14-
func (d *DevfileV2) SetDevfileWorkspace(content v1.DevWorkspaceTemplateSpecContent) {
13+
// SetDevfileWorkspaceSpecContent sets the workspace spec content
14+
func (d *DevfileV2) SetDevfileWorkspaceSpecContent(content v1.DevWorkspaceTemplateSpecContent) {
1515
d.DevWorkspaceTemplateSpecContent = content
1616
}
17+
18+
// SetDevfileWorkspaceSpec sets the workspace spec
19+
func (d *DevfileV2) SetDevfileWorkspaceSpec(spec v1.DevWorkspaceTemplateSpec) {
20+
d.DevWorkspaceTemplateSpec = spec
21+
}

‎pkg/devfile/parser/data/v2/workspace_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
88
)
99

10-
func TestDevfile200_SetDevfileWorkspace(t *testing.T) {
10+
func TestDevfile200_SetDevfileWorkspaceSpecContent(t *testing.T) {
1111

1212
type args struct {
1313
name string
@@ -65,9 +65,9 @@ func TestDevfile200_SetDevfileWorkspace(t *testing.T) {
6565
}
6666
for _, tt := range tests {
6767
t.Run(tt.name, func(t *testing.T) {
68-
tt.devfilev2.SetDevfileWorkspace(tt.workspace)
68+
tt.devfilev2.SetDevfileWorkspaceSpecContent(tt.workspace)
6969
if !reflect.DeepEqual(tt.devfilev2, tt.expectedDevfilev2) {
70-
t.Errorf("TestDevfile200_SetDevfileWorkspace() expected %v, got %v", tt.expectedDevfilev2, tt.devfilev2)
70+
t.Errorf("TestDevfile200_SetDevfileWorkspaceSpecContent() expected %v, got %v", tt.expectedDevfilev2, tt.devfilev2)
7171
}
7272
})
7373
}

‎pkg/devfile/parser/data/versions.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ type supportedApiVersion string
1313

1414
// Supported devfile API versions
1515
const (
16-
APIVersion200 supportedApiVersion = "2.0.0"
17-
APIVersion210 supportedApiVersion = "2.1.0"
18-
V2APIVersion supportedApiVersion = "v1alpha2"
16+
APISchemaVersion200 supportedApiVersion = "2.0.0"
17+
APISchemaVersion210 supportedApiVersion = "2.1.0"
18+
APIVersionAlpha2 supportedApiVersion = "v1alpha2"
1919
)
2020

2121
// ------------- Init functions ------------- //
@@ -26,9 +26,9 @@ var apiVersionToDevfileStruct map[supportedApiVersion]reflect.Type
2626
// Initializes a map of supported devfile api versions and devfile structs
2727
func init() {
2828
apiVersionToDevfileStruct = make(map[supportedApiVersion]reflect.Type)
29-
apiVersionToDevfileStruct[APIVersion200] = reflect.TypeOf(v2.DevfileV2{})
30-
apiVersionToDevfileStruct[APIVersion210] = reflect.TypeOf(v2.DevfileV2{})
31-
apiVersionToDevfileStruct[V2APIVersion] = reflect.TypeOf(v2.DevfileV2{})
29+
apiVersionToDevfileStruct[APISchemaVersion200] = reflect.TypeOf(v2.DevfileV2{})
30+
apiVersionToDevfileStruct[APISchemaVersion210] = reflect.TypeOf(v2.DevfileV2{})
31+
apiVersionToDevfileStruct[APIVersionAlpha2] = reflect.TypeOf(v2.DevfileV2{})
3232
}
3333

3434
// Map to store mappings between supported devfile API versions and respective devfile JSON schemas
@@ -37,8 +37,8 @@ var devfileApiVersionToJSONSchema map[supportedApiVersion]string
3737
// init initializes a map of supported devfile apiVersions with it's respective devfile JSON schema
3838
func init() {
3939
devfileApiVersionToJSONSchema = make(map[supportedApiVersion]string)
40-
devfileApiVersionToJSONSchema[APIVersion200] = v200.JsonSchema200
41-
devfileApiVersionToJSONSchema[APIVersion210] = v210.JsonSchema210
40+
devfileApiVersionToJSONSchema[APISchemaVersion200] = v200.JsonSchema200
41+
devfileApiVersionToJSONSchema[APISchemaVersion210] = v210.JsonSchema210
4242
// should use hightest v2 schema version since it is expected to be backward compatible with the same api version
43-
devfileApiVersionToJSONSchema[V2APIVersion] = v210.JsonSchema210
43+
devfileApiVersionToJSONSchema[APIVersionAlpha2] = v210.JsonSchema210
4444
}

‎pkg/devfile/parser/parse.go

+27-27
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ type ParserArgs struct {
8686
// Creates devfile context and runtime objects
8787
func ParseDevfile(args ParserArgs) (d DevfileObj, err error) {
8888
if args.Data != nil {
89-
d.Ctx = devfileCtx.DevfileCtx{}
90-
err = d.Ctx.SetDevfileContentFromBytes(args.Data)
89+
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(args.Data)
9190
if err != nil {
9291
return d, errors.Wrap(err, "failed to set devfile content from bytes")
9392
}
@@ -177,8 +176,7 @@ func ParseFromURL(url string) (d DevfileObj, err error) {
177176
// Creates devfile context and runtime objects
178177
// Deprecated, use ParseDevfile() instead
179178
func ParseFromData(data []byte) (d DevfileObj, err error) {
180-
d.Ctx = devfileCtx.DevfileCtx{}
181-
err = d.Ctx.SetDevfileContentFromBytes(data)
179+
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(data)
182180
if err != nil {
183181
return d, errors.Wrap(err, "failed to set devfile content from bytes")
184182
}
@@ -209,7 +207,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
209207
return fmt.Errorf("devfile parent does not define any resources")
210208
}
211209

212-
parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspace()
210+
parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspaceSpecContent()
213211
if !reflect.DeepEqual(parent.ParentOverrides, v1.ParentOverrides{}) {
214212
flattenedParent, err = apiOverride.OverrideDevWorkspaceTemplateSpec(parentWorkspaceContent, parent.ParentOverrides)
215213
if err != nil {
@@ -248,7 +246,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
248246
default:
249247
return fmt.Errorf("plugin %s does not define any resources", component.Name)
250248
}
251-
pluginWorkspaceContent := pluginDevfileObj.Data.GetDevfileWorkspace()
249+
pluginWorkspaceContent := pluginDevfileObj.Data.GetDevfileWorkspaceSpecContent()
252250
flattenedPlugin := pluginWorkspaceContent
253251
if !reflect.DeepEqual(plugin.PluginOverrides, v1.PluginOverrides{}) {
254252
flattenedPlugin, err = apiOverride.OverrideDevWorkspaceTemplateSpec(pluginWorkspaceContent, plugin.PluginOverrides)
@@ -260,11 +258,11 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
260258
}
261259
}
262260

263-
mergedContent, err := apiOverride.MergeDevWorkspaceTemplateSpec(d.Data.GetDevfileWorkspace(), flattenedParent, flattenedPlugins...)
261+
mergedContent, err := apiOverride.MergeDevWorkspaceTemplateSpec(d.Data.GetDevfileWorkspaceSpecContent(), flattenedParent, flattenedPlugins...)
264262
if err != nil {
265263
return err
266264
}
267-
d.Data.SetDevfileWorkspace(*mergedContent)
265+
d.Data.SetDevfileWorkspaceSpecContent(*mergedContent)
268266
// remove parent from flatterned devfile
269267
d.Data.SetParent(nil)
270268

@@ -302,21 +300,20 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D
302300
d.Ctx = devfileCtx.NewURLDevfileCtx(newUri)
303301
}
304302
importReference.Uri = newUri
305-
newCtx := resolveCtx.appendNode(importReference)
303+
newResolveCtx := resolveCtx.appendNode(importReference)
306304

307-
return populateAndParseDevfile(d, newCtx, tool, true)
305+
return populateAndParseDevfile(d, newResolveCtx, tool, true)
308306
}
309307

310308
func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutionContextTree, tool resolverTools) (d DevfileObj, err error) {
311-
d.Ctx = devfileCtx.DevfileCtx{}
312309
id := importReference.Id
313310
registryURL := importReference.RegistryUrl
314311
if registryURL != "" {
315312
devfileContent, err := getDevfileFromRegistry(id, registryURL)
316313
if err != nil {
317314
return DevfileObj{}, err
318315
}
319-
err = d.Ctx.SetDevfileContentFromBytes(devfileContent)
316+
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent)
320317
if err != nil {
321318
return d, errors.Wrap(err, "failed to set devfile content from bytes")
322319
}
@@ -328,7 +325,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio
328325
for _, registryURL := range tool.registryURLs {
329326
devfileContent, err := getDevfileFromRegistry(id, registryURL)
330327
if devfileContent != nil && err == nil {
331-
err = d.Ctx.SetDevfileContentFromBytes(devfileContent)
328+
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent)
332329
if err != nil {
333330
return d, errors.Wrap(err, "failed to set devfile content from bytes")
334331
}
@@ -361,20 +358,23 @@ func parseFromKubeCRD(importReference v1.ImportReference, resolveCtx *resolution
361358
return DevfileObj{}, fmt.Errorf("Kubernetes client and context are required to parse from Kubernetes CRD")
362359
}
363360
namespace := importReference.Kubernetes.Namespace
364-
// if namespace is not set in devfile, use default namespace provided in by consumer
365-
if namespace == "" {
366-
namespace = tool.defaultNamespace
367-
}
368-
// use current namespace if namespace is not set in devfile and not provided by consumer
361+
369362
if namespace == "" {
370-
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
371-
configOverrides := &clientcmd.ConfigOverrides{}
372-
config := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, configOverrides)
373-
namespace, _, err = config.Namespace()
374-
if err != nil {
375-
return DevfileObj{}, fmt.Errorf("kubernetes namespace is not provided, and cannot get current running cluster's namespace: %v", err)
363+
// if namespace is not set in devfile, use default namespace provided in by consumer
364+
if tool.defaultNamespace != "" {
365+
namespace = tool.defaultNamespace
366+
} else {
367+
// use current namespace if namespace is not set in devfile and not provided by consumer
368+
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
369+
configOverrides := &clientcmd.ConfigOverrides{}
370+
config := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, configOverrides)
371+
namespace, _, err = config.Namespace()
372+
if err != nil {
373+
return DevfileObj{}, fmt.Errorf("kubernetes namespace is not provided, and cannot get current running cluster's namespace: %v", err)
374+
}
376375
}
377376
}
377+
378378
var dwTemplate v1.DevWorkspaceTemplate
379379
namespacedName := types.NamespacedName{
380380
Name: importReference.Kubernetes.Name,
@@ -391,9 +391,9 @@ func parseFromKubeCRD(importReference v1.ImportReference, resolveCtx *resolution
391391
}
392392

393393
importReference.Kubernetes.Namespace = namespace
394-
newCtx := resolveCtx.appendNode(importReference)
394+
newResolveCtx := resolveCtx.appendNode(importReference)
395395

396-
err = parseParentAndPlugin(d, newCtx, tool)
396+
err = parseParentAndPlugin(d, newResolveCtx, tool)
397397
return d, err
398398

399399
}
@@ -407,7 +407,7 @@ func convertDevWorskapceTemplateToDevObj(dwTemplate v1.DevWorkspaceTemplate) (d
407407
if err != nil {
408408
return DevfileObj{}, err
409409
}
410-
d.Data.SetDevfileWorkspace(dwTemplate.Spec.DevWorkspaceTemplateSpecContent)
410+
d.Data.SetDevfileWorkspaceSpec(dwTemplate.Spec)
411411

412412
return d, nil
413413

0 commit comments

Comments
 (0)