Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Iflib newKO #122

Merged
merged 9 commits into from
Apr 30, 2023
Merged

Iflib newKO #122

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b392ab0
new approach to interface with the parser interface
henderiw Apr 16, 2023
595cfe1
Merge branch 'nephio-project:main' into iflib-newparser
henderiw Apr 21, 2023
1f718b7
refactored based on new strategy
henderiw Apr 21, 2023
6b8553e
refactored based on new strategy
henderiw Apr 21, 2023
6ba0aa5
Merge branch 'nephio-project:main' into iflib-newparser
henderiw Apr 28, 2023
9072c5a
Merge branch 'nephio-project:main' into iflib-newparser
henderiw Apr 28, 2023
77238e8
aligned interface with new KubeObject
henderiw Apr 28, 2023
ab5a87d
added setspec/set status
henderiw Apr 29, 2023
1da9d38
updated go fmt
henderiw Apr 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 48 additions & 136 deletions krm-functions/lib/interface/v1alpha1/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ import (

"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
nephioreqv1alpha1 "github.com/nephio-project/api/nf_requirements/v1alpha1"
"sigs.k8s.io/yaml"
)

const (
// errors
errKubeObjectNotInitialized = "KubeObject not initialized"
"github.com/nephio-project/nephio/krm-functions/lib/kubeobject"
)

