From a45eca7e22ba69a6991047c7f10c9a4b33d2e2f1 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Fri, 1 Nov 2019 17:20:50 -0700 Subject: [PATCH] move load restrictions --- api/go.sum | 1 + api/krusty/kustomizer.go | 3 +- api/krusty/loadrestrictions.go | 68 ------------------- api/krusty/loadrestrictions_string.go | 25 ------- api/krusty/options.go | 4 +- api/pgmconfig/pluginconfig.go | 32 +-------- api/plugins/loader/loader.go | 4 +- api/target/transformerplugin_test.go | 6 +- api/types/erronlybuiltinpluginsallowed.go | 32 +++++++++ api/types/loadrestrictions.go | 24 +++++++ api/types/loadrestrictions_string.go | 25 +++++++ api/types/pluginconfig.go | 5 +- api/types/pluginrestrictions.go | 28 ++++++++ api/types/pluginrestrictions_string.go | 25 +++++++ kustomize/internal/commands/build/build.go | 16 +++-- .../commands/build/flagenableplugins.go | 29 ++++++++ .../commands/build/flagloadrestrictor.go | 51 ++++++++++++++ travis/pre-commit.sh | 4 ++ 18 files changed, 243 insertions(+), 139 deletions(-) delete mode 100644 api/krusty/loadrestrictions.go delete mode 100644 api/krusty/loadrestrictions_string.go create mode 100644 api/types/erronlybuiltinpluginsallowed.go create mode 100644 api/types/loadrestrictions.go create mode 100644 api/types/loadrestrictions_string.go create mode 100644 api/types/pluginrestrictions.go create mode 100644 api/types/pluginrestrictions_string.go create mode 100644 kustomize/internal/commands/build/flagenableplugins.go create mode 100644 kustomize/internal/commands/build/flagloadrestrictor.go diff --git a/api/go.sum b/api/go.sum index 05066c163..33756e512 100644 --- a/api/go.sum +++ b/api/go.sum @@ -128,6 +128,7 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20181011042414-1f849cf54d09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20181030221726-6c7e314b6563/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59 h1:QjA/9ArTfVTLfEhClDCG7SGrZkZixxWpwNCDiwJfh88= golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index b6bcdf3ca..c8b69aaa6 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/target" + "sigs.k8s.io/kustomize/api/types" ) // Kustomizer performs kustomizations. It's meant to behave @@ -55,7 +56,7 @@ func (b *Kustomizer) Run(path string) (resmap.ResMap, error) { kunstruct.NewKunstructuredFactoryImpl()), pf) lr := fLdr.RestrictionNone - if b.options.LoadRestrictions == rootOnly { + if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly { lr = fLdr.RestrictionRootOnly } ldr, err := fLdr.NewLoader(lr, path, b.fSys) diff --git a/api/krusty/loadrestrictions.go b/api/krusty/loadrestrictions.go deleted file mode 100644 index 8312a9407..000000000 --- a/api/krusty/loadrestrictions.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -package krusty - -import ( - "fmt" - "github.com/spf13/pflag" -) - -//go:generate stringer -type=loadRestrictions -type loadRestrictions int - -const ( - unknown loadRestrictions = iota - - // With this restriction, the files referenced by a - // kustomization file must be in or under the directory - // holding the kustomization file itself. - rootOnly - - // The kustomization file may specify absolute or - // relative paths to patch or resources files outside - // its own tree. - 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 AddFlagLoadRestrictor(set *pflag.FlagSet) { - set.StringVar( - &flagValue, flagName, - rootOnly.String(), flagHelp) -} - -func ValidateFlagLoadRestrictor() error { - switch flagValue { - case rootOnly.String(): - return nil - case none.String(): - return nil - default: - return fmt.Errorf( - "illegal flag value --%s %s; legal values: %v", - flagName, flagValue, - []string{rootOnly.String(), none.String()}) - } -} - -func GetFlagLoadRestrictorValue() loadRestrictions { - switch flagValue { - case rootOnly.String(): - return rootOnly - case none.String(): - return none - default: - return unknown - } -} diff --git a/api/krusty/loadrestrictions_string.go b/api/krusty/loadrestrictions_string.go deleted file mode 100644 index 1f1d099ae..000000000 --- a/api/krusty/loadrestrictions_string.go +++ /dev/null @@ -1,25 +0,0 @@ -// Code generated by "stringer -type=loadRestrictions"; DO NOT EDIT. - -package krusty - -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/api/krusty/options.go b/api/krusty/options.go index ebea73ff1..d6db3d7b4 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -20,7 +20,7 @@ type Options struct { // Restrictions on what can be loaded from the file system. // See type definition. - LoadRestrictions loadRestrictions + LoadRestrictions types.LoadRestrictions // Create an inventory object for pruning. DoPrune bool @@ -33,7 +33,7 @@ type Options struct { func MakeDefaultOptions() *Options { return &Options{ DoLegacyResourceSort: true, - LoadRestrictions: rootOnly, + LoadRestrictions: types.LoadRestrictionsRootOnly, DoPrune: false, PluginConfig: pgmconfig.DefaultPluginConfig(), } diff --git a/api/pgmconfig/pluginconfig.go b/api/pgmconfig/pluginconfig.go index fd7f211b0..8abfdc386 100644 --- a/api/pgmconfig/pluginconfig.go +++ b/api/pgmconfig/pluginconfig.go @@ -4,12 +4,10 @@ package pgmconfig import ( - "fmt" "os" "path/filepath" "runtime" - "github.com/spf13/pflag" "sigs.k8s.io/kustomize/api/types" ) @@ -31,45 +29,21 @@ const ( // Name of directory housing all plugins. PluginRoot = "plugin" - - flagEnablePluginsName = "enable_alpha_plugins" - flagEnablePluginsHelp = `enable plugins, an alpha feature. -See https://github.com/kubernetes-sigs/kustomize/blob/master/docs/plugins/README.md -` - flagErrorFmt = ` -unable to load external plugin %s because plugins disabled -specify the flag - --%s -to %s` ) func ActivePluginConfig() *types.PluginConfig { pc := DefaultPluginConfig() - pc.Enabled = true + pc.PluginRestrictions = types.PluginRestrictionsNone return pc } func DefaultPluginConfig() *types.PluginConfig { return &types.PluginConfig{ - Enabled: false, - DirectoryPath: filepath.Join(configRoot(), PluginRoot), + PluginRestrictions: types.PluginRestrictionsBuiltinsOnly, + DirectoryPath: filepath.Join(configRoot(), PluginRoot), } } -func NotEnabledErr(name string) error { - return fmt.Errorf( - flagErrorFmt, - name, - flagEnablePluginsName, - flagEnablePluginsHelp) -} - -func AddFlagEnablePlugins(set *pflag.FlagSet, v *bool) { - set.BoolVar( - v, flagEnablePluginsName, - false, flagEnablePluginsHelp) -} - // Use https://github.com/kirsle/configdir instead? func configRoot() string { dir := os.Getenv(XdgConfigHomeEnv) diff --git a/api/plugins/loader/loader.go b/api/plugins/loader/loader.go index 7fb214618..2c1ccc697 100644 --- a/api/plugins/loader/loader.go +++ b/api/plugins/loader/loader.go @@ -114,10 +114,10 @@ func (l *Loader) loadAndConfigurePlugin( // function (see "pluginator"). Being able to do this // is what makes a plugin "builtin". c, err = l.makeBuiltinPlugin(res.GetGvk()) - } else if l.pc.Enabled { + } else if l.pc.PluginRestrictions == types.PluginRestrictionsNone { c, err = l.loadPlugin(res.OrgId()) } else { - err = pgmconfig.NotEnabledErr(res.OrgId().Kind) + err = types.NewErrOnlyBuiltinPluginsAllowed(res.OrgId().Kind) } if err != nil { return nil, err diff --git a/api/target/transformerplugin_test.go b/api/target/transformerplugin_test.go index 98c6cfd91..421fe0fe0 100644 --- a/api/target/transformerplugin_test.go +++ b/api/target/transformerplugin_test.go @@ -4,10 +4,10 @@ package target_test import ( - "strings" "testing" - "sigs.k8s.io/kustomize/api/testutils/kusttest" + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" + "sigs.k8s.io/kustomize/api/types" ) func writeDeployment(th *kusttest_test.KustTestHarness, path string) { @@ -112,7 +112,7 @@ transformers: if err == nil { t.Fatalf("expected error") } - if !strings.Contains(err.Error(), "unable to load external plugin StringPrefixer") { + if !types.IsErrOnlyBuiltinPluginsAllowed(err) { t.Fatalf("unexpected err: %v", err) } } diff --git a/api/types/erronlybuiltinpluginsallowed.go b/api/types/erronlybuiltinpluginsallowed.go new file mode 100644 index 000000000..41e0488b0 --- /dev/null +++ b/api/types/erronlybuiltinpluginsallowed.go @@ -0,0 +1,32 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "fmt" + "github.com/pkg/errors" +) + +type errOnlyBuiltinPluginsAllowed struct { + name string +} + +func (e *errOnlyBuiltinPluginsAllowed) Error() string { + return fmt.Sprintf( + "external plugins disabled; unable to load external plugin '%s'", + e.name) +} + +func NewErrOnlyBuiltinPluginsAllowed(n string) *errOnlyBuiltinPluginsAllowed { + return &errOnlyBuiltinPluginsAllowed{name: n} +} + +func IsErrOnlyBuiltinPluginsAllowed(err error) bool { + _, ok := err.(*errOnlyBuiltinPluginsAllowed) + if ok { + return true + } + _, ok = errors.Cause(err).(*errOnlyBuiltinPluginsAllowed) + return ok +} diff --git a/api/types/loadrestrictions.go b/api/types/loadrestrictions.go new file mode 100644 index 000000000..6617abdac --- /dev/null +++ b/api/types/loadrestrictions.go @@ -0,0 +1,24 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package types + +// Restrictions on what things can be referred to +// in a kustomization file. +// +//go:generate stringer -type=LoadRestrictions +type LoadRestrictions int + +const ( + LoadRestrictionsUnknown LoadRestrictions = iota + + // Files referenced by a kustomization file must be in + // or under the directory holding the kustomization + // file itself. + LoadRestrictionsRootOnly + + // The kustomization file may specify absolute or + // relative paths to patch or resources files outside + // its own tree. + LoadRestrictionsNone +) diff --git a/api/types/loadrestrictions_string.go b/api/types/loadrestrictions_string.go new file mode 100644 index 000000000..d2355950b --- /dev/null +++ b/api/types/loadrestrictions_string.go @@ -0,0 +1,25 @@ +// Code generated by "stringer -type=LoadRestrictions"; DO NOT EDIT. + +package types + +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[LoadRestrictionsUnknown-0] + _ = x[LoadRestrictionsRootOnly-1] + _ = x[LoadRestrictionsNone-2] +} + +const _LoadRestrictions_name = "LoadRestrictionsUnknownLoadRestrictionsRootOnlyLoadRestrictionsNone" + +var _LoadRestrictions_index = [...]uint8{0, 23, 47, 67} + +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/api/types/pluginconfig.go b/api/types/pluginconfig.go index 46327a0d3..f28b97027 100644 --- a/api/types/pluginconfig.go +++ b/api/types/pluginconfig.go @@ -11,6 +11,7 @@ type PluginConfig struct { // further categorizing plugins. DirectoryPath string - // Enabled is true if plugins are enabled. - Enabled bool + // PluginRestrictions defines the plugin restriction state. + // See type for more information. + PluginRestrictions PluginRestrictions } diff --git a/api/types/pluginrestrictions.go b/api/types/pluginrestrictions.go new file mode 100644 index 000000000..166086757 --- /dev/null +++ b/api/types/pluginrestrictions.go @@ -0,0 +1,28 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package types + +// Some plugin classes +// - builtin: plugins defined in the kustomize repo. +// May be freely used and re-configured. +// - local: plugins that aren't builtin but are +// locally defined (presumably by the user), meaning +// the kustomization refers to them via a relative +// file path, not a URL. +// - remote: require a build-time download to obtain. +// Unadvised, unless one controls the +// serving site. +// +//go:generate stringer -type=PluginRestrictions +type PluginRestrictions int + +const ( + PluginRestrictionsUnknown PluginRestrictions = iota + + // Non-builtin plugins completely disabled. + PluginRestrictionsBuiltinsOnly + + // No restrictions, do whatever you want. + PluginRestrictionsNone +) diff --git a/api/types/pluginrestrictions_string.go b/api/types/pluginrestrictions_string.go new file mode 100644 index 000000000..b9dba7dfc --- /dev/null +++ b/api/types/pluginrestrictions_string.go @@ -0,0 +1,25 @@ +// Code generated by "stringer -type=PluginRestrictions"; DO NOT EDIT. + +package types + +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[PluginRestrictionsUnknown-0] + _ = x[PluginRestrictionsBuiltinsOnly-1] + _ = x[PluginRestrictionsNone-2] +} + +const _PluginRestrictions_name = "PluginRestrictionsUnknownPluginRestrictionsBuiltinsOnlyPluginRestrictionsNone" + +var _PluginRestrictions_index = [...]uint8{0, 25, 55, 77} + +func (i PluginRestrictions) String() string { + if i < 0 || i >= PluginRestrictions(len(_PluginRestrictions_index)-1) { + return "PluginRestrictions(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _PluginRestrictions_name[_PluginRestrictions_index[i]:_PluginRestrictions_index[i+1]] +} diff --git a/kustomize/internal/commands/build/build.go b/kustomize/internal/commands/build/build.go index d8c185abc..538527852 100644 --- a/kustomize/internal/commands/build/build.go +++ b/kustomize/internal/commands/build/build.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/kustomize/api/pgmconfig" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/yaml" ) @@ -23,7 +24,6 @@ type Options struct { kustomizationPath string outputPath string outOrder reorderOutput - pluginsEnabled bool } // NewOptions creates a Options object @@ -73,9 +73,8 @@ func NewCmdBuild(out io.Writer) *cobra.Command { &o.outputPath, "output", "o", "", "If specified, write the build output to this path.") - krusty.AddFlagLoadRestrictor(cmd.Flags()) - pgmconfig.AddFlagEnablePlugins( - cmd.Flags(), &o.pluginsEnabled) + addFlagLoadRestrictor(cmd.Flags()) + addFlagEnablePlugins(cmd.Flags()) addFlagReorderOutput(cmd.Flags()) cmd.AddCommand(NewCmdBuildPrune(out)) return cmd @@ -95,7 +94,7 @@ func (o *Options) Validate(args []string) (err error) { } else { o.kustomizationPath = args[0] } - err = krusty.ValidateFlagLoadRestrictor() + err = validateFlagLoadRestrictor() if err != nil { return err } @@ -105,9 +104,12 @@ func (o *Options) Validate(args []string) (err error) { func (o *Options) makeOptions() *krusty.Options { opts := krusty.MakeDefaultOptions() - opts.LoadRestrictions = krusty.GetFlagLoadRestrictorValue() + opts.LoadRestrictions = getFlagLoadRestrictorValue() opts.DoLegacyResourceSort = o.outOrder == legacy - opts.PluginConfig.Enabled = o.pluginsEnabled + opts.PluginConfig.PluginRestrictions = types.PluginRestrictionsBuiltinsOnly + if isFlagEnablePluginsSet() { + opts.PluginConfig.PluginRestrictions = types.PluginRestrictionsNone + } return opts } diff --git a/kustomize/internal/commands/build/flagenableplugins.go b/kustomize/internal/commands/build/flagenableplugins.go new file mode 100644 index 000000000..a9c0eed05 --- /dev/null +++ b/kustomize/internal/commands/build/flagenableplugins.go @@ -0,0 +1,29 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package build + +import ( + "github.com/spf13/pflag" +) + +const ( + flagEnablePluginsName = "enable_alpha_plugins" + flagEnablePluginsHelp = `enable plugins, an alpha feature. +See https://github.com/kubernetes-sigs/kustomize/blob/master/docs/plugins/README.md +` +) + +var ( + flagPluginsEnabledValue = false +) + +func addFlagEnablePlugins(set *pflag.FlagSet) { + set.BoolVar( + &flagPluginsEnabledValue, flagEnablePluginsName, + false, flagEnablePluginsHelp) +} + +func isFlagEnablePluginsSet() bool { + return flagPluginsEnabledValue +} diff --git a/kustomize/internal/commands/build/flagloadrestrictor.go b/kustomize/internal/commands/build/flagloadrestrictor.go new file mode 100644 index 000000000..059d5118e --- /dev/null +++ b/kustomize/internal/commands/build/flagloadrestrictor.go @@ -0,0 +1,51 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package build + +import ( + "fmt" + + "github.com/spf13/pflag" + "sigs.k8s.io/kustomize/api/types" +) + +const ( + flagName = "load_restrictor" +) + +var ( + flagLrValue = types.LoadRestrictionsRootOnly.String() + flagLrHelp = "if set to '" + types.LoadRestrictionsNone.String() + + "', local kustomizations may load files from outside their root. " + + "This does, however, break the relocatability of the kustomization." +) + +func addFlagLoadRestrictor(set *pflag.FlagSet) { + set.StringVar( + &flagLrValue, flagName, + types.LoadRestrictionsRootOnly.String(), flagLrHelp) +} + +func validateFlagLoadRestrictor() error { + switch getFlagLoadRestrictorValue() { + case types.LoadRestrictionsRootOnly, types.LoadRestrictionsNone: + return nil + default: + return fmt.Errorf( + "illegal flag value --%s %s; legal values: %v", + flagName, flagLrValue, + []string{types.LoadRestrictionsRootOnly.String(), types.LoadRestrictionsNone.String()}) + } +} + +func getFlagLoadRestrictorValue() types.LoadRestrictions { + switch flagLrValue { + case types.LoadRestrictionsRootOnly.String(), "rootOnly": + return types.LoadRestrictionsRootOnly + case types.LoadRestrictionsNone.String(), "none": + return types.LoadRestrictionsNone + default: + return types.LoadRestrictionsUnknown + } +} diff --git a/travis/pre-commit.sh b/travis/pre-commit.sh index 3c74b8b78..5a2277bf1 100755 --- a/travis/pre-commit.sh +++ b/travis/pre-commit.sh @@ -34,6 +34,10 @@ function removeBin { /bin/rm -f $d } +# TODO: go install golang.org/x/tools/cmd/stringer +# TODO: kubeval? +# TODO: helm? no. except then we should not requirekubeval either, +# since they're both only used in demo (non-builtin) plugins. function installTools { # TODO(2019/Oct/19): After the API is in place, and # there's a new pluginator compiled against that API,