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

use Strict unmarshal when read TransformerConfig #5550

Merged
merged 1 commit into from
Feb 22, 2024
Merged

use Strict unmarshal when read TransformerConfig #5550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion api/internal/plugins/builtinconfig/loaddefaultconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func loadDefaultConfig(
// makeTransformerConfigFromBytes returns a TransformerConfig object from bytes
func makeTransformerConfigFromBytes(data []byte) (*TransformerConfig, error) {
var t TransformerConfig
err := yaml.Unmarshal(data, &t)
err := yaml.UnmarshalStrict(data, &t)
if err != nil {
return nil, err
}
Expand Down
53 changes: 53 additions & 0 deletions api/internal/plugins/builtinconfig/loaddefaultconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package builtinconfig

import (
"reflect"
"strings"
"testing"

"sigs.k8s.io/kustomize/api/internal/loader"
Expand Down Expand Up @@ -44,3 +45,55 @@ namePrefix:
t.Fatalf("expected %v\n but go6t %v\n", expected, tCfg)
}
}

func TestLoadDefaultConfigsFromFilesWithMissingFields(t *testing.T) {
fSys := filesys.MakeFsInMemory()
filePathContainsTypo := "config_contains_typo.yaml"
if err := fSys.WriteFile(filePathContainsTypo, []byte(`
namoPrefix:
- path: nameprefix/path
kind: SomeKind
`)); err != nil {
t.Fatal(err)
}
ldr, err := loader.NewLoader(
loader.RestrictionRootOnly, filesys.Separator, fSys)
if err != nil {
t.Fatal(err)
}
errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
if err == nil {
t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
}
if !strings.Contains(err.Error(), errMsg) {
t.Fatalf("expected error %s, but got %s", errMsg, err)
}
}

// please remove this failing test after implements the labels support
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the comment 🙏

