Require CloudInit .spec.contents to be valid YAML

The Elemental cloud-init executor will fail in disastrous ways instead
of just discarding the document if the document's contents are not YAML.
However, it does ignore instructions that it doesn't understand if the
document is valid YAML.

So, require that the CloudInit resource's .spec.contents are valid YAML.

Ideally we could use the yip package to parse and perform semantic
analysis on the document as a richer form of validation here, but that
package does not expose a way of reading those diagnostics. In client
code, it only logs what the errors are (in fact, the function signature
doesn't return an error.)

Some future work upstream in that package could be beneficial here to
create an API such that the yip package can return errors to its caller
instead of simply logging them.

Signed-off-by: Connor Kuehl <[email protected]>

pkg/admitter/cloudinit.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 
 	"github.com/harvester/webhook/pkg/server/admission"
+	"gopkg.in/yaml.v3"
 	admissionregv1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -21,6 +22,7 @@ var (
 	errFilenameTaken     = errors.New("filename already in use")
 	errProtectedFilename = errors.New("filename conflicts with a critical system-owned file")
 	errMissingExt        = errors.New("filename does not end in .yaml or .yml")
+	errNotYAML           = errors.New("could not parse document as yaml")
 )
 
 var builtinFilenameDenyList = []string{
@@ -85,6 +87,16 @@ func (v *CloudInit) validate(cloudinit *v1beta1.CloudInit) error {
 		return errFilenameTaken
 	}
 
+	var obj map[string]any
+	var typeErr *yaml.TypeError
+	err = yaml.Unmarshal([]byte(cloudinit.Spec.Contents), &obj)
+	if errors.As(err, &typeErr) {
+		return errNotYAML
+	}
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 

pkg/admitter/cloudinit_test.go

@@ -56,6 +56,7 @@ func TestCreate(t *testing.T) {
 		{"filename collision", v1beta1.CloudInitSpec{Filename: "99_ssh.yaml"}, errFilenameTaken},
 		{"conflicts with protected file", v1beta1.CloudInitSpec{Filename: "helloworld.yaml"}, errProtectedFilename},
 		{"not yaml or yml file ext", v1beta1.CloudInitSpec{Filename: "a"}, errMissingExt},
+		{"not yaml contents", v1beta1.CloudInitSpec{Filename: "not.yaml", Contents: "hello, there"}, errNotYAML},
 	}
 
 	for _, tt := range tests {
@@ -96,6 +97,7 @@ func TestUpdate(t *testing.T) {
 		{"filename collision", v1beta1.CloudInitSpec{Filename: "99_ssh.yaml"}, errFilenameTaken},
 		{"conflicts with protected file", v1beta1.CloudInitSpec{Filename: "helloworld.yaml"}, errProtectedFilename},
 		{"not yaml or yml file ext", v1beta1.CloudInitSpec{Filename: "a"}, errMissingExt},
+		{"not yaml contents", v1beta1.CloudInitSpec{Filename: "not.yaml", Contents: "hello, there"}, errNotYAML},
 	}
 
 	for _, tt := range tests {