Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
Move Storage.New() to the Serializer Defaulter
Browse files Browse the repository at this point in the history
  • Loading branch information
luxas committed Aug 21, 2020
1 parent c46586b commit 0121e59
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 30 deletions.
18 changes: 18 additions & 0 deletions pkg/serializer/defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package serializer

import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/errors"
)

Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions pkg/serializer/serializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 42 additions & 0 deletions pkg/serializer/serializer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 0 additions & 30 deletions pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import (
)

type ReadStorage interface {
// New creates a new Object for the specified kind
New(kind KindKey) (runtime.Object, error)

// Get returns a new Object for the resource at the specified kind/uid path, based on the file content
Get(key ObjectKey) (runtime.Object, error)
// GetMeta returns a new Object's APIType representation for the resource at the specified kind/uid path
Expand Down Expand Up @@ -86,33 +83,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)
Expand Down

0 comments on commit 0121e59

Please sign in to comment.