From 865169971252adc911cf703f54b3220ff7fbdcc2 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Thu, 14 Mar 2019 14:15:54 -0700 Subject: [PATCH 1/2] clean paths before comparison --- pkg/patch/patcher.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/patch/patcher.go b/pkg/patch/patcher.go index 412895078..318636995 100644 --- a/pkg/patch/patcher.go +++ b/pkg/patch/patcher.go @@ -2,6 +2,7 @@ package patch import ( "encoding/json" + "fmt" "os" "path" "path/filepath" @@ -232,7 +233,7 @@ func (p *ShipPatcher) writeTempKustomization(step api.Kustomize, resource string return errors.Wrap(err, "failed to get relative path") } - if targetPath == resource { + if filepath.Clean(targetPath) == filepath.Clean(resource) { tempBaseKustomization.Resources = append(tempBaseKustomization.Resources, relativePath) } return nil @@ -243,7 +244,7 @@ func (p *ShipPatcher) writeTempKustomization(step api.Kustomize, resource string if len(tempBaseKustomization.Resources) == 0 { level.Error(p.Logger).Log("event", "unable to find", "resource", resource) - return errors.New("Temp base directory is empty - base resource not found") + return fmt.Errorf("temp base directory is empty - base resource not found in %s", step.Base) } marshalled, err := yaml.Marshal(tempBaseKustomization) From b8722d494944c0c677fbcc83a5e233f6d9abfcca Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Thu, 14 Mar 2019 14:53:01 -0700 Subject: [PATCH 2/2] add test for writeTempKustomziation --- pkg/patch/patcher.go | 2 +- pkg/patch/patcher_test.go | 165 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/pkg/patch/patcher.go b/pkg/patch/patcher.go index 318636995..66c9e538f 100644 --- a/pkg/patch/patcher.go +++ b/pkg/patch/patcher.go @@ -244,7 +244,7 @@ func (p *ShipPatcher) writeTempKustomization(step api.Kustomize, resource string if len(tempBaseKustomization.Resources) == 0 { level.Error(p.Logger).Log("event", "unable to find", "resource", resource) - return fmt.Errorf("temp base directory is empty - base resource not found in %s", step.Base) + return fmt.Errorf("temp base directory is empty - base resource %s not found in %s", resource, step.Base) } marshalled, err := yaml.Marshal(tempBaseKustomization) diff --git a/pkg/patch/patcher_test.go b/pkg/patch/patcher_test.go index 7adc4838d..cc2c3bb6d 100644 --- a/pkg/patch/patcher_test.go +++ b/pkg/patch/patcher_test.go @@ -6,11 +6,15 @@ import ( "path" "testing" + "github.com/ghodss/yaml" + "github.com/go-kit/kit/log" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/replicatedhq/ship/pkg/api" "github.com/replicatedhq/ship/pkg/testing/logger" "github.com/spf13/afero" + "github.com/stretchr/testify/require" + k8stypes "sigs.k8s.io/kustomize/pkg/types" ) func TestShipPatcher(t *testing.T) { @@ -140,3 +144,164 @@ var _ = Describe("ShipPatcher", func() { }) }) }) + +func TestShipPatcher_writeTempKustomization(t *testing.T) { + type testFile struct { + path string + contents string + } + tests := []struct { + name string + step api.Kustomize + resource string + testFiles []testFile + expectKustomization k8stypes.Kustomization + expectErr bool + }{ + { + name: "no matching resource", + step: api.Kustomize{Base: "base/"}, + resource: "./base/file.yaml", + testFiles: []testFile{ + { + path: "./base/strawberry.yaml", + contents: `apiVersion: apps/v1beta2 +kind: Deployment +metadata: + labels: + app: strawberry + heritage: Tiller + chart: strawberry-1.0.0 + name: strawberry`, + }, + }, + expectErr: true, + }, + { + name: "matching resource", + step: api.Kustomize{Base: "base/"}, + resource: "./base/strawberry.yaml", + testFiles: []testFile{ + { + path: "base/strawberry.yaml", + contents: `apiVersion: apps/v1beta2 +kind: Deployment +metadata: + labels: + app: strawberry + heritage: Tiller + chart: strawberry-1.0.0 + name: strawberry`, + }, + }, + expectErr: false, + expectKustomization: k8stypes.Kustomization{ + Resources: []string{"strawberry.yaml"}, + }, + }, + { + name: "matching resource, unclean path", + step: api.Kustomize{Base: "base/"}, + resource: "./base/strawberry.yaml", + testFiles: []testFile{ + { + path: "./base/strawberry.yaml", + contents: `apiVersion: apps/v1beta2 +kind: Deployment +metadata: + labels: + app: strawberry + heritage: Tiller + chart: strawberry-1.0.0 + name: strawberry`, + }, + }, + expectErr: false, + expectKustomization: k8stypes.Kustomization{ + Resources: []string{"strawberry.yaml"}, + }, + }, + { + name: "matching resource, unclean path in subdir", + step: api.Kustomize{Base: "base/"}, + resource: "./base/flowers/rose.yml", + testFiles: []testFile{ + { + path: "./base/strawberry.yaml", + contents: `apiVersion: apps/v1beta2 +kind: Deployment +metadata: + labels: + app: strawberry + heritage: Tiller + chart: strawberry-1.0.0 + name: strawberry`, + }, + { + path: "./base/flowers/rose.yml", + contents: `apiVersion: v1 +kind: Service +metadata: + labels: + app: rose + name: rose`, + }, + }, + expectErr: false, + expectKustomization: k8stypes.Kustomization{ + Resources: []string{"flowers/rose.yml"}, + }, + }, + { + name: "alternate base", + step: api.Kustomize{Base: "another/base/path/"}, + resource: "another/base/path/raspberry.yaml", + testFiles: []testFile{ + { + path: "another/base/path/raspberry.yaml", + contents: `apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: raspberry + name: raspberry`, + }, + }, + expectErr: false, + expectKustomization: k8stypes.Kustomization{ + Resources: []string{"raspberry.yaml"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := require.New(t) + + mockFs := afero.Afero{Fs: afero.NewMemMapFs()} + for _, testFile := range tt.testFiles { + err := mockFs.WriteFile(testFile.path, []byte(testFile.contents), 0755) + req.NoError(err) + } + p := &ShipPatcher{ + Logger: log.NewNopLogger(), + FS: mockFs, + } + + err := p.writeTempKustomization(tt.step, tt.resource) + + if !tt.expectErr { + req.NoError(err) + + kustomizationB, err := mockFs.ReadFile(path.Join(tt.step.Base, "kustomization.yaml")) + req.NoError(err) + + kustomizationYaml := k8stypes.Kustomization{} + err = yaml.Unmarshal(kustomizationB, &kustomizationYaml) + req.NoError(err) + req.Equal(tt.expectKustomization, kustomizationYaml) + } else { + req.Error(err) + } + }) + } +}