From a3d280298cf08d090da741e2f6d0ee17b09e9d5f Mon Sep 17 00:00:00 2001 From: Shivam Mukhade Date: Wed, 15 Sep 2021 09:51:50 +0530 Subject: [PATCH] Cleanup: moves common code for components to common pkg Signed-off-by: Shivam Mukhade --- .../test-fetch-version-from-crds.yaml | 42 ++++++++++ pkg/reconciler/common/utils.go | 55 ++++++++++++ pkg/reconciler/common/utils_test.go | 84 +++++++++++++++++++ .../kubernetes/tektonpipeline/controller.go | 18 +--- .../tektonpipeline/tektonpipeline.go | 16 +--- .../kubernetes/tektontrigger/controller.go | 18 +--- .../kubernetes/tektontrigger/tektontrigger.go | 16 +--- 7 files changed, 187 insertions(+), 62 deletions(-) create mode 100644 pkg/reconciler/common/testdata/test-fetch-version-from-crds.yaml create mode 100644 pkg/reconciler/common/utils.go create mode 100644 pkg/reconciler/common/utils_test.go diff --git a/pkg/reconciler/common/testdata/test-fetch-version-from-crds.yaml b/pkg/reconciler/common/testdata/test-fetch-version-from-crds.yaml new file mode 100644 index 0000000000..982538b886 --- /dev/null +++ b/pkg/reconciler/common/testdata/test-fetch-version-from-crds.yaml @@ -0,0 +1,42 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: tasks.tekton.dev + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-pipelines + pipeline.tekton.dev/release: "devel" + version: "devel" +spec: + group: tekton.dev + preserveUnknownFields: false + names: + kind: Task + plural: tasks + categories: + - tekton + - tekton-pipelines + scope: Namespaced + versions: [] + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: pipelines.tekton.dev + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-pipelines + pipeline.tekton.dev/release: "devel" + version: "devel" +spec: + group: tekton.dev + preserveUnknownFields: false + names: + kind: Pipeline + plural: pipelines + categories: + - tekton + - tekton-pipelines + scope: Namespaced + versions: [] \ No newline at end of file diff --git a/pkg/reconciler/common/utils.go b/pkg/reconciler/common/utils.go new file mode 100644 index 0000000000..9e9ff80b0f --- /dev/null +++ b/pkg/reconciler/common/utils.go @@ -0,0 +1,55 @@ +/* +Copyright 2021 The Tekton 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 common + +import ( + "crypto/sha256" + "encoding/json" + "fmt" + + mf "github.com/manifestival/manifestival" +) + +// FetchVersionFromCRD finds the component version from the crd labels, it looks for the +// label on the first crd it finds after filtering +// It will return error if crds are not found in manifest or label is not found +func FetchVersionFromCRD(manifest mf.Manifest, releaseLabel string) (string, error) { + crds := manifest.Filter(mf.CRDs) + if len(crds.Resources()) == 0 { + return "", fmt.Errorf("failed to find crds to get release version") + } + + crd := crds.Resources()[0] + version, ok := crd.GetLabels()[releaseLabel] + if !ok { + return version, fmt.Errorf("failed to find release label on crd") + } + + return version, nil +} + +// ComputeHashOf generates an unique hash/string for the +// object pass to it. +func ComputeHashOf(obj interface{}) (string, error) { + h := sha256.New() + d, err := json.Marshal(obj) + if err != nil { + return "", err + } + h.Write(d) + return fmt.Sprintf("%x", h.Sum(nil)), nil +} diff --git a/pkg/reconciler/common/utils_test.go b/pkg/reconciler/common/utils_test.go new file mode 100644 index 0000000000..24221805b0 --- /dev/null +++ b/pkg/reconciler/common/utils_test.go @@ -0,0 +1,84 @@ +/* +Copyright 2021 The Tekton 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 common + +import ( + "path" + "testing" + + mf "github.com/manifestival/manifestival" + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" +) + +func TestFetchVersionFromCRD(t *testing.T) { + + testData := path.Join("testdata", "test-fetch-version-from-crds.yaml") + manifest, err := mf.ManifestFrom(mf.Recursive(testData)) + assertNoEror(t, err) + + version, err := FetchVersionFromCRD(manifest, "pipeline.tekton.dev/release") + if err != nil { + t.Fatal("Unexpected Error: ", err) + } + + if version != "devel" { + t.Fatal("invalid label fetched from crd: ", version) + } +} + +func TestComputeHashOf(t *testing.T) { + tp := &v1alpha1.TektonPipeline{ + Spec: v1alpha1.TektonPipelineSpec{ + CommonSpec: v1alpha1.CommonSpec{TargetNamespace: "tekton"}, + Config: v1alpha1.Config{ + NodeSelector: map[string]string{ + "abc": "xyz", + }, + }, + }, + } + + hash, err := ComputeHashOf(tp.Spec) + if err != nil { + t.Fatal("unexpected error while computing hash of obj") + } + + // Again, calculate the hash without changing object + + hash2, err := ComputeHashOf(tp.Spec) + if err != nil { + t.Fatal("unexpected error while computing hash of obj") + } + + if hash != hash2 { + t.Fatal("hash changed without changing the object") + } + + // Now, change the object + + tp.Spec.TargetNamespace = "changed" + + hash3, err := ComputeHashOf(tp.Spec) + if err != nil { + t.Fatal("unexpected error while computing hash of obj") + } + + // Hash should be changed now + if hash == hash3 { + t.Fatal("hash not changed after changing the object") + } +} diff --git a/pkg/reconciler/kubernetes/tektonpipeline/controller.go b/pkg/reconciler/kubernetes/tektonpipeline/controller.go index e2639512da..116cff6e1a 100644 --- a/pkg/reconciler/kubernetes/tektonpipeline/controller.go +++ b/pkg/reconciler/kubernetes/tektonpipeline/controller.go @@ -18,7 +18,6 @@ package tektonpipeline import ( "context" - "fmt" "os" "path/filepath" @@ -68,7 +67,7 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro } // Read the release version of pipelines - releaseVersion, err := fetchVersion(manifest) + releaseVersion, err := common.FetchVersionFromCRD(manifest, releaseLabel) if err != nil { logger.Fatalw("failed to read release version from manifest", err) } @@ -113,18 +112,3 @@ func addProxy(manifest *mf.Manifest) error { proxyLocation := filepath.Join(koDataDir, "webhook") return common.AppendManifest(manifest, proxyLocation) } - -func fetchVersion(manifest mf.Manifest) (string, error) { - crds := manifest.Filter(mf.CRDs) - if len(crds.Resources()) == 0 { - return "", fmt.Errorf("failed to find crds to get release version") - } - - crd := crds.Resources()[0] - version, ok := crd.GetLabels()[releaseLabel] - if !ok { - return version, fmt.Errorf("failed to find release label on crd") - } - - return version, nil -} diff --git a/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go b/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go index ee25721e4c..727a8fccdd 100644 --- a/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go +++ b/pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go @@ -18,8 +18,6 @@ package tektonpipeline import ( "context" - "crypto/sha256" - "encoding/json" "fmt" "time" @@ -172,7 +170,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel // TektonInstallerSet with computing new hash of TektonPipeline Spec // Hash of TektonPipeline Spec - expectedSpecHash, err := computeHashOf(tp.Spec) + expectedSpecHash, err := common.ComputeHashOf(tp.Spec) if err != nil { return err } @@ -255,7 +253,7 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tp *v1alpha1.Tekton // in further reconciliation we compute hash of tp spec and check with // annotation, if they are same then we skip updating the object // otherwise we update the manifest - specHash, err := computeHashOf(tp.Spec) + specHash, err := common.ComputeHashOf(tp.Spec) if err != nil { return err } @@ -324,13 +322,3 @@ func (r *Reconciler) transform(ctx context.Context, manifest *mf.Manifest, comp trns = append(trns, extra...) return common.Transform(ctx, manifest, instance, trns...) } - -func computeHashOf(obj interface{}) (string, error) { - h := sha256.New() - d, err := json.Marshal(obj) - if err != nil { - return "", err - } - h.Write(d) - return fmt.Sprintf("%x", h.Sum(nil)), nil -} diff --git a/pkg/reconciler/kubernetes/tektontrigger/controller.go b/pkg/reconciler/kubernetes/tektontrigger/controller.go index 576b4fb685..c9393e5b73 100644 --- a/pkg/reconciler/kubernetes/tektontrigger/controller.go +++ b/pkg/reconciler/kubernetes/tektontrigger/controller.go @@ -18,7 +18,6 @@ package tektontrigger import ( "context" - "fmt" "github.com/go-logr/zapr" mfc "github.com/manifestival/client-go-client" @@ -68,7 +67,7 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro } // Read the release version of pipelines - releaseVersion, err := fetchVersion(manifest) + releaseVersion, err := common.FetchVersionFromCRD(manifest, releaseLabel) if err != nil { logger.Fatalw("failed to read release version from manifest", err) } @@ -104,18 +103,3 @@ func fetchSourceManifests(ctx context.Context, manifest *mf.Manifest) error { var trigger *v1alpha1.TektonTrigger return common.AppendTarget(ctx, manifest, trigger) } - -func fetchVersion(manifest mf.Manifest) (string, error) { - crds := manifest.Filter(mf.CRDs) - if len(crds.Resources()) == 0 { - return "", fmt.Errorf("failed to find crds to get release version") - } - - crd := crds.Resources()[0] - version, ok := crd.GetLabels()[releaseLabel] - if !ok { - return version, fmt.Errorf("failed to find release label on crd") - } - - return version, nil -} diff --git a/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go b/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go index 72b55eba26..7ebf4c2cf3 100644 --- a/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go +++ b/pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go @@ -18,8 +18,6 @@ package tektontrigger import ( "context" - "crypto/sha256" - "encoding/json" "fmt" "time" @@ -186,7 +184,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tt *v1alpha1.TektonTrigg // TektonInstallerSet with computing new hash of TektonTrigger Spec // Hash of TektonPipeline Spec - expectedSpecHash, err := computeHashOf(tt.Spec) + expectedSpecHash, err := common.ComputeHashOf(tt.Spec) if err != nil { return err } @@ -289,7 +287,7 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tt *v1alpha1.Tekton // in further reconciliation we compute hash of tt spec and check with // annotation, if they are same then we skip updating the object // otherwise we update the manifest - specHash, err := computeHashOf(tt.Spec) + specHash, err := common.ComputeHashOf(tt.Spec) if err != nil { return err } @@ -338,13 +336,3 @@ func makeInstallerSet(tt *v1alpha1.TektonTrigger, manifest mf.Manifest, ttSpecHa }, } } - -func computeHashOf(obj interface{}) (string, error) { - h := sha256.New() - d, err := json.Marshal(obj) - if err != nil { - return "", err - } - h.Write(d) - return fmt.Sprintf("%x", h.Sum(nil)), nil -}