var (
Expand All @@ -35,146 +30,104 @@ var (
networkInstanceName = []string{"spec", "networkInstance", "name"}
)

type Interface interface {
// GetKubeObject returns the present kubeObject
GetKubeObject() *fn.KubeObject
// GetGoStruct returns a go struct representing the present KRM resource
GetGoStruct() (*nephioreqv1alpha1.Interface, error)
// GetAttachmentType returns the attachmentType from the spec
// if an error occurs or the attribute is not present an empty string is returned
GetAttachmentType() string
// GetCNIType returns the cniType from the spec
// if an error occurs or the attribute is not present an empty string is returned
GetCNIType() string
// GetNetworkInstanceName returns the name of the networkInstance from the spec
// if an error occurs or the attribute is not present an empty string is returned
GetNetworkInstanceName() string
// SetAttachmentType sets the attachmentType in the spec
SetAttachmentType(s string) error
// SetCNIType sets the cniType in the spec
SetCNIType(s string) error
// SetNetworkInstanceName sets the name of the networkInstance in the spec
SetNetworkInstanceName(s string) error
// SetSpec sets the spec attributes in the kubeobject according the go struct
SetSpec(*nephioreqv1alpha1.InterfaceSpec) error
// DeleteAttachmentType deletes the attachmentType from the spec
DeleteAttachmentType() error
// DeleteAttachmentType deletes the attachmentType from the spec
DeleteCNIType() error
type Interface struct {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need to create this structure? Can the functions developers just directly create the base KubeObjectExt structure? All the methods are used from there anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value is you get explicit type checking for the object and we need this in several libraries. This is the reason to do this. otherwise people need to implement this in each function independently again and again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I did not do so far is have the explicit SeptSpec and SetStatus with their specific type, so this would add even more type awareness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the way to look at this is a the specific KubeObj extensions for the InterfaceReq CRD

kubeobject.KubeObjectExt[*nephioreqv1alpha1.Interface]
}

// NewFromKubeObject creates a new parser interface
// It expects a *fn.KubeObject as input representing the serialized yaml file
func NewFromKubeObject(o *fn.KubeObject) (*Interface, error) {
henderiw marked this conversation as resolved.
Show resolved Hide resolved
r, err := kubeobject.NewFromKubeObject[*nephioreqv1alpha1.Interface](o)
if err != nil {
return nil, err
}
return &Interface{*r}, nil
}

// NewFromYAML creates a new parser interface
// It expects a raw byte slice as input representing the serialized yaml file
func NewFromYAML(b []byte) (Interface, error) {
o, err := fn.ParseKubeObject(b)
func NewFromYAML(b []byte) (*Interface, error) {
r, err := kubeobject.NewFromYaml[*nephioreqv1alpha1.Interface](b)
if err != nil {
return nil, err
}
return &itfce{
o: o,
}, nil
return &Interface{*r}, nil
}

// NewFromGoStruct creates a new parser interface
// It expects a go struct representing the interface krm resource
func NewFromGoStruct(x *nephioreqv1alpha1.Interface) (Interface, error) {
b, err := yaml.Marshal(x)
func NewFromGoStruct(x *nephioreqv1alpha1.Interface) (*Interface, error) {
r, err := kubeobject.NewFromGoStruct[*nephioreqv1alpha1.Interface](x)
if err != nil {
return nil, err
}
return NewFromYAML(b)
}

type itfce struct {
o *fn.KubeObject
}

// GetKubeObject returns the present kubeObject
func (r *itfce) GetKubeObject() *fn.KubeObject {
return r.o
return &Interface{*r}, nil
}

// GetGoStruct returns a go struct representing the present KRM resource
func (r *itfce) GetGoStruct() (*nephioreqv1alpha1.Interface, error) {
x := &nephioreqv1alpha1.Interface{}
if err := yaml.Unmarshal([]byte(r.o.String()), x); err != nil {
return nil, err
func (r *Interface) GetNestedString(fields ...string) string {
s, ok, err := r.NestedString(fields...)
if err != nil {
return ""
}
if !ok {
return ""
}
return x, nil
return s
}

// GetAttachmentType returns the attachmentType from the spec
// if an error occurs or the attribute is not present an empty string is returned
func (r *itfce) GetAttachmentType() string {
return r.getStringValue(attachmentType...)
func (r *Interface) GetAttachmentType() string {
return r.GetNestedString(attachmentType...)
}

// GetCNIType returns the cniType from the spec
// if an error occurs or the attribute is not present an empty string is returned
func (r *itfce) GetCNIType() string {
return r.getStringValue(cniType...)
func (r *Interface) GetCNIType() string {
return r.GetNestedString(cniType...)
}

// GetNetworkInstanceName returns the name of the networkInstance from the spec
// if an error occurs or the attribute is not present an empty string is returned
func (r *itfce) GetNetworkInstanceName() string {
return r.getStringValue(networkInstanceName...)
func (r *Interface) GetNetworkInstanceName() string {
return r.GetNestedString(networkInstanceName...)
}

// SetAttachmentType sets the attachmentType in the spec
func (r *itfce) SetAttachmentType(s string) error {

// validation -> should be part of the interface api repo
switch s {
case string(nephioreqv1alpha1.AttachmentTypeNone):
case string(nephioreqv1alpha1.AttachmentTypeVLAN):
default:
return fmt.Errorf("unknown attachmentType")
}

return r.setNestedField(s, attachmentType...)
func (r *Interface) SetAttachmentType(s nephioreqv1alpha1.AttachmentType) error {
return r.SetNestedString(string(s), attachmentType...)
}

// SetCNIType sets the cniType in the spec
func (r *itfce) SetCNIType(s string) error {

// validation -> should be part of the interface api repo
switch s {
case string(nephioreqv1alpha1.CNITypeIPVLAN):
case string(nephioreqv1alpha1.CNITypeSRIOV):
case string(nephioreqv1alpha1.CNITypeMACVLAN):
default:
return fmt.Errorf("unknown cniType")
}

return r.setNestedField(s, cniType...)
func (r *Interface) SetCNIType(s nephioreqv1alpha1.CNIType) error {
return r.SetNestedString(string(s), cniType...)
}

// SetNetworkInstanceName sets the name of the networkInstance in the spec
func (r *itfce) SetNetworkInstanceName(s string) error {
return r.setNestedField(s, networkInstanceName...)
func (r *Interface) SetNetworkInstanceName(s string) error {
return r.SetNestedString(s, networkInstanceName...)
}

// SetSpec sets the spec attributes in the kubeobject according the go struct
func (r *itfce) SetSpec(spec *nephioreqv1alpha1.InterfaceSpec) error {
func (r *Interface) SetSpec(spec *nephioreqv1alpha1.InterfaceSpec) error {
if spec == nil {
return nil
}
if spec.AttachmentType != "" {
if err := r.SetAttachmentType(string(spec.AttachmentType)); err != nil {
if err := r.SetAttachmentType(spec.AttachmentType); err != nil {
return err
}
} else {
if err := r.DeleteAttachmentType(); err != nil {
if _, err := r.DeleteAttachmentType(); err != nil {
return err
}
}
if spec.CNIType != "" {
if err := r.SetCNIType(string(spec.CNIType)); err != nil {
if err := r.SetCNIType(spec.CNIType); err != nil {
return err
}
} else {
if err := r.DeleteCNIType(); err != nil {
if _, err := r.DeleteCNIType(); err != nil {
return err
}
}
Expand All @@ -189,52 +142,11 @@ func (r *itfce) SetSpec(spec *nephioreqv1alpha1.InterfaceSpec) error {
}

// DeleteAttachmentType deletes the attachmentType from the spec
func (r *itfce) DeleteAttachmentType() error {
return r.deleteNestedField(attachmentType...)
func (r *Interface) DeleteAttachmentType() (bool, error) {
return r.RemoveNestedField(attachmentType...)
}

// DeleteAttachmentType deletes the attachmentType from the spec
func (r *itfce) DeleteCNIType() error {
return r.deleteNestedField(cniType...)
}

// getStringValue is a generic utility function that returns a string from
// a string slice representing the path in the yaml doc
func (r *itfce) getStringValue(fields ...string) string {
if r.o == nil {
return ""
}
s, ok, err := r.o.NestedString(fields...)
if err != nil {
return ""
}
if !ok {
return ""
}
return s
}

// setNestedField is a generic utility function that sets a string on
// a string slice representing the path in the yaml doc
func (r *itfce) setNestedField(s string, fields ...string) error {
if r.o == nil {
return fmt.Errorf(errKubeObjectNotInitialized)
}
if err := r.o.SetNestedField(s, fields...); err != nil {
return err
}
return nil
}

// deleteNestedField is a generic utility function that deletes
// a string slice representing the path from the yaml doc
func (r *itfce) deleteNestedField(fields ...string) error {
if r.o == nil {
return fmt.Errorf(errKubeObjectNotInitialized)
}
_, err := r.o.RemoveNestedField(fields...)
if err != nil {
return err
}
return nil
func (r *Interface) DeleteCNIType() (bool, error) {
return r.RemoveNestedField(cniType...)
}
34 changes: 11 additions & 23 deletions krm-functions/lib/interface/v1alpha1/interface_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestNewFromGoStruct(t *testing.T) {
},
"TestNewFromGoStructNil": {
input: nil,
errExpected: true,
errExpected: false, // new approach does not return an error
},
}

Expand Down Expand Up @@ -140,10 +140,10 @@ func TestGetKubeObject(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {

if diff := cmp.Diff(tc.wantKind, i.GetKubeObject().GetKind()); diff != "" {
if diff := cmp.Diff(tc.wantKind, i.GetKind()); diff != "" {
t.Errorf("TestGetKubeObject: -want, +got:\n%s", diff)
}
if diff := cmp.Diff(tc.wantName, i.GetKubeObject().GetName()); diff != "" {
if diff := cmp.Diff(tc.wantName, i.GetName()); diff != "" {
t.Errorf("TestGetKubeObject: -want, +got:\n%s", diff)
}
})
Expand Down Expand Up @@ -275,7 +275,7 @@ func TestGetNetworkInstanceName(t *testing.T) {
func TestSetAttachmentType(t *testing.T) {
cases := map[string]struct {
file string
value string
value nephioreqv1alpha1.AttachmentType
errExpected bool
}{
"SetAttachmentTypeNormal": {
Expand All @@ -288,11 +288,6 @@ func TestSetAttachmentType(t *testing.T) {
value: "vlan",
errExpected: false,
},
"SetAttachmentTypeUnknown": {
file: itfaceEmpty,
value: "a",
errExpected: true,
},
}

for name, tc := range cases {
Expand All @@ -308,7 +303,7 @@ func TestSetAttachmentType(t *testing.T) {
} else {
assert.NoError(t, err)
got := i.GetAttachmentType()
if diff := cmp.Diff(tc.value, got); diff != "" {
if diff := cmp.Diff(tc.value, nephioreqv1alpha1.AttachmentType(got)); diff != "" {
t.Errorf("TestSetAttachmentType: -want, +got:\n%s", diff)
}
}
Expand All @@ -320,7 +315,7 @@ func TestSetAttachmentType(t *testing.T) {
func TestSetCNIType(t *testing.T) {
cases := map[string]struct {
file string
value string
value nephioreqv1alpha1.CNIType
errExpected bool
}{
"SetCNITypeNormal": {
Expand All @@ -333,11 +328,6 @@ func TestSetCNIType(t *testing.T) {
value: "sriov",
errExpected: false,
},
"SetCNITypeUnknown": {
file: itfaceEmpty,
value: "a",
errExpected: true,
},
}

for name, tc := range cases {
Expand All @@ -353,7 +343,7 @@ func TestSetCNIType(t *testing.T) {
} else {
assert.NoError(t, err)
got := i.GetCNIType()
if diff := cmp.Diff(tc.value, got); diff != "" {
if diff := cmp.Diff(tc.value, nephioreqv1alpha1.CNIType(got)); diff != "" {
t.Errorf("TestSetCNIType: -want, +got:\n%s", diff)
}
}
Expand Down Expand Up @@ -509,7 +499,7 @@ func TestDeleteCNIType(t *testing.T) {
}

t.Run(name, func(t *testing.T) {
err := i.DeleteCNIType()
_, err := i.DeleteCNIType()
assert.NoError(t, err)

})
Expand All @@ -535,7 +525,7 @@ func TestDeleteAttachmentType(t *testing.T) {
}

t.Run(name, func(t *testing.T) {
err := i.DeleteAttachmentType()
_, err := i.DeleteAttachmentType()
assert.NoError(t, err)

})
Expand Down Expand Up @@ -582,10 +572,8 @@ func TestYamlComments(t *testing.T) {
assert.NoError(t, err)
}

o := i.GetKubeObject()

if o.String() != string(tc.input) {
t.Errorf("expected output to be %q, but got %q", string(tc.input), o.String())
if i.String() != string(tc.input) {
t.Errorf("expected output to be %q, but got %q", string(tc.input), i.String())
}
})
}
Expand Down