func TestLoadDefaultConfigsFromFilesWithMissingFieldsLabels(t *testing.T) {
fSys := filesys.MakeFsInMemory()
filePathContainsTypo := "config_contains_typo.yaml"
if err := fSys.WriteFile(filePathContainsTypo, []byte(`
labels:
- path: spec/podTemplate/metadata/labels
create: true
kind: FlinkDeployment
`)); err != nil {
t.Fatal(err)
}
ldr, err := loader.NewLoader(
loader.RestrictionRootOnly, filesys.Separator, fSys)
if err != nil {
t.Fatal(err)
}
errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
if err == nil {
t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
}
if !strings.Contains(err.Error(), errMsg) {
t.Fatalf("expected error %s, but got %s", errMsg, err)
}
}
10 changes: 6 additions & 4 deletions api/internal/target/kusttarget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,19 +313,21 @@ configurations:
th.WriteF("/merge-config/name-prefix-rules.yaml", `
namePrefix:
- path: metadata/name
apiVersion: v1
group: apps
version: v1
kind: Deployment
- path: metadata/name
apiVersion: v1
version: v1
kind: Secret
`)
th.WriteF("/merge-config/name-suffix-rules.yaml", `
nameSuffix:
- path: metadata/name
apiVersion: v1
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why we use apiVersion here.
Because we don't allow the use of the apiVersion field, we must use the group and version fields.

kustomize/api/types/fieldspec.go

Line 30 in 3771586

resid.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"`

Copy link
Member

Choose a reason for hiding this comment

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

This is strange indeed. Perhaps a residue from a previous implementation?

version: v1
kind: ConfigMap
- path: metadata/name
apiVersion: v1
group: apps
version: v1
kind: Deployment
`)
th.WriteF("/merge-config/deployment.yaml", `
Expand Down
37 changes: 29 additions & 8 deletions api/krusty/customconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ commonLabels:
vars:
- name: APRIL_DIET
objref:
group: foo
version: v1
kind: Giraffe
name: april
fieldref:
fieldpath: spec.diet
- name: KOKO_DIET
objref:
group: foo
version: v1
kind: Gorilla
name: koko
fieldref:
Expand All @@ -36,13 +40,15 @@ configurations:
- config/custom.yaml
`)
th.WriteF("base/giraffes.yaml", `
apiVersion: foo/v1
kind: Giraffe
metadata:
name: april
spec:
diet: mimosa
location: NE
---
apiVersion: foo/v1
kind: Giraffe
metadata:
name: may
Expand All @@ -51,6 +57,7 @@ spec:
location: SE
`)
th.WriteF("base/gorilla.yaml", `
apiVersion: foo/v1
kind: Gorilla
metadata:
name: koko
Expand All @@ -59,7 +66,7 @@ spec:
location: SW
`)
th.WriteF("base/animalPark.yaml", `
apiVersion: foo
apiVersion: foo/v1
kind: AnimalPark
metadata:
name: sandiego
Expand Down Expand Up @@ -94,7 +101,7 @@ varReference:
`)
m := th.Run("base", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: foo
apiVersion: foo/v1
kind: AnimalPark
metadata:
labels:
Expand All @@ -109,6 +116,7 @@ spec:
gorillaRef:
name: x-koko
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -118,6 +126,7 @@ spec:
diet: mimosa
location: NE
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -127,6 +136,7 @@ spec:
diet: acacia
location: SE
---
apiVersion: foo/v1
kind: Gorilla
metadata:
labels:
Expand Down Expand Up @@ -163,7 +173,7 @@ varReference:
`)
m := th.Run("base", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: foo
apiVersion: foo/v1
kind: AnimalPark
metadata:
labels:
Expand All @@ -178,6 +188,7 @@ spec:
gorillaRef:
name: x-koko
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -187,6 +198,7 @@ spec:
diet: mimosa
location: NE
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -196,6 +208,7 @@ spec:
diet: acacia
location: SE
---
apiVersion: foo/v1
kind: Gorilla
metadata:
labels:
Expand All @@ -215,17 +228,20 @@ func TestFixedBug605_BaseCustomizationAvailableInOverlay(t *testing.T) {
nameReference:
- kind: Gorilla
fieldSpecs:
- apiVersion: foo
- group: foo
version: v1
kind: AnimalPark
path: spec/gorillaRef/name
- kind: Giraffe
fieldSpecs:
- apiVersion: foo
- group: foo
version: v1
kind: AnimalPark
path: spec/giraffeRef/name
varReference:
- path: spec/food
apiVersion: foo
group: foo
version: v1
kind: AnimalPark
`)
th.WriteK("overlay", `
Expand All @@ -239,6 +255,7 @@ resources:
- ursus.yaml
`)
th.WriteF("overlay/ursus.yaml", `
apiVersion: foo/v1
kind: Gorilla
metadata:
name: ursus
Expand All @@ -248,7 +265,7 @@ spec:
`)
// The following replaces the gorillaRef in the AnimalPark.
th.WriteF("overlay/animalPark.yaml", `
apiVersion: foo
apiVersion: foo/v1
kind: AnimalPark
metadata:
name: sandiego
Expand All @@ -258,7 +275,7 @@ spec:
`)
m := th.Run("overlay", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: foo
apiVersion: foo/v1
kind: AnimalPark
metadata:
labels:
Expand All @@ -274,6 +291,7 @@ spec:
gorillaRef:
name: o-ursus
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -284,6 +302,7 @@ spec:
diet: mimosa
location: NE
---
apiVersion: foo/v1
kind: Giraffe
metadata:
labels:
Expand All @@ -294,6 +313,7 @@ spec:
diet: acacia
location: SE
---
apiVersion: foo/v1
kind: Gorilla
metadata:
labels:
Expand All @@ -304,6 +324,7 @@ spec:
diet: bambooshoots
location: SW
---
apiVersion: foo/v1
kind: Gorilla
metadata:
labels:
Expand Down
3 changes: 2 additions & 1 deletion api/krusty/transformersimage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,8 @@ spec:
th.WriteF("base/config/knative.yaml", `
images:
- path: spec/runLatest/configuration/revisionTemplate/spec/container/image
apiVersion: serving.knative.dev/v1alpha1
group: serving.knative.dev
version: v1alpha1
kind: Service
`)
}
Expand Down
8 changes: 5 additions & 3 deletions api/krusty/variableref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ vars:
objref: &config-map-ref
kind: ConfigMap
name: kustomize-vars
apiVersion: v1
version: v1
fieldref:
fieldpath: data.DBT_TARGET
- name: SUSPENDED
Expand Down Expand Up @@ -500,10 +500,12 @@ nameReference:
varReference:
- path: spec/workflowSpec/arguments/parameters/value
kind: CronWorkflow
apiVersion: argoproj.io/v1alpha1
group: argoproj.io
version: v1alpha1
- path: spec
kind: CronWorkflow
apiVersion: argoproj.io/v1alpha1
group: argoproj.io
version: v1alpha1
`)
th.WriteF("vars.env", `
DBT_TARGET=development
Expand Down
Loading