Create CRDs with schema

Fixes an issue where CRDs were being created without schema, allowing
resources with invalid content to be created, later stalling the
controller ListWatch event channel when the invalid resources could not
be deserialized.

This also requires moving Addon GVK tracking from a status field to
an annotation, as the GroupVersionKind type has special handling
internal to Kubernetes that prevents it from being serialized to the CRD
when schema validation is enabled.

Signed-off-by: Brad Davidson <[email protected]>

go.mod

@@ -124,7 +124,7 @@ require (
 	github.com/rancher/lasso v0.0.0-20221227210133-6ea88ca2fbcc
 	github.com/rancher/remotedialer v0.3.0
 	github.com/rancher/wharfie v0.5.3
-	github.com/rancher/wrangler v1.1.1
+	github.com/rancher/wrangler v1.1.1-0.20230419173538-80fdf092be3b
 	github.com/robfig/cron/v3 v3.0.1
 	github.com/rootless-containers/rootlesskit v1.0.1
 	github.com/sirupsen/logrus v1.9.0

go.sum

@@ -939,8 +939,8 @@ github.com/rancher/remotedialer v0.3.0 h1:y1EO8JCsgZo0RcqTUp6U8FXcBAv27R+TLnWRcp
 github.com/rancher/remotedialer v0.3.0/go.mod h1:BwwztuvViX2JrLLUwDlsYt5DiyUwHLlzynRwkZLAY0Q=
 github.com/rancher/wharfie v0.5.3 h1:6hiO26H7YTgChbLAE6JppxFRjaH3tbKfMItv/LqV0Q0=
 github.com/rancher/wharfie v0.5.3/go.mod h1:Ebpai7digxegLroBseeC54XRBt5we3DgFS6kAE2ho+o=
-github.com/rancher/wrangler v1.1.1 h1:wmqUwqc2M7ADfXnBCJTFkTB5ZREWpD78rnZMzmxwMvM=
-github.com/rancher/wrangler v1.1.1/go.mod h1:ioVbKupzcBOdzsl55MvEDN0R1wdGggj8iNCYGFI5JvM=
+github.com/rancher/wrangler v1.1.1-0.20230419173538-80fdf092be3b h1:rs3WYld8iaRcSzCmM/CrCIVz9uVgfd96o7FsufIdoVI=
+github.com/rancher/wrangler v1.1.1-0.20230419173538-80fdf092be3b/go.mod h1:D6Tu6oVX8aGtCHsMCtYaysgVK3ad920MTSeAu7rzb5U=
 github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2/go.mod h1:Nfe4efndBz4TibWycNE+lqyJZiMX4ycx+QKV8Ta0f/o=
 github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs=
 github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro=

pkg/apis/k3s.cattle.io/v1/types.go

@@ -2,7 +2,6 @@ package v1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 // +genclient
@@ -12,15 +11,10 @@ type Addon struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   AddonSpec   `json:"spec,omitempty"`
-	Status AddonStatus `json:"status,omitempty"`
+	Spec AddonSpec `json:"spec,omitempty"`
 }
 
 type AddonSpec struct {
 	Source   string `json:"source,omitempty"`
 	Checksum string `json:"checksum,omitempty"`
 }
-
-type AddonStatus struct {
-	GVKs []schema.GroupVersionKind `json:"gvks,omitempty"`
-}

pkg/crd/crds.go

@@ -0,0 +1,15 @@
+package crd
+
+import (
+	v1 "github.com/k3s-io/k3s/pkg/apis/k3s.cattle.io/v1"
+	"github.com/rancher/wrangler/pkg/crd"
+)
+
+func List() []crd.CRD {
+	addon := crd.NamespacedType("Addon.k3s.cattle.io/v1").
+		WithSchemaFromStruct(v1.Addon{}).
+		WithColumn("Source", ".spec.source").
+		WithColumn("Checksum", ".spec.checksum")
+
+	return []crd.CRD{addon}
+}

