k3s server does not ignore empty YAML documents in manifests

[mortenlj]

Describe the bug
k3s fails to load YAML-manifests with an "empty" document, failing with this error:
failed to process /var/lib/rancher/k3s/server/manifests/metallb.yaml: Object 'Kind' is missing in 'null'

To Reproduce

1. Start the k3s server
2. Place the following content in a YAML-file in /var/lib/rancher/k3s/server/manifests:

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: default
  name: config
data:
  config: some_data
---

Notice the extra --- and blank line at the end, which indicates the start of a new document, which happens to be empty.

3. Observe errors in k3s server logs

Expected behavior
It would be nice if empty documents could just be ignored.

Additional context
Similar problem in a different project: kubernetes-sigs/kustomize#178

[matej-g]

Seems like the bug originates in controller.go - function yamlToObjects breaks only at the end of input, but if in the YAML ends with a separator sign (and a newline), it goes on and passes it further, causing the error.

Maybe adding a condition to check whether raw is empty and break if true would suffice?

[matej-g]

I tried my hand at a solution (see PR #225 ), happy to receive feedback

[epicfilemcnulty]

@matej-g Unfortunately, your solution won't really work, cause

1. In case of an empty yaml object raw will have not only newlines, but three initial dashes, so bytes.Count(raw, []byte{'\n'}) == len(raw) will never be true.
2. If the empty yaml object is not the last one in the manifest, your break will prevent all the following yaml objects to be processed.
3. We also should consider the case when there are not only newlines, but also comments in the yaml object. helm template usually generates this kind of yaml objects:

---
# Source: acmedns/templates/configmaps.yaml
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: acmedns-config
...

[epicfilemcnulty]

I've tried another approach here, see #229

[matej-g]

More elegant but are you sure your solution will actually work? From what I gather:

Ad 1) the separator is not included in the raw reader, with the exception of the first object if the YAML file contains the separator at the beginning of the file (as is the example provided by the bug reporter). So actually condition "three dashes + newline(s)" will always be false.

Ad 2) True, this is a case I did not consider

Ad 3) AFAIK helm template generates something akin to:

# Source: /path/to/template.yaml
apiVersion: v1
kind: ConfigMap
metadata:
 name: example
---
#Source: /path/to/second_template.yaml
apiVersion: v1
...

So it does not separate the comment, it includes it as part of every YAML. Not saying that we could not also address an edge case where YAML is comment only, but it does not seem to be common case.

[epicfilemcnulty]

@matej-g Well, I am pretty much sure my solution works (although there might be more edge cases we have not thought of yet), cause I did test it with a couple of manifests having empty yaml objects (or yaml objects with comments only) in the beginning/middle/end of a manifest. Your solution did not work with them.
As for helm template -- I did generate manifests for tests using helm template and local charts, and my previous example was literally copypasted from the output that helm template generated.

[matej-g]

You are right, I was commenting before I actually had a closer look, seems like your solution covers all the scenarios (regardless of whether raw YAML starts with --- or it's just "empty"), so my apologies.

Helm chart just surprised me a bit because normally I don't see separation between # Source: ... comment and the actual content, could be just an oddity in particular template.

In that case I will close my PR and would suggest to accept your solution.

[erikwilson]

Reopening for testing

[dnoland1]

Tried several variations of "empty" documents in the manifest:

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: default
  name: mytest
data:
  config: test_data
---

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: default
  name: mytest
data:
  config: test_data
---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: default
  name: mytest2
data:
  config: test_data2
---

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: default
  name: mytest
data:
  config: test_data
---
---

No errors and the configmaps were created:

vagrant@k3s-node1:~$ k3s kubectl get configmap
NAME      DATA   AGE
mytest    1      10m
mytest2   1      44s

[erikwilson]

Thanks for testing @dnoland1!