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.

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

Add TrackableFilter interface #4410

Merged
merged 4 commits into from Jan 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 6 additions & 13 deletions api/filters/annotations/annotations.go
Expand Up @@ -20,22 +20,15 @@ type Filter struct {
// FsSlice contains the FieldSpecs to locate the namespace field
FsSlice types.FsSlice

// SetEntryCallback is invoked each time an annotation is applied
// Example use cases:
// - Tracking all paths where annotations have been applied
SetEntryCallback func(key, value, tag string, node *yaml.RNode)
trackableSetter filtersutil.TrackableSetter
}

var _ kio.Filter = Filter{}
var _ kio.TrackableFilter = &Filter{}
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved

func (f Filter) setEntry(key, value, tag string) filtersutil.SetFn {
baseSetEntryFunc := filtersutil.SetEntry(key, value, tag)
return func(node *yaml.RNode) error {
if f.SetEntryCallback != nil {
f.SetEntryCallback(key, value, tag, node)
}
return baseSetEntryFunc(node)
}
// WithMutationTracker registers a callback which will be invoked each time a field is mutated
func (f *Filter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could alternatively be implemented using embedding rather than the private setter attribute. I see the tradeoff as less boilerplate vs unnecessarily exposing the SetScalar and SetEntry methods.

I chose to not use embedding because I figured we wanted to keep the public api as minimal as possible, but I am open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with your decision with keeping setter private rather than embedding it. In addition to avoiding unnecessarily exposing public methods, IMO it is clearer to understand what is happening when each filter that implements TrackableFilter is explicitly defining their own WithMutationTracker.

f.trackableSetter.WithMutationTracker(callback)
}

func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
Expand All @@ -45,7 +38,7 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
for _, k := range keys {
if err := node.PipeE(fsslice.Filter{
FsSlice: f.FsSlice,
SetValue: f.setEntry(
SetValue: f.trackableSetter.SetEntry(
k, f.Annotations[k], yaml.NodeTagString),
CreateKind: yaml.MappingNode, // Annotations are MappingNodes.
CreateTag: yaml.NodeTagMap,
Expand Down
4 changes: 3 additions & 1 deletion api/filters/annotations/annotations_test.go
Expand Up @@ -40,6 +40,7 @@ func TestAnnotations_Filter(t *testing.T) {
expectedOutput string
filter Filter
fsslice types.FsSlice
setEntryCallback func(key, value, tag string, node *yaml.RNode)
expectedSetEntryArgs []setEntryArg
}{
"add": {
Expand Down Expand Up @@ -259,8 +260,8 @@ spec:
"a": "a1",
"b": "b1",
},
SetEntryCallback: setEntryCallbackStub,
},
setEntryCallback: setEntryCallbackStub,
fsslice: []types.FieldSpec{
{
Path: "spec/template/metadata/annotations",
Expand Down Expand Up @@ -300,6 +301,7 @@ spec:
setEntryArgs = nil
t.Run(tn, func(t *testing.T) {
filter := tc.filter
filter.WithMutationTracker(tc.setEntryCallback)
filter.FsSlice = append(annosFs, tc.fsslice...)
if !assert.Equal(t,
strings.TrimSpace(tc.expectedOutput),
Expand Down
36 changes: 36 additions & 0 deletions api/filters/filtersutil/setters.go
Expand Up @@ -31,3 +31,39 @@ func SetEntry(key, value, tag string) SetFn {
})
}
}

type TrackableSetter struct {
// SetValueCallback will be invoked each time a field is set
setValueCallback func(key, value, tag string, node *yaml.RNode)
}

// WithMutationTracker registers a callback which will be invoked each time a field is mutated
func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) {
s.setValueCallback = callback
}

// SetScalar returns a SetFn to set a scalar value
// if a mutation tracker has been registered, the tracker will be invoked each
// time a scalar is set
func (s TrackableSetter) SetScalar(value string) SetFn {
sdowell marked this conversation as resolved.
Show resolved Hide resolved
origSetScalar := SetScalar(value)
return func(node *yaml.RNode) error {
if s.setValueCallback != nil {
s.setValueCallback("", value, "", node)
}
return origSetScalar(node)
}
}

// SetEntry returns a SetFn to set an entry in a map
// if a mutation tracker has been registered, the tracker will be invoked each
// time an entry is set
func (s TrackableSetter) SetEntry(key, value, tag string) SetFn {
origSetEntry := SetEntry(key, value, tag)
return func(node *yaml.RNode) error {
if s.setValueCallback != nil {
s.setValueCallback(key, value, tag, node)
}
return origSetEntry(node)
}
}
19 changes: 6 additions & 13 deletions api/filters/labels/labels.go
Expand Up @@ -21,22 +21,15 @@ type Filter struct {
// FsSlice identifies the label fields.
FsSlice types.FsSlice

// SetEntryCallback is invoked each time a label is applied
// Example use cases:
// - Tracking all paths where labels have been applied
SetEntryCallback func(key, value, tag string, node *yaml.RNode)
trackableSetter filtersutil.TrackableSetter
}

var _ kio.Filter = Filter{}
var _ kio.TrackableFilter = &Filter{}

func (f Filter) setEntry(key, value, tag string) filtersutil.SetFn {
baseSetEntryFunc := filtersutil.SetEntry(key, value, tag)
return func(node *yaml.RNode) error {
if f.SetEntryCallback != nil {
f.SetEntryCallback(key, value, tag, node)
}
return baseSetEntryFunc(node)
}
// WithMutationTracker registers a callback which will be invoked each time a field is mutated
func (f *Filter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) {
f.trackableSetter.WithMutationTracker(callback)
}

func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
Expand All @@ -46,7 +39,7 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
for _, k := range keys {
if err := node.PipeE(fsslice.Filter{
FsSlice: f.FsSlice,
SetValue: f.setEntry(
SetValue: f.trackableSetter.SetEntry(
k, f.Labels[k], yaml.NodeTagString),
CreateKind: yaml.MappingNode, // Labels are MappingNodes.
CreateTag: yaml.NodeTagMap,
Expand Down
4 changes: 3 additions & 1 deletion api/filters/labels/labels_test.go
Expand Up @@ -37,6 +37,7 @@ func TestLabels_Filter(t *testing.T) {
input string
expectedOutput string
filter Filter
setEntryCallback func(key, value, tag string, node *yaml.RNode)
expectedSetEntryArgs []setEntryArg
}{
"add": {
Expand Down Expand Up @@ -456,8 +457,8 @@ a:
CreateIfNotPresent: true,
},
},
SetEntryCallback: setEntryCallbackStub,
},
setEntryCallback: setEntryCallbackStub,
expectedSetEntryArgs: []setEntryArg{
{
Key: "mage",
Expand All @@ -478,6 +479,7 @@ a:
for tn, tc := range testCases {
setEntryArgs = nil
t.Run(tn, func(t *testing.T) {
tc.filter.WithMutationTracker(tc.setEntryCallback)
if !assert.Equal(t,
strings.TrimSpace(tc.expectedOutput),
strings.TrimSpace(filtertest_test.RunFilter(t, tc.input, tc.filter))) {
Expand Down
7 changes: 7 additions & 0 deletions kyaml/kio/kio.go
Expand Up @@ -57,6 +57,13 @@ type Filter interface {
Filter([]*yaml.RNode) ([]*yaml.RNode, error)
}

// TrackableFilter is an extension of Filter which is also capable of tracking
// which fields were mutated by the filter.
type TrackableFilter interface {
Filter
WithMutationTracker(func(key, value, tag string, node *yaml.RNode))
}

// FilterFunc implements a Filter as a function.
type FilterFunc func([]*yaml.RNode) ([]*yaml.RNode, error)

Expand Down