Move Storage.New() -> Serializer.Defaulter().NewDefaultedObject()

pkg/serializer/defaulter.go

@@ -2,6 +2,7 @@ package serializer
 
 import (
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/errors"
 )
 
@@ -13,6 +14,23 @@ type defaulter struct {
 	scheme *runtime.Scheme
 }
 
+// NewDefaultedObject returns a new, defaulted object. It is essentially scheme.New() and
+// scheme.Default(obj), but with extra logic to cover also internal versions.
+// Important to note here is that the TypeMeta information is NOT applied automatically here.
+func (d *defaulter) NewDefaultedObject(gvk schema.GroupVersionKind) (runtime.Object, error) {
+	obj, err := d.scheme.New(gvk)
+	if err != nil {
+		return nil, err
+	}
+
+	// Default the new object, this will take care of internal defaulting automatically
+	if err := d.Default(obj); err != nil {
+		return nil, err
+	}
+
+	return obj, nil
+}
+
 func (d *defaulter) Default(objs ...runtime.Object) error {
 	errs := []error{}
 	for _, obj := range objs {

pkg/serializer/serializer.go

@@ -165,7 +165,13 @@ type Defaulter interface {
 	// If the given object is internal, it will be automatically defaulted using the preferred external
 	// version's defaults (i.e. converted to the preferred external version, defaulted there, and converted
 	// back to internal).
+	// Important to note here is that the TypeMeta information is NOT applied automatically.
 	Default(objs ...runtime.Object) error
+
+	// NewDefaultedObject returns a new, defaulted object. It is essentially scheme.New() and
+	// scheme.Default(obj), but with extra logic to cover also internal versions.
+	// Important to note here is that the TypeMeta information is NOT applied automatically.
+	NewDefaultedObject(gvk schema.GroupVersionKind) (runtime.Object, error)
 }
 
 // NewSerializer constructs a new serializer based on a scheme, and optionally a codecfactory

pkg/serializer/serializer_test.go

@@ -622,6 +622,48 @@ func TestDefaulter(t *testing.T) {
 	}
 }
 
+func Test_defaulter_NewDefaultedObject(t *testing.T) {
+	tests := []struct {
+		name    string
+		gvk     schema.GroupVersionKind
+		want    runtime.Object
+		wantErr bool
+	}{
+		{
+			name: "internal complex",
+			gvk:  intgv.WithKind("Complex"),
+			// TODO: Now the v2 defaults are applied, because both v1 and v2 defaulting functions are
+			// applied on the same reflect Type (the same ExternalComplex struct), and hence both functions
+			// are run.
+			// This test illustrates though that an internal object can be created and defaulted though, using
+			// the NewDefaultedObject function.
+			want: &runtimetest.InternalComplex{Integer64: 5},
+		},
+		{
+			name: "crdoldversion",
+			gvk:  ext1gv.WithKind("CRD"),
+			want: &CRDOldVersion{TestString: "foo"},
+		},
+		{
+			name: "crdnewversion",
+			gvk:  ext2gv.WithKind("CRD"),
+			want: &CRDNewVersion{OtherString: "bar"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ourserializer.Defaulter().NewDefaultedObject(tt.gvk)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("defaulter.NewDefaultedObject() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("defaulter.NewDefaultedObject() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 /*
 // TODO: If we ever support keeping comments on the List -> YAML documents conversion, re-enable this unit test
 

pkg/storage/storage.go

@@ -75,9 +75,6 @@ type ReadStorage interface {
 	// Misc methods.
 	//
 
-	// New creates a new Object for the specified kind
-	// TODO: Move this to the Serializer's Defaulter interface? Defaulter.New(gvk)?
-	New(kind KindKey) (runtime.Object, error)
 	// ObjectKeyFor returns the ObjectKey for the given object
 	ObjectKeyFor(obj runtime.Object) (ObjectKey, error)
 	// Close closes all underlying resources (e.g. goroutines) used; before the application exits
@@ -120,33 +117,6 @@ func (s *GenericStorage) Serializer() serializer.Serializer {
 	return s.serializer
 }
 
-// New creates a new Object for the specified kind
-// TODO: Create better error handling if the GVK specified is not recognized
-func (s *GenericStorage) New(kind KindKey) (runtime.Object, error) {
-	obj, err := s.serializer.Scheme().New(kind.GetGVK())
-	if err != nil {
-		return nil, err
-	}
-
-	// Default the new object, this will take care of internal defaulting automatically
-	if err := s.serializer.Defaulter().Default(obj); err != nil {
-		return nil, err
-	}
-
-	// Cast to runtime.Object, and make sure it works
-	metaObj, ok := obj.(runtime.Object)
-	if !ok {
-		return nil, fmt.Errorf("can't convert to libgitops.runtime.Object")
-	}
-	// Set the desired gvk from the caller of this Object
-	// In practice, this means, although we created an internal type,
-	// from defaulting external TypeMeta information was set. Set the
-	// desired gvk here so it's correctly handled in all code that gets
-	// the gvk from the Object
-	metaObj.GetObjectKind().SetGroupVersionKind(kind.GetGVK())
-	return metaObj, nil
-}
-
 // Get returns a new Object for the resource at the specified kind/uid path, based on the file content
 func (s *GenericStorage) Get(key ObjectKey) (runtime.Object, error) {
 	content, err := s.raw.Read(key)