Implement webhook

Signed-off-by: Connor Kuehl <[email protected]>
harvester · Jan 11, 2024 · 76a5933 · 76a5933
1 parent ba65cc2
commit 76a5933
Show file tree

Hide file tree

Showing 5 changed files with 422 additions and 15 deletions.
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -92,8 +92,13 @@ func main() {
 func runWebhookServer(ctx context.Context, cfg *rest.Config, options *config.Options) error {
 	webhookServer := server.NewWebhookServer(ctx, cfg, webhookName, options)
 
+	cloudinitValidator, err := admitter.NewCloudInitValidator(cfg)
+	if err != nil {
+		return err
+	}
+
 	var validators = []admission.Validator{
-		admitter.NewCloudInitValidator(),
+		cloudinitValidator,
 	}
 
 	if err := webhookServer.RegisterValidators(validators...); err != nil {

diff --git a/pkg/admitter/cloudinit.go b/pkg/admitter/cloudinit.go
@@ -1,31 +1,120 @@
 package admitter
 
 import (
+	"context"
 	"errors"
+	"fmt"
+	"path/filepath"
 
 	"github.com/harvester/webhook/pkg/server/admission"
 	admissionregv1 "k8s.io/api/admissionregistration/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/rest"
 
 	v1beta1 "github.com/harvester/node-manager/pkg/apis/node.harvesterhci.io/v1beta1"
+	clientset "github.com/harvester/node-manager/pkg/generated/clientset/versioned"
+	cloudinitv1beta1 "github.com/harvester/node-manager/pkg/generated/clientset/versioned/typed/node.harvesterhci.io/v1beta1"
 )
 
+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")
+)
+
+var builtinFilenameDenyList = []string{
+	"90_custom.yaml",
+	"99_settings.yaml",
+	"elemental.config",
+	"grubenv",
+	"harvester.config",
+	"install",
+}
+
 type CloudInit struct {
 	admission.DefaultValidator
+
+	cloudinits cloudinitv1beta1.CloudInitInterface
+}
+
+func NewCloudInitValidator(config *rest.Config) (*CloudInit, error) {
+	client, err := clientset.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	cloudinits := client.NodeV1beta1().CloudInits()
+
+	return &CloudInit{
+		cloudinits: cloudinits,
+	}, nil
+}
+
+func (v *CloudInit) Create(_ *admission.Request, newObj runtime.Object) error {
+	newCloudInit := newObj.(*v1beta1.CloudInit)
+	return v.validate(newCloudInit)
+}
+
+func (v *CloudInit) Update(_ *admission.Request, oldObj runtime.Object, newObj runtime.Object) error {
+	oldCloudInit := oldObj.(*v1beta1.CloudInit)
+	newCloudInit := newObj.(*v1beta1.CloudInit)
+
+	if oldCloudInit.Spec.Filename == newCloudInit.Spec.Filename {
+		return nil
+	}
+
+	return v.validate(newCloudInit)
+}
+
+func (v *CloudInit) validate(cloudinit *v1beta1.CloudInit) error {
+	if v.missingExtension(cloudinit.Spec.Filename) {
+		return errMissingExt
+	}
+
+	if v.isProtectedFilename(cloudinit.Spec.Filename) {
+		return errProtectedFilename
+	}
+
+	taken, err := v.isFilenameTaken(cloudinit.Spec.Filename)
+	if err != nil {
+		return fmt.Errorf("check for duplicate filename: %w", err)
+	}
+
+	if taken {
+		return errFilenameTaken
+	}
+
+	return nil
 }
 
-func NewCloudInitValidator() *CloudInit {
-	return &CloudInit{}
+func (v *CloudInit) missingExtension(name string) bool {
+	ext := filepath.Ext(name)
+	return ext != ".yaml" && ext != ".yml"
 }
 
-func (v *CloudInit) Create(request *admission.Request, newObj runtime.Object) error {
-	_, _ = request, newObj
-	return errors.New("not implemented")
+func (v *CloudInit) isFilenameTaken(name string) (bool, error) {
+	cloudinits, err := v.cloudinits.List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return true, err
+	}
+
+	for _, cloudinit := range cloudinits.Items {
+		if cloudinit.Spec.Filename == name {
+			return true, nil
+		}
+	}
+
+	return false, nil
 }
 
-func (v *CloudInit) Update(request *admission.Request, oldObj runtime.Object, newObj runtime.Object) error {
-	_, _, _ = request, oldObj, newObj
-	return errors.New("not implemented")
+func (v *CloudInit) isProtectedFilename(name string) bool {
+	for _, protected := range builtinFilenameDenyList {
+		if name == protected {
+			return true
+		}
+	}
+	return false
 }
 
 func (v *CloudInit) Resource() admission.Resource {

diff --git a/pkg/admitter/cloudinit_test.go b/pkg/admitter/cloudinit_test.go
@@ -0,0 +1,164 @@
+package admitter
+
+import (
+	"context"
+	"errors"
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/watch"
+
+	v1beta1 "github.com/harvester/node-manager/pkg/apis/node.harvesterhci.io/v1beta1"
+	"github.com/harvester/webhook/pkg/server/admission"
+)
+
+func TestProtectedFilenames(t *testing.T) {
+	want := map[string]struct{}{
+		"90_custom.yaml":   {},
+		"99_settings.yaml": {},
+		"elemental.config": {},
+		"grubenv":          {},
+		"harvester.config": {},
+		"install":          {},
+	}
+
+	got := make(map[string]struct{})
+	for _, f := range builtinFilenameDenyList {
+		got[f] = struct{}{}
+	}
+
+	if !reflect.DeepEqual(want, got) {
+		t.Errorf("want %v, got %v", want, got)
+	}
+}
+
+func TestCreate(t *testing.T) {
+	origDenyList := builtinFilenameDenyList
+	defer func() { builtinFilenameDenyList = origDenyList }()
+	builtinFilenameDenyList = []string{
+		"helloworld.yaml",
+	}
+
+	existing := []v1beta1.CloudInit{
+		{ObjectMeta: metav1.ObjectMeta{Name: "ssh-access"}, Spec: v1beta1.CloudInitSpec{Filename: "99_ssh.yaml"}},
+	}
+
+	tests := []struct {
+		name  string
+		input v1beta1.CloudInitSpec
+		want  error
+	}{
+		{"allow yaml", v1beta1.CloudInitSpec{Filename: "hi.yaml"}, nil},
+		{"allow yml", v1beta1.CloudInitSpec{Filename: "hi.yml"}, nil},
+		{"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},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctl := &CloudInit{cloudinits: &mockClient{list: existing}}
+
+			cloudinit := &v1beta1.CloudInit{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-cloudinit"},
+				Spec:       tt.input,
+			}
+
+			got := ctl.Create(new(admission.Request), cloudinit)
+			if !errors.Is(got, tt.want) {
+				t.Errorf("want err=%v, got err=%v", tt.want, got)
+			}
+		})
+	}
+}
+
+func TestUpdate(t *testing.T) {
+	origDenyList := builtinFilenameDenyList
+	defer func() { builtinFilenameDenyList = origDenyList }()
+	builtinFilenameDenyList = []string{
+		"helloworld.yaml",
+	}
+
+	existing := []v1beta1.CloudInit{
+		{ObjectMeta: metav1.ObjectMeta{Name: "ssh-access"}, Spec: v1beta1.CloudInitSpec{Filename: "99_ssh.yaml"}},
+	}
+
+	tests := []struct {
+		name  string
+		input v1beta1.CloudInitSpec
+		want  error
+	}{
+		{"allow yaml", v1beta1.CloudInitSpec{Filename: "hi.yaml"}, nil},
+		{"allow yml", v1beta1.CloudInitSpec{Filename: "hi.yml"}, nil},
+		{"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},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctl := &CloudInit{cloudinits: &mockClient{list: existing}}
+
+			cloudinit := &v1beta1.CloudInit{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-cloudinit"},
+				Spec:       tt.input,
+			}
+
+			old := &v1beta1.CloudInit{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-cloudinit"},
+				Spec:       v1beta1.CloudInitSpec{Filename: "specifically-not-in-use.yaml"},
+			}
+
+			got := ctl.Update(new(admission.Request), old, cloudinit)
+			if !errors.Is(got, tt.want) {
+				t.Errorf("want err=%v, got err=%v", tt.want, got)
+			}
+		})
+	}
+}
+
+// Sadly, github.com/rancher/wrangler/pkg/generic/fake package generates mock clients that lack
+// the ctx parameter that is required by the CloudInitInterface.
+
+type mockClient struct {
+	list []v1beta1.CloudInit
+}
+
+func (m *mockClient) Create(_ context.Context, _ *v1beta1.CloudInit, _ v1.CreateOptions) (*v1beta1.CloudInit, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (m *mockClient) Update(_ context.Context, _ *v1beta1.CloudInit, _ v1.UpdateOptions) (*v1beta1.CloudInit, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (m *mockClient) UpdateStatus(_ context.Context, _ *v1beta1.CloudInit, _ v1.UpdateOptions) (*v1beta1.CloudInit, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (m *mockClient) Delete(_ context.Context, _ string, _ v1.DeleteOptions) error {
+	return errors.New("not implemented")
+}
+
+func (m *mockClient) DeleteCollection(_ context.Context, _ v1.DeleteOptions, _ v1.ListOptions) error {
+	return errors.New("not implemented")
+}
+
+func (m *mockClient) Get(_ context.Context, _ string, _ v1.GetOptions) (*v1beta1.CloudInit, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (m *mockClient) List(_ context.Context, _ v1.ListOptions) (*v1beta1.CloudInitList, error) {
+	return &v1beta1.CloudInitList{Items: m.list}, nil
+}
+
+func (m *mockClient) Watch(_ context.Context, _ v1.ListOptions) (watch.Interface, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (m *mockClient) Patch(_ context.Context, _ string, _ types.PatchType, _ []byte, _ v1.PatchOptions, _ ...string) (result *v1beta1.CloudInit, err error) {
+	return nil, errors.New("not implemented")
+}
diff --git a/pkg/mutator/cloudinit.go b/pkg/mutator/cloudinit.go
@@ -1,6 +1,9 @@
 package mutator
 
 import (
+	"fmt"
+	"path/filepath"
+
 	"github.com/harvester/webhook/pkg/server/admission"
 	admissionregv1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -16,18 +19,50 @@ func NewCloudInitMutator() *CloudInit {
 	return &CloudInit{}
 }
 
-func (m *CloudInit) Create(_ *admission.Request, _ runtime.Object) (admission.Patch, error) {
-	var patch admission.Patch
-	// Not implemented, validator will fail request
-	return patch, nil
+func (m *CloudInit) Create(_ *admission.Request, newObj runtime.Object) (admission.Patch, error) {
+	newCloudInit := newObj.(*v1beta1.CloudInit)
+	return patchFilenameIfNecessary(newCloudInit)
 }
 
-func (m *CloudInit) Update(_ *admission.Request, _ runtime.Object, _ runtime.Object) (admission.Patch, error) {
+func (m *CloudInit) Update(_ *admission.Request, _ runtime.Object, newObj runtime.Object) (admission.Patch, error) {
+	newCloudInit := newObj.(*v1beta1.CloudInit)
+	return patchFilenameIfNecessary(newCloudInit)
+}
+
+func patchFilenameIfNecessary(newCloudInit *v1beta1.CloudInit) (admission.Patch, error) {
 	var patch admission.Patch
-	// Not implemented, validator will fail request
+
+	filename := ensureFileExtension(filepath.Base(newCloudInit.Spec.Filename))
+	if filename == newCloudInit.Spec.Filename {
+		return patch, nil
+	}
+
+	p := admission.PatchOp{
+		Op:    admission.PatchOpReplace,
+		Path:  "/spec/filename",
+		Value: filename,
+	}
+	patch = append(patch, p)
+
 	return patch, nil
 }
 
+func ensureFileExtension(s string) string {
+	accept := func(extension string) bool {
+		extensions := []string{".yaml", ".yml"}
+		for _, ext := range extensions {
+			if ext == extension {
+				return true
+			}
+		}
+		return false
+	}
+	if accept(filepath.Ext(s)) {
+		return s
+	}
+	return fmt.Sprintf("%s.yaml", s)
+}
+
 func (m *CloudInit) Resource() admission.Resource {
 	return admission.Resource{
 		Names:      []string{v1beta1.CloudInitResourceName},