Skip to content

Commit

Permalink
Address global attribute PR feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
  • Loading branch information
maysunfaisal committed Mar 10, 2021
1 parent fb6e078 commit a2940cb
Show file tree
Hide file tree
Showing 65 changed files with 233 additions and 384 deletions.
15 changes: 9 additions & 6 deletions crds/workspace.devfile.io_devworkspaces.v1beta1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4141,15 +4141,17 @@ spec:
of a workspace template.
properties:
attributes:
additionalProperties:
type: string
description: Map of implementation-dependant free-form YAML attributes.
Attribute values can be referenced throughout the devfile in
string type fields in the form {{attribute-key}} except for
schemaVersion, metadata and events. Exception to the string
field include element's key identifiers(command id, component
name, endpoint name, project name, etc.) and string enums(command
group kind, endpoint exposure, etc.)
schemaVersion, metadata, parent source. Exception to the string
field also include element's key identifiers (command id, component
name, endpoint name, project name, etc.) and their references(events,
command's component, container's volume mount name, etc.) and
string enums(command group kind, endpoint exposure, etc.)
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Predefined, ready-to-use, workspace-related commands
items:
Expand Down Expand Up @@ -5572,11 +5574,12 @@ spec:
- kubernetes
properties:
attributes:
additionalProperties:
type: string
description: Overrides of attributes encapsulated in a parent
devfile. Overriding is done according to K8S strategic merge
patch standard rules.
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Overrides of commands encapsulated in a parent
devfile or a plugin. Overriding is done according to K8S
Expand Down
15 changes: 9 additions & 6 deletions crds/workspace.devfile.io_devworkspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4139,15 +4139,17 @@ spec:
of a workspace template.
properties:
attributes:
additionalProperties:
type: string
description: Map of implementation-dependant free-form YAML attributes.
Attribute values can be referenced throughout the devfile in
string type fields in the form {{attribute-key}} except for
schemaVersion, metadata and events. Exception to the string
field include element's key identifiers(command id, component
name, endpoint name, project name, etc.) and string enums(command
group kind, endpoint exposure, etc.)
schemaVersion, metadata, parent source. Exception to the string
field also include element's key identifiers (command id, component
name, endpoint name, project name, etc.) and their references(events,
command's component, container's volume mount name, etc.) and
string enums(command group kind, endpoint exposure, etc.)
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Predefined, ready-to-use, workspace-related commands
items:
Expand Down Expand Up @@ -5577,11 +5579,12 @@ spec:
- kubernetes
properties:
attributes:
additionalProperties:
type: string
description: Overrides of attributes encapsulated in a parent
devfile. Overriding is done according to K8S strategic merge
patch standard rules.
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Overrides of commands encapsulated in a parent
devfile or a plugin. Overriding is done according to K8S
Expand Down
15 changes: 9 additions & 6 deletions crds/workspace.devfile.io_devworkspacetemplates.v1beta1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3915,15 +3915,17 @@ spec:
of a workspace template.
properties:
attributes:
additionalProperties:
type: string
description: Map of implementation-dependant free-form YAML attributes.
Attribute values can be referenced throughout the devfile in string
type fields in the form {{attribute-key}} except for schemaVersion,
metadata and events. Exception to the string field include element's
key identifiers(command id, component name, endpoint name, project
name, etc.) and string enums(command group kind, endpoint exposure,
etc.)
metadata, parent source. Exception to the string field also include
element's key identifiers (command id, component name, endpoint
name, project name, etc.) and their references(events, command's
component, container's volume mount name, etc.) and string enums(command
group kind, endpoint exposure, etc.)
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Predefined, ready-to-use, workspace-related commands
items:
Expand Down Expand Up @@ -5294,11 +5296,12 @@ spec:
- kubernetes
properties:
attributes:
additionalProperties:
type: string
description: Overrides of attributes encapsulated in a parent
devfile. Overriding is done according to K8S strategic merge
patch standard rules.
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Overrides of commands encapsulated in a parent devfile
or a plugin. Overriding is done according to K8S strategic merge
Expand Down
15 changes: 9 additions & 6 deletions crds/workspace.devfile.io_devworkspacetemplates.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3913,15 +3913,17 @@ spec:
of a workspace template.
properties:
attributes:
additionalProperties:
type: string
description: Map of implementation-dependant free-form YAML attributes.
Attribute values can be referenced throughout the devfile in string
type fields in the form {{attribute-key}} except for schemaVersion,
metadata and events. Exception to the string field include element's
key identifiers(command id, component name, endpoint name, project
name, etc.) and string enums(command group kind, endpoint exposure,
etc.)
metadata, parent source. Exception to the string field also include
element's key identifiers (command id, component name, endpoint
name, project name, etc.) and their references(events, command's
component, container's volume mount name, etc.) and string enums(command
group kind, endpoint exposure, etc.)
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Predefined, ready-to-use, workspace-related commands
items:
Expand Down Expand Up @@ -5299,11 +5301,12 @@ spec:
- kubernetes
properties:
attributes:
additionalProperties:
type: string
description: Overrides of attributes encapsulated in a parent
devfile. Overriding is done according to K8S strategic merge
patch standard rules.
type: object
x-kubernetes-preserve-unknown-fields: true
commands:
description: Overrides of commands encapsulated in a parent devfile
or a plugin. Overriding is done according to K8S strategic merge
Expand Down
9 changes: 4 additions & 5 deletions pkg/apis/workspaces/v1alpha2/devworkspaceTemplateSpec.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package v1alpha2

import attributes "github.com/devfile/api/v2/pkg/attributes"

// Structure of the workspace. This is also the specification of a workspace template.
// +devfile:jsonschema:generate
type DevWorkspaceTemplateSpec struct {
Expand All @@ -16,12 +14,13 @@ type DevWorkspaceTemplateSpec struct {
type DevWorkspaceTemplateSpecContent struct {
// Map of implementation-dependant free-form YAML attributes.
// Attribute values can be referenced throughout the devfile in string type fields in the form {{attribute-key}}
// except for schemaVersion, metadata and events. Exception to the string field include element's key identifiers(command id,
// component name, endpoint name, project name, etc.) and string enums(command group kind, endpoint exposure, etc.)
// except for schemaVersion, metadata, parent source. Exception to the string field also include element's key identifiers
// (command id, component name, endpoint name, project name, etc.) and their references(events, command's component, container's
// volume mount name, etc.) and string enums(command group kind, endpoint exposure, etc.)
// +optional
// +patchStrategy=merge
// +devfile:overrides:include:omitInPlugin=true,description=Overrides of attributes encapsulated in a parent devfile.
Attributes attributes.Attributes `json:"attributes,omitempty" patchStrategy:"merge"`
Attributes map[string]string `json:"attributes,omitempty" patchStrategy:"merge"`

// List of the workspace components, such as editor and plugins,
// user-provided containers, or other types of components
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/workspaces/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 10 additions & 8 deletions pkg/utils/overriding/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"reflect"

workspaces "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
attributesPkg "github.com/devfile/api/v2/pkg/attributes"
"github.com/hashicorp/go-multierror"
"k8s.io/apimachinery/pkg/util/sets"
)
Expand Down Expand Up @@ -42,14 +41,17 @@ func checkKeys(doCheck checkFn, toplevelListContainers ...workspaces.TopLevelLis
attributeValue = value.FieldByName("Attributes")
}

if attributeValue.IsValid() && attributeValue.CanInterface() {
attributes, ok := attributeValue.Interface().(attributesPkg.Attributes)
if !ok {
return fmt.Errorf("unable to fetch Attributes from the devfile data")
}
if attributeValue.IsValid() && attributeValue.Kind() == reflect.Map {
mapIter := attributeValue.MapRange()

var attributeKeys []string
for k := range attributes {
attributeKeys = append(attributeKeys, k)
for mapIter.Next() {
k := mapIter.Key()
v := mapIter.Value()
if k.Kind() != reflect.String || v.Kind() != reflect.String {
return fmt.Errorf("unable to fetch Global Attributes, Global Attributes should be map of strings")
}
attributeKeys = append(attributeKeys, k.String())
}
listTypeToKeys["Attributes"] = append(listTypeToKeys["Attributes"], sets.NewString(attributeKeys...))
}
Expand Down
11 changes: 2 additions & 9 deletions pkg/utils/overriding/merging.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"strings"

workspaces "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/api/v2/pkg/attributes"
"k8s.io/apimachinery/pkg/util/json"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/yaml"
Expand Down Expand Up @@ -107,18 +106,12 @@ func MergeDevWorkspaceTemplateSpec(
postStopCommands = postStopCommands.Union(sets.NewString(content.Events.PostStop...))
}

var err error
if len(content.Attributes) > 0 {
if len(result.Attributes) == 0 {
result.Attributes = attributes.Attributes{}
result.Attributes = make(map[string]string)
}
for k, v := range content.Attributes {
result.Attributes.FromMap(map[string]interface{}{
k: v,
}, &err)
if err != nil {
return nil, err
}
result.Attributes[k] = v
}
}
}
Expand Down
25 changes: 12 additions & 13 deletions pkg/utils/overriding/merging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"testing"

workspaces "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
attributesPkg "github.com/devfile/api/v2/pkg/attributes"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/json"
yamlMachinery "k8s.io/apimachinery/pkg/util/yaml"
Expand All @@ -27,9 +26,9 @@ func TestBasicMerging(t *testing.T) {
{
name: "Basic Merging",
mainContent: &workspaces.DevWorkspaceTemplateSpecContent{
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
"main": true,
}, nil),
Attributes: map[string]string{
"version1": "main",
},
Commands: []workspaces.Command{
{
Id: "mainCommand",
Expand Down Expand Up @@ -72,9 +71,9 @@ func TestBasicMerging(t *testing.T) {
},
pluginFlattenedContents: []*workspaces.DevWorkspaceTemplateSpecContent{
{
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
Attributes: map[string]string{
"version2": "plugin",
}, nil),
},
Commands: []workspaces.Command{
{
Id: "pluginCommand",
Expand Down Expand Up @@ -105,9 +104,9 @@ func TestBasicMerging(t *testing.T) {
},
},
parentFlattenedContent: &workspaces.DevWorkspaceTemplateSpecContent{
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
"version": "parent",
}, nil),
Attributes: map[string]string{
"version3": "parent",
},
Commands: []workspaces.Command{
{
Id: "parentCommand",
Expand Down Expand Up @@ -138,11 +137,11 @@ func TestBasicMerging(t *testing.T) {
},
},
expected: &workspaces.DevWorkspaceTemplateSpecContent{
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
"version": "parent",
Attributes: map[string]string{
"version3": "parent",
"version2": "plugin",
"main": true,
}, nil),
"version1": "main",
},
Commands: []workspaces.Command{
{
Id: "parentCommand",
Expand Down
13 changes: 6 additions & 7 deletions pkg/utils/overriding/overriding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"

workspaces "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
attributesPkg "github.com/devfile/api/v2/pkg/attributes"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/json"
yamlMachinery "k8s.io/apimachinery/pkg/util/yaml"
Expand Down Expand Up @@ -52,10 +51,10 @@ func TestBasicToplevelOverriding(t *testing.T) {
},
},
},
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
Attributes: map[string]string{
"version": "main",
"xyz": "xyz",
}, nil),
},
}

patch := workspaces.ParentOverrides{
Expand Down Expand Up @@ -86,9 +85,9 @@ func TestBasicToplevelOverriding(t *testing.T) {
},
},
},
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
Attributes: map[string]string{
"version": "patch",
}, nil),
},
}

expected := &workspaces.DevWorkspaceTemplateSpecContent{
Expand Down Expand Up @@ -133,10 +132,10 @@ func TestBasicToplevelOverriding(t *testing.T) {
},
},
},
Attributes: attributesPkg.Attributes{}.FromMap(map[string]interface{}{
Attributes: map[string]string{
"version": "patch",
"xyz": "xyz",
}, nil),
},
}

result, err := OverrideDevWorkspaceTemplateSpec(&original, &patch)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
parent:
uri: "anyParent"
attributes:
objectAttribute:
attributeField: 9.9
objectAttribute: mainValue
components:
- container:
image: "aDifferentValue"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
attributes:
objectAttribute:
attributeField: 10.10
objectAttribute: parentValue
components:
- container:
image: "aValue"
Expand Down

0 comments on commit a2940cb

Please sign in to comment.