pkg/deploy/controller.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -20,6 +21,7 @@ import (
 	pkgutil "github.com/k3s-io/k3s/pkg/util"
 	errors2 "github.com/pkg/errors"
 	"github.com/rancher/wrangler/pkg/apply"
+	"github.com/rancher/wrangler/pkg/kv"
 	"github.com/rancher/wrangler/pkg/merr"
 	"github.com/rancher/wrangler/pkg/objectset"
 	"github.com/sirupsen/logrus"
@@ -38,7 +40,9 @@ import (
 
 const (
 	ControllerName = "deploy"
+	GVKAnnotation  = "addon.k3s.cattle.io/gvks"
 	startKey       = "_start_"
+	gvkSep         = ";"
 )
 
 // WatchFiles sets up an OnChange callback to start a periodic goroutine to watch files for changes once the controller has started up.
@@ -206,11 +210,17 @@ func (w *watcher) deploy(path string, compareChecksum bool) error {
 	}
 
 	// Merge GVK list early for validation
-	addon.Status.GVKs = append(addon.Status.GVKs, objects.GVKs()...)
+	addonGVKs := objects.GVKs()
+	for _, gvkString := range strings.Split(addon.Annotations[GVKAnnotation], gvkSep) {
+		if gvk, err := getGVK(gvkString); err == nil {
+			addonGVKs = append(addonGVKs, *gvk)
+		}
+	}
 
 	// Ensure that we don't try to prune using GVKs that the server doesn't have.
 	// This can happen when CRDs are removed or when core types are removed - PodSecurityPolicy, for example.
-	if err := w.validateGVKs(&addon); err != nil {
+	addonGVKs, err = w.validateGVKs(addonGVKs)
+	if err != nil {
 		w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ValidateManifestFailed", "Validate GVKs for manifest at %q failed: %v", path, err)
 		return err
 	}
@@ -222,15 +232,18 @@ func (w *watcher) deploy(path string, compareChecksum bool) error {
 	// doesn't know to search that GVK for owner references, it won't find and delete them.
 	w.recorder.Eventf(&addon, corev1.EventTypeNormal, "ApplyingManifest", "Applying manifest at %q", path)
 
-	if err := w.apply.WithOwner(&addon).WithGVK(addon.Status.GVKs...).Apply(objects); err != nil {
+	if err := w.apply.WithOwner(&addon).WithGVK(addonGVKs...).Apply(objects); err != nil {
 		w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ApplyManifestFailed", "Applying manifest at %q failed: %v", path, err)
 		return err
 	}
 
 	// Emit event, Update Addon checksum and GVKs only if apply was successful
 	w.recorder.Eventf(&addon, corev1.EventTypeNormal, "AppliedManifest", "Applied manifest at %q", path)
+	if addon.Annotations == nil {
+		addon.Annotations = map[string]string{}
+	}
 	addon.Spec.Checksum = checksum
-	addon.Status.GVKs = objects.GVKs()
+	addon.Annotations[GVKAnnotation] = getGVKString(objects.GVKs())
 	_, err = w.addons.Update(&addon)
 	return err
 }
@@ -244,6 +257,13 @@ func (w *watcher) delete(path string) error {
 		return err
 	}
 
+	addonGVKs := []schema.GroupVersionKind{}
+	for _, gvkString := range strings.Split(addon.Annotations[GVKAnnotation], gvkSep) {
+		if gvk, err := getGVK(gvkString); err == nil {
+			addonGVKs = append(addonGVKs, *gvk)
+		}
+	}
+
 	content, err := os.ReadFile(path)
 	if err != nil {
 		w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ReadManifestFailed", "Read manifest at %q failed: %v", path, err)
@@ -253,13 +273,14 @@ func (w *watcher) delete(path string) error {
 		} else {
 			// Search for objects using both GVKs currently listed in the file, as well as GVKs previously applied.
 			// This ensures that any conflicts between competing deploy controllers are handled properly.
-			addon.Status.GVKs = append(addon.Status.GVKs, o.GVKs()...)
+			addonGVKs = append(addonGVKs, o.GVKs()...)
 		}
 	}
 
 	// Ensure that we don't try to delete using GVKs that the server doesn't have.
 	// This can happen when CRDs are removed or when core types are removed - PodSecurityPolicy, for example.
-	if err := w.validateGVKs(&addon); err != nil {
+	addonGVKs, err = w.validateGVKs(addonGVKs)
+	if err != nil {
 		return err
 	}
 
@@ -271,7 +292,7 @@ func (w *watcher) delete(path string) error {
 	}
 
 	// apply an empty set with owner & gvk data to delete
-	if err := w.apply.WithOwner(&addon).WithGVK(addon.Status.GVKs...).ApplyObjects(); err != nil {
+	if err := w.apply.WithOwner(&addon).WithGVK(addonGVKs...).ApplyObjects(); err != nil {
 		return err
 	}
 
@@ -290,22 +311,19 @@ func (w *watcher) getOrCreateAddon(name string) (apisv1.Addon, error) {
 	return *addon, nil
 }
 
-// validateGVKs removes from the Addon status any GVKs that the server does not support
-func (w *watcher) validateGVKs(addon *apisv1.Addon) error {
+// validateGVKs removes from the list any GVKs that the server does not support
+func (w *watcher) validateGVKs(addonGVKs []schema.GroupVersionKind) ([]schema.GroupVersionKind, error) {
 	gvks := []schema.GroupVersionKind{}
-	for _, gvk := range addon.Status.GVKs {
+	for _, gvk := range addonGVKs {
 		found, err := w.serverHasGVK(gvk)
 		if err != nil {
-			return err
+			return gvks, err
 		}
 		if found {
 			gvks = append(gvks, gvk)
-		} else {
-			logrus.Warnf("Pruned unknown GVK from %s %s/%s: %s", addon.TypeMeta.GroupVersionKind(), addon.Namespace, addon.Name, gvk)
 		}
 	}
-	addon.Status.GVKs = gvks
-	return nil
+	return gvks, nil
 }
 
 // serverHasGVK uses a positive cache of GVKs that the cluster is known to have supported at some
@@ -462,3 +480,22 @@ func shouldDisableFile(base, fileName string, disables map[string]bool) bool {
 	baseName := strings.TrimSuffix(baseFile, suffix)
 	return disables[baseName]
 }
+
+func getGVK(s string) (*schema.GroupVersionKind, error) {
+	parts := strings.Split(s, ", Kind=")
+	if len(parts) != 2 {
+		return nil, fmt.Errorf("invalid GVK format: %s", s)
+	}
+	gvk := &schema.GroupVersionKind{}
+	gvk.Group, gvk.Version = kv.Split(parts[0], "/")
+	gvk.Kind = parts[1]
+	return gvk, nil
+}
+
+func getGVKString(gvks []schema.GroupVersionKind) string {
+	strs := make([]string, len(gvks))
+	for i, gvk := range gvks {
+		strs[i] = gvk.String()
+	}
+	return strings.Join(strs, gvkSep)
+}