From cedb4695ed8e31081625289c7dafc315ff9c3780 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Fri, 21 Jan 2022 10:59:10 +0200 Subject: [PATCH] Fix preflight validation Validate that the resources built with kustomize conform to the Kubernetes API conventions before passing them to the server-side apply engine. Signed-off-by: Stefan Prodan --- controllers/kustomization_controller.go | 7 + controllers/kustomization_decryptor_test.go | 12 +- controllers/kustomization_validation_test.go | 142 ++++++++++++++++++ .../testdata/invalid/overlay/deployment.yaml | 29 ++++ .../invalid/overlay/kustomization.yaml | 6 + .../testdata/invalid/plain/deployment.yaml | 29 ++++ go.mod | 2 +- go.sum | 4 +- 8 files changed, 225 insertions(+), 6 deletions(-) create mode 100644 controllers/kustomization_validation_test.go create mode 100644 controllers/testdata/invalid/overlay/deployment.yaml create mode 100644 controllers/testdata/invalid/overlay/kustomization.yaml create mode 100644 controllers/testdata/invalid/plain/deployment.yaml diff --git a/controllers/kustomization_controller.go b/controllers/kustomization_controller.go index 7f728c71e..60daf0e57 100644 --- a/controllers/kustomization_controller.go +++ b/controllers/kustomization_controller.go @@ -360,6 +360,8 @@ func (r *KustomizationReconciler) reconcile( ), err } + fmt.Println(string(resources)) + // convert the build result into Kubernetes unstructured objects objects, err := ssa.ReadObjects(bytes.NewReader(resources)) if err != nil { @@ -631,6 +633,11 @@ func (r *KustomizationReconciler) build(ctx context.Context, kustomization kusto } for _, res := range m.Resources() { + // check if resource conforms to the Kubernetes API conventions + if res.GetName() == "" || res.GetKind() == "" || res.GetApiVersion() == "" { + return nil, fmt.Errorf("failed to decode Kubernetes apiVersion, kind and name from: %v", res.String()) + } + // check if resources are encrypted and decrypt them before generating the final YAML if kustomization.Spec.Decryption != nil { outRes, err := dec.Decrypt(res) diff --git a/controllers/kustomization_decryptor_test.go b/controllers/kustomization_decryptor_test.go index 300006aaa..d7888a6a6 100644 --- a/controllers/kustomization_decryptor_test.go +++ b/controllers/kustomization_decryptor_test.go @@ -123,7 +123,7 @@ func TestKustomizationReconciler_Decryptor(t *testing.T) { Namespace: kustomizationKey.Namespace, }, Spec: kustomizev1.KustomizationSpec{ - Interval: metav1.Duration{Duration: reconciliationInterval}, + Interval: metav1.Duration{Duration: 2 * time.Minute}, Path: "./", KubeConfig: &kustomizev1.KubeConfig{ SecretRef: meta.LocalObjectReference{ @@ -146,6 +146,12 @@ func TestKustomizationReconciler_Decryptor(t *testing.T) { } g.Expect(k8sClient.Create(context.TODO(), kustomization)).To(Succeed()) + g.Eventually(func() bool { + var obj kustomizev1.Kustomization + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(kustomization), &obj) + return obj.Status.LastAppliedRevision == "main/"+artifactChecksum + }, timeout, time.Second).Should(BeTrue()) + overlayKustomizationName := fmt.Sprintf("sops-%s", randStringRunes(5)) overlayKs := kustomization.DeepCopy() overlayKs.ResourceVersion = "" @@ -158,8 +164,8 @@ func TestKustomizationReconciler_Decryptor(t *testing.T) { g.Eventually(func() bool { var obj kustomizev1.Kustomization - _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(kustomization), &obj) - return obj.Status.LastAppliedRevision == "main/"+artifactChecksum + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(overlayKs), &obj) + return obj.Status.LastAppliedRevision == "main/"+overlayChecksum }, timeout, time.Second).Should(BeTrue()) t.Run("decrypts SOPS secrets", func(t *testing.T) { diff --git a/controllers/kustomization_validation_test.go b/controllers/kustomization_validation_test.go new file mode 100644 index 000000000..503167865 --- /dev/null +++ b/controllers/kustomization_validation_test.go @@ -0,0 +1,142 @@ +/* +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 ( + "context" + "fmt" + "testing" + "time" + + kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta2" + "github.com/fluxcd/pkg/apis/meta" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestKustomizationReconciler_Validation(t *testing.T) { + g := NewWithT(t) + + id := "val-" + randStringRunes(5) + + err := createNamespace(id) + g.Expect(err).NotTo(HaveOccurred(), "failed to create test namespace") + + err = createKubeConfigSecret(id) + g.Expect(err).NotTo(HaveOccurred(), "failed to create kubeconfig secret") + + artifactName := "val-" + randStringRunes(5) + artifactChecksum, err := createArtifact(testServer, "testdata/invalid/plain", artifactName) + g.Expect(err).ToNot(HaveOccurred()) + + overlayArtifactName := "val-" + randStringRunes(5) + overlayChecksum, err := createArtifact(testServer, "testdata/invalid/overlay", overlayArtifactName) + g.Expect(err).ToNot(HaveOccurred()) + + repositoryName := types.NamespacedName{ + Name: fmt.Sprintf("val-%s", randStringRunes(5)), + Namespace: id, + } + + overlayRepositoryName := types.NamespacedName{ + Name: fmt.Sprintf("val-%s", randStringRunes(5)), + Namespace: id, + } + + err = applyGitRepository(repositoryName, artifactName, "main/"+artifactChecksum) + g.Expect(err).NotTo(HaveOccurred()) + + err = applyGitRepository(overlayRepositoryName, overlayArtifactName, "main/"+overlayChecksum) + g.Expect(err).NotTo(HaveOccurred()) + + kustomizationKey := types.NamespacedName{ + Name: fmt.Sprintf("val-%s", randStringRunes(5)), + Namespace: id, + } + kustomization := &kustomizev1.Kustomization{ + ObjectMeta: metav1.ObjectMeta{ + Name: kustomizationKey.Name, + Namespace: kustomizationKey.Namespace, + }, + Spec: kustomizev1.KustomizationSpec{ + Interval: metav1.Duration{Duration: 2 * time.Minute}, + Path: "./", + KubeConfig: &kustomizev1.KubeConfig{ + SecretRef: meta.LocalObjectReference{ + Name: "kubeconfig", + }, + }, + SourceRef: kustomizev1.CrossNamespaceSourceReference{ + Name: repositoryName.Name, + Namespace: repositoryName.Namespace, + Kind: sourcev1.GitRepositoryKind, + }, + TargetNamespace: id, + }, + } + g.Expect(k8sClient.Create(context.TODO(), kustomization)).To(Succeed()) + + g.Eventually(func() bool { + var obj kustomizev1.Kustomization + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(kustomization), &obj) + return obj.Status.LastAttemptedRevision == "main/"+artifactChecksum + }, timeout, time.Second).Should(BeTrue()) + + overlayKustomizationName := fmt.Sprintf("val-%s", randStringRunes(5)) + overlayKs := kustomization.DeepCopy() + overlayKs.ResourceVersion = "" + overlayKs.Name = overlayKustomizationName + overlayKs.Spec.SourceRef.Name = overlayRepositoryName.Name + overlayKs.Spec.SourceRef.Namespace = overlayRepositoryName.Namespace + + g.Expect(k8sClient.Create(context.TODO(), overlayKs)).To(Succeed()) + + g.Eventually(func() bool { + var obj kustomizev1.Kustomization + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(overlayKs), &obj) + return obj.Status.LastAttemptedRevision == "main/"+overlayChecksum + }, timeout, time.Second).Should(BeTrue()) + + t.Run("fails to build invalid plain yamls", func(t *testing.T) { + var resultK kustomizev1.Kustomization + g.Eventually(func() bool { + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(kustomization), &resultK) + for _, c := range resultK.Status.Conditions { + if c.Reason == kustomizev1.BuildFailedReason { + return true + } + } + return false + }, timeout, interval).Should(BeTrue()) + }) + + t.Run("fails to build invalid overlay", func(t *testing.T) { + var resultK kustomizev1.Kustomization + g.Eventually(func() bool { + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(overlayKs), &resultK) + for _, c := range resultK.Status.Conditions { + if c.Reason == kustomizev1.BuildFailedReason { + return true + } + } + return false + }, timeout, interval).Should(BeTrue()) + }) +} diff --git a/controllers/testdata/invalid/overlay/deployment.yaml b/controllers/testdata/invalid/overlay/deployment.yaml new file mode 100644 index 000000000..224aeab05 --- /dev/null +++ b/controllers/testdata/invalid/overlay/deployment.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: podinfo + labels: + app: podinfo +spec: + replicas: 1 + selector: + matchLabels: + app: podinfo + template: + metadata: + labels: + app: podinfo + spec: + containers: + - name: podinfo + image: podinfo +--- +piVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: crossplane-provider-aws1 +spec: + package: crossplane/provider-aws:v0.23.0 + controllerConfigRef: + name: provider-aws diff --git a/controllers/testdata/invalid/overlay/kustomization.yaml b/controllers/testdata/invalid/overlay/kustomization.yaml new file mode 100644 index 000000000..a41bdf0e0 --- /dev/null +++ b/controllers/testdata/invalid/overlay/kustomization.yaml @@ -0,0 +1,6 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - deployment.yaml + + diff --git a/controllers/testdata/invalid/plain/deployment.yaml b/controllers/testdata/invalid/plain/deployment.yaml new file mode 100644 index 000000000..224aeab05 --- /dev/null +++ b/controllers/testdata/invalid/plain/deployment.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: podinfo + labels: + app: podinfo +spec: + replicas: 1 + selector: + matchLabels: + app: podinfo + template: + metadata: + labels: + app: podinfo + spec: + containers: + - name: podinfo + image: podinfo +--- +piVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: crossplane-provider-aws1 +spec: + package: crossplane/provider-aws:v0.23.0 + controllerConfigRef: + name: provider-aws diff --git a/go.mod b/go.mod index 85756fdcd..deeadcb9c 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/fluxcd/pkg/apis/kustomize v0.3.1 github.com/fluxcd/pkg/apis/meta v0.10.2 github.com/fluxcd/pkg/runtime v0.12.3 - github.com/fluxcd/pkg/ssa v0.10.0 + github.com/fluxcd/pkg/ssa v0.11.1 github.com/fluxcd/pkg/testserver v0.2.0 github.com/fluxcd/pkg/untar v0.1.0 github.com/fluxcd/source-controller/api v0.20.1 diff --git a/go.sum b/go.sum index d157381de..566645c39 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,8 @@ github.com/fluxcd/pkg/apis/meta v0.10.2 h1:pnDBBEvfs4HaKiVAYgz+e/AQ8dLvcgmVfSeBr github.com/fluxcd/pkg/apis/meta v0.10.2/go.mod h1:KQ2er9xa6koy7uoPMZjIjNudB5p4tXs+w0GO6fRcy7I= github.com/fluxcd/pkg/runtime v0.12.3 h1:h21AZ3YG5MAP7DxFF9hfKrP+vFzys2L7CkUbPFjbP/0= github.com/fluxcd/pkg/runtime v0.12.3/go.mod h1:imJ2xYy/d4PbSinX2IefmZk+iS2c1P5fY0js8mCE4SM= -github.com/fluxcd/pkg/ssa v0.10.0 h1:dhgWDeqz0/zAs5guzmPx/DMPCkzZdlEiPvCs1NChAQM= -github.com/fluxcd/pkg/ssa v0.10.0/go.mod h1:S+qig7BTOxop0c134y8Yv8/iQST4Kt7S2xXiFkP4VMA= +github.com/fluxcd/pkg/ssa v0.11.1 h1:iZMMe6Pdgt/sv3pZPJ5y4oRDa+8IXHbpPYgpjEmaq/8= +github.com/fluxcd/pkg/ssa v0.11.1/go.mod h1:S+qig7BTOxop0c134y8Yv8/iQST4Kt7S2xXiFkP4VMA= github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4= github.com/fluxcd/pkg/testserver v0.2.0/go.mod h1:bgjjydkXsZTeFzjz9Cr4heGANr41uTB1Aj1Q5qzuYVk= github.com/fluxcd/pkg/untar v0.1.0 h1:k97V/xV5hFrAkIkVPuv5AVhyxh1ZzzAKba/lbDfGo6o=