From c8c8246342a1c15c5d9b4cad7efa7866854c0131 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 3 Feb 2021 23:12:18 +0100 Subject: [PATCH 1/2] Optimize Kustomize post renderer * Use constants for APIVersion and Kind in Kustomization config * Remove redundant JSON marshal of strategic merge patches * Factor out Kustomization build wrapper and add notes about settings Signed-off-by: Hidde Beydals --- internal/runner/post_renderer_kustomize.go | 57 ++++++++++++++-------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/internal/runner/post_renderer_kustomize.go b/internal/runner/post_renderer_kustomize.go index cc43ec0b1..8f164ad30 100644 --- a/internal/runner/post_renderer_kustomize.go +++ b/internal/runner/post_renderer_kustomize.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/krusty" + "sigs.k8s.io/kustomize/api/resmap" kustypes "sigs.k8s.io/kustomize/api/types" "github.com/fluxcd/pkg/apis/kustomize" @@ -91,35 +92,25 @@ func adaptSelector(selector *kustomize.Selector) (output *kustypes.Selector) { } func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedManifests *bytes.Buffer, err error) { - buildOptions := &krusty.Options{ - UseKyaml: false, - DoLegacyResourceSort: true, - LoadRestrictions: kustypes.LoadRestrictionsNone, - AddManagedbyLabel: false, - DoPrune: false, - PluginConfig: konfig.DisabledPluginConfig(), - AllowResourceIdChanges: false, - } fs := filesys.MakeFsInMemory() cfg := kustypes.Kustomization{} - cfg.APIVersion = "kustomize.config.k8s.io/v1beta1" - cfg.Kind = "Kustomization" + cfg.APIVersion = kustypes.KustomizationVersion + cfg.Kind = kustypes.KustomizationKind cfg.Images = adaptImages(k.spec.Images) - // add rendered Helm output as input resource to the Kustomization. + + // Add rendered Helm output as input resource to the Kustomization. const input = "helm-output.yaml" cfg.Resources = append(cfg.Resources, input) if err := writeFile(fs, input, renderedManifests); err != nil { return nil, err } - // add strategic merge patches + + // Add strategic merge patches. for _, m := range k.spec.PatchesStrategicMerge { - patch, err := json.Marshal(m) - if err != nil { - return nil, err - } - cfg.PatchesStrategicMerge = append(cfg.PatchesStrategicMerge, kustypes.PatchStrategicMerge(patch)) + cfg.PatchesStrategicMerge = append(cfg.PatchesStrategicMerge, kustypes.PatchStrategicMerge(m.Raw)) } - // add JSON patches + + // Add JSON 6902 patches. for _, m := range k.spec.PatchesJSON6902 { patch, err := json.Marshal(m.Patch) if err != nil { @@ -130,6 +121,8 @@ func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedMa Target: adaptSelector(&m.Target), }) } + + // Write kustomization config to file. kustomization, err := json.Marshal(cfg) if err != nil { return nil, err @@ -137,8 +130,7 @@ func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedMa if err := writeToFile(fs, "kustomization.yaml", kustomization); err != nil { return nil, err } - kustomizer := krusty.MakeKustomizer(fs, buildOptions) - resMap, err := kustomizer.Run(".") + resMap, err := buildKustomization(fs, ".") if err != nil { return nil, err } @@ -148,3 +140,26 @@ func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedMa } return bytes.NewBuffer(yaml), nil } + +// buildKustomization wraps krusty.MakeKustomizer with the following settings: +// - disable kyaml due to critical bugs like: +// - https://github.com/kubernetes-sigs/kustomize/issues/3446 +// - https://github.com/kubernetes-sigs/kustomize/issues/3480 +// - reorder the resources just before output (Namespaces and Cluster roles/role bindings first, CRDs before CRs, Webhooks last) +// - load files from outside the kustomization.yaml root +// - disable plugins except for the builtin ones +// - prohibit changes to resourceIds, patch name/kind don't overwrite target name/kind +func buildKustomization(fs filesys.FileSystem, dirPath string) (resmap.ResMap, error) { + buildOptions := &krusty.Options{ + UseKyaml: false, + DoLegacyResourceSort: true, + LoadRestrictions: kustypes.LoadRestrictionsNone, + AddManagedbyLabel: false, + DoPrune: false, + PluginConfig: konfig.DisabledPluginConfig(), + AllowResourceIdChanges: false, + } + + k := krusty.MakeKustomizer(fs, buildOptions) + return k.Run(dirPath) +} From dcf0d93e8470747f24c1ca920671dff6909a648d Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 4 Feb 2021 09:23:54 +0100 Subject: [PATCH 2/2] Add tests for Kustomize post renderer Signed-off-by: Hidde Beydals --- controllers/helmrelease_types_test.go | 82 ------- .../runner/post_renderer_kustomize_test.go | 224 ++++++++++++++++++ 2 files changed, 224 insertions(+), 82 deletions(-) delete mode 100644 controllers/helmrelease_types_test.go create mode 100644 internal/runner/post_renderer_kustomize_test.go diff --git a/controllers/helmrelease_types_test.go b/controllers/helmrelease_types_test.go deleted file mode 100644 index ac2d7d65a..000000000 --- a/controllers/helmrelease_types_test.go +++ /dev/null @@ -1,82 +0,0 @@ -/* -Copyright 2021 The Flux 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 controllers - -import ( - "encoding/json" - "testing" - - "github.com/fluxcd/pkg/apis/kustomize" - - v2 "github.com/fluxcd/helm-controller/api/v2beta1" -) - -func TestHelmReleaseTypes_unmarshal_PatchJSON6902(t *testing.T) { - var p kustomize.JSON6902Patch - err := json.Unmarshal([]byte(`{"target": {"namespace": "ns", "name": "x", "kind": "k", "version": "v"},"patch": [{"op": "add", "path": "/some/new/path", "value": "value"}]}`), &p) - if err != nil { - t.Error(err) - } - if p.Target.Kind != "k" { - t.Logf("Invalid Kind: epected 'k' got %s", p.Target.Kind) - t.Fail() - } - if p.Target.Version != "v" { - t.Logf("Invalid Version: epected 'v' got %s", p.Target.Version) - t.Fail() - } - if p.Target.Name != "x" { - t.Logf("Invalid Name: epected 'x got %s", p.Target.Name) - t.Fail() - } - if p.Target.Namespace != "ns" { - t.Logf("Invalid Namespace: epected 'ns' got %s", p.Target.Namespace) - t.Fail() - } - if len(p.Patch) != 1 { - t.Logf("Failed to unmarshal Patch: got %s", p.Patch) - t.Fail() - } -} - -// Ensure the generic JSON fields are unmarshaled. -func TestHelmReleaseTypes_unmarshal_Kustomize(t *testing.T) { - var p v2.Kustomize - err := json.Unmarshal([]byte(`{"patchesStrategicMerge": [{"apiVersion": "apps/v1", "kind": "Deployment", "metadata": {"name": "test"}}]}`), &p) - if err != nil { - t.Error(err) - } - if len(p.PatchesStrategicMerge) != 1 { - t.Logf("Failed to unmarshal PatchesStrategicMerge: got %s", p.PatchesStrategicMerge) - t.Fail() - } else { - sm := p.PatchesStrategicMerge[0] - s, err := json.Marshal(sm) - if err != nil { - t.Error(err) - } - var m map[string]interface{} - err = json.Unmarshal(s, &m) - if err != nil { - t.Error(err) - } - if m["apiVersion"] != "apps/v1" { - t.Logf("expected 'apps/v1' got %s", m["apiVersion"]) - t.Fail() - } - } -} diff --git a/internal/runner/post_renderer_kustomize_test.go b/internal/runner/post_renderer_kustomize_test.go new file mode 100644 index 000000000..65add38ab --- /dev/null +++ b/internal/runner/post_renderer_kustomize_test.go @@ -0,0 +1,224 @@ +/* +Copyright 2021 The Flux 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 runner + +import ( + "bytes" + "encoding/json" + "reflect" + "testing" + + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "sigs.k8s.io/yaml" + + "github.com/fluxcd/pkg/apis/kustomize" + + v2 "github.com/fluxcd/helm-controller/api/v2beta1" +) + +const replaceImageMock = `apiVersion: v1 +kind: Pod +metadata: + name: image +spec: + containers: + - image: repository/image:tag +` + +const json6902Mock = `apiVersion: v1 +kind: Pod +metadata: + annotations: + c: foo + name: json6902 +` + +const strategicMergeMock = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + template: + spec: + containers: + - name: nginx + image: nignx:v1.0.0 +` + +func Test_postRendererKustomize_Run(t *testing.T) { + tests := []struct { + name string + renderedManifests string + patchesStrategicMerge string + patchesJson6902 string + images string + expectManifests string + expectErr bool + }{ + { + name: "image tag", + renderedManifests: replaceImageMock, + images: ` +- name: repository/image + newTag: 0.1.0 +`, + expectManifests: `apiVersion: v1 +kind: Pod +metadata: + name: image +spec: + containers: + - image: repository/image:0.1.0 +`, + }, + { + name: "image name", + renderedManifests: replaceImageMock, + images: ` +- name: repository/image + newName: repository/new-image +`, + expectManifests: `apiVersion: v1 +kind: Pod +metadata: + name: image +spec: + containers: + - image: repository/new-image:tag +`, + }, + { + name: "image digest", + renderedManifests: replaceImageMock, + images: ` +- name: repository/image + digest: sha256:24a0c4b4a4c0eb97a1aabb8e29f18e917d05abfe1b7a7c07857230879ce7d3d3 +`, + expectManifests: `apiVersion: v1 +kind: Pod +metadata: + name: image +spec: + containers: + - image: repository/image@sha256:24a0c4b4a4c0eb97a1aabb8e29f18e917d05abfe1b7a7c07857230879ce7d3d3 +`, + }, + { + name: "json 6902", + renderedManifests: json6902Mock, + patchesJson6902: ` +- target: + version: v1 + kind: Pod + name: json6902 + patch: + - op: test + path: /metadata/annotations/c + value: foo + - op: remove + path: /metadata/annotations/c + - op: add + path: /metadata/annotations/c + value: [ "foo", "bar" ] + - op: replace + path: /metadata/annotations/c + value: 42 + - op: move + from: /metadata/annotations/c + path: /metadata/annotations/d + - op: copy + from: /metadata/annotations/d + path: /metadata/annotations/e +`, + expectManifests: `apiVersion: v1 +kind: Pod +metadata: + annotations: + d: 42 + e: 42 + name: json6902 +`, + }, + { + name: "strategic merge test", + renderedManifests: strategicMergeMock, + patchesStrategicMerge: ` +- apiVersion: apps/v1 + kind: Deployment + metadata: + name: nginx + spec: + template: + spec: + containers: + - name: nginx + image: nignx:latest +`, + expectManifests: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + template: + spec: + containers: + - image: nignx:latest + name: nginx +`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + spec, err := mockKustomize(tt.patchesStrategicMerge, tt.patchesJson6902, tt.images) + k := &postRendererKustomize{ + spec: spec, + } + gotModifiedManifests, err := k.Run(bytes.NewBufferString(tt.renderedManifests)) + if (err != nil) != tt.expectErr { + t.Errorf("Run() error = %v, expectErr %v", err, tt.expectErr) + return + } + if !reflect.DeepEqual(gotModifiedManifests, bytes.NewBufferString(tt.expectManifests)) { + t.Errorf("Run() gotModifiedManifests = %v, want %v", gotModifiedManifests, tt.expectManifests) + } + }) + } +} + +func mockKustomize(patchesStrategicMerge, patchesJson6902, images string) (*v2.Kustomize, error) { + b, err := yaml.YAMLToJSON([]byte(patchesStrategicMerge)) + if err != nil { + return nil, err + } + var strategicMerge []v1.JSON + if err := json.Unmarshal(b, &strategicMerge); err != nil { + return nil, err + } + var json6902 []kustomize.JSON6902Patch + if err := yaml.Unmarshal([]byte(patchesJson6902), &json6902); err != nil { + return nil, err + } + var imgs []kustomize.Image + if err := yaml.Unmarshal([]byte(images), &imgs); err != nil { + return nil, err + } + return &v2.Kustomize{ + PatchesStrategicMerge: strategicMerge, + PatchesJSON6902: json6902, + Images: imgs, + }, nil +}