vmware-archive / operator-builder

A Kubebuilder plugin to accelerate the development of Kubernetes operators
MIT License
41 stars 6 forks source link

bug: manifest parser sees spaces in comments as multiple results #271

Open scottd018 opened 2 years ago

scottd018 commented 2 years ago

Consider the following manifest:

---
# +operator-builder:resource:collectionField=provider,value="aws",include
apiVersion: v1
kind: ConfigMap
metadata:
  name: contour-config-test-collection-field
  namespace: ingress-system  # +operator-builder:field:name=namespace,default=ingress-system,type=string
data:
  this: "serves no purpose other than to test resource markers on collection fields"
---
# +operator-builder:resource:collectionField=provider,value="aws",include
# Note: Comments with spaces can cause problems with parsing.  Leave this here 
# for functional testing purposes.

# The space above is actually what causes the problem.
apiVersion: v1
kind: ConfigMap
metadata:
  name: contour-config-test-parse-comment
  namespace: ingress-system  # +operator-builder:field:name=namespace,default=ingress-system,type=string
data:
  this: "serves no purpose other than to test comment spaces for resource markers on collection fields"

With a space in the comment, this will actually return multiple marker results for the contour-config-test-parse-comment ConfigMap. While this is not necessarily an issue if we handle the result properly, the marker package should only return one result as there is only one marker.

UPDATE: this is actually in the manifest parser, not the marker parser, as the static content while parsing looks like this:

(this is from a debug console)

"\n# +operator-builder:resource:collectionField=provider,value=\"aws\",include\napiVersion: v1\nkind: ConfigMap\nmetadata:\n    name: contour-config-test-collection-field\n    namespace: !!var parent.Spec.Namespace # controlled by field: namespace\ndata:\n    this: \"serves no purpose other than to test resource markers on collection fields\"\n\n# +operator-builder:resource:collectionField=provider,value=\"aws\",include\n# Note: Comments with spaces can cause problems with parsing.  Leave this here \n# for functional testing purposes."
scottd018 commented 2 years ago

This appears to be a bug in the upstream yaml.v3 package. Given the above set of manifests, we get:

Manifest 0:

---
# +operator-builder:resource:collectionField=provider,value="aws",include
apiVersion: v1
kind: ConfigMap
metadata:
  name: contour-config-test-collection-field
  namespace: ingress-system  # +operator-builder:field:name=namespace,default=ingress-system,type=string
data:
  this: "serves no purpose other than to test resource markers on collection fields"
---
# +operator-builder:resource:collectionField=provider,value="aws",include
# Note: Comments with spaces can cause problems with parsing.  Leave this here 
# for functional testing purposes.

Manifest 1:

# The space above is actually what causes the problem.
apiVersion: v1
kind: ConfigMap
metadata:
  name: contour-config-test-parse-comment
  namespace: ingress-system  # +operator-builder:field:name=namespace,default=ingress-system,type=string
data:
  this: "serves no purpose other than to test comment spaces for resource markers on collection fields"

The newline character causes the yaml marshaler to see the resource marker as a foot comment to the first manifest and takes the newline as the start of a new manifest.