From ba2d2e7714498b5743017f2d52af2aef877ad016 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Tue, 1 Dec 2020 11:53:03 +0100 Subject: [PATCH] pkg/apis: remove duplicate MergeStepsWithStepTemplate function Signed-off-by: Vincent Demeester --- pkg/apis/pipeline/v1alpha1/merge.go | 92 ------------ pkg/apis/pipeline/v1alpha1/merge_test.go | 138 ------------------ pkg/apis/pipeline/v1alpha1/task_validation.go | 2 +- 3 files changed, 1 insertion(+), 231 deletions(-) delete mode 100644 pkg/apis/pipeline/v1alpha1/merge.go delete mode 100644 pkg/apis/pipeline/v1alpha1/merge_test.go diff --git a/pkg/apis/pipeline/v1alpha1/merge.go b/pkg/apis/pipeline/v1alpha1/merge.go deleted file mode 100644 index 2384bd041de..00000000000 --- a/pkg/apis/pipeline/v1alpha1/merge.go +++ /dev/null @@ -1,92 +0,0 @@ -/* -Copyright 2019 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 v1alpha1 - -import ( - "encoding/json" - - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/strategicpatch" -) - -// MergeStepsWithStepTemplate takes a possibly nil container template and a -// list of steps, merging each of the steps with the container template, if -// it's not nil, and returning the resulting list. -// Deprecated -func MergeStepsWithStepTemplate(template *v1.Container, steps []Step) ([]Step, error) { - if template == nil { - return steps, nil - } - - // We need JSON bytes to generate a patch to merge the step containers - // onto the template container, so marshal the template. - templateAsJSON, err := json.Marshal(template) - if err != nil { - return nil, err - } - // We need to do a three-way merge to actually merge the template and - // step containers, so we need an empty container as the "original" - emptyAsJSON, err := json.Marshal(&v1.Container{}) - if err != nil { - return nil, err - } - - for i, s := range steps { - // Marshal the step's to JSON - stepAsJSON, err := json.Marshal(s.Container) - if err != nil { - return nil, err - } - - // Get the patch meta for Container, which is needed for generating and applying the merge patch. - patchSchema, err := strategicpatch.NewPatchMetaFromStruct(template) - - if err != nil { - return nil, err - } - - // Create a merge patch, with the empty JSON as the original, the step JSON as the modified, and the template - // JSON as the current - this lets us do a deep merge of the template and step containers, with awareness of - // the "patchMerge" tags. - patch, err := strategicpatch.CreateThreeWayMergePatch(emptyAsJSON, stepAsJSON, templateAsJSON, patchSchema, true) - if err != nil { - return nil, err - } - - // Actually apply the merge patch to the template JSON. - mergedAsJSON, err := strategicpatch.StrategicMergePatchUsingLookupPatchMeta(templateAsJSON, patch, patchSchema) - if err != nil { - return nil, err - } - - // Unmarshal the merged JSON to a Container pointer, and return it. - merged := &v1.Container{} - err = json.Unmarshal(mergedAsJSON, merged) - if err != nil { - return nil, err - } - - // If the container's args is nil, reset it to empty instead - if merged.Args == nil && s.Args != nil { - merged.Args = []string{} - } - - // Pass through original step Script, for later conversion. - steps[i] = Step{Container: *merged, Script: s.Script} - } - return steps, nil -} diff --git a/pkg/apis/pipeline/v1alpha1/merge_test.go b/pkg/apis/pipeline/v1alpha1/merge_test.go deleted file mode 100644 index 23f41367d30..00000000000 --- a/pkg/apis/pipeline/v1alpha1/merge_test.go +++ /dev/null @@ -1,138 +0,0 @@ -/* -Copyright 2019 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 v1alpha1 - -import ( - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/tektoncd/pipeline/test/diff" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -func TestMergeStepsWithStepTemplate(t *testing.T) { - resourceQuantityCmp := cmp.Comparer(func(x, y resource.Quantity) bool { - return x.Cmp(y) == 0 - }) - - for _, tc := range []struct { - name string - template *corev1.Container - steps []Step - expected []Step - }{{ - name: "nil-template", - template: nil, - steps: []Step{{Container: corev1.Container{ - Image: "some-image", - }}}, - expected: []Step{{Container: corev1.Container{ - Image: "some-image", - }}}, - }, { - name: "not-overlapping", - template: &corev1.Container{ - Command: []string{"/somecmd"}, - }, - steps: []Step{{Container: corev1.Container{ - Image: "some-image", - }}}, - expected: []Step{{Container: corev1.Container{ - Command: []string{"/somecmd"}, - Image: "some-image", - }}}, - }, { - name: "overwriting-one-field", - template: &corev1.Container{ - Image: "some-image", - Command: []string{"/somecmd"}, - }, - steps: []Step{{Container: corev1.Container{ - Image: "some-other-image", - }}}, - expected: []Step{{Container: corev1.Container{ - Command: []string{"/somecmd"}, - Image: "some-other-image", - }}}, - }, { - name: "merge-and-overwrite-slice", - template: &corev1.Container{ - Env: []corev1.EnvVar{{ - Name: "KEEP_THIS", - Value: "A_VALUE", - }, { - Name: "SOME_KEY", - Value: "ORIGINAL_VALUE", - }}, - }, - steps: []Step{{Container: corev1.Container{ - Env: []corev1.EnvVar{{ - Name: "NEW_KEY", - Value: "A_VALUE", - }, { - Name: "SOME_KEY", - Value: "NEW_VALUE", - }}, - }}}, - expected: []Step{{Container: corev1.Container{ - Env: []corev1.EnvVar{{ - Name: "NEW_KEY", - Value: "A_VALUE", - }, { - Name: "KEEP_THIS", - Value: "A_VALUE", - }, { - Name: "SOME_KEY", - Value: "NEW_VALUE", - }}, - }}}, - }, { - name: "merge-preserves-script", - template: &corev1.Container{ - Env: []corev1.EnvVar{{ - Name: "FOO", - Value: "bar", - }}, - }, - steps: []Step{{ - Script: "my-script", - Container: corev1.Container{Image: "image"}, - }}, - expected: []Step{{ - Script: "my-script", - Container: corev1.Container{ - Image: "image", - Env: []corev1.EnvVar{{ - Name: "FOO", - Value: "bar", - }}, - }, - }}, - }} { - t.Run(tc.name, func(t *testing.T) { - result, err := MergeStepsWithStepTemplate(tc.template, tc.steps) - if err != nil { - t.Errorf("expected no error. Got error %v", err) - } - - if d := cmp.Diff(tc.expected, result, resourceQuantityCmp); d != "" { - t.Errorf("merged steps don't match, diff: %s", diff.PrintWantGot(d)) - } - }) - } -} diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index b8a6fa6056a..549cc53ee1e 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -51,7 +51,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { if err := validateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate); err != nil { return err } - mergedSteps, err := MergeStepsWithStepTemplate(ts.StepTemplate, ts.Steps) + mergedSteps, err := v1beta1.MergeStepsWithStepTemplate(ts.StepTemplate, ts.Steps) if err != nil { return &apis.FieldError{ Message: fmt.Sprintf("error merging step template and steps: %s", err),