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 check that kustomization is empty #4949

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ func (kt *KustTarget) Load() error {

k.FixKustomization()

// check that Kustomization is empty
if err := k.CheckEmpty(); err != nil {
return err
}

errs := k.EnforceFields()
if len(errs) > 0 {
return fmt.Errorf(
Expand Down
1 change: 1 addition & 0 deletions api/internal/target/kusttarget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestLoad(t *testing.T) {
k: types.Kustomization{
TypeMeta: expectedTypeMeta,
},
errContains: "kustomization.yaml is empty",
},
"nonsenseLatin": {
errContains: "found a tab character that violates indentation",
Expand Down
15 changes: 13 additions & 2 deletions api/krusty/directoryarrangement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,20 @@ import (
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)

const expectedResources = `apiVersion: v1
kind: Service
metadata:
name: myService
spec:
ports:
- port: 7002
`

func TestIssue596AllowDirectoriesThatAreSubstringsOfEachOther(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK("base", "")
th.WriteF("base/service.yaml", expectedResources)
th.WriteK("base", `resources:
- service.yaml`)
th.WriteK("overlays/aws", `
resources:
- ../../base
Expand All @@ -25,5 +36,5 @@ resources:
- ../aws-nonprod
`)
m := th.Run("overlays/aws-sandbox2.us-east-1", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, "")
th.AssertActualEqualsExpected(m, expectedResources)
}
15 changes: 15 additions & 0 deletions api/types/kustomization.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"reflect"

"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/filesys"
Expand Down Expand Up @@ -298,6 +299,20 @@ func (k *Kustomization) FixKustomizationPreMarshalling(fSys filesys.FileSystem)
return nil
}

func (k *Kustomization) CheckEmpty() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I right to think that we want to throw an error on any file that does not contain any transformers? The code below effectively excludes files that specifically contain only Kustomization type metadata, but I suspect it would actually allow fully empty files or files with partial type meta (which is currently allowed, for better or for worse). I suggest changing the tests to be against raw yaml input rather than struct input to demonstrate the various cases more thoroughly. I also suggest trying the opposite approach for the comparison: deep copy the input Kustomization, clear its type meta, and then compare it to a completely empty object (in object form, not as yaml). LMK what you think of that approach.

Copy link
Member Author

@koba1t koba1t Apr 5, 2023

Choose a reason for hiding this comment

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

Thanks for your advice!
I agree with your opinions.

I fixed these codes.

// generate empty Kustomization
emptyKustomization := &Kustomization{}

// k.TypeMeta is metadata. It Isn't related to whether empty or not.
emptyKustomization.TypeMeta = k.TypeMeta

if reflect.DeepEqual(k, emptyKustomization) {
return fmt.Errorf("kustomization.yaml is empty")
}

return nil
}

func (k *Kustomization) EnforceFields() []string {
var errs []string
if k.Kind != "" && k.Kind != KustomizationKind && k.Kind != ComponentKind {
Expand Down
66 changes: 58 additions & 8 deletions api/types/kustomization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,64 @@ unknown: foo`)
}
}

func TestUnmarshal_InvalidYaml(t *testing.T) {
y := []byte(`
apiVersion: kustomize.config.k8s.io/v1beta1
func TestUnmarshal_Failed(t *testing.T) {
tests := []struct {
name string
kustomizationYamls []byte
errMsg string
}{
{
name: "invalid yaml",
kustomizationYamls: []byte(`apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
unknown`)
var k Kustomization
err := k.Unmarshal(y)
if err == nil {
t.Fatalf("expect an error")
unknown`),
errMsg: "invalid Kustomization: yaml: line 4: could not find expected ':'",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var k Kustomization
if err := k.Unmarshal(tt.kustomizationYamls); err == nil || err.Error() != tt.errMsg {
t.Errorf("Kustomization.Unmarshal() error = %v, wantErr %v", err, tt.errMsg)
}
})
}
}

func TestKustomization_CheckEmpty(t *testing.T) {
tests := []struct {
name string
kustomization *Kustomization
wantErr bool
}{
{
name: "empty kustomization.yaml",
kustomization: &Kustomization{},
wantErr: true,
},
{
name: "empty kustomization.yaml",
kustomization: &Kustomization{
TypeMeta: TypeMeta{
Kind: KustomizationKind,
APIVersion: KustomizationVersion,
},
},
wantErr: true,
},
{
name: "non empty kustomization.yaml",
kustomization: &Kustomization{Resources: []string{"res"}},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k := tt.kustomization
k.FixKustomization()
if err := k.CheckEmpty(); (err != nil) != tt.wantErr {
t.Errorf("Kustomization.CheckEmpty() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}