diff --git a/README.md b/README.md index c86d420b2..b6139f72a 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ these [instructions](docs/INSTALL.md). Browse the [docs](docs) or jump right into the tested [examples](examples). -kustomize [v2.0.3] is available in [kubectl v1.14][kubectl]. +kustomize [v2.0.3] is available in [kubectl v1.15][kubectl]. ## Usage diff --git a/docs/bugs.md b/docs/bugs.md index d3a0c311c..02dcb4428 100644 --- a/docs/bugs.md +++ b/docs/bugs.md @@ -20,7 +20,10 @@ following to improve response time. kustomize has a simple test harness in the [target package] for specifying a kustomization's input and the expected output. -See this [example of a target test]. + +See this [example of a target test], and contribution +[#971](https://github.com/kubernetes-sigs/kustomize/pull/971), +which does exactly the right thing. The pattern is * call `NewKustTestHarness` diff --git a/docs/plugins/README.md b/docs/plugins/README.md index 36fa977c9..730897a0e 100644 --- a/docs/plugins/README.md +++ b/docs/plugins/README.md @@ -26,9 +26,9 @@ or [transformer configs] doesn't meet your needs. * A _transformer_ plugin might perform special container command line edits, or any other - transformation that exceeds the power of the - builtin transformations (`namePrefix`, - `commonLabels`, etc.). + transformation beyond those provided by the + builtin (`namePrefix`, `commonLabels`, etc.) + transformers. ## Specification in `kustomization.yaml` @@ -76,7 +76,8 @@ Given this, the kustomization process would expect to find a file called `chartInflator.yaml` in the kustomization [root](../glossary.md#kustomization-root). -This is the _plugin's configuration file_. +This is the plugin's configuration file; +it contains a YAML configuration object. The file `chartInflator.yaml` could contain: @@ -115,16 +116,18 @@ e.g. the tests for [ChartInflator] or Each plugin gets its own dedicated directory named +[`XDG_CONFIG_HOME`]: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html + ``` $XDG_CONFIG_HOME/kustomize/plugin /${apiVersion}/LOWERCASE(${kind}) ``` -The default value of `XDG_CONFIG_HOME` is +The default value of [`XDG_CONFIG_HOME`] is `$HOME/.config`. The one-plugin-per-directory requirement eases -creation of a plugin bundle (source, test, plugin +creation of a plugin bundle (source, tests, plugin data files, etc.) for sharing. In the case of a [Go plugin](#go-plugins), it also @@ -169,23 +172,30 @@ The order specified in the `transformers` field is respected, as transformers cannot be expected to be commutative. +#### No Security + +Kustomize plugins do not run in any kind of +kustomize-provided sandbox. There's no notion +of _"plugin security"_. + A `kustomize build` that tries to use plugins but omits the flag > `--enable_alpha_plugins` -will fail with a warning about plugin use. +will not load plugins and will fail with a +warning about plugin use. -Flag use is an opt-in acknowledging both tthe -unstable (alpha) plugin API, and the absence of -plugin provenance. It's meant to give pause to -someone who blindly downloads a kustomization from -the internet and attempts to run it, without -realizing that it might attempt to run 3rd party -code in plugin form. The plugin would have to be -installed already, but nevertheless the flag is a -reminder. +The use of this flag is an opt-in acknowledging +the unstable (alpha) plugin API, the absence of +plugin provenance, and the fact that a plugin +is not part of kustomize. +To be clear, some kustomize plugin downloaded +from the internet might wonderfully transform +k8s config in a desired manner, while also +quietly doing anything the user could do to the +system running `kustomize build`. ## Authoring diff --git a/docs/plugins/goPluginCaveats.md b/docs/plugins/goPluginCaveats.md index 06d9588ca..e9d2e4bd5 100644 --- a/docs/plugins/goPluginCaveats.md +++ b/docs/plugins/goPluginCaveats.md @@ -54,7 +54,6 @@ This means a one-time run of ``` GOPATH=${whatever} go get \ sigs.k8s.io/kustomize/cmd/kustomize@${releaseVersion} - ``` and then a normal development cycle using @@ -75,37 +74,44 @@ must do to write a [tensorflow plugin]. The Go plugin developer sees the same API offered to native kustomize operations, assuring certain -semantics, invariants, checks, etc. An exec +semantics, invariants, checks, etc. An exec plugin sub-process dealing with this via stdin/stdout will have an easier time screwing things up for downstream transformers and consumers. +Minor point: if the plugin reads files via +the kustomize-provided file `Loader` interface, it +will be constrained by kustomize file loading +restrictions. Of course, nothing but a code audit +prevents a Go plugin from importing the `io` package +and doing whatever it wants. + ### Debugging A Go plugin developer can debug the plugin _in situ_, setting breakpoints inside the plugin and elsewhere while running a plugin in feature tests. -### Unit of contribution - -All the builtin generators and transformers -are themselves Go plugins. This means it's -trivial for the kustomize maintainers to -promote a contributed Go plugin to -_builtin_ status. - To get the best of both worlds (shareability and safety), a developer can write an `.go` program that functions as an _exec plugin_, but can be processed by `go generate` to emit a _Go plugin_ (or vice versa). +### Unit of contribution + +All the builtin generators and transformers +are themselves Go plugins. This means that +the kustomize maintainers can promote a contributed +plugin to a builtin without needing code changes +(beyond those mandated by normal code review). + ### Ecosystems grow through use Tooling could ease Go plugin _sharing_, but this requires some critical mass of Go plugin _authoring_, which in turn is hampered by confusion around sharing. [Go modules], once they -are more widely adopted, will solve one of the -biggest plugin sharing difficulties - ambiguous +are more widely adopted, will solve the +biggest plugin sharing difficulty: ambiguous plugin vs host dependencies. diff --git a/go.sum b/go.sum index 5b4e8a845..86f575653 100644 --- a/go.sum +++ b/go.sum @@ -121,6 +121,7 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= 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-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 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/k8sdeps/kunstruct/kunstruct.go b/k8sdeps/kunstruct/kunstruct.go index 24cad01e1..dc3a100d6 100644 --- a/k8sdeps/kunstruct/kunstruct.go +++ b/k8sdeps/kunstruct/kunstruct.go @@ -20,6 +20,8 @@ package kunstruct import ( "encoding/json" "fmt" + + "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/kustomize/v3/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -267,3 +269,19 @@ func (fs *UnstructAdapter) GetMap(path string) (map[string]interface{}, error) { } return nil, types.NoFieldError{Field: path} } + +func (fs *UnstructAdapter) MatchesLabelSelector(selector string) (bool, error) { + s, err := labels.Parse(selector) + if err != nil { + return false, err + } + return s.Matches(labels.Set(fs.GetLabels())), nil +} + +func (fs *UnstructAdapter) MatchesAnnotationSelector(selector string) (bool, error) { + s, err := labels.Parse(selector) + if err != nil { + return false, err + } + return s.Matches(labels.Set(fs.GetAnnotations())), nil +} diff --git a/k8sdeps/transformer/patch/transformer.go b/k8sdeps/transformer/patch/transformer.go index 2fbc2738a..692b0b75e 100644 --- a/k8sdeps/transformer/patch/transformer.go +++ b/k8sdeps/transformer/patch/transformer.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/kustomize/v3/pkg/gvk" - "sigs.k8s.io/kustomize/v3/pkg/resid" "sigs.k8s.io/kustomize/v3/pkg/resmap" "sigs.k8s.io/kustomize/v3/pkg/resource" "sigs.k8s.io/kustomize/v3/pkg/transformers" @@ -44,7 +43,7 @@ func (tf *transformer) Transform(m resmap.ResMap) error { return err } for _, patch := range patches.Resources() { - target, err := tf.findPatchTarget(m, patch.OrgId()) + target, err := m.GetById(patch.OrgId()) if err != nil { return err } @@ -96,20 +95,6 @@ func (tf *transformer) Transform(m resmap.ResMap) error { return nil } -func (tf *transformer) findPatchTarget( - m resmap.ResMap, id resid.ResId) (*resource.Resource, error) { - match, err := m.GetByOriginalId(id) - if err == nil { - return match, nil - } - match, err = m.GetByCurrentId(id) - if err == nil { - return match, nil - } - return nil, fmt.Errorf( - "failed to find target for patch %s", id.GvknString()) -} - // mergePatches merge and index patches by OrgId. // It errors out if there is conflict between patches. func (tf *transformer) mergePatches() (resmap.ResMap, error) { diff --git a/k8sdeps/transformer/patch/transformer_test.go b/k8sdeps/transformer/patch/transformer_test.go index f258efe1d..4a9cec645 100644 --- a/k8sdeps/transformer/patch/transformer_test.go +++ b/k8sdeps/transformer/patch/transformer_test.go @@ -380,7 +380,7 @@ func TestPatchesWithWrongNamespace(t *testing.T) { if err == nil { t.Fatalf("did not get expected error") } - if !strings.Contains(err.Error(), "failed to find target for patch") { + if !strings.Contains(err.Error(), "failed to find unique target for patch") { t.Fatalf("expected error to contain %q but get %v", "failed to find target for patch", err) } } diff --git a/pkg/git/repospec.go b/pkg/git/repospec.go index 2a1fc2731..f6f762cb5 100644 --- a/pkg/git/repospec.go +++ b/pkg/git/repospec.go @@ -19,6 +19,7 @@ package git import ( "fmt" "path/filepath" + "regexp" "strings" "sigs.k8s.io/kustomize/v3/pkg/fs" @@ -53,6 +54,9 @@ type RepoSpec struct { // Branch or tag reference. ref string + + // e.g. .git or empty in case of _git is present + gitSuffix string } // CloneSpec returns a string suitable for "git clone {spec}". @@ -60,7 +64,7 @@ func (x *RepoSpec) CloneSpec() string { if isAzureHost(x.host) || isAWSHost(x.host) { return x.host + x.orgRepo } - return x.host + x.orgRepo + gitSuffix + return x.host + x.orgRepo + x.gitSuffix } func (x *RepoSpec) CloneDir() fs.ConfirmedDir { @@ -86,7 +90,7 @@ func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { if filepath.IsAbs(n) { return nil, fmt.Errorf("uri looks like abs path: %s", n) } - host, orgRepo, path, gitRef := parseGithubUrl(n) + host, orgRepo, path, gitRef, gitSuffix := parseGithubUrl(n) if orgRepo == "" { return nil, fmt.Errorf("url lacks orgRepo: %s", n) } @@ -95,21 +99,32 @@ func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { } return &RepoSpec{ raw: n, host: host, orgRepo: orgRepo, - cloneDir: notCloned, path: path, ref: gitRef}, nil + cloneDir: notCloned, path: path, ref: gitRef, gitSuffix: gitSuffix}, nil } const ( - refQuery = "?ref=" - gitSuffix = ".git" + refQuery = "?ref=" + refQueryRegex = "\\?(version|ref)=" + gitSuffix = ".git" + gitDelimiter = "_git/" ) // From strings like git@github.com:someOrg/someRepo.git or // https://github.com/someOrg/someRepo?ref=someHash, extract // the parts. func parseGithubUrl(n string) ( - host string, orgRepo string, path string, gitRef string) { - host, n = parseHostSpec(n) + host string, orgRepo string, path string, gitRef string, gitSuff string) { + if strings.Contains(n, gitDelimiter) { + index := strings.Index(n, gitDelimiter) + // Adding _git/ to host + host = n[:index+len(gitDelimiter)] + orgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0] + path, gitRef = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):]) + return + } + host, n = parseHostSpec(n) + gitSuff = gitSuffix if strings.Contains(n, gitSuffix) { index := strings.Index(n, gitSuffix) orgRepo = n[0:index] @@ -120,24 +135,27 @@ func parseGithubUrl(n string) ( i := strings.Index(n, "/") if i < 1 { - return "", "", "", "" + return "", "", "", "", "" } j := strings.Index(n[i+1:], "/") if j >= 0 { j += i + 1 orgRepo = n[:j] path, gitRef = peelQuery(n[j+1:]) - } else { - path = "" - orgRepo, gitRef = peelQuery(n) + return } - return + path = "" + orgRepo, gitRef = peelQuery(n) + return host, orgRepo, path, gitRef, gitSuff } func peelQuery(arg string) (string, string) { - j := strings.Index(arg, refQuery) - if j >= 0 { - return arg[:j], arg[j+len(refQuery):] + + r, _ := regexp.Compile(refQueryRegex) + j := r.FindStringIndex(arg) + + if len(j) > 0 { + return arg[:j[0]], arg[j[0]+len(r.FindString(arg)):] } return arg, "" } diff --git a/pkg/git/repospec_test.go b/pkg/git/repospec_test.go index c8ee5e2da..8bd23928a 100644 --- a/pkg/git/repospec_test.go +++ b/pkg/git/repospec_test.go @@ -169,6 +169,24 @@ func TestNewRepoSpecFromUrl_CloneSpecs(t *testing.T) { absPath: notCloned.Join("path"), ref: "branch", }, + { + input: "https://itfs.mycompany.com/collection/project/_git/somerepos", + cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", + absPath: notCloned.String(), + ref: "", + }, + { + input: "https://itfs.mycompany.com/collection/project/_git/somerepos?version=v1.0.0", + cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", + absPath: notCloned.String(), + ref: "v1.0.0", + }, + { + input: "https://itfs.mycompany.com/collection/project/_git/somerepos/somedir?version=v1.0.0", + cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", + absPath: notCloned.Join("somedir"), + ref: "v1.0.0", + }, } for _, testcase := range testcases { rs, err := NewRepoSpecFromUrl(testcase.input) @@ -220,6 +238,32 @@ func TestIsAzureHost(t *testing.T) { } } +func TestPeelQuery(t *testing.T) { + testcases := []struct { + input string + expect [2]string + }{ + { + input: "somerepos?ref=v1.0.0", + expect: [2]string{"somerepos", "v1.0.0"}, + }, + { + input: "somerepos?version=master", + expect: [2]string{"somerepos", "master"}, + }, + { + input: "somerepos", + expect: [2]string{"somerepos", ""}, + }, + } + for _, testcase := range testcases { + path, ref := peelQuery(testcase.input) + if path != testcase.expect[0] || ref != testcase.expect[1] { + t.Errorf("peelQuery: expected (%s, %s) got (%s, %s) on %s", testcase.expect[0], testcase.expect[1], path, ref, testcase.input) + } + } +} + func TestIsAWSHost(t *testing.T) { testcases := []struct { input string diff --git a/pkg/ifc/ifc.go b/pkg/ifc/ifc.go index 190d77aaa..3c839d962 100644 --- a/pkg/ifc/ifc.go +++ b/pkg/ifc/ifc.go @@ -61,6 +61,8 @@ type Kunstructured interface { SetLabels(map[string]string) GetAnnotations() map[string]string SetAnnotations(map[string]string) + MatchesLabelSelector(selector string) (bool, error) + MatchesAnnotationSelector(selector string) (bool, error) } // KunstructuredFactory makes instances of Kunstructured. diff --git a/pkg/kusttest/kusttestharness.go b/pkg/kusttest/kusttestharness.go index 7b2e49e36..2e5adf943 100644 --- a/pkg/kusttest/kusttestharness.go +++ b/pkg/kusttest/kusttestharness.go @@ -31,19 +31,18 @@ type KustTestHarness struct { } func NewKustTestHarness(t *testing.T, path string) *KustTestHarness { - return newHarness( - t, path, plugins.DefaultPluginConfig()) + return NewKustTestHarnessFull( + t, path, loader.RestrictionRootOnly, plugins.DefaultPluginConfig()) } func NewKustTestPluginHarness(t *testing.T, path string) *KustTestHarness { - return newHarness( - t, path, plugins.ActivePluginConfig()) + return NewKustTestHarnessFull( + t, path, loader.RestrictionRootOnly, plugins.ActivePluginConfig()) } -func newHarness( - t *testing.T, path string, - pc *types.PluginConfig) *KustTestHarness { - return NewKustTestHarnessFull(t, path, loader.RestrictionRootOnly, pc) +func NewKustTestNoLoadRestrictorHarness(t *testing.T, path string) *KustTestHarness { + return NewKustTestHarnessFull( + t, path, loader.RestrictionNone, plugins.DefaultPluginConfig()) } func NewKustTestHarnessFull( @@ -124,7 +123,7 @@ func (th *KustTestHarness) LoadAndRunGenerator( func (th *KustTestHarness) LoadAndRunTransformer( config, input string) resmap.ResMap { - resMap, err := th.runTransformer(config, input) + resMap, err := th.RunTransformer(config, input) if err != nil { th.t.Fatalf("Err: %v", err) } @@ -133,11 +132,11 @@ func (th *KustTestHarness) LoadAndRunTransformer( func (th *KustTestHarness) ErrorFromLoadAndRunTransformer( config, input string) error { - _, err := th.runTransformer(config, input) + _, err := th.RunTransformer(config, input) return err } -func (th *KustTestHarness) runTransformer( +func (th *KustTestHarness) RunTransformer( config, input string) (resmap.ResMap, error) { transConfig, err := th.rf.RF().FromBytes([]byte(config)) if err != nil { @@ -149,7 +148,7 @@ func (th *KustTestHarness) runTransformer( } g, err := th.pl.LoadTransformer(th.ldr, transConfig) if err != nil { - th.t.Fatalf("Err: %v", err) + return nil, err } err = g.Transform(resMap) return resMap, err diff --git a/pkg/patch/transformer/factory.go b/pkg/patch/transformer/factory.go deleted file mode 100644 index 69aff144d..000000000 --- a/pkg/patch/transformer/factory.go +++ /dev/null @@ -1,82 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transformer - -import ( - "fmt" - - "sigs.k8s.io/kustomize/v3/pkg/gvk" - "sigs.k8s.io/kustomize/v3/pkg/ifc" - "sigs.k8s.io/kustomize/v3/pkg/resid" - "sigs.k8s.io/kustomize/v3/pkg/transformers" - "sigs.k8s.io/kustomize/v3/pkg/types" -) - -// PatchJson6902Factory makes PatchJson6902 transformers -type PatchJson6902Factory struct { - loader ifc.Loader -} - -// NewPatchJson6902Factory returns a new PatchJson6902Factory. -func NewPatchJson6902Factory(l ifc.Loader) PatchJson6902Factory { - return PatchJson6902Factory{loader: l} -} - -// MakePatchJson6902Transformer returns a transformer for applying PatchJson6902 patch -func (f PatchJson6902Factory) MakePatchJson6902Transformer(patches []types.PatchJson6902) (transformers.Transformer, error) { - var ts []transformers.Transformer - for _, p := range patches { - t, err := f.makeOnePatchJson6902Transformer(p) - if err != nil { - return nil, err - } - if t != nil { - ts = append(ts, t) - } - } - return transformers.NewMultiTransformerWithConflictCheck(ts), nil -} - -func (f PatchJson6902Factory) makeOnePatchJson6902Transformer(p types.PatchJson6902) (transformers.Transformer, error) { - if p.Target == nil { - return nil, fmt.Errorf("must specify the target field in patchesJson6902") - } - if p.Path == "" { - return nil, fmt.Errorf("must specify the path for a json patch file") - } - - targetId := resid.NewResIdWithNamespace( - gvk.Gvk{ - Group: p.Target.Group, - Version: p.Target.Version, - Kind: p.Target.Kind, - }, - p.Target.Name, - p.Target.Namespace, - ) - - rawOp, err := f.loader.Load(p.Path) - if err != nil { - return nil, err - } - - return newPatchJson6902JSONTransformer(targetId, rawOp) -} - -func isJsonFormat(data []byte) bool { - return data[0] == '[' -} diff --git a/pkg/patch/transformer/factory_test.go b/pkg/patch/transformer/factory_test.go deleted file mode 100644 index 256d7b125..000000000 --- a/pkg/patch/transformer/factory_test.go +++ /dev/null @@ -1,326 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transformer - -import ( - "reflect" - "strings" - "testing" - - "gopkg.in/yaml.v2" - "sigs.k8s.io/kustomize/v3/internal/loadertest" - "sigs.k8s.io/kustomize/v3/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/v3/pkg/resmaptest" - "sigs.k8s.io/kustomize/v3/pkg/resource" - "sigs.k8s.io/kustomize/v3/pkg/types" -) - -var rf = resource.NewFactory( - kunstruct.NewKunstructuredFactoryImpl()) - -func TestNewPatchJson6902FactoryNoTarget(t *testing.T) { - p := types.PatchJson6902{} - _, err := NewPatchJson6902Factory(nil).makeOnePatchJson6902Transformer(p) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "must specify the target field in patchesJson6902") { - t.Fatalf("incorrect error returned: %v", err) - } -} - -func TestNewPatchJson6902FactoryConflict(t *testing.T) { - jsonPatch := []byte(` -target: - name: some-name - kind: Deployment -`) - p := types.PatchJson6902{} - err := yaml.Unmarshal(jsonPatch, &p) - if err != nil { - t.Fatalf("expected error %v", err) - } - f := NewPatchJson6902Factory(nil) - _, err = f.makeOnePatchJson6902Transformer(p) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "must specify the path for a json patch file") { - t.Fatalf("incorrect error returned %v", err) - } -} - -func TestNewPatchJson6902FactoryJSON(t *testing.T) { - ldr := loadertest.NewFakeLoader("/testpath") - operations := []byte(`[ - {"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, - {"op": "add", "path": "/spec/replica", "value": "3"}, - {"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} -]`) - err := ldr.AddFile("/testpath/patch.json", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - - jsonPatch := []byte(` -target: - kind: Deployment - name: some-name -path: patch.json -`) - p := types.PatchJson6902{} - err = yaml.Unmarshal(jsonPatch, &p) - if err != nil { - t.Fatal("expected error") - } - - tr, err := NewPatchJson6902Factory(ldr).makeOnePatchJson6902Transformer(p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if tr == nil { - t.Fatal("the returned transformer should not be nil") - } -} - -func TestNewPatchJson6902FactoryYAML(t *testing.T) { - ldr := loadertest.NewFakeLoader("/testpath") - operations := []byte(` -- op: replace - path: /spec/template/spec/containers/0/name - value: my-nginx -- op: add - path: /spec/replica - value: 3 -- op: add - path: /spec/template/spec/containers/0/command - value: ["arg1", "arg2", "arg3"] -`) - err := ldr.AddFile("/testpath/patch.yaml", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - jsonPatch := []byte(` -target: - name: some-name - kind: Deployment -path: patch.yaml -`) - p := types.PatchJson6902{} - err = yaml.Unmarshal(jsonPatch, &p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - - tr, err := NewPatchJson6902Factory(ldr).makeOnePatchJson6902Transformer(p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if tr == nil { - t.Fatal("the returned transformer should not be nil") - } -} - -func TestNewPatchJson6902FactoryMulti(t *testing.T) { - ldr := loadertest.NewFakeLoader("/testpath") - operations := []byte(`[ - {"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, - {"op": "add", "path": "/spec/replica", "value": "3"} -]`) - err := ldr.AddFile("/testpath/patch.json", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - - operations = []byte(` -- op: add - path: /spec/template/spec/containers/0/command - value: ["arg1", "arg2", "arg3"] -`) - err = ldr.AddFile("/testpath/patch.yaml", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - - jsonPatches := []byte(` -- target: - kind: foo - name: some-name - path: patch.json - -- target: - kind: foo - name: some-name - path: patch.yaml -`) - var p []types.PatchJson6902 - err = yaml.Unmarshal(jsonPatches, &p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - - f := NewPatchJson6902Factory(ldr) - tr, err := f.MakePatchJson6902Transformer(p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if tr == nil { - t.Fatal("the returned transformer should not be nil") - } - - base := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "some-name", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "nginx", - }, - }, - }, - }, - }, - }).ResMap() - expected := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "some-name", - }, - "spec": map[string]interface{}{ - "replica": "3", - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "my-nginx", - "command": []interface{}{ - "arg1", - "arg2", - "arg3", - }, - }, - }, - }, - }, - }, - }).ResMap() - err = tr.Transform(base) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if !reflect.DeepEqual(base, expected) { - err = expected.ErrorIfNotEqualSets(base) - t.Fatalf("actual doesn't match expected: %v", err) - } -} - -func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { - ldr := loadertest.NewFakeLoader("/testpath") - operations := []byte(`[ - {"op": "add", "path": "/spec/replica", "value": "3"} -]`) - err := ldr.AddFile("/testpath/patch.json", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - operations = []byte(` -- op: add - path: /spec/replica - value: 4 -`) - err = ldr.AddFile("/testpath/patch.yaml", operations) - if err != nil { - t.Fatalf("Failed to setup fake ldr.") - } - - jsonPatches := []byte(` -- target: - kind: foo - name: somename - path: patch.json - -- target: - kind: foo - name: somename - path: patch.yaml -`) - var p []types.PatchJson6902 - err = yaml.Unmarshal(jsonPatches, &p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - - f := NewPatchJson6902Factory(ldr) - tr, err := f.MakePatchJson6902Transformer(p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if tr == nil { - t.Fatal("the returned transformer should not be nil") - } - - m := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "somename", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "nginx", - }, - }, - }, - }, - }, - }).ResMap() - - err = tr.Transform(m) - if err == nil { - t.Fatal("expected conflict") - } - if !strings.Contains(err.Error(), "found conflict between different patches") { - t.Fatalf("incorrect error happened %v", err) - } -} diff --git a/pkg/patch/transformer/patchjson6902json.go b/pkg/patch/transformer/patchjson6902json.go deleted file mode 100644 index 8a76952f1..000000000 --- a/pkg/patch/transformer/patchjson6902json.go +++ /dev/null @@ -1,113 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transformer - -import ( - "fmt" - - jsonpatch "github.com/evanphx/json-patch" - "github.com/pkg/errors" - "sigs.k8s.io/kustomize/v3/pkg/resid" - "sigs.k8s.io/kustomize/v3/pkg/resmap" - "sigs.k8s.io/kustomize/v3/pkg/resource" - "sigs.k8s.io/kustomize/v3/pkg/transformers" - "sigs.k8s.io/yaml" -) - -// patchJson6902JSONTransformer applies patches. -type patchJson6902JSONTransformer struct { - target resid.ResId - patch jsonpatch.Patch - rawOp []byte -} - -var _ transformers.Transformer = &patchJson6902JSONTransformer{} - -// newPatchJson6902JSONTransformer constructs a PatchJson6902 transformer. -func newPatchJson6902JSONTransformer( - id resid.ResId, rawOp []byte) (transformers.Transformer, error) { - op := rawOp - var err error - - if len(op) == 0 { - return nil, fmt.Errorf("json patch file is empty %v", id) - } - - if !isJsonFormat(op) { - // if it isn't JSON, try to parse it as YAML - op, err = yaml.YAMLToJSON(rawOp) - if err != nil { - return nil, err - } - } - decodedPatch, err := jsonpatch.DecodePatch(op) - if err != nil { - return nil, err - } - if len(decodedPatch) == 0 { - return transformers.NewNoOpTransformer(), nil - } - return &patchJson6902JSONTransformer{target: id, patch: decodedPatch, rawOp: rawOp}, nil -} - -// Transform apply the json patches on top of the base resources. -func (t *patchJson6902JSONTransformer) Transform(m resmap.ResMap) error { - obj, err := t.findTargetObj(m) - if err != nil { - return err - } - rawObj, err := obj.MarshalJSON() - if err != nil { - return err - } - modifiedObj, err := t.patch.Apply(rawObj) - if err != nil { - return errors.Wrapf(err, "failed to apply json patch '%s'", string(t.rawOp)) - } - err = obj.UnmarshalJSON(modifiedObj) - if err != nil { - return err - } - return nil -} - -func (t *patchJson6902JSONTransformer) findTargetObj( - m resmap.ResMap) (*resource.Resource, error) { - var matched []*resource.Resource - // TODO(monopole): namespace bug in json patch? - // Since introduction in PR #300 - // (see pkg/patch/transformer/util.go), - // this code has treated an empty namespace like a wildcard - // rather than like an additional restriction to match - // only the empty namespace. No test coverage to confirm. - // Not sure if desired, keeping it for now. - if t.target.Namespace != "" { - matched = m.GetMatchingResourcesByOriginalId(t.target.Equals) - } else { - matched = m.GetMatchingResourcesByOriginalId(t.target.GvknEquals) - } - if len(matched) == 0 { - return nil, fmt.Errorf( - "couldn't find target %v for json patch", t.target) - } - if len(matched) > 1 { - return nil, fmt.Errorf( - "found multiple targets %v matching %v for json patch", - matched, t.target) - } - return matched[0], nil -} diff --git a/pkg/patch/transformer/patchjson6902json_test.go b/pkg/patch/transformer/patchjson6902json_test.go deleted file mode 100644 index 132ca9c13..000000000 --- a/pkg/patch/transformer/patchjson6902json_test.go +++ /dev/null @@ -1,180 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transformer - -import ( - "reflect" - "strings" - "testing" - - "sigs.k8s.io/kustomize/v3/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/v3/pkg/gvk" - "sigs.k8s.io/kustomize/v3/pkg/resid" - "sigs.k8s.io/kustomize/v3/pkg/resmaptest" - "sigs.k8s.io/kustomize/v3/pkg/resource" -) - -var deploy = gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"} - -func TestJsonPatchJSONTransformer_Transform(t *testing.T) { - rf := resource.NewFactory( - kunstruct.NewKunstructuredFactoryImpl()) - m := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, - }, - }, - }, - }, - }).ResMap() - - operations := []byte(`[ - {"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, - {"op": "add", "path": "/spec/replica", "value": "3"}, - {"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} -]`) - - expected := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "replica": "3", - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "my-nginx", - "command": []interface{}{ - "arg1", - "arg2", - "arg3", - }, - }, - }, - }, - }, - }, - }).ResMap() - - patchId := m.GetByIndex(0).OrgId() - - jpt, err := newPatchJson6902JSONTransformer(patchId, operations) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - err = jpt.Transform(m) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqualSets(m) - t.Fatalf("actual doesn't match expected: %v", err) - } -} - -func TestJsonPatchJSONTransformer_UnHappyTransform(t *testing.T) { - rf := resource.NewFactory( - kunstruct.NewKunstructuredFactoryImpl()) - m := resmaptest_test.NewRmBuilder(t, rf). - Add(map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, - }, - }, - }, - }, - }).ResMap() - - operations := []byte(`[ - {"op": "add", "path": "/spec/template/spec/containers/0/command/", "value": ["arg1", "arg2", "arg3"]} -]`) - - jpt, err := newPatchJson6902JSONTransformer( - m.GetByIndex(0).OrgId(), operations) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - err = jpt.Transform(m) - if err == nil { - t.Fatalf("expected error didn't happen") - } - if !strings.HasPrefix( - err.Error(), "failed to apply json patch") || - !strings.Contains(err.Error(), string(operations)) { - t.Fatalf("expected error didn't happen, but got %v", err) - } -} - -func TestJsonPatchJSONTransformer_EmptyPatchFile(t *testing.T) { - id := resid.NewResId(deploy, "deploy1") - operations := []byte(``) - - _, err := newPatchJson6902JSONTransformer(id, operations) - - if err == nil { - t.Fatalf("expected an error") - } - - if err != nil { - if !strings.HasPrefix(err.Error(), "json patch file is empty") { - t.Fatalf("expected %s, but got %v", "json patch file is empty", err) - } - } -} diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index 821b5b34c..58e552137 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -8,6 +8,8 @@ package resmap import ( "bytes" "fmt" + "regexp" + "github.com/pkg/errors" "sigs.k8s.io/kustomize/v3/pkg/resid" @@ -37,8 +39,23 @@ type ResMap interface { // as appended. Resources() []*resource.Resource - // Append adds a Resource. - // Error on OrgId collision. + // Append adds a Resource. Error on CurId collision. + // + // A class invariant of ResMap is that all of its + // resources must differ in their value of + // CurId(), aka current Id. The Id is the tuple + // of {namespace, group, version, kind, name} + // (see ResId). + // + // This invariant reflects the invariant of a + // kubernetes cluster, where if one tries to add + // a resource to the cluster whose Id matches + // that of a resource already in the cluster, + // only two outcomes are allowed. Either the + // incoming resource is _merged_ into the existing + // one, or the incoming resource is rejected. + // One cannot end up with two resources + // in the cluster with the same Id. Append(*resource.Resource) error // AppendAll appends another ResMap to self, @@ -89,8 +106,10 @@ type ResMap interface { // an exact match, returning an error on multiple or no matches. GetByOriginalId(resid.ResId) (*resource.Resource, error) - // Deprecated. - // Same as GetByOriginalId. + // GetById is a helper function which first + // attempts GetByOriginalId, then GetByCurrentId, + // returning an error if both fail to find a single + // match. GetById(resid.ResId) (*resource.Resource, error) // GroupedByNamespace returns a map of namespace @@ -158,6 +177,10 @@ type ResMap interface { // Debug prints the ResMap. Debug(title string) + + // Select returns a list of resources that + // are selected by a Selector + Select(types.Selector) ([]*resource.Resource, error) } // resWrangler holds the content manipulated by kustomize. @@ -337,6 +360,22 @@ func (m *resWrangler) GetByOriginalId( return demandOneMatch(m.GetMatchingResourcesByOriginalId, id, "Original") } +// GetById implements ResMap. +func (m *resWrangler) GetById( + id resid.ResId) (*resource.Resource, error) { + match, err1 := m.GetByOriginalId(id) + if err1 == nil { + return match, nil + } + match, err2 := m.GetByCurrentId(id) + if err2 == nil { + return match, nil + } + return nil, fmt.Errorf( + "%s; %s; failed to find unique target for patch %s", + err1.Error(), err2.Error(), id.GvknString()) +} + type resFinder func(IdMatcher) []*resource.Resource func demandOneMatch( @@ -351,11 +390,6 @@ func demandOneMatch( return nil, fmt.Errorf("no matches for %sId %s", s, id) } -// GetById implements ResMap. -func (m *resWrangler) GetById(id resid.ResId) (*resource.Resource, error) { - return m.GetByCurrentId(id) -} - // GroupedByNamespace implements ResMap.GroupByNamespace func (m *resWrangler) GroupedByNamespace() map[string][]*resource.Resource { items := m.groupedByNamespace() @@ -587,3 +621,66 @@ func (m *resWrangler) appendReplaceOrMerge( } return nil } + +// Select returns a list of resources that +// are selected by a Selector +func (m *resWrangler) Select(s types.Selector) ([]*resource.Resource, error) { + ns := regexp.MustCompile(s.Namespace) + nm := regexp.MustCompile(s.Name) + var result []*resource.Resource + for _, r := range m.Resources() { + curId := r.CurId() + orgId := r.OrgId() + + // matches the namespace when namespace is not empty in the selector + // It first tries to match with the original namespace + // then matches with the current namespace + if r.GetNamespace() != "" { + matched := ns.MatchString(orgId.EffectiveNamespace()) + if !matched { + matched = ns.MatchString(curId.EffectiveNamespace()) + if !matched { + continue + } + } + } + + // matches the name when name is not empty in the selector + // It first tries to match with the original name + // then matches with the current name + if r.GetName() != "" { + matched := nm.MatchString(orgId.Name) + if !matched { + matched = nm.MatchString(curId.Name) + if !matched { + continue + } + } + } + + // matches the GVK + if !r.GetGvk().IsSelected(&s.Gvk) { + continue + } + + // matches the label selector + matched, err := r.MatchesLabelSelector(s.LabelSelector) + if err != nil { + return nil, err + } + if !matched { + continue + } + + // matches the annotation selector + matched, err = r.MatchesAnnotationSelector(s.AnnotationSelector) + if err != nil { + return nil, err + } + if !matched { + continue + } + result = append(result, r) + } + return result, nil +} diff --git a/pkg/resmap/resmap_test.go b/pkg/resmap/resmap_test.go index babe8cb20..cb8742599 100644 --- a/pkg/resmap/resmap_test.go +++ b/pkg/resmap/resmap_test.go @@ -6,6 +6,7 @@ package resmap_test import ( "fmt" "reflect" + "strings" "testing" "sigs.k8s.io/kustomize/v3/k8sdeps/kunstruct" @@ -45,6 +46,24 @@ func makeCm(i int) *resource.Resource { }) } +// Maintain the class invariant that no two +// resources can have the same CurId(). +func TestAppendRejectsDuplicateResId(t *testing.T) { + w := New() + if err := w.Append(makeCm(1)); err != nil { + t.Fatalf("append error: %v", err) + } + err := w.Append(makeCm(1)) + if err == nil { + t.Fatalf("expected append error") + } + if !strings.Contains( + err.Error(), + "may not add resource with an already registered id") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestAppendRemove(t *testing.T) { w1 := New() doAppend(t, w1, makeCm(1)) diff --git a/pkg/resmap/selector_test.go b/pkg/resmap/selector_test.go new file mode 100644 index 000000000..e1b078e09 --- /dev/null +++ b/pkg/resmap/selector_test.go @@ -0,0 +1,130 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2. + +package resmap_test + +import ( + "testing" + + "sigs.k8s.io/kustomize/v3/pkg/gvk" + "sigs.k8s.io/kustomize/v3/pkg/resmap" + "sigs.k8s.io/kustomize/v3/pkg/types" +) + +func setupRMForPatchTargets(t *testing.T) resmap.ResMap { + result, err := rmF.NewResMapFromBytes([]byte(` +apiVersion: group1/v1 +kind: Kind1 +metadata: + name: name1 + namespace: ns1 + labels: + app: name1 + annotations: + foo: bar +--- +apiVersion: group1/v1 +kind: Kind1 +metadata: + name: name2 + namespace: default + labels: + app: name2 + annotations: + foo: bar +--- +apiVersion: group1/v1 +kind: Kind2 +metadata: + name: name3 + labels: + app: name3 + annotations: + bar: baz +`)) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + return result +} + +func TestFindPatchTargets(t *testing.T) { + rm := setupRMForPatchTargets(t) + testcases := []struct { + target types.Selector + count int + }{ + { + target: types.Selector{ + Name: "name*", + }, + count: 3, + }, + { + target: types.Selector{ + Name: "name*", + AnnotationSelector: "foo=bar", + }, + count: 2, + }, + { + target: types.Selector{ + LabelSelector: "app=name1", + }, + count: 1, + }, + { + target: types.Selector{ + Gvk: gvk.Gvk{ + Kind: "Kind1", + }, + Name: "name*", + }, + count: 2, + }, + { + target: types.Selector{ + Name: "NotMatched", + }, + count: 0, + }, + { + target: types.Selector{ + Name: "", + }, + count: 3, + }, + { + target: types.Selector{ + Namespace: "default", + }, + count: 2, + }, + { + target: types.Selector{ + Namespace: "", + }, + count: 3, + }, + { + target: types.Selector{ + Namespace: "default", + Name: "name*", + Gvk: gvk.Gvk{ + Kind: "Kind1", + }, + }, + count: 1, + }, + } + for _, testcase := range testcases { + actual, err := rm.Select(testcase.target) + if err != nil { + t.Errorf("unexpected error %v", err) + } + if len(actual) != testcase.count { + t.Errorf("expected %d objects, but got %d:\n%v", testcase.count, len(actual), actual) + } + } + +} diff --git a/pkg/target/baseandoverlaymedium_test.go b/pkg/target/baseandoverlaymedium_test.go index 9b614e31c..7d7752be5 100644 --- a/pkg/target/baseandoverlaymedium_test.go +++ b/pkg/target/baseandoverlaymedium_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package target_test @@ -47,7 +34,7 @@ spec: app: mungebot `) th.WriteF("/app/base/deployment/deployment.yaml", ` -apiVersion: extensions/v1beta1 +apiVersion: apps/v1 kind: Deployment metadata: name: mungebot @@ -79,7 +66,7 @@ func TestMediumBase(t *testing.T) { t.Fatalf("Err: %v", err) } th.AssertActualEqualsExpected(m, ` -apiVersion: extensions/v1beta1 +apiVersion: apps/v1 kind: Deployment metadata: annotations: @@ -174,7 +161,7 @@ adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. `) th.WriteF("/app/overlay/deployment/deployment.yaml", ` -apiVersion: extensions/v1beta1 +apiVersion: apps/v1 kind: Deployment metadata: name: mungebot @@ -211,7 +198,7 @@ spec: t.Fatalf("Err: %v", err) } th.AssertActualEqualsExpected(m, ` -apiVersion: extensions/v1beta1 +apiVersion: apps/v1 kind: Deployment metadata: annotations: diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index 3bc99b75f..73d71c645 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -419,7 +419,7 @@ patchesJson6902: - target: version: v1 kind: Service - name: myService # BUG (https://github.com/kubernetes-sigs/kustomize/issues/972): this should be a-myService, because that is what the output for the base contains + name: a-myService path: service-patch.yaml `) diff --git a/pkg/target/complexcomposition_test.go b/pkg/target/complexcomposition_test.go new file mode 100644 index 000000000..a5d3a5eb0 --- /dev/null +++ b/pkg/target/complexcomposition_test.go @@ -0,0 +1,339 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package target_test + +import ( + "strings" + "testing" + + "sigs.k8s.io/kustomize/v3/pkg/kusttest" +) + +// Base +// ---- +const baseKustomization = ` +resources: +- statefulset.yaml +` + +const baseStatefulSet = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + serviceName: my-svc + selector: + matchLabels: + app: my-app + template: + metadata: + labels: + app: my-app + spec: + containers: + - name: app + image: my-image + volumeClaimTemplates: + - spec: + storageClassName: default +` + +// Storage overlay +// --------------- +const storageKustomization = ` +resources: +- ../base +patchesJson6902: +- target: + group: apps + version: v1 + kind: StatefulSet + name: my-sts + path: sts-patch.json +` + +const patchJsonPVCTemplate = ` +[{"op": "replace", "path": "/spec/volumeClaimTemplates/0/spec/storageClassName", "value": "my-sc"}] +` + +// Config overlay +// -------------- +const configKustomization = ` +resources: +- ../base +configMapGenerator: +- name: my-config + literals: + - MY_ENV=foo +generatorOptions: + disableNameSuffixHash: true +patchesStrategicMerge: +- sts-patch.yaml +` + +const patchEnvFromConfig = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + template: + spec: + containers: + - name: app + envFrom: + - configMapRef: + name: my-config +` + +// Tolerations overlay +// ------------------- +const tolerationsKustomization = ` +resources: +- ../base +patchesStrategicMerge: +- sts-patch.yaml +` + +const patchTolerations = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + template: + spec: + tolerations: + - effect: NoExecute + key: node.kubernetes.io/not-ready + tolerationSeconds: 30 +` + +// HTTPS overlay +// ------------- +const httpsKustomization = ` +resources: +- ../base +- https-svc.yaml +patchesStrategicMerge: +- sts-patch.yaml +` + +const patchService = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + serviceName: my-https-svc +` + +const httpsService = ` +apiVersion: v1 +kind: Service +metadata: + name: my-https-svc +spec: + ports: + - port: 443 + protocol: TCP + name: https + selector: + app: my-app +` + +func writeStatefulSetBase(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/base", baseKustomization) + th.WriteF("/app/base/statefulset.yaml", baseStatefulSet) +} + +func writeHTTPSOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/https", httpsKustomization) + th.WriteF("/app/https/https-svc.yaml", httpsService) + th.WriteF("/app/https/sts-patch.yaml", patchService) +} + +func writeConfigOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/config", configKustomization) + th.WriteF("/app/config/sts-patch.yaml", patchEnvFromConfig) +} + +func writeTolerationsOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/tolerations", tolerationsKustomization) + th.WriteF("/app/tolerations/sts-patch.yaml", patchTolerations) +} + +func writeStorageOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/storage", storageKustomization) + th.WriteF("/app/storage/sts-patch.json", patchJsonPVCTemplate) +} + +// Here's a complex kustomization scenario that combines multiple overlays +// on a common base: +// +// dev prod +// | | +// | | +// + ------- + + ------------ + ------------- + +// | | | | | +// | | | | | +// v | v v v +// storage + -----> config tolerations https +// | | | | +// | | | | +// | + --- + + --- + | +// | | | | +// | v v | +// + -----------------------> base <------------------ + +// +// The base resource is a statefulset. Each intermediate overlay manages or +// generates new resources and patches different aspects of the same base +// resource, without using any of the `namePrefix`, `nameSuffix` or `namespace` +// kustomization keywords. +// +// Intermediate overlays: +// - storage: Changes the storage class of the stateful set with a JSON patch. +// - config: Generates a config map and adds a field as an environment +// variable. +// - tolerations: Adds a new tolerations field in the spec. +// - https: Adds a new service resource and changes the service name in the +// stateful set. +// +// Top overlays: +// - dev: Combines the storage and config intermediate overlays. +// - prod: Combines the config, tolerations and https intermediate overlays. + +func TestComplexComposition_Dev_Failure(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/dev") + writeStatefulSetBase(th) + writeStorageOverlay(th) + writeConfigOverlay(th) + writeTolerationsOverlay(th) + writeHTTPSOverlay(th) + + th.WriteK("/app/dev", ` +resources: +- ../storage +- ../config +`) + + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("Expected resource accumulation error") + } + if !strings.Contains( + err.Error(), "already registered id: apps_v1_StatefulSet|~X|my-sts") { + t.Fatalf("Unexpected err: %v", err) + } + + // Expected Output + const devMergeResult = ` +apiVersion: v1 +data: + MY_ENV: foo +kind: ConfigMap +metadata: + name: my-config +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + serviceName: my-svc + selector: + matchLabels: + app: my-app + template: + metadata: + labels: + app: my-app + spec: + containers: + - name: app + image: my-image + envFrom: + - configMapRef: + name: my-config + volumeClaimTemplates: + - spec: + storageClassName: my-sc + ` +} + +func TestComplexComposition_Prod_Failure(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/prod") + writeStatefulSetBase(th) + writeStorageOverlay(th) + writeConfigOverlay(th) + writeTolerationsOverlay(th) + writeHTTPSOverlay(th) + + th.WriteK("/app/prod", ` +resources: +- ../config +- ../tolerations +- ../https +`) + + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("Expected resource accumulation error") + } + if !strings.Contains( + err.Error(), "already registered id: apps_v1_StatefulSet|~X|my-sts") { + t.Fatalf("Unexpected err: %v", err) + } + + // Expected Output + const prodMergeResult = ` +apiVersion: v1 +data: + MY_ENV: foo +kind: ConfigMap +metadata: + name: my-config +--- +apiVersion: v1 +kind: Service +metadata: + name: my-https-svc +spec: + ports: + - name: https + port: 443 + protocol: TCP + selector: + app: my-app +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: my-sts +spec: + serviceName: my-https-svc + selector: + matchLabels: + app: my-app + template: + metadata: + labels: + app: my-app + spec: + containers: + - image: my-image + envFrom: + - configMapRef: + name: my-config + name: app + tolerations: + - effect: NoExecute + key: node.kubernetes.io/not-ready + tolerationSeconds: 30 + volumeClaimTemplates: + - spec: + storageClassName: default + ` +} diff --git a/pkg/target/diamondcomposition_test.go b/pkg/target/diamondcomposition_test.go new file mode 100644 index 000000000..137f2e21c --- /dev/null +++ b/pkg/target/diamondcomposition_test.go @@ -0,0 +1,491 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package target_test + +import ( + "fmt" + "path/filepath" + "strings" + "testing" + + "sigs.k8s.io/kustomize/v3/pkg/kusttest" + "sigs.k8s.io/kustomize/v3/pkg/plugins" +) + +const patchAddProbe = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + containers: + - name: my-deployment + livenessProbe: + httpGet: + path: /healthz + port: 8080 +` + +const container = `{ "image": "my-image", "livenessProbe": { "httpGet" : {"path": "/healthz", "port": 8080 } }, "name": "my-deployment"}` + +const patchJsonAddProbe = `[{"op": "replace", "path": "/spec/template/spec/containers/0", "value": ` + + container + `}]` + +const patchDnsPolicy = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + dnsPolicy: ClusterFirst +` +const patchJsonDnsPolicy = `[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "ClusterFirst"}]` + +const patchRestartPolicy = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + restartPolicy: Always +` +const patchJsonRestartPolicy = `[{"op": "add", "path": "/spec/template/spec/restartPolicy", "value": "Always"}]` + +func writeDeploymentBase(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/base", ` +resources: +- deployment.yaml +`) + + th.WriteF("/app/base/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + dnsPolicy: "None" + containers: + - name: my-deployment + image: my-image +`) +} + +func writeProbeOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/probe", ` +resources: +- ../base +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/probe/dep-patch.yaml", patchAddProbe) +} + +func writeDNSOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/dns", ` +resources: +- ../base +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/dns/dep-patch.yaml", patchDnsPolicy) +} + +func writeRestartOverlay(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/restart", ` +resources: +- ../base +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/restart/dep-patch.yaml", patchRestartPolicy) +} + +// Here's a composite kustomization, that combines multiple overlays +// (replicas, dns and metadata) which patch the same base resource. +// +// The base resource is a deployment and the overlays patch aspects +// of it, without using any of the `namePrefix`, `nameSuffix` or `namespace` +// kustomization keywords. +// +// composite +// / | \ +// probe dns restart +// \ | / +// base +// +func TestIssue1251_CompositeDiamond_Failure(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/composite") + writeDeploymentBase(th) + writeProbeOverlay(th) + writeDNSOverlay(th) + writeRestartOverlay(th) + + th.WriteK("/app/composite", ` +resources: +- ../probe +- ../dns +- ../restart +`) + + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("Expected resource accumulation error") + } + if !strings.Contains( + err.Error(), "already registered id: apps_v1_Deployment|~X|my-deployment") { + t.Fatalf("Unexpected err: %v", err) + } +} + +const expectedPatchedDeployment = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + containers: + - image: my-image + livenessProbe: + httpGet: + path: /healthz + port: 8080 + name: my-deployment + dnsPolicy: ClusterFirst + restartPolicy: Always +` + +// This test reuses some methods from TestIssue1251_CompositeDiamond, +// but overwrites the kustomization files in the overlays. +func TestIssue1251_Patches_Overlayed(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/restart") + writeDeploymentBase(th) + + // probe overlays base. + writeProbeOverlay(th) + + // dns overlays probe. + writeDNSOverlay(th) + th.WriteK("/app/dns", ` +resources: +- ../probe +patchesStrategicMerge: +- dep-patch.yaml +`) + + // restart overlays dns. + writeRestartOverlay(th) + th.WriteK("/app/restart", ` +resources: +- ../dns +patchesStrategicMerge: +- dep-patch.yaml +`) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, expectedPatchedDeployment) +} + +func TestIssue1251_Patches_Local(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/composite") + writeDeploymentBase(th) + + th.WriteK("/app/composite", ` +resources: +- ../base +patchesStrategicMerge: +- patchAddProbe.yaml +- patchDnsPolicy.yaml +- patchRestartPolicy.yaml +`) + th.WriteF("/app/composite/patchRestartPolicy.yaml", patchRestartPolicy) + th.WriteF("/app/composite/patchDnsPolicy.yaml", patchDnsPolicy) + th.WriteF("/app/composite/patchAddProbe.yaml", patchAddProbe) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, expectedPatchedDeployment) +} + +func definePatchDirStructure(th *kusttest_test.KustTestHarness) { + writeDeploymentBase(th) + + th.WriteF("/app/patches/patchRestartPolicy.yaml", patchRestartPolicy) + th.WriteF("/app/patches/patchDnsPolicy.yaml", patchDnsPolicy) + th.WriteF("/app/patches/patchAddProbe.yaml", patchAddProbe) +} + +// Fails due to file load restrictor. +func TestIssue1251_Patches_ProdVsDev_Failure(t *testing.T) { + th := kusttest_test.NewKustTestPluginHarness(t, "/app/prod") + definePatchDirStructure(th) + + th.WriteK("/app/prod", ` +resources: +- ../base +patchesStrategicMerge: +- ../patches/patchAddProbe.yaml +- ../patches/patchDnsPolicy.yaml +`) + + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains( + err.Error(), + "security; file '/app/patches/patchAddProbe.yaml' is not in or below '/app/prod'") { + t.Fatalf("unexpected error: %v", err) + } +} + +const prodDevMergeResult1 = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + containers: + - image: my-image + livenessProbe: + httpGet: + path: /healthz + port: 8080 + name: my-deployment + dnsPolicy: ClusterFirst +` + +const prodDevMergeResult2 = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment +spec: + template: + spec: + containers: + - image: my-image + name: my-deployment + dnsPolicy: ClusterFirst + restartPolicy: Always +` + +// This test does what +// TestIssue1251_Patches_ProdVsDev_Failure +// failed to do, because this test does the equivalent +// os specifying `--load_restrictor none` on the build. +// +// This allows the use patch files located outside the +// kustomization root, and not in a kustomization +// themselves. +// +// Doing so means the kustomization using them is no +// longer relocatable, not addressible via a git URL, +// and not git clonable. It's no longer self-contained. +// +// Likewise suppressing load restrictions happens for +// the entire build (i.e. everything can reach outside +// the kustomization root), opening the user to whatever +// threat the load restrictor was meant to address. +func TestIssue1251_Patches_ProdVsDev(t *testing.T) { + th := kusttest_test.NewKustTestNoLoadRestrictorHarness(t, "/app/prod") + definePatchDirStructure(th) + + th.WriteK("/app/prod", ` +resources: +- ../base +patchesStrategicMerge: +- ../patches/patchAddProbe.yaml +- ../patches/patchDnsPolicy.yaml +`) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + th.AssertActualEqualsExpected(m, prodDevMergeResult1) + + th = kusttest_test.NewKustTestNoLoadRestrictorHarness(t, "/app/dev") + definePatchDirStructure(th) + + th.WriteK("/app/dev", ` +resources: +- ../base +patchesStrategicMerge: +- ../patches/patchDnsPolicy.yaml +- ../patches/patchRestartPolicy.yaml +`) + + m, err = th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, prodDevMergeResult2) +} + +func TestIssue1251_Plugins_ProdVsDev(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app/prod") + defineTransformerDirStructure(th) + th.WriteK("/app/prod", ` +resources: +- ../base +transformers: +- ../patches/addProbe +- ../patches/addDnsPolicy +`) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, prodDevMergeResult1) + + th = kusttest_test.NewKustTestPluginHarness(t, "/app/dev") + defineTransformerDirStructure(th) + th.WriteK("/app/dev", ` +resources: +- ../base +transformers: +- ../patches/addRestartPolicy +- ../patches/addDnsPolicy +`) + + m, err = th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, prodDevMergeResult2) +} + +func TestIssue1251_Plugins_Local(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app/composite") + writeDeploymentBase(th) + + writeJsonTransformerPluginConfig( + th, "/app/composite", "addDnsPolicy", patchJsonDnsPolicy) + writeJsonTransformerPluginConfig( + th, "/app/composite", "addRestartPolicy", patchJsonRestartPolicy) + writeJsonTransformerPluginConfig( + th, "/app/composite", "addProbe", patchJsonAddProbe) + + th.WriteK("/app/composite", ` +resources: +- ../base +transformers: +- addDnsPolicyConfig.yaml +- addRestartPolicyConfig.yaml +- addProbeConfig.yaml +`) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, expectedPatchedDeployment) +} + +func writeJsonTransformerPluginConfig( + th *kusttest_test.KustTestHarness, path, name, patch string) { + th.WriteF(filepath.Join(path, name+"Config.yaml"), + fmt.Sprintf(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: %s +target: + group: apps + version: v1 + kind: Deployment + name: my-deployment +jsonOp: '%s' +`, name, patch)) +} + +// Remote in the sense that they are bundled in a different kustomization. +func TestIssue1251_Plugins_Bundled(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app/composite") + writeDeploymentBase(th) + + th.WriteK("/app/patches", ` +resources: +- addDnsPolicyConfig.yaml +- addRestartPolicyConfig.yaml +- addProbeConfig.yaml +`) + writeJsonTransformerPluginConfig( + th, "/app/patches", "addDnsPolicy", patchJsonDnsPolicy) + writeJsonTransformerPluginConfig( + th, "/app/patches", "addRestartPolicy", patchJsonRestartPolicy) + writeJsonTransformerPluginConfig( + th, "/app/patches", "addProbe", patchJsonAddProbe) + + th.WriteK("/app/composite", ` +resources: +- ../base +transformers: +- ../patches +`) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, expectedPatchedDeployment) +} + +func defineTransformerDirStructure(th *kusttest_test.KustTestHarness) { + writeDeploymentBase(th) + + th.WriteK("/app/patches/addDnsPolicy", ` +resources: +- addDnsPolicyConfig.yaml +`) + writeJsonTransformerPluginConfig( + th, "/app/patches/addDnsPolicy", "addDnsPolicy", patchJsonDnsPolicy) + + th.WriteK("/app/patches/addRestartPolicy", ` +resources: +- addRestartPolicyConfig.yaml +`) + writeJsonTransformerPluginConfig( + th, "/app/patches/addRestartPolicy", "addRestartPolicy", patchJsonRestartPolicy) + + th.WriteK("/app/patches/addProbe", ` +resources: +- addProbeConfig.yaml +`) + writeJsonTransformerPluginConfig( + th, "/app/patches/addProbe", "addProbe", patchJsonAddProbe) +} diff --git a/pkg/target/diamonds_test.go b/pkg/target/diamonds_test.go new file mode 100644 index 000000000..be6755365 --- /dev/null +++ b/pkg/target/diamonds_test.go @@ -0,0 +1,224 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package target_test + +import ( + kusttest_test "sigs.k8s.io/kustomize/v3/pkg/kusttest" + "testing" +) + +// Here's a structure of two kustomizations, +// `dev` and `prod`, individually deployable, +// that depend on a diamond that combines +// multiple tenants (kirk, spock and bones), +// each sharing a common base. +// +// The objects used are contrived to avoid +// clouding the example with authentic +// but verbose Deployment boilerplate. +// +// Patches are applied at various levels, +// requiring more specificity as needed. +// +// dev prod +// \ / +// tenants +// / | \ +// kirk spock bones +// \ | / +// base +// +func writeDiamondBase(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/base", ` +resources: +- deploy.yaml +`) + th.WriteF("/app/base/deploy.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + numReplicas: 1 +`) +} + +func writeKirk(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/kirk", ` +namePrefix: kirk- +resources: +- ../base +- configmap.yaml +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/kirk/dep-patch.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + type: Confident +`) + th.WriteF("/app/kirk/configmap.yaml", ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: settings +data: + phaser: caress +`) +} + +func writeSpock(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/spock", ` +namePrefix: spock- +resources: +- ../base +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/spock/dep-patch.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + type: Logical +`) +} + +func writeBones(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/bones", ` +namePrefix: bones- +resources: +- ../base +patchesStrategicMerge: +- dep-patch.yaml +`) + th.WriteF("/app/bones/dep-patch.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + type: Concerned +`) +} + +func writeTenants(th *kusttest_test.KustTestHarness) { + th.WriteK("/app/tenants", ` +namePrefix: t- +resources: +- ../kirk +- ../spock +- ../bones +- configMap.yaml +patchesStrategicMerge: +- bones-patch.yaml +`) + th.WriteF("/app/tenants/bones-patch.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: bones-storefront +spec: + mood: Cantankerous +`) + th.WriteF("/app/tenants/configMap.yaml", ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: federation +data: + zone: neutral + guardian: forever +`) +} + +func TestBasicDiamond(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/prod") + writeDiamondBase(th) + writeKirk(th) + writeSpock(th) + writeBones(th) + writeTenants(th) + th.WriteK("/app/prod", ` +namePrefix: prod- +resources: +- ../tenants +patchesStrategicMerge: +- patches.yaml +`) + // The patch only has to be specific enough + // to match the item. + th.WriteF("/app/prod/patches.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: t-kirk-storefront +spec: + numReplicas: 10000 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: federation +data: + guardian: ofTheGalaxy +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: t-federation +data: + zone: twilight +`) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: prod-t-kirk-storefront +spec: + numReplicas: 10000 + type: Confident +--- +apiVersion: v1 +data: + phaser: caress +kind: ConfigMap +metadata: + name: prod-t-kirk-settings +--- +apiVersion: v1 +kind: Deployment +metadata: + name: prod-t-spock-storefront +spec: + numReplicas: 1 + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: prod-t-bones-storefront +spec: + mood: Cantankerous + numReplicas: 1 + type: Concerned +--- +apiVersion: v1 +data: + guardian: ofTheGalaxy + zone: twilight +kind: ConfigMap +metadata: + name: prod-t-federation +`) +} diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index 3005acee7..42215cc92 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -183,9 +183,8 @@ func (kt *KustTarget) computeInventory( p := builtin.NewInventoryTransformerPlugin() var c struct { - Policy string - Name string - Namespace string + Policy string + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"` } c.Name = inv.ConfigMap.Name c.Namespace = inv.ConfigMap.Namespace diff --git a/pkg/target/kusttarget_configplugin.go b/pkg/target/kusttarget_configplugin.go index 1cf9e143f..f3a8d76cb 100644 --- a/pkg/target/kusttarget_configplugin.go +++ b/pkg/target/kusttarget_configplugin.go @@ -65,11 +65,11 @@ func (kt *KustTarget) configureBuiltinTransformers( configurators := []transformerConfigurator{ kt.configureBuiltinNamespaceTransformer, kt.configureBuiltinNameTransformer, - kt.configureBuiltinImageTagTransformer, kt.configureBuiltinLabelTransformer, kt.configureBuiltinAnnotationsTransformer, kt.configureBuiltinPatchJson6902Transformer, kt.configureBuiltinReplicaCountTransformer, + kt.configureBuiltinImageTagTransformer, } var result []transformers.Transformer for _, f := range configurators { @@ -129,8 +129,8 @@ func (kt *KustTarget) configureBuiltinNamespaceTransformer( tConfig *config.TransformerConfig) ( result []transformers.Transformer, err error) { var c struct { - Namespace string - FieldSpecs []config.FieldSpec + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"` + FieldSpecs []config.FieldSpec } c.Namespace = kt.kustomization.Namespace c.FieldSpecs = tConfig.NameSpace @@ -147,15 +147,21 @@ func (kt *KustTarget) configureBuiltinPatchJson6902Transformer( tConfig *config.TransformerConfig) ( result []transformers.Transformer, err error) { var c struct { - Patches []types.PatchJson6902 + Target types.PatchTarget `json:"target,omitempty" yaml:"target,omitempty"` + Path string `json:"path,omitempty" yaml:"path,omitempty"` + JsonOp string `json:"jsonOp,omitempty" yaml:"jsonOp,omitempty"` } - c.Patches = kt.kustomization.PatchesJson6902 - p := builtin.NewPatchJson6902TransformerPlugin() - err = kt.configureBuiltinPlugin(p, c, "patchJson6902") - if err != nil { - return nil, err + for _, args := range kt.kustomization.PatchesJson6902 { + c.Target = *args.Target + c.Path = args.Path + c.JsonOp = "" // Not implemented for kustomization file yet. + p := builtin.NewPatchJson6902TransformerPlugin() + err = kt.configureBuiltinPlugin(p, c, "patchJson6902") + if err != nil { + return nil, err + } + result = append(result, p) } - result = append(result, p) return } diff --git a/pkg/target/namespaces_test.go b/pkg/target/namespaces_test.go new file mode 100644 index 000000000..084221947 --- /dev/null +++ b/pkg/target/namespaces_test.go @@ -0,0 +1,69 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package target_test + +import ( + "sigs.k8s.io/kustomize/v3/pkg/kusttest" + "strings" + "testing" +) + +func TestNamespacedSecrets(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app") + + th.WriteF("/app/secrets.yaml", ` +apiVersion: v1 +kind: Secret +metadata: + name: dummy + namespace: default +type: Opaque +data: + dummy: "" +--- +apiVersion: v1 +kind: Secret +metadata: + name: dummy + namespace: kube-system +type: Opaque +data: + dummy: "" +`) + + // This should find the proper secret. + th.WriteF("/app/role.yaml", ` +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: dummy +rules: +- apiGroups: [""] + resources: ["secrets"] + resourceNames: ["dummy"] + verbs: ["get"] +`) + + th.WriteK("/app", ` +resources: +- secrets.yaml +- role.yaml +`) + + _, err := th.MakeKustTarget().MakeCustomizedResMap() + // TODO: Fix #1044 + // This should not be an error - + // the secrets have the same name but are in different namespaces. + // The ClusterRole (by def) is not in a namespace, + // an in this case applies to *any* Secret resource + // named "dummy" + if err == nil { + t.Fatalf("unexpected lack of error") + } + if !strings.Contains( + err.Error(), + "slice case - multiple matches for ~G_v1_Secret|default|dummy") { + t.Fatalf("unexpected error: %s", err) + } +} diff --git a/pkg/target/transformersimage_test.go b/pkg/target/transformersimage_test.go index 900900117..a77f00d7f 100644 --- a/pkg/target/transformersimage_test.go +++ b/pkg/target/transformersimage_test.go @@ -106,6 +106,90 @@ spec3: `) } +func TestIssue1281_JsonPatchAndImageTag(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app") + + th.WriteK("/app", ` +resources: +- deployment.yaml + +images: +- name: abbott + newTag: v2 +- name: costello + newTag: v8 + +patchesJson6902: +- target: + group: apps + version: v1 + kind: Deployment + name: ben + path: patch.json +`) + th.WriteF("/app/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: ben +spec: + template: + spec: + dnsPolicy: "None" + containers: + - name: awesome + image: abbott +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: alice +spec: + template: + spec: + containers: + - name: tomato + image: abbott +`) + + th.WriteF("/app/patch.json", ` +[ {"op": "add", + "path": "/spec/replica", "value": "3"}, + {"op": "replace", + "path": "/spec/template/spec/containers/0", + "value": { "image": "costello" } } ] +`) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: ben +spec: + replica: "3" + template: + spec: + containers: + - image: costello:v8 + dnsPolicy: None +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: alice +spec: + template: + spec: + containers: + - image: abbott:v2 + name: tomato +`) +} + func TestTransfomersImageDefaultConfig(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/app/base") makeTransfomersImageBase(th) diff --git a/pkg/types/kustomization.go b/pkg/types/kustomization.go index 08487230d..0d46fd90d 100644 --- a/pkg/types/kustomization.go +++ b/pkg/types/kustomization.go @@ -1,18 +1,5 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 // Package types holds struct definitions that should find a better home. package types @@ -29,13 +16,18 @@ const ( KustomizationKind = "Kustomization" ) -// TypeMeta copies apimachinery/pkg/apis/meta/v1.TypeMeta +// TypeMeta partially copies apimachinery/pkg/apis/meta/v1.TypeMeta +// No need for a direct dependence; the fields are stable. type TypeMeta struct { - // Kind copies apimachinery/pkg/apis/meta/v1.Typemeta.Kind - Kind string `json:"kind,omitempty" protobuf:"bytes,1,opt,name=kind"` + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + APIVersion string `json:"apiVersion,omitempty" yaml:"apiversion,omitempty"` +} - // APIVersion copies apimachinery/pkg/apis/meta/v1.Typemeta.APIVersion - APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,2,opt,name=apiVersion"` +// ObjectMeta partially copies apimachinery/pkg/apis/meta/v1.ObjectMeta +// No need for a direct dependence; the fields are stable. +type ObjectMeta struct { + Name string `json:"name,omitempty" yaml:"name,omitempty"` + Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } // Kustomization holds the information needed to generate customized k8s api resources. @@ -394,13 +386,13 @@ type Patch struct { Patch string `json:"patch,omitempty" yaml:"patch,omitempty"` // Target points to the resources that the patch is applied to - Target Targets `json:"target,omitempty" yaml:"target,omitempty"` + Target Selector `json:"target,omitempty" yaml:"target,omitempty"` } -// Targets specifies a set of resources. +// Selector specifies a set of resources. // Any resource that matches intersection of all conditions // is included in this set. -type Targets struct { +type Selector struct { gvk.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` Name string `json:"name,omitempty" yaml:"name,omitempty"` diff --git a/plugin/builtin/ConfigMapGenerator.go b/plugin/builtin/ConfigMapGenerator.go index b737a4eda..5bb0c1eb6 100644 --- a/plugin/builtin/ConfigMapGenerator.go +++ b/plugin/builtin/ConfigMapGenerator.go @@ -9,8 +9,9 @@ import ( ) type ConfigMapGeneratorPlugin struct { - ldr ifc.Loader - rf *resmap.Factory + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` types.GeneratorOptions types.ConfigMapArgs } @@ -25,6 +26,12 @@ func (p *ConfigMapGeneratorPlugin) Config( p.GeneratorOptions = types.GeneratorOptions{} p.ConfigMapArgs = types.ConfigMapArgs{} err = yaml.Unmarshal(config, p) + if p.ConfigMapArgs.Name == "" { + p.ConfigMapArgs.Name = p.Name + } + if p.ConfigMapArgs.Namespace == "" { + p.ConfigMapArgs.Namespace = p.Namespace + } p.ldr = ldr p.rf = rf return diff --git a/plugin/builtin/InventoryTransformer.go b/plugin/builtin/InventoryTransformer.go index fdc455108..93419be94 100644 --- a/plugin/builtin/InventoryTransformer.go +++ b/plugin/builtin/InventoryTransformer.go @@ -16,11 +16,10 @@ import ( ) type InventoryTransformerPlugin struct { - ldr ifc.Loader - rf *resmap.Factory - Policy string `json:"policy,omitempty" yaml:"policy,omitempty"` - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + Policy string `json:"policy,omitempty" yaml:"policy,omitempty"` } //noinspection GoUnusedGlobalVariable diff --git a/plugin/builtin/NamespaceTransformer.go b/plugin/builtin/NamespaceTransformer.go index 1a47bb842..dd45a6a7f 100644 --- a/plugin/builtin/NamespaceTransformer.go +++ b/plugin/builtin/NamespaceTransformer.go @@ -9,13 +9,14 @@ import ( "sigs.k8s.io/kustomize/v3/pkg/resource" "sigs.k8s.io/kustomize/v3/pkg/transformers" "sigs.k8s.io/kustomize/v3/pkg/transformers/config" + "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/yaml" ) // Change or set the namespace of non-cluster level resources. type NamespaceTransformerPlugin struct { - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` - FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } //noinspection GoUnusedGlobalVariable diff --git a/plugin/builtin/PatchJson6902Transformer.go b/plugin/builtin/PatchJson6902Transformer.go index fc8214020..40f162900 100644 --- a/plugin/builtin/PatchJson6902Transformer.go +++ b/plugin/builtin/PatchJson6902Transformer.go @@ -2,16 +2,23 @@ package builtin import ( + "fmt" + jsonpatch "github.com/evanphx/json-patch" + "github.com/pkg/errors" + "sigs.k8s.io/kustomize/v3/pkg/gvk" "sigs.k8s.io/kustomize/v3/pkg/ifc" - "sigs.k8s.io/kustomize/v3/pkg/patch/transformer" + "sigs.k8s.io/kustomize/v3/pkg/resid" "sigs.k8s.io/kustomize/v3/pkg/resmap" "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/yaml" ) type PatchJson6902TransformerPlugin struct { - ldr ifc.Loader - Patches []types.PatchJson6902 `json:"patches,omitempty" yaml:"patches,omitempty"` + ldr ifc.Loader + decodedPatch jsonpatch.Patch + Target types.PatchTarget `json:"target,omitempty" yaml:"target,omitempty"` + Path string `json:"path,omitempty" yaml:"path,omitempty"` + JsonOp string `json:"jsonOp,omitempty" yaml:"jsonOp,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -22,15 +29,71 @@ func NewPatchJson6902TransformerPlugin() *PatchJson6902TransformerPlugin { func (p *PatchJson6902TransformerPlugin) Config( ldr ifc.Loader, rf *resmap.Factory, c []byte) (err error) { p.ldr = ldr - p.Patches = nil - return yaml.Unmarshal(c, p) -} - -func (p *PatchJson6902TransformerPlugin) Transform(m resmap.ResMap) error { - t, err := transformer.NewPatchJson6902Factory(p.ldr). - MakePatchJson6902Transformer(p.Patches) + err = yaml.Unmarshal(c, p) if err != nil { return err } - return t.Transform(m) + if p.Target.Name == "" { + return fmt.Errorf("must specify the target name") + } + if p.Path == "" && p.JsonOp == "" { + return fmt.Errorf("empty file path and empty jsonOp") + } + if p.Path != "" { + if p.JsonOp != "" { + return fmt.Errorf("must specify a file path or jsonOp, not both") + } + rawOp, err := p.ldr.Load(p.Path) + if err != nil { + return err + } + p.JsonOp = string(rawOp) + if p.JsonOp == "" { + return fmt.Errorf("patch file '%s' empty seems to be empty", p.Path) + } + } + if p.JsonOp[0] != '[' { + // if it doesn't seem to be JSON, imagine + // it is YAML, and convert to JSON. + op, err := yaml.YAMLToJSON([]byte(p.JsonOp)) + if err != nil { + return err + } + p.JsonOp = string(op) + } + p.decodedPatch, err = jsonpatch.DecodePatch([]byte(p.JsonOp)) + if err != nil { + return errors.Wrapf(err, "decoding %s", p.JsonOp) + } + if len(p.decodedPatch) == 0 { + return fmt.Errorf( + "patch appears to be empty; file=%s, JsonOp=%s", p.Path, p.JsonOp) + } + return err +} + +func (p *PatchJson6902TransformerPlugin) Transform(m resmap.ResMap) error { + id := resid.NewResIdWithNamespace( + gvk.Gvk{ + Group: p.Target.Group, + Version: p.Target.Version, + Kind: p.Target.Kind, + }, + p.Target.Name, + p.Target.Namespace, + ) + obj, err := m.GetById(id) + if err != nil { + return err + } + rawObj, err := obj.MarshalJSON() + if err != nil { + return err + } + modifiedObj, err := p.decodedPatch.Apply(rawObj) + if err != nil { + return errors.Wrapf( + err, "failed to apply json patch '%s'", p.JsonOp) + } + return obj.UnmarshalJSON(modifiedObj) } diff --git a/plugin/builtin/SecretGenerator.go b/plugin/builtin/SecretGenerator.go index 48910e830..ce954f5b4 100644 --- a/plugin/builtin/SecretGenerator.go +++ b/plugin/builtin/SecretGenerator.go @@ -9,8 +9,9 @@ import ( ) type SecretGeneratorPlugin struct { - ldr ifc.Loader - rf *resmap.Factory + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` types.GeneratorOptions types.SecretArgs } @@ -25,6 +26,12 @@ func (p *SecretGeneratorPlugin) Config( p.GeneratorOptions = types.GeneratorOptions{} p.SecretArgs = types.SecretArgs{} err = yaml.Unmarshal(config, p) + if p.SecretArgs.Name == "" { + p.SecretArgs.Name = p.Name + } + if p.SecretArgs.Namespace == "" { + p.SecretArgs.Namespace = p.Namespace + } p.ldr = ldr p.rf = rf return diff --git a/plugin/builtin/configmapgenerator/ConfigMapGenerator.go b/plugin/builtin/configmapgenerator/ConfigMapGenerator.go index 721f4bb9f..596cf6da9 100644 --- a/plugin/builtin/configmapgenerator/ConfigMapGenerator.go +++ b/plugin/builtin/configmapgenerator/ConfigMapGenerator.go @@ -12,8 +12,9 @@ import ( ) type plugin struct { - ldr ifc.Loader - rf *resmap.Factory + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` types.GeneratorOptions types.ConfigMapArgs } @@ -26,6 +27,12 @@ func (p *plugin) Config( p.GeneratorOptions = types.GeneratorOptions{} p.ConfigMapArgs = types.ConfigMapArgs{} err = yaml.Unmarshal(config, p) + if p.ConfigMapArgs.Name == "" { + p.ConfigMapArgs.Name = p.Name + } + if p.ConfigMapArgs.Namespace == "" { + p.ConfigMapArgs.Namespace = p.Namespace + } p.ldr = ldr p.rf = rf return diff --git a/plugin/builtin/configmapgenerator/ConfigMapGenerator_test.go b/plugin/builtin/configmapgenerator/ConfigMapGenerator_test.go index 2848e1b50..95d0ab844 100644 --- a/plugin/builtin/configmapgenerator/ConfigMapGenerator_test.go +++ b/plugin/builtin/configmapgenerator/ConfigMapGenerator_test.go @@ -30,8 +30,7 @@ COLOR=red apiVersion: builtin kind: ConfigMapGenerator metadata: - name: myMapGen -name: myMap + name: myMap envs: - devops.env - uxteam.env diff --git a/plugin/builtin/inventorytransformer/InventoryTransformer.go b/plugin/builtin/inventorytransformer/InventoryTransformer.go index a5e0cb892..69f90432b 100644 --- a/plugin/builtin/inventorytransformer/InventoryTransformer.go +++ b/plugin/builtin/inventorytransformer/InventoryTransformer.go @@ -19,11 +19,10 @@ import ( ) type plugin struct { - ldr ifc.Loader - rf *resmap.Factory - Policy string `json:"policy,omitempty" yaml:"policy,omitempty"` - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + Policy string `json:"policy,omitempty" yaml:"policy,omitempty"` } //noinspection GoUnusedGlobalVariable diff --git a/plugin/builtin/inventorytransformer/InventoryTransformer_test.go b/plugin/builtin/inventorytransformer/InventoryTransformer_test.go index a73692e81..df3be48f9 100644 --- a/plugin/builtin/inventorytransformer/InventoryTransformer_test.go +++ b/plugin/builtin/inventorytransformer/InventoryTransformer_test.go @@ -71,10 +71,9 @@ func TestInventoryTransformerCollect(t *testing.T) { apiVersion: builtin kind: InventoryTransformer metadata: - name: notImportantHere + name: pruneCM + namespace: default policy: GarbageCollect -name: pruneCM -namespace: default `, content) th.AssertActualEqualsExpected(rm, inv) @@ -93,10 +92,9 @@ func TestInventoryTransformerIgnore(t *testing.T) { apiVersion: builtin kind: InventoryTransformer metadata: - name: notImportantHere + name: pruneCM + namespace: default policy: GarbageIgnore -name: pruneCM -namespace: default `, content) th.AssertActualEqualsExpected(rm, content+"---"+inv) @@ -115,9 +113,8 @@ func TestInventoryTransformerDefaultPolicy(t *testing.T) { apiVersion: builtin kind: InventoryTransformer metadata: - name: notImportantHere -name: pruneCM -namespace: default + name: pruneCM + namespace: default `, content) th.AssertActualEqualsExpected(rm, content+"---"+inv) diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index 4f2f4c1ad..eb0e6732a 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -12,13 +12,14 @@ import ( "sigs.k8s.io/kustomize/v3/pkg/resource" "sigs.k8s.io/kustomize/v3/pkg/transformers" "sigs.k8s.io/kustomize/v3/pkg/transformers/config" + "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/yaml" ) // Change or set the namespace of non-cluster level resources. type plugin struct { - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` - FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } //noinspection GoUnusedGlobalVariable diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index 4aa1b608f..9a38f66a4 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go @@ -24,7 +24,7 @@ apiVersion: builtin kind: NamespaceTransformer metadata: name: notImportantHere -namespace: test + namespace: test fieldSpecs: - path: metadata/namespace create: true @@ -166,7 +166,7 @@ apiVersion: builtin kind: NamespaceTransformer metadata: name: notImportantHere -namespace: test + namespace: test fieldSpecs: - path: metadata/namespace create: true diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go index b22562980..e21e88d11 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go @@ -5,16 +5,23 @@ package main import ( + "fmt" + jsonpatch "github.com/evanphx/json-patch" + "github.com/pkg/errors" + "sigs.k8s.io/kustomize/v3/pkg/gvk" "sigs.k8s.io/kustomize/v3/pkg/ifc" - "sigs.k8s.io/kustomize/v3/pkg/patch/transformer" + "sigs.k8s.io/kustomize/v3/pkg/resid" "sigs.k8s.io/kustomize/v3/pkg/resmap" "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/yaml" ) type plugin struct { - ldr ifc.Loader - Patches []types.PatchJson6902 `json:"patches,omitempty" yaml:"patches,omitempty"` + ldr ifc.Loader + decodedPatch jsonpatch.Patch + Target types.PatchTarget `json:"target,omitempty" yaml:"target,omitempty"` + Path string `json:"path,omitempty" yaml:"path,omitempty"` + JsonOp string `json:"jsonOp,omitempty" yaml:"jsonOp,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -23,15 +30,71 @@ var KustomizePlugin plugin func (p *plugin) Config( ldr ifc.Loader, rf *resmap.Factory, c []byte) (err error) { p.ldr = ldr - p.Patches = nil - return yaml.Unmarshal(c, p) -} - -func (p *plugin) Transform(m resmap.ResMap) error { - t, err := transformer.NewPatchJson6902Factory(p.ldr). - MakePatchJson6902Transformer(p.Patches) + err = yaml.Unmarshal(c, p) if err != nil { return err } - return t.Transform(m) + if p.Target.Name == "" { + return fmt.Errorf("must specify the target name") + } + if p.Path == "" && p.JsonOp == "" { + return fmt.Errorf("empty file path and empty jsonOp") + } + if p.Path != "" { + if p.JsonOp != "" { + return fmt.Errorf("must specify a file path or jsonOp, not both") + } + rawOp, err := p.ldr.Load(p.Path) + if err != nil { + return err + } + p.JsonOp = string(rawOp) + if p.JsonOp == "" { + return fmt.Errorf("patch file '%s' empty seems to be empty", p.Path) + } + } + if p.JsonOp[0] != '[' { + // if it doesn't seem to be JSON, imagine + // it is YAML, and convert to JSON. + op, err := yaml.YAMLToJSON([]byte(p.JsonOp)) + if err != nil { + return err + } + p.JsonOp = string(op) + } + p.decodedPatch, err = jsonpatch.DecodePatch([]byte(p.JsonOp)) + if err != nil { + return errors.Wrapf(err, "decoding %s", p.JsonOp) + } + if len(p.decodedPatch) == 0 { + return fmt.Errorf( + "patch appears to be empty; file=%s, JsonOp=%s", p.Path, p.JsonOp) + } + return err +} + +func (p *plugin) Transform(m resmap.ResMap) error { + id := resid.NewResIdWithNamespace( + gvk.Gvk{ + Group: p.Target.Group, + Version: p.Target.Version, + Kind: p.Target.Kind, + }, + p.Target.Name, + p.Target.Namespace, + ) + obj, err := m.GetById(id) + if err != nil { + return err + } + rawObj, err := obj.MarshalJSON() + if err != nil { + return err + } + modifiedObj, err := p.decodedPatch.Apply(rawObj) + if err != nil { + return errors.Wrapf( + err, "failed to apply json patch '%s'", p.JsonOp) + } + return obj.UnmarshalJSON(modifiedObj) } diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer_test.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer_test.go index 8ebdba540..6e820c13b 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer_test.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer_test.go @@ -4,13 +4,117 @@ package main_test import ( + "strings" "testing" "sigs.k8s.io/kustomize/v3/pkg/kusttest" "sigs.k8s.io/kustomize/v3/pkg/plugins" ) -func TestPatchJson6902Transformer(t *testing.T) { +const target = ` +apiVersion: apps/v1 +metadata: + name: myDeploy +kind: Deployment +spec: + replica: 2 + template: + metadata: + labels: + old-label: old-value + spec: + containers: + - name: nginx + image: nginx +` + +func TestPatchJson6902TransformerMissingFile(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + _, err := th.RunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +path: jsonpatch.json +`, target) + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), "cannot read file \"/app/jsonpatch.json\"") { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestBadPatchJson6902Transformer(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + _, err := th.RunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +jsonOp: 'thisIsNotAPatch' +`, target) + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), "cannot unmarshal string into Go value of type jsonpatch") { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestBothEmptyJson6902Transformer(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + _, err := th.RunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +`, target) + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), "empty file path and empty jsonOp") { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestBothSpecifiedJson6902Transformer(t *testing.T) { tc := plugins.NewEnvForTest(t).Set() defer tc.Reset() @@ -20,7 +124,45 @@ func TestPatchJson6902Transformer(t *testing.T) { th := kusttest_test.NewKustTestPluginHarness(t, "/app") th.WriteF("/app/jsonpatch.json", `[ - {"op": "add", "path": "/spec/replica", "value": "3"} +{"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, +{"op": "add", "path": "/spec/replica", "value": "999"}, +{"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} +]`) + + _, err := th.RunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +path: jsonpatch.json +jsonOp: '[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "ClusterFirst"}]' +`, target) + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), "must specify a file path or jsonOp, not both") { + t.Fatalf("unexpected err: %v", err) + } +} + +func TestPatchJson6902TransformerFromJsonFile(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + th.WriteF("/app/jsonpatch.json", `[ +{"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, +{"op": "add", "path": "/spec/replica", "value": "999"}, +{"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} ]`) rm := th.LoadAndRunTransformer(` @@ -28,21 +170,13 @@ apiVersion: builtin kind: PatchJson6902Transformer metadata: name: notImportantHere -patches: -- target: - group: apps - version: v1 - kind: Deployment - name: myDeploy - path: jsonpatch.json -`, ` -apiVersion: apps/v1 -metadata: +target: + group: apps + version: v1 + kind: Deployment name: myDeploy -kind: Deployment -spec: - replica: 1 -`) +path: jsonpatch.json +`, target) th.AssertActualEqualsExpected(rm, ` apiVersion: apps/v1 @@ -50,6 +184,109 @@ kind: Deployment metadata: name: myDeploy spec: - replica: "3" + replica: "999" + template: + metadata: + labels: + old-label: old-value + spec: + containers: + - command: + - arg1 + - arg2 + - arg3 + image: nginx + name: my-nginx +`) +} + +func TestPatchJson6902TransformerFromYamlFile(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + th.WriteF("/app/jsonpatch.json", ` +- op: add + path: /spec/template/spec/containers/0/command + value: ["arg1", "arg2", "arg3"] +`) + + rm := th.LoadAndRunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +path: jsonpatch.json +`, target) + + th.AssertActualEqualsExpected(rm, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: myDeploy +spec: + replica: 2 + template: + metadata: + labels: + old-label: old-value + spec: + containers: + - command: + - arg1 + - arg2 + - arg3 + image: nginx + name: nginx +`) +} + +func TestPatchJson6902TransformerWithInline(t *testing.T) { + tc := plugins.NewEnvForTest(t).Set() + defer tc.Reset() + + tc.BuildGoPlugin( + "builtin", "", "PatchJson6902Transformer") + + th := kusttest_test.NewKustTestPluginHarness(t, "/app") + + rm := th.LoadAndRunTransformer(` +apiVersion: builtin +kind: PatchJson6902Transformer +metadata: + name: notImportantHere +target: + group: apps + version: v1 + kind: Deployment + name: myDeploy +jsonOp: '[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "ClusterFirst"}]' +`, target) + + th.AssertActualEqualsExpected(rm, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: myDeploy +spec: + replica: 2 + template: + metadata: + labels: + old-label: old-value + spec: + containers: + - image: nginx + name: nginx + dnsPolicy: ClusterFirst `) } diff --git a/plugin/builtin/secretgenerator/SecretGenerator.go b/plugin/builtin/secretgenerator/SecretGenerator.go index 61c5b5020..1dd7e2995 100644 --- a/plugin/builtin/secretgenerator/SecretGenerator.go +++ b/plugin/builtin/secretgenerator/SecretGenerator.go @@ -12,8 +12,9 @@ import ( ) type plugin struct { - ldr ifc.Loader - rf *resmap.Factory + ldr ifc.Loader + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` types.GeneratorOptions types.SecretArgs } @@ -26,6 +27,12 @@ func (p *plugin) Config( p.GeneratorOptions = types.GeneratorOptions{} p.SecretArgs = types.SecretArgs{} err = yaml.Unmarshal(config, p) + if p.SecretArgs.Name == "" { + p.SecretArgs.Name = p.Name + } + if p.SecretArgs.Namespace == "" { + p.SecretArgs.Namespace = p.Namespace + } p.ldr = ldr p.rf = rf return diff --git a/plugin/builtin/secretgenerator/SecretGenerator_test.go b/plugin/builtin/secretgenerator/SecretGenerator_test.go index 78c450c90..eb62bcb41 100644 --- a/plugin/builtin/secretgenerator/SecretGenerator_test.go +++ b/plugin/builtin/secretgenerator/SecretGenerator_test.go @@ -34,9 +34,8 @@ consectetur adipiscing elit. apiVersion: builtin kind: SecretGenerator metadata: - name: exampleSecGen -name: mySecret -namespace: whatever + name: mySecret + namespace: whatever behavior: merge envs: - a.env diff --git a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go index 30afb574d..49d3031c0 100644 --- a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go +++ b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go @@ -13,10 +13,9 @@ import ( // A secret generator example that gets data // from a database (simulated by a hardcoded map). type plugin struct { - rf *resmap.Factory - ldr ifc.Loader - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` + rf *resmap.Factory + ldr ifc.Loader + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` // List of keys to use in database lookups Keys []string `json:"keys,omitempty" yaml:"keys,omitempty"` } diff --git a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase_test.go b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase_test.go index 8d87bc4b9..1730910d2 100644 --- a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase_test.go +++ b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase_test.go @@ -23,9 +23,8 @@ func TestSecretsFromDatabasePlugin(t *testing.T) { apiVersion: someteam.example.com/v1 kind: SecretsFromDatabase metadata: - name: mySecretGenerator -name: forbiddenValues -namespace: production + name: forbiddenValues + namespace: production keys: - ROCKET - VEGETABLE diff --git a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go index 3df992053..12ff51884 100644 --- a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go +++ b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go @@ -9,14 +9,15 @@ import ( "sigs.k8s.io/kustomize/v3/pkg/ifc" "sigs.k8s.io/kustomize/v3/pkg/resmap" + "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/yaml" ) // A simple generator example. Makes one service. type plugin struct { - rf *resmap.Factory - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Port string `json:"port,omitempty" yaml:"port,omitempty"` + rf *resmap.Factory + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + Port string `json:"port,omitempty" yaml:"port,omitempty"` } //nolint: golint @@ -30,6 +31,7 @@ metadata: labels: app: dev name: {{.Name}} + namespace: {{.Namespace}} spec: ports: - port: {{.Port}} diff --git a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator_test.go b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator_test.go index 91a0d0bba..a7413c374 100644 --- a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator_test.go +++ b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator_test.go @@ -23,8 +23,8 @@ func TestSomeServiceGeneratorPlugin(t *testing.T) { apiVersion: someteam.example.com/v1 kind: SomeServiceGenerator metadata: - name: myGenerator -name: my-service + name: my-service + namespace: test port: "12345" `) th.AssertActualEqualsExpected(m, ` @@ -34,6 +34,7 @@ metadata: labels: app: dev name: my-service + namespace: test spec: ports: - port: 12345 diff --git a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go index e33712bac..e34d620b1 100644 --- a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go +++ b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/kustomize/v3/pkg/resmap" "sigs.k8s.io/kustomize/v3/pkg/transformers" "sigs.k8s.io/kustomize/v3/pkg/transformers/config" + "sigs.k8s.io/kustomize/v3/pkg/types" "sigs.k8s.io/kustomize/v3/plugin/builtin" "sigs.k8s.io/yaml" ) @@ -16,12 +17,8 @@ import ( // Add a string prefix to the name. // A plugin that adapts another plugin. type plugin struct { - Metadata metaData `json:"metadata,omitempty" yaml:"metadata,omitempty"` - t transformers.Transformer -} - -type metaData struct { - Name string `json:"name,omitempty" yaml:"name,omitempty"` + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + t transformers.Transformer } //nolint: golint @@ -47,7 +44,7 @@ func (p *plugin) Config( if err != nil { return err } - c, err = p.makePrefixSuffixPluginConfig(p.Metadata.Name) + c, err = p.makePrefixSuffixPluginConfig(p.Name) if err != nil { return err }