From 6d374d26a1b8e4be3aab5f3eb44ae736fdb6cbd4 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Wed, 2 Jun 2021 10:55:24 -0700 Subject: [PATCH] remove `config` in function spec (#2122) --- .../inline-fnconfig/.expected/diff.patch | 30 ------------------- .../fn-render/inline-fnconfig/.krmignore | 1 - .../fn-render/inline-fnconfig/Kptfile | 15 ---------- .../fn-render/inline-fnconfig/resources.yaml | 26 ---------------- .../.expected/config.yaml | 16 ---------- .../invalid-inline-fnconfig/.krmignore | 1 - .../invalid-inline-fnconfig/resources.yaml | 26 ---------------- .../multiple-fnconfig/.expected/config.yaml | 2 +- internal/fnruntime/runner.go | 3 -- internal/fnruntime/runner_test.go | 16 ---------- package-examples/pipeline-validate/Kptfile | 9 ++---- package-examples/pipeline-validate/README.md | 9 ++---- pkg/api/kptfile/v1alpha2/types.go | 6 ---- pkg/api/kptfile/v1alpha2/validation.go | 29 +++++------------- pkg/api/kptfile/v1alpha2/validation_test.go | 10 ------- pkg/kptfile/kptfileutil/util_test.go | 14 +++------ 16 files changed, 16 insertions(+), 197 deletions(-) delete mode 100644 e2e/testdata/fn-render/inline-fnconfig/.expected/diff.patch delete mode 100644 e2e/testdata/fn-render/inline-fnconfig/.krmignore delete mode 100644 e2e/testdata/fn-render/inline-fnconfig/Kptfile delete mode 100644 e2e/testdata/fn-render/inline-fnconfig/resources.yaml delete mode 100644 e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml delete mode 100644 e2e/testdata/fn-render/invalid-inline-fnconfig/.krmignore delete mode 100644 e2e/testdata/fn-render/invalid-inline-fnconfig/resources.yaml diff --git a/e2e/testdata/fn-render/inline-fnconfig/.expected/diff.patch b/e2e/testdata/fn-render/inline-fnconfig/.expected/diff.patch deleted file mode 100644 index 9997352594..0000000000 --- a/e2e/testdata/fn-render/inline-fnconfig/.expected/diff.patch +++ /dev/null @@ -1,30 +0,0 @@ -diff --git a/resources.yaml b/resources.yaml -index 7a494c9..a9dd224 100644 ---- a/resources.yaml -+++ b/resources.yaml -@@ -15,12 +15,25 @@ apiVersion: apps/v1 - kind: Deployment - metadata: - name: nginx-deployment -+ namespace: staging -+ labels: -+ tier: backend - spec: - replicas: 3 -+ selector: -+ matchLabels: -+ tier: backend -+ template: -+ metadata: -+ labels: -+ tier: backend - --- - apiVersion: custom.io/v1 - kind: Custom - metadata: - name: custom -+ namespace: staging -+ labels: -+ tier: backend - spec: - image: nginx:1.2.3 diff --git a/e2e/testdata/fn-render/inline-fnconfig/.krmignore b/e2e/testdata/fn-render/inline-fnconfig/.krmignore deleted file mode 100644 index 9d7a4007d6..0000000000 --- a/e2e/testdata/fn-render/inline-fnconfig/.krmignore +++ /dev/null @@ -1 +0,0 @@ -.expected diff --git a/e2e/testdata/fn-render/inline-fnconfig/Kptfile b/e2e/testdata/fn-render/inline-fnconfig/Kptfile deleted file mode 100644 index dcf80258cb..0000000000 --- a/e2e/testdata/fn-render/inline-fnconfig/Kptfile +++ /dev/null @@ -1,15 +0,0 @@ -apiVersion: kpt.dev/v1alpha2 -kind: Kptfile -metadata: - name: app -pipeline: - mutators: - - image: gcr.io/kpt-fn/set-namespace:v0.1.3 - configMap: - namespace: staging - - image: gcr.io/kpt-fn/set-labels:v0.1.4 - config: - apiVersion: v1 - kind: ConfigMap - data: - tier: backend diff --git a/e2e/testdata/fn-render/inline-fnconfig/resources.yaml b/e2e/testdata/fn-render/inline-fnconfig/resources.yaml deleted file mode 100644 index 7a494c9ca7..0000000000 --- a/e2e/testdata/fn-render/inline-fnconfig/resources.yaml +++ /dev/null @@ -1,26 +0,0 @@ -# Copyright 2021 Google LLC -# -# 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. -apiVersion: apps/v1 -kind: Deployment -metadata: - name: nginx-deployment -spec: - replicas: 3 ---- -apiVersion: custom.io/v1 -kind: Custom -metadata: - name: custom -spec: - image: nginx:1.2.3 diff --git a/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml deleted file mode 100644 index 89dcbcda4b..0000000000 --- a/e2e/testdata/fn-render/invalid-inline-fnconfig/.expected/config.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2021 Google LLC -# -# 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. - -exitCode: 1 -stdErr: "functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields" diff --git a/e2e/testdata/fn-render/invalid-inline-fnconfig/.krmignore b/e2e/testdata/fn-render/invalid-inline-fnconfig/.krmignore deleted file mode 100644 index 9d7a4007d6..0000000000 --- a/e2e/testdata/fn-render/invalid-inline-fnconfig/.krmignore +++ /dev/null @@ -1 +0,0 @@ -.expected diff --git a/e2e/testdata/fn-render/invalid-inline-fnconfig/resources.yaml b/e2e/testdata/fn-render/invalid-inline-fnconfig/resources.yaml deleted file mode 100644 index 7a494c9ca7..0000000000 --- a/e2e/testdata/fn-render/invalid-inline-fnconfig/resources.yaml +++ /dev/null @@ -1,26 +0,0 @@ -# Copyright 2021 Google LLC -# -# 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. -apiVersion: apps/v1 -kind: Deployment -metadata: - name: nginx-deployment -spec: - replicas: 3 ---- -apiVersion: custom.io/v1 -kind: Custom -metadata: - name: custom -spec: - image: nginx:1.2.3 diff --git a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml index 188ee54259..0f4b9e9e93 100644 --- a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml +++ b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml @@ -13,4 +13,4 @@ # limitations under the License. exitCode: 1 -stdErr: 'only one of ''config'', ''configMap'', ''configPath'' can be specified. Got "configPath, configMap"' +stdErr: 'functionConfig must not specify both `configMap` and `configPath` at the same time' diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go index b30e6a98eb..9f164ee3e6 100644 --- a/internal/fnruntime/runner.go +++ b/internal/fnruntime/runner.go @@ -425,9 +425,6 @@ func newFnConfig(f *kptfilev1alpha2.Function, pkgPath types.UniquePath) (*yaml.R } // directly use the config from file return node, nil - case !kptfilev1alpha2.IsNodeZero(&f.Config): - // directly use the inline config - return yaml.NewRNode(&f.Config), nil case len(f.ConfigMap) != 0: node = yaml.NewMapRNode(&f.ConfigMap) if node == nil { diff --git a/internal/fnruntime/runner_test.go b/internal/fnruntime/runner_test.go index c2d36c4d9d..c234e97c6d 100644 --- a/internal/fnruntime/runner_test.go +++ b/internal/fnruntime/runner_test.go @@ -47,22 +47,6 @@ func TestFunctionConfig(t *testing.T) { fn: kptfilev1alpha2.Function{}, expected: "", }, - { - name: "inline config", - fn: kptfilev1alpha2.Function{ - Config: *yaml.MustParse(`apiVersion: cft.dev/v1alpha1 -kind: ResourceHierarchy -metadata: - name: root-hierarchy - namespace: hierarchy`).YNode(), - }, - expected: `apiVersion: cft.dev/v1alpha1 -kind: ResourceHierarchy -metadata: - name: root-hierarchy - namespace: hierarchy -`, - }, { name: "file config", fn: kptfilev1alpha2.Function{}, diff --git a/package-examples/pipeline-validate/Kptfile b/package-examples/pipeline-validate/Kptfile index 1f867aac8a..b81e172e41 100644 --- a/package-examples/pipeline-validate/Kptfile +++ b/package-examples/pipeline-validate/Kptfile @@ -5,12 +5,7 @@ metadata: pipeline: mutators: - image: gcr.io/kpt-fn/set-labels:unstable - config: - apiVersion: fn.kpt.dev/v1alpha1 - kind: SetLabelConfig - metadata: - name: label-color-blue - labels: - color: blue + configMap: + color: blue validators: - image: gcr.io/kpt-fn/enforce-gatekeeper:unstable diff --git a/package-examples/pipeline-validate/README.md b/package-examples/pipeline-validate/README.md index 08cdcb2276..bf7ae4afd4 100644 --- a/package-examples/pipeline-validate/README.md +++ b/package-examples/pipeline-validate/README.md @@ -60,13 +60,8 @@ you to use gatekeeper for checks on the configuration. pipeline: mutators: - image: gcr.io/kpt-fn/set-labels:unstable - config: - apiVersion: fn.kpt.dev/v1alpha1 - kind: SetLabelConfig - metadata: - name: label-color-blue - labels: - color: blue + configMap: + color: blue validators: - image: gcr.io/kpt-fn/enforce-gatekeeper:unstable ``` diff --git a/pkg/api/kptfile/v1alpha2/types.go b/pkg/api/kptfile/v1alpha2/types.go index b0e199030f..431d00d1d1 100644 --- a/pkg/api/kptfile/v1alpha2/types.go +++ b/pkg/api/kptfile/v1alpha2/types.go @@ -267,12 +267,6 @@ type Function struct { // image: set-labels Image string `yaml:"image,omitempty"` - // `Config` specifies an inline KRM resource used as the function config. - // Config, ConfigPath, and ConfigMap fields are mutually exclusive. - // We cannot use a pointer to yaml.Node because it will cause errors when - // we try to unmarshal the YAML. - Config yaml.Node `yaml:"config,omitempty"` - // `ConfigPath` specifies a slash-delimited relative path to a file in the current directory // containing a KRM resource used as the function config. This resource is // excluded when resolving 'sources', and as a result cannot be operated on diff --git a/pkg/api/kptfile/v1alpha2/validation.go b/pkg/api/kptfile/v1alpha2/validation.go index d629ae66ea..aa3f53b767 100644 --- a/pkg/api/kptfile/v1alpha2/validation.go +++ b/pkg/api/kptfile/v1alpha2/validation.go @@ -65,7 +65,13 @@ func (f *Function) validate(fnType string, idx int) error { } } - var configFields []string + if len(f.ConfigMap) != 0 && f.ConfigPath != "" { + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d]", fnType, idx), + Reason: "functionConfig must not specify both `configMap` and `configPath` at the same time", + } + } + if f.ConfigPath != "" { if err := validateFnConfigPath(f.ConfigPath); err != nil { return &ValidateError{ @@ -74,27 +80,6 @@ func (f *Function) validate(fnType string, idx int) error { Reason: err.Error(), } } - configFields = append(configFields, "configPath") - } - if len(f.ConfigMap) != 0 { - configFields = append(configFields, "configMap") - } - if !IsNodeZero(&f.Config) { - config := yaml.NewRNode(&f.Config) - if _, err := config.GetMeta(); err != nil { - return &ValidateError{ - Field: fmt.Sprintf("pipeline.%s[%d].config", fnType, idx), - Reason: "functionConfig must be a valid KRM resource with `apiVersion` and `kind` fields", - } - } - configFields = append(configFields, "config") - } - if len(configFields) > 1 { - return &ValidateError{ - Field: fmt.Sprintf("pipeline.%s[%d]", fnType, idx), - Reason: fmt.Sprintf("only one of 'config', 'configMap', 'configPath' can be specified. Got %q", - strings.Join(configFields, ", ")), - } } return nil diff --git a/pkg/api/kptfile/v1alpha2/validation_test.go b/pkg/api/kptfile/v1alpha2/validation_test.go index 3c04002f05..b9dd032e1b 100644 --- a/pkg/api/kptfile/v1alpha2/validation_test.go +++ b/pkg/api/kptfile/v1alpha2/validation_test.go @@ -15,8 +15,6 @@ package v1alpha2 import ( "testing" - - "sigs.k8s.io/kustomize/kyaml/yaml" ) func TestKptfileValidate(t *testing.T) { @@ -39,14 +37,6 @@ func TestKptfileValidate(t *testing.T) { kptfile: KptFile{ Pipeline: &Pipeline{ Mutators: []Function{ - { - Image: "gcr.io/kpt-functions/generate-folders", - Config: *yaml.MustParse(`apiVersion: cft.dev/v1alpha1 -kind: ResourceHierarchy -metadata: - name: root-hierarchy - namespace: hierarchy # {"$kpt-set":"namespace"}`).YNode(), - }, { Image: "patch-strategic-merge", ConfigPath: "./patch.yaml", diff --git a/pkg/kptfile/kptfileutil/util_test.go b/pkg/kptfile/kptfileutil/util_test.go index 4455ed5016..ea71a8de3c 100644 --- a/pkg/kptfile/kptfileutil/util_test.go +++ b/pkg/kptfile/kptfileutil/util_test.go @@ -377,11 +377,8 @@ metadata: pipeline: mutators: - image: my:image - config: - apiVersion: kpt.dev/v1alpha2 - kind: Function - spec: - foo: bar + configMap: + foo: bar - image: foo:bar `, updateUpstream: true, @@ -393,11 +390,8 @@ metadata: pipeline: mutators: - image: my:image - config: - apiVersion: kpt.dev/v1alpha2 - kind: Function - spec: - foo: bar + configMap: + foo: bar - image: foo:bar `, },