From 95f568b85769de74af777141597af3abaa47cab9 Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 6 Jun 2018 13:16:10 -0700 Subject: [PATCH] add comments in refvar transformer Refactor change --- pkg/app/application.go | 22 ++++---- pkg/app/var.go | 54 ------------------- pkg/resource/resource.go | 27 ++++++++++ .../var_test.go => resource/resource_test.go} | 47 ++++++++++------ pkg/transformers/refvars.go | 20 +++---- pkg/types/defaulting.go | 27 ---------- pkg/types/kustomization.go | 24 --------- pkg/types/var.go | 47 ++++++++++++++++ 8 files changed, 126 insertions(+), 142 deletions(-) delete mode 100644 pkg/app/var.go rename pkg/{app/var_test.go => resource/resource_test.go} (53%) delete mode 100644 pkg/types/defaulting.go create mode 100644 pkg/types/var.go diff --git a/pkg/app/application.go b/pkg/app/application.go index 6c4bffc5b..012c297af 100644 --- a/pkg/app/application.go +++ b/pkg/app/application.go @@ -79,7 +79,7 @@ func (a *applicationImpl) Resources() (resmap.ResMap, error) { if err != nil { return nil, err } - t, err := a.getHashAndReferenceTransformer(res) + t, err := a.newHashAndReferenceTransformer(res) if err != nil { return nil, err } @@ -125,7 +125,7 @@ func (a *applicationImpl) SemiResources() (resmap.ResMap, error) { return nil, errs } - t, err := a.getTransformer(patches) + t, err := a.newTransformer(patches) if err != nil { return nil, err } @@ -144,7 +144,7 @@ func (a *applicationImpl) RawResources() (resmap.ResMap, error) { if err != nil { return nil, err } - t, err := a.getHashAndReferenceTransformer(res) + t, err := a.newHashAndReferenceTransformer(res) if err != nil { return nil, err } @@ -220,12 +220,12 @@ func (a *applicationImpl) subApp() ([]Application, error) { return apps, nil } -// getTransformer generates the following transformers: +// newTransformer generates the following transformers: // 1) apply overlay // 2) name prefix // 3) apply labels // 4) apply annotations -func (a *applicationImpl) getTransformer(patches []*resource.Resource) (transformers.Transformer, error) { +func (a *applicationImpl) newTransformer(patches []*resource.Resource) (transformers.Transformer, error) { ts := []transformers.Transformer{} ot, err := transformers.NewOverlayTransformer(patches) @@ -257,11 +257,11 @@ func (a *applicationImpl) getTransformer(patches []*resource.Resource) (transfor return transformers.NewMultiTransformer(ts), nil } -// getHashAndReferenceTransformer generates the following transformers: +// newHashAndReferenceTransformer generates the following transformers: // 1) name hash for configmap and secrests // 2) apply name reference // 3) apply reference variables -func (a *applicationImpl) getHashAndReferenceTransformer(allRes resmap.ResMap) (transformers.Transformer, error) { +func (a *applicationImpl) newHashAndReferenceTransformer(allRes resmap.ResMap) (transformers.Transformer, error) { ts := []transformers.Transformer{} nht := transformers.NewNameHashTransformer() ts = append(ts, nht) @@ -271,7 +271,7 @@ func (a *applicationImpl) getHashAndReferenceTransformer(allRes resmap.ResMap) ( return nil, err } ts = append(ts, nrt) - t, err := a.getVariableReferenceTransformer(allRes) + t, err := a.newVariableReferenceTransformer(allRes) if err != nil { return nil, err } @@ -280,7 +280,7 @@ func (a *applicationImpl) getHashAndReferenceTransformer(allRes resmap.ResMap) ( return transformers.NewMultiTransformer(ts), nil } -func (a *applicationImpl) getVariableReferenceTransformer(allRes resmap.ResMap) (transformers.Transformer, error) { +func (a *applicationImpl) newVariableReferenceTransformer(allRes resmap.ResMap) (transformers.Transformer, error) { refvars, err := a.resolveRefVars(allRes) if err != nil { return nil, err @@ -314,9 +314,9 @@ func (a *applicationImpl) resolveRefVars(resources resmap.ResMap) (map[string]st } for _, refvar := range vars { - refGVKN := gvkn(refvar) + refGVKN := resource.NewResId(refvar.ObjRef.GroupVersionKind(), refvar.ObjRef.Name) if r, found := resources[refGVKN]; found { - s, err := getFieldAsString(r.Unstruct().UnstructuredContent(), strings.Split(refvar.FieldRef.FieldPath, ".")) + s, err := resource.GetFieldValue(r.Unstruct().UnstructuredContent(), strings.Split(refvar.FieldRef.FieldPath, ".")) if err != nil { return nil, fmt.Errorf("failed to resolve referred var: %+v", refvar) } diff --git a/pkg/app/var.go b/pkg/app/var.go deleted file mode 100644 index 4444c9cef..000000000 --- a/pkg/app/var.go +++ /dev/null @@ -1,54 +0,0 @@ -/* -Copyright 2018 The Kubernetes 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 app - -import ( - "fmt" - - "github.com/kubernetes-sigs/kustomize/pkg/resource" - "github.com/kubernetes-sigs/kustomize/pkg/types" -) - -func gvkn(rv types.Var) resource.ResId { - return resource.NewResId(rv.ObjRef.GroupVersionKind(), rv.ObjRef.Name) -} - -func getFieldAsString(m map[string]interface{}, pathToField []string) (string, error) { - if len(pathToField) == 0 { - return "", fmt.Errorf("Field not found") - } - - if len(pathToField) == 1 { - if v, found := m[pathToField[0]]; found { - if s, ok := v.(string); ok { - return s, nil - } - return "", fmt.Errorf("value at fieldpath is not of string type") - } - return "", fmt.Errorf("field at given fieldpath does not exist") - } - - curr, rest := pathToField[0], pathToField[1] - - v := m[curr] - switch typedV := v.(type) { - case map[string]interface{}: - return getFieldAsString(typedV, []string{rest}) - default: - return "", fmt.Errorf("%#v is not expected to be a primitive type", typedV) - } -} diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index f129f711b..a73a16289 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -18,6 +18,7 @@ package resource import ( "encoding/json" + "fmt" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -108,3 +109,29 @@ func newUnstructuredFromObject(in runtime.Object) (*unstructured.Unstructured, e err = out.UnmarshalJSON(marshaled) return &out, err } + +func GetFieldValue(m map[string]interface{}, pathToField []string) (string, error) { + if len(pathToField) == 0 { + return "", fmt.Errorf("Field not found") + } + + if len(pathToField) == 1 { + if v, found := m[pathToField[0]]; found { + if s, ok := v.(string); ok { + return s, nil + } + return "", fmt.Errorf("value at fieldpath is not of string type") + } + return "", fmt.Errorf("field at given fieldpath does not exist") + } + + curr, rest := pathToField[0], pathToField[1] + + v := m[curr] + switch typedV := v.(type) { + case map[string]interface{}: + return GetFieldValue(typedV, []string{rest}) + default: + return "", fmt.Errorf("%#v is not expected to be a primitive type", typedV) + } +} diff --git a/pkg/app/var_test.go b/pkg/resource/resource_test.go similarity index 53% rename from pkg/app/var_test.go rename to pkg/resource/resource_test.go index e16c24f14..4ccf9184e 100644 --- a/pkg/app/var_test.go +++ b/pkg/resource/resource_test.go @@ -14,10 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -package app +package resource import ( - "strings" "testing" ) @@ -31,21 +30,37 @@ func TestGetFieldAsString(t *testing.T) { "name": "service-name", }, } - p := []string{"Kind"} - s, _ := getFieldAsString(m, p) - if s != "Service" { - t.Errorf("Expected to get Service, but actually got %s", s) + + tests := []struct { + pathToField []string + expectedName string + expectedError bool + }{ + { + pathToField: []string{"Kind"}, + expectedName: "Service", + expectedError: false, + }, + { + pathToField: []string{"metadata", "name"}, + expectedName: "service-name", + expectedError: false, + }, + { + pathToField: []string{"metadata", "non-existing-field"}, + expectedName: "", + expectedError: true, + }, } - p = []string{"metadata", "name"} - s, _ = getFieldAsString(m, p) - if s != "service-name" { - t.Errorf("Expected to get service-name, but actually got %s", s) - } - - p = []string{"metadata", "non-existing-field"} - s, err := getFieldAsString(m, p) - if !strings.HasSuffix(err.Error(), "field at given fieldpath does not exist") { - t.Errorf("Unexpected failure due to incorrect error message %s", err.Error()) + for _, test := range tests { + s, err := GetFieldValue(m, test.pathToField) + if test.expectedError && err == nil { + t.Fatalf("should return error, but no error returned") + } else { + if test.expectedName != s { + t.Fatalf("Got:%s expected:%s", s, test.expectedName) + } + } } } diff --git a/pkg/transformers/refvars.go b/pkg/transformers/refvars.go index 9a885467b..b572178d0 100644 --- a/pkg/transformers/refvars.go +++ b/pkg/transformers/refvars.go @@ -13,6 +13,7 @@ type refvarTransformer struct { vars map[string]string } +// NewRefVarTransformer returns a Trasformer that replaces $(VAR) style variables with values. func NewRefVarTransformer(vars map[string]string) (Transformer, error) { return &refvarTransformer{ vars: vars, @@ -33,17 +34,16 @@ func NewRefVarTransformer(vars map[string]string) (Transformer, error) { }, nil } +// Transform determines the final values of variables: +// +// 1. Determine the final value of each variable: +// a. If the variable's Value is set, expand the `$(var)` references to other +// variables in the .Value field; the sources of variables are the declared +// variables of the container and the service environment variables +// b. If a source is defined for an environment variable, resolve the source +// 2. Create the container's environment in the order variables are declared +// 3. Add remaining service environment vars func (rv *refvarTransformer) Transform(resources resmap.ResMap) error { - // Determine the final values of variables: - // - // 1. Determine the final value of each variable: - // a. If the variable's Value is set, expand the `$(var)` references to other - // variables in the .Value field; the sources of variables are the declared - // variables of the container and the service environment variables - // b. If a source is defined for an environment variable, resolve the source - // 2. Create the container's environment in the order variables are declared - // 3. Add remaining service environment vars - for GVKn := range resources { obj := resources[GVKn].Unstruct() objMap := obj.UnstructuredContent() diff --git a/pkg/types/defaulting.go b/pkg/types/defaulting.go deleted file mode 100644 index 6f49240f0..000000000 --- a/pkg/types/defaulting.go +++ /dev/null @@ -1,27 +0,0 @@ -/* -Copyright 2017 The Kubernetes 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 types - -import ( - corev1 "k8s.io/api/core/v1" -) - -func (v *Var) Defaulting() { - if (corev1.ObjectFieldSelector{}) == v.FieldRef { - v.FieldRef = corev1.ObjectFieldSelector{FieldPath: "metadata.name"} - } -} diff --git a/pkg/types/kustomization.go b/pkg/types/kustomization.go index da3ca4982..920652294 100644 --- a/pkg/types/kustomization.go +++ b/pkg/types/kustomization.go @@ -16,10 +16,6 @@ limitations under the License. package types -import ( - corev1 "k8s.io/api/core/v1" -) - // Kustomization holds the information needed to generate customized k8s api resources. type Kustomization struct { // NamePrefix will prefix the names of all resources mentioned in the kustomization @@ -136,23 +132,3 @@ type DataSources struct { // i.e. a Docker .env file or a .ini file. EnvSource string `json:"env,omitempty" yaml:"env,omitempty"` } - -// Var represents a variable whose value will be sourced -// from a field in a Kubernetes object. -type Var struct { - // Value of identifier name e.g. FOO used in container args, annotations - // Appears in pod template as $(FOO) - Name string `json:"name" yaml:"name"` - - // ObjRef must refer to a Kubernetes resource under the - // purview of this kustomization. ObjRef should use the - // raw name of the object (the name specified in its YAML, - // before addition of a namePrefix). - ObjRef corev1.ObjectReference `json:"objref" yaml:"objref"` - - // FieldRef refers to the field of the object referred to by - // ObjRef whose value will be extracted for use in - // replacing $(FOO). - // If unspecified, this defaults to fieldpath: metadata.name - FieldRef corev1.ObjectFieldSelector `json:"fieldref,omitempty" yaml:"objref,omitempty"` -} diff --git a/pkg/types/var.go b/pkg/types/var.go new file mode 100644 index 000000000..294d4d318 --- /dev/null +++ b/pkg/types/var.go @@ -0,0 +1,47 @@ +/* +Copyright 2017 The Kubernetes 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 types + +import ( + corev1 "k8s.io/api/core/v1" +) + +// Var represents a variable whose value will be sourced +// from a field in a Kubernetes object. +type Var struct { + // Value of identifier name e.g. FOO used in container args, annotations + // Appears in pod template as $(FOO) + Name string `json:"name" yaml:"name"` + + // ObjRef must refer to a Kubernetes resource under the + // purview of this kustomization. ObjRef should use the + // raw name of the object (the name specified in its YAML, + // before addition of a namePrefix). + ObjRef corev1.ObjectReference `json:"objref" yaml:"objref"` + + // FieldRef refers to the field of the object referred to by + // ObjRef whose value will be extracted for use in + // replacing $(FOO). + // If unspecified, this defaults to fieldpath: metadata.name + FieldRef corev1.ObjectFieldSelector `json:"fieldref,omitempty" yaml:"objref,omitempty"` +} + +func (v *Var) Defaulting() { + if (corev1.ObjectFieldSelector{}) == v.FieldRef { + v.FieldRef = corev1.ObjectFieldSelector{FieldPath: "metadata.name"} + } +}