906- variable substitution in kubernetes uri content #138
Conversation
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
pkg/devfile/parser/devfileobj.go
Outdated
| @@ -7,7 +7,8 @@ import ( | |||
|
|
|||
| // Default filenames for create devfile | |||
| const ( | |||
| OutputDevfileYamlPath = "devfile.yaml" | |||
| OutputDevfileYamlPath = "devfile.yaml" | |||
| K8sLikeComponentOriginalURIKey = "devfile.io/k8sLikeComponent-originalURI" | |||
There was a problem hiding this comment.
wonder if we should keep it consistent like
// attribute key of the plugin overridden element resource information
PluginOverrideAttribute = "api.devfile.io/plugin-override-from"
pkg/devfile/parser/parse.go
Outdated
| return err | ||
| } | ||
| for _, kubeComp := range kubeComponents { | ||
| if kubeComp.Kubernetes.Uri != "" { |
There was a problem hiding this comment.
| if kubeComp.Kubernetes.Uri != "" { | |
| if kubeComp.Kubernetes != nil && kubeComp.Kubernetes.Uri != "" { |
tech not needed but i would still put it in place to avoid any panics if somehow GetComponents(getKubeCompOptions) fails
pkg/devfile/parser/writer.go
Outdated
| @@ -11,6 +14,11 @@ import ( | |||
| // WriteYamlDevfile creates a devfile.yaml file | |||
| func (d *DevfileObj) WriteYamlDevfile() error { | |||
|
|
|||
| // Check kubernetes components, and restore original uri content | |||
| err := restoreK8sCompURI(d) | |||
There was a problem hiding this comment.
just a thought:
this func would be checking for the attribute every time it is called to write devfile. I wonder if we should introduce some property in the object that we can use to keep track:
type DevfileObj struct {
// Ctx has devfile context info
Ctx devfileCtx.DevfileCtx
// Data has the devfile data
Data data.DevfileData
// convert
ConvertUriToInlined bool
}
There was a problem hiding this comment.
I do not want the value to be direct accessible for all devobj consumers.. so added into DevfileObj.ctx.convertUriToInlined instead.
There was a problem hiding this comment.
Thinking if we should really keep this context? Because context refers to a devfile context but otherwise its ok i guess
pkg/devfile/parser/writer.go
Outdated
| if err != nil && err.Error() != keyNotFoundErr.Error() { | ||
| return err | ||
| } |
There was a problem hiding this comment.
you can also do it this way:
| if err != nil && err.Error() != keyNotFoundErr.Error() { | |
| return err | |
| } | |
| if err != nil { | |
| if _, ok := err.(*attributes.KeyNotFoundError); !ok { | |
| return err | |
| } | |
| } |
Signed-off-by: Stephanie <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maysunfaisal, yangcao77 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What does this PR do?:
This PR converts kubernetest
urifield to theinlinedfield with actual content in thedevfileObjobject in memory; so that the grobal variable defined can be substituted. An attributeapi.devfile.io/kubeComponent-originalURIwill be added to the kubernetes component as well.The PR also modified writer to check for the specific attribute and restore the uri back before write to the devfile; so that we are not modifying user's devfile content.
Which issue(s) this PR fixes:
fixes devfile/api#906
PR acceptance criteria:
Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.
Unit/Functional tests
QE Integration test
Documentation
Client Impact
How to test changes / Special notes to the reviewer: