From 3b8c5ee96d86ee5807cfc994076619b3331142a6 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Thu, 18 Apr 2019 19:25:06 -0700 Subject: [PATCH] Add load_restrictor flag. --- go.mod | 2 +- pkg/commands/build/build.go | 15 ++++++---- pkg/internal/loadertest/fakeloader.go | 14 +++++++-- pkg/loader/fileloader_test.go | 2 +- pkg/loader/loader.go | 9 +++--- pkg/loader/loadrestrictions.go | 41 ++++++++++++++++++++++++++ pkg/loader/loadrestrictions_string.go | 25 ++++++++++++++++ pkg/target/baseandoverlaysmall_test.go | 40 ++++++++++++++++++++++++- pkg/target/kusttestharness_test.go | 9 +++++- 9 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 pkg/loader/loadrestrictions_string.go diff --git a/go.mod b/go.mod index dd6a43102f..da97c1e5b0 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/onsi/gomega v1.5.0 // indirect github.com/pkg/errors v0.8.0 github.com/spf13/cobra v0.0.2 - github.com/spf13/pflag v1.0.1 // indirect + github.com/spf13/pflag v1.0.1 github.com/stretchr/testify v1.3.0 // indirect golang.org/x/net v0.0.0-20190225153610-fe579d43d832 // indirect golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6 // indirect diff --git a/pkg/commands/build/build.go b/pkg/commands/build/build.go index b8cc1254dd..ed77fbb262 100644 --- a/pkg/commands/build/build.go +++ b/pkg/commands/build/build.go @@ -38,6 +38,7 @@ import ( type Options struct { kustomizationPath string outputPath string + loadRestrictor loader.LoadRestrictorFunc } // NewOptions creates a Options object @@ -45,6 +46,7 @@ func NewOptions(p, o string) *Options { return &Options{ kustomizationPath: p, outputPath: o, + loadRestrictor: loader.RestrictionRootOnly, } } @@ -88,13 +90,14 @@ func NewCmdBuild( &o.outputPath, "output", "o", "", "If specified, write the build output to this path.") + loader.AddLoadRestrictionsFlag(cmd.Flags()) cmd.AddCommand(NewCmdBuildPrune(out, fs, rf, ptf, pc)) return cmd } // Validate validates build command. -func (o *Options) Validate(args []string) error { +func (o *Options) Validate(args []string) (err error) { if len(args) > 1 { return errors.New( "specify one path to " + pgmconfig.KustomizationFileNames[0]) @@ -104,8 +107,8 @@ func (o *Options) Validate(args []string) error { } else { o.kustomizationPath = args[0] } - - return nil + o.loadRestrictor, err = loader.ValidateLoadRestrictorFlag() + return } // RunBuild runs build command. @@ -113,7 +116,8 @@ func (o *Options) RunBuild( out io.Writer, fSys fs.FileSystem, rf *resmap.Factory, ptf transformer.Factory, pc *types.PluginConfig) error { - ldr, err := loader.NewLoader(o.kustomizationPath, fSys) + ldr, err := loader.NewLoader( + o.loadRestrictor, o.kustomizationPath, fSys) if err != nil { return err } @@ -133,7 +137,8 @@ func (o *Options) RunBuildPrune( out io.Writer, fSys fs.FileSystem, rf *resmap.Factory, ptf transformer.Factory, pc *types.PluginConfig) error { - ldr, err := loader.NewLoader(o.kustomizationPath, fSys) + ldr, err := loader.NewLoader( + o.loadRestrictor, o.kustomizationPath, fSys) if err != nil { return err } diff --git a/pkg/internal/loadertest/fakeloader.go b/pkg/internal/loadertest/fakeloader.go index c882c239e8..df5a03cc66 100644 --- a/pkg/internal/loadertest/fakeloader.go +++ b/pkg/internal/loadertest/fakeloader.go @@ -31,12 +31,22 @@ type FakeLoader struct { } // NewFakeLoader returns a Loader that uses a fake filesystem. -// The argument should be an absolute file path. +// The loader will be restricted to root only. +// The initialDir argument should be an absolute file path. func NewFakeLoader(initialDir string) FakeLoader { + return NewFakeLoaderWithRestrictor( + loader.RestrictionRootOnly, initialDir) +} + +// NewFakeLoaderWithRestrictor returns a Loader that +// uses a fake filesystem. +// The initialDir argument should be an absolute file path. +func NewFakeLoaderWithRestrictor( + lr loader.LoadRestrictorFunc, initialDir string) FakeLoader { // Create fake filesystem and inject it into initial Loader. fSys := fs.MakeFakeFS() fSys.Mkdir(initialDir) - ldr, err := loader.NewLoader(initialDir, fSys) + ldr, err := loader.NewLoader(lr, initialDir, fSys) if err != nil { log.Fatalf("Unable to make loader: %v", err) } diff --git a/pkg/loader/fileloader_test.go b/pkg/loader/fileloader_test.go index ed504de0d0..b0f8c81cb5 100644 --- a/pkg/loader/fileloader_test.go +++ b/pkg/loader/fileloader_test.go @@ -288,7 +288,7 @@ func doSanityChecksAndDropIntoBase( return l } -func TestRestrictionRootInRealLoader(t *testing.T) { +func TestRestrictionRootOnlyInRealLoader(t *testing.T) { dir, fSys, err := commonSetupForLoaderRestrictionTest() if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go index d930323eae..5bc23a58b7 100644 --- a/pkg/loader/loader.go +++ b/pkg/loader/loader.go @@ -26,10 +26,11 @@ import ( // NewLoader returns a Loader pointed at the given target. // If the target is remote, the loader will be restricted // to the root and below only. If the target is local, the -// loader will have no restrictions. If the local target -// attempts to transitively load remote bases, they will all -// be root-only restricted. +// loader will have the restrictions passed in. Regardless, +// if a local target attempts to transitively load remote bases, +// the remote bases will all be root-only restricted. func NewLoader( + lr LoadRestrictorFunc, target string, fSys fs.FileSystem) (ifc.Loader, error) { repoSpec, err := git.NewRepoSpecFromUrl(target) if err == nil { @@ -42,5 +43,5 @@ func NewLoader( return nil, err } return newLoaderAtConfirmedDir( - RestrictionNone, root, fSys, nil, git.ClonerUsingGitExec), nil + lr, root, fSys, nil, git.ClonerUsingGitExec), nil } diff --git a/pkg/loader/loadrestrictions.go b/pkg/loader/loadrestrictions.go index 135cd8afc0..33fed8d244 100644 --- a/pkg/loader/loadrestrictions.go +++ b/pkg/loader/loadrestrictions.go @@ -19,9 +19,50 @@ package loader import ( "fmt" + "github.com/spf13/pflag" "sigs.k8s.io/kustomize/pkg/fs" ) +//go:generate stringer -type=loadRestrictions +type loadRestrictions int + +const ( + unknown loadRestrictions = iota + rootOnly + none +) + +const ( + flagName = "load_restrictor" +) + +var ( + flagValue = rootOnly.String() + flagHelp = "if set to '" + none.String() + + "', local kustomizations may load files from outside their root. " + + "This does, however, break the relocatability of the kustomization." +) + +func AddLoadRestrictionsFlag(set *pflag.FlagSet) { + set.StringVar( + &flagValue, flagName, + rootOnly.String(), flagHelp) +} + +func ValidateLoadRestrictorFlag() (LoadRestrictorFunc, error) { + switch flagValue { + case rootOnly.String(): + return RestrictionRootOnly, nil + case none.String(): + return RestrictionNone, nil + default: + return nil, fmt.Errorf( + "illegal flag value --%s %s; legal values: %v", + flagName, flagValue, + []string{rootOnly.String(), none.String()}) + } +} + type LoadRestrictorFunc func( fs.FileSystem, fs.ConfirmedDir, string) (string, error) diff --git a/pkg/loader/loadrestrictions_string.go b/pkg/loader/loadrestrictions_string.go new file mode 100644 index 0000000000..126ce36139 --- /dev/null +++ b/pkg/loader/loadrestrictions_string.go @@ -0,0 +1,25 @@ +// Code generated by "stringer -type=loadRestrictions"; DO NOT EDIT. + +package loader + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[unknown-0] + _ = x[rootOnly-1] + _ = x[none-2] +} + +const _loadRestrictions_name = "unknownrootOnlynone" + +var _loadRestrictions_index = [...]uint8{0, 7, 15, 19} + +func (i loadRestrictions) String() string { + if i < 0 || i >= loadRestrictions(len(_loadRestrictions_index)-1) { + return "loadRestrictions(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _loadRestrictions_name[_loadRestrictions_index[i]:_loadRestrictions_index[i+1]] +} diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index c9c1a004a2..5743f4adf4 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -17,7 +17,11 @@ limitations under the License. package target_test import ( + "strings" "testing" + + "sigs.k8s.io/kustomize/k8sdeps/kv/plugin" + "sigs.k8s.io/kustomize/pkg/loader" ) func writeSmallBase(th *KustTestHarness) { @@ -181,8 +185,42 @@ spec: `) } +func TestSharedPatchDisAllowed(t *testing.T) { + th := NewKustTestHarnessFull( + t, "/app/overlay", + loader.RestrictionRootOnly, plugin.DefaultPluginConfig()) + writeSmallBase(th) + th.writeK("/app/overlay", ` +commonLabels: + env: prod +bases: +- ../base +patchesStrategicMerge: +- ../shared/deployment-patch.yaml +`) + th.writeF("/app/shared/deployment-patch.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: myDeployment +spec: + replicas: 1000 +`) + _, err := th.makeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains( + err.Error(), + "security; file '/app/shared/deployment-patch.yaml' is not in or below '/app/overlay'") { + t.Fatalf("unexpected error: %s", err) + } +} + func TestSharedPatchAllowed(t *testing.T) { - th := NewKustTestHarness(t, "/app/overlay") + th := NewKustTestHarnessFull( + t, "/app/overlay", + loader.RestrictionNone, plugin.DefaultPluginConfig()) writeSmallBase(th) th.writeK("/app/overlay", ` commonLabels: diff --git a/pkg/target/kusttestharness_test.go b/pkg/target/kusttestharness_test.go index d7a760d88e..e60d193ecf 100644 --- a/pkg/target/kusttestharness_test.go +++ b/pkg/target/kusttestharness_test.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/kustomize/k8sdeps/kv/plugin" "sigs.k8s.io/kustomize/k8sdeps/transformer" "sigs.k8s.io/kustomize/pkg/internal/loadertest" + "sigs.k8s.io/kustomize/pkg/loader" "sigs.k8s.io/kustomize/pkg/pgmconfig" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" @@ -51,12 +52,18 @@ func NewKustTestHarness(t *testing.T, path string) *KustTestHarness { func NewKustTestHarnessWithPluginConfig( t *testing.T, path string, pc *types.PluginConfig) *KustTestHarness { + return NewKustTestHarnessFull(t, path, loader.RestrictionRootOnly, pc) +} + +func NewKustTestHarnessFull( + t *testing.T, path string, + lr loader.LoadRestrictorFunc, pc *types.PluginConfig) *KustTestHarness { return &KustTestHarness{ t: t, rf: resmap.NewFactory(resource.NewFactory( kunstruct.NewKunstructuredFactoryWithGeneratorArgs( &types.GeneratorMetaArgs{PluginConfig: pc}))), - ldr: loadertest.NewFakeLoader(path), + ldr: loadertest.NewFakeLoaderWithRestrictor(lr, path), pc: pc} }