redhat-developer
`odo add binding` drops all YAML comments from the Devfile
/kind bug
What versions of software are you using?
Operating System: Fedora release 36 (Thirty Six)
Output of odo version:
odo v3.0.0-alpha2 (ba7f1a1cf)
How did you run odo exactly?
❯ odo init --name my-sample-go --devfile go --starter go-starter
❯ git init && git add -A . && git commit -m "initial commit"
Now add a few comments in the devfile.yaml file. This is just an example, but we can imagine users having more-complex Devfiles with many comments/documentation in them.
diff --git a/devfile.yaml b/devfile.yaml
index d39ff93..af9a6cd 100644
--- a/devfile.yaml
+++ b/devfile.yaml
@@ -1,5 +1,7 @@
+# User comments in the Devfile will get removed by running 'odo add binding'
commands:
- exec:
+ # Description of the build command
commandLine: GOCACHE=${PROJECT_SOURCE}/.cache go build main.go
component: runtime
group:
@@ -8,6 +10,7 @@ commands:
workingDir: ${PROJECT_SOURCE}
id: build
- exec:
+ # Description of the run command
commandLine: ./main
component: runtime
group:
❯ git commit -am "Add comments to the Devfile"
[main 0718673] Add comments to the Devfile
1 file changed, 3 insertions(+)
Now run odo add binding (either interactively or not), e.g.:
❯ odo add binding --service redis-standalone.Redis.redis.redis.opstreelabs.in --name my-sample-go-redis-standalone
✓ Successfully added the binding to the devfile.
Run `odo dev` to create it on the cluster.
Actual behavior
The Devfile has been updated, as expected, to include the ServiceBinding Kubernetes Component. But all YAML comments in it have been surprisingly removed at the same time:
diff --git a/devfile.yaml b/devfile.yaml
index af9a6cd..3112651 100644
--- a/devfile.yaml
+++ b/devfile.yaml
@@ -1,32 +1,58 @@
-# User comments in the Devfile will get removed by running 'odo add binding'
commands:
- exec:
- # Description of the build command
commandLine: GOCACHE=${PROJECT_SOURCE}/.cache go build main.go
component: runtime
group:
isDefault: true
kind: build
+ hotReloadCapable: false
workingDir: ${PROJECT_SOURCE}
id: build
- exec:
- # Description of the run command
commandLine: ./main
component: runtime
group:
isDefault: true
kind: run
+ hotReloadCapable: false
workingDir: ${PROJECT_SOURCE}
id: run
components:
- container:
+ dedicatedPod: false
endpoints:
- name: http
+ secure: false
targetPort: 8080
image: golang:latest
memoryLimit: 1024Mi
mountSources: true
name: runtime
+- kubernetes:
+ inlined: |
+ apiVersion: binding.operators.coreos.com/v1alpha1
+ kind: ServiceBinding
+ metadata:
+ creationTimestamp: null
+ name: my-sample-go-redis-standalone
+ spec:
+ application:
+ group: apps
+ name: my-sample-go-app
+ resource: deployments
+ version: v1
+ bindAsFiles: true
+ detectBindingResources: true
+ services:
+ - group: redis.redis.opstreelabs.in
+ id: my-sample-go-redis-standalone
+ kind: Redis
+ name: redis-standalone
+ resource: redis
+ version: v1beta1
+ status:
+ secret: ""
+ name: my-sample-go-redis-standalone
metadata:
description: Stack with the latest Go version
displayName: Go Runtime
Expected behavior
Users might have spent some time adding comments and documentation to their Devfiles, which might be quite complex. So existing comments should not be removed, especially since we want the Devfile to be the main source of truth.
I guess we will have the same issue with any other commands that edit the Devfile.
TL;DR This might need to remain as a known issue for now 😕
Summary of my quick investigations
Devfile marshaling and unmarshaling are currently not handled directly by odo, but done via the Devfile library, which uses sigs.k8s.io/yaml, which in turn relies on JSON to convert YAML to/from Go structs (like in parse.go).
The rationale behind this is explained in detail in this blog post.
In a nutshell, this allows to reuse JSON tags and any custom MarshalJSON and UnmarshalJSON methods, thus allowing consistent handling of both YAML and JSON (YAML being a superset of JSON).
Since JSON is data-only and does not have any such notion of comments, all comments in a YAML document are lost when converting it into JSON (via yaml.YAMLToJSON).
sigs.k8s.io/yaml is a fork of ghodss/yaml, which is a wrapper around go-yaml (v2). go-yaml v3 brings in comment handling, via a new Node representation that allows preserving comments near the data they describe.
So the recommended way to have comments preserved is via this Node representation, but I guess this will not allow consistent handling of both YAML and JSON.
Found the following open issues and discussions related to these:
- https://github.com/redhat-developer/odo/issues/5119 and https://github.com/devfile/api/issues/371#issuecomment-801342890 (different effect, on ordering, but seems related to this as it mentions why the Devfile library chose to use sigs.k8s.io/yaml and JSON encoding)
- https://github.com/kubernetes-sigs/yaml/issues/18, but I don't think this will change anything, as the point behind sigs.k8s.io/yaml is to purposely reuse JSON tags.
- https://stackoverflow.com/questions/55674853/modify-existing-yaml-file-and-add-new-data-and-comments
While it certainly makes sense for the Devfile library to use sigs.k8s.io/yaml (to effectively reuse the JSON tags defined in the Devfile API objects or {Un,}MarshalJSON methods, like DevWorkspaceTemplateSpecContent and Attributes structs), if we want to preserve comments on the odo side, we might not need JSON as an intermediate pivot format, but maybe we could leverage gopkg.in/yaml.v3 directly. This would require more effort on our side.
But I am not sure it actually makes sense to handle this on our side. This should certainly be an issue on the Devfile library side.
Added this to the Devfile community call (just to open the discussion), but we didn't have enough time to start discussing it. We will discuss it at a later time.
/status blocked
Issue created on the Devfile library side: https://github.com/devfile/api/issues/919