Skip to content

feat: add optional strict unmarshalling for LoadFunctionConfig and SimpleProcessor #5926

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 14 additions & 5 deletions kyaml/fn/framework/processors.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ type SimpleProcessor struct {
// Config must be a struct capable of receiving the data from ResourceList.functionConfig.
// Filter functions may close over this struct to access its data.
Config interface{}
// Strict indicates whether the processor should use strict unmarshalling when loading the Config.
Strict bool
}

// Process makes SimpleProcessor implement the ResourceListProcessor interface.
// It loads the ResourceList.functionConfig into the provided Config type, applying
// defaulting and validation if supported by Config. It then executes the processor's filter.
func (p SimpleProcessor) Process(rl *ResourceList) error {
if err := LoadFunctionConfig(rl.FunctionConfig, p.Config); err != nil {
if err := LoadFunctionConfig(rl.FunctionConfig, p.Config, p.Strict); err != nil {
return errors.WrapPrefixf(err, "loading function config")
}
return errors.WrapPrefixf(rl.Filter(p.Filter), "processing filter")
Expand Down Expand Up @@ -111,7 +113,7 @@ func (p *VersionedAPIProcessor) Process(rl *ResourceList) error {
if err != nil {
return errors.WrapPrefixf(err, "unable to identify provider for resource")
}
if err := LoadFunctionConfig(rl.FunctionConfig, api); err != nil {
if err := LoadFunctionConfig(rl.FunctionConfig, api, false); err != nil {
return errors.Wrap(err)
}
return errors.Wrap(rl.Filter(api))
Expand Down Expand Up @@ -139,7 +141,7 @@ func extractGVK(src *yaml.RNode) (apiVersion, kind string) {
// LoadFunctionConfig reads a configuration resource from YAML into the provided data structure
// and then prepares it for use by running defaulting and validation on it, if supported.
// ResourceListProcessors should use this function to load ResourceList.functionConfig.
func LoadFunctionConfig(src *yaml.RNode, api interface{}) error {
func LoadFunctionConfig(src *yaml.RNode, api interface{}, strict bool) error {
if api == nil {
return nil
}
Expand All @@ -156,7 +158,14 @@ func LoadFunctionConfig(src *yaml.RNode, api interface{}) error {

// using sigs.k8s.io/yaml here lets the custom types embed core types
// that only have json tags, notably types from k8s.io/apimachinery/pkg/apis/meta/v1
if err := k8syaml.Unmarshal([]byte(src.MustString()), api); err != nil {

var err error
if strict {
err = k8syaml.UnmarshalStrict([]byte(src.MustString()), api)
} else {
err = k8syaml.Unmarshal([]byte(src.MustString()), api)
}
if err != nil {
if schemaValidationError != nil {
// if we got a validation error, report it instead as it is likely a nicer version of the same message
return schemaValidationError
Expand Down Expand Up @@ -295,7 +304,7 @@ func (tp TemplateProcessor) Filter(items []*yaml.RNode) ([]*yaml.RNode, error) {
// directly as a processor. As a Processor, it loads the ResourceList.functionConfig into the
// TemplateData field, exposing it to all templates by default.
func (tp TemplateProcessor) Process(rl *ResourceList) error {
if err := LoadFunctionConfig(rl.FunctionConfig, tp.TemplateData); err != nil {
if err := LoadFunctionConfig(rl.FunctionConfig, tp.TemplateData, false); err != nil {
return errors.Wrap(err)
}
return errors.Wrap(rl.Filter(tp))
Expand Down
45 changes: 44 additions & 1 deletion kyaml/fn/framework/processors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,7 @@ func TestLoadFunctionConfig(t *testing.T) {
name string
src *yaml.RNode
api interface{}
strict bool
want interface{}
wantErrMsgs []string
}{
Expand Down Expand Up @@ -825,6 +826,16 @@ test: true
api: &jsonTagTest{},
want: &jsonTagTest{Name: "tester", Test: true},
},
{
name: "successfully loads types that include fields only tagged with json markers, strict mode",
src: yaml.MustParse(`
name: tester
test: true
`),
api: &jsonTagTest{},
strict: true,
want: &jsonTagTest{Name: "tester", Test: true},
},
{
name: "successfully loads types that include fields only tagged with yaml markers",
src: yaml.MustParse(`
Expand All @@ -834,11 +845,43 @@ test: true
api: &yamlTagTest{},
want: &yamlTagTest{Name: "tester", Test: true},
},
{
name: "successfully loads types that include fields only tagged with yaml markers, strict mode",
src: yaml.MustParse(`
name: tester
test: true
`),
api: &yamlTagTest{},
strict: true,
want: &yamlTagTest{Name: "tester", Test: true},
},
{
name: "strict mode fails on unknown fields on types with yaml tags",
src: yaml.MustParse(`
name: tester
test: true
unknown: field
`),
api: &yamlTagTest{},
strict: true,
wantErrMsgs: []string{`error unmarshaling JSON: while decoding JSON: json: unknown field "unknown"`},
},
{
name: "strict mode fails on unknown fields on types with json tags",
src: yaml.MustParse(`
name: tester
test: true
unknown: field
`),
api: &jsonTagTest{},
strict: true,
wantErrMsgs: []string{`error unmarshaling JSON: while decoding JSON: json: unknown field "unknown"`},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := framework.LoadFunctionConfig(tt.src, tt.api)
err := framework.LoadFunctionConfig(tt.src, tt.api, tt.strict)
if len(tt.wantErrMsgs) == 0 {
require.NoError(t, err)
require.Equal(t, tt.want, tt.api)
Expand Down