From 032fffe111145b2cccaf9a0bc628566dc3dd8210 Mon Sep 17 00:00:00 2001 From: jregan Date: Sun, 6 Jan 2019 09:17:53 -0800 Subject: [PATCH] Drain more code from kusttarget. --- pkg/target/kusttarget.go | 78 ++----- pkg/target/resaccumulator_test.go | 2 +- pkg/transformers/config/constants.go | 42 ---- pkg/transformers/config/factory.go | 59 +++-- pkg/transformers/config/factory_test.go | 35 ++- pkg/transformers/config/factorycrd.go | 203 ++++++++++-------- pkg/transformers/config/factorycrd_test.go | 3 +- pkg/transformers/config/transformerconfig.go | 18 ++ pkg/transformers/labelsandannotations_test.go | 2 +- 9 files changed, 211 insertions(+), 231 deletions(-) delete mode 100644 pkg/transformers/config/constants.go diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index d79791682..b362737f2 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -50,7 +50,8 @@ type KustTarget struct { // NewKustTarget returns a new instance of KustTarget primed with a Loader. func NewKustTarget( - ldr ifc.Loader, fSys fs.FileSystem, + ldr ifc.Loader, + fSys fs.FileSystem, rFactory *resmap.Factory, tFactory transformer.Factory) (*KustTarget, error) { content, err := loadKustFile(ldr) @@ -80,6 +81,21 @@ func NewKustTarget( }, nil } +func loadKustFile(ldr ifc.Loader) ([]byte, error) { + for _, kf := range []string{ + constants.KustomizationFileName, + constants.SecondaryKustomizationFileName} { + content, err := ldr.Load(kf) + if err == nil { + return content, nil + } + if !strings.Contains(err.Error(), "no such file or directory") { + return nil, err + } + } + return nil, fmt.Errorf("no kustomization.yaml file under %s", ldr.Root()) +} + func unmarshal(y []byte, o interface{}) error { j, err := yaml.YAMLToJSON(y) if err != nil { @@ -90,47 +106,6 @@ func unmarshal(y []byte, o interface{}) error { return dec.Decode(o) } -// TODO(#6060) Maybe switch to the false path permanently -// (desired by #606), or expose this as a new customization -// directive. -const demandExplicitConfig = true - -func makeTransformerConfig( - ldr ifc.Loader, paths []string) (*config.TransformerConfig, error) { - if demandExplicitConfig { - return loadConfigFromDiskOrDefaults(ldr, paths) - } - return mergeCustomConfigWithDefaults(ldr, paths) -} - -// loadConfigFromDiskOrDefaults returns a TransformerConfig object -// built from either files or the hardcoded default configs. -// There's no merging, it's one or the other. This is preferred if one -// wants all configuration to be explicit in version control, as -// opposed to relying on a mix of files and hard coded config. -func loadConfigFromDiskOrDefaults( - ldr ifc.Loader, paths []string) (*config.TransformerConfig, error) { - if paths == nil || len(paths) == 0 { - return config.NewFactory(nil).DefaultConfig(), nil - } - return config.NewFactory(ldr).FromFiles(paths) -} - -// mergeCustomConfigWithDefaults returns a merger of custom config, -// if any, with default config. -func mergeCustomConfigWithDefaults( - ldr ifc.Loader, paths []string) (*config.TransformerConfig, error) { - t1 := config.NewFactory(nil).DefaultConfig() - if len(paths) == 0 { - return t1, nil - } - t2, err := config.NewFactory(ldr).FromFiles(paths) - if err != nil { - return nil, err - } - return t1.Merge(t2) -} - // MakeCustomizedResMap creates a ResMap per kustomization instructions. // The Resources in the returned ResMap are fully customized. func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) { @@ -186,7 +161,7 @@ func (kt *KustTarget) accumulateTarget() ( if err != nil { errs.Append(errors.Wrap(err, "MergeResourcesWithErrorOnIdCollision")) } - tConfig, err := makeTransformerConfig( + tConfig, err := config.MakeTransformerConfig( kt.ldr, kt.kustomization.Configurations) if err != nil { return nil, err @@ -199,7 +174,7 @@ func (kt *KustTarget) accumulateTarget() ( if err != nil { errs.Append(errors.Wrap(err, "MergeVars")) } - crdTc, err := config.NewFactory(kt.ldr).LoadCRDs(kt.kustomization.Crds) + crdTc, err := config.LoadConfigFromCRDs(kt.ldr, kt.kustomization.Crds) if err != nil { errs.Append(errors.Wrap(err, "LoadCRDs")) } @@ -334,18 +309,3 @@ func (kt *KustTarget) newTransformer( r = append(r, t) return transformers.NewMultiTransformer(r), nil } - -func loadKustFile(ldr ifc.Loader) ([]byte, error) { - for _, kf := range []string{ - constants.KustomizationFileName, - constants.SecondaryKustomizationFileName} { - content, err := ldr.Load(kf) - if err == nil { - return content, nil - } - if !strings.Contains(err.Error(), "no such file or directory") { - return nil, err - } - } - return nil, fmt.Errorf("no kustomization.yaml file under %s", ldr.Root()) -} diff --git a/pkg/target/resaccumulator_test.go b/pkg/target/resaccumulator_test.go index 0d7c7036f..69ec0b791 100644 --- a/pkg/target/resaccumulator_test.go +++ b/pkg/target/resaccumulator_test.go @@ -31,7 +31,7 @@ import ( func TestResolveVars(t *testing.T) { ra := MakeEmptyAccumulator() - err := ra.MergeConfig(config.NewFactory(nil).DefaultConfig()) + err := ra.MergeConfig(config.MakeDefaultConfig()) if err != nil { t.Fatalf("unexpected err: %v", err) } diff --git a/pkg/transformers/config/constants.go b/pkg/transformers/config/constants.go deleted file mode 100644 index 357b8950f..000000000 --- a/pkg/transformers/config/constants.go +++ /dev/null @@ -1,42 +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 config - -// Annotation is to mark a field as annotations. -// "x-kubernetes-annotation": "" -const Annotation = "x-kubernetes-annotation" - -// LabelSelector is to mark a field as LabelSelector -// "x-kubernetes-label-selector": "" -const LabelSelector = "x-kubernetes-label-selector" - -// Identity is to mark a field as Identity -// "x-kubernetes-identity": "" -const Identity = "x-kubernetes-identity" - -// Version marks the type version of an object ref field -// "x-kubernetes-object-ref-api-version": -const Version = "x-kubernetes-object-ref-api-version" - -// Kind marks the type name of an object ref field -// "x-kubernetes-object-ref-kind": -const Kind = "x-kubernetes-object-ref-kind" - -// NameKey marks the field key that refers to an object of an object ref field -// "x-kubernetes-object-ref-name-key": "name" -// default is "name" -const NameKey = "x-kubernetes-object-ref-name-key" diff --git a/pkg/transformers/config/factory.go b/pkg/transformers/config/factory.go index 644fbcae6..9d912dd3c 100644 --- a/pkg/transformers/config/factory.go +++ b/pkg/transformers/config/factory.go @@ -21,7 +21,6 @@ import ( "github.com/ghodss/yaml" "sigs.k8s.io/kustomize/pkg/ifc" - "sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig" ) // Factory makes instances of TransformerConfig. @@ -29,6 +28,48 @@ type Factory struct { ldr ifc.Loader } +// TODO(#6060) Maybe switch to the false path permanently +// (desired by #606), or expose this as a new customization +// directive. +const demandExplicitConfig = true + +func MakeTransformerConfig( + ldr ifc.Loader, paths []string) (*TransformerConfig, error) { + if demandExplicitConfig { + return loadConfigFromDiskOrDefaults(ldr, paths) + } + return mergeCustomConfigWithDefaults(ldr, paths) +} + +// loadConfigFromDiskOrDefaults returns a TransformerConfig object +// built from either files or the hardcoded default configs. +// There's no merging, it's one or the other. This is preferred +// if one wants all configuration to be explicit in version +// control, as opposed to relying on a mix of files and +// hard-coded config. +func loadConfigFromDiskOrDefaults( + ldr ifc.Loader, paths []string) (*TransformerConfig, error) { + if paths == nil || len(paths) == 0 { + return MakeDefaultConfig(), nil + } + return NewFactory(ldr).FromFiles(paths) +} + +// mergeCustomConfigWithDefaults returns a merger of custom config, +// if any, with default config. +func mergeCustomConfigWithDefaults( + ldr ifc.Loader, paths []string) (*TransformerConfig, error) { + t1 := MakeDefaultConfig() + if len(paths) == 0 { + return t1, nil + } + t2, err := NewFactory(ldr).FromFiles(paths) + if err != nil { + return nil, err + } + return t1.Merge(t2) +} + func NewFactory(l ifc.Loader) *Factory { return &Factory{ldr: l} } @@ -71,19 +112,3 @@ func makeTransformerConfigFromBytes(data []byte) (*TransformerConfig, error) { t.sortFields() return &t, nil } - -// EmptyConfig returns an empty TransformerConfig object -func (tf *Factory) EmptyConfig() *TransformerConfig { - return &TransformerConfig{} -} - -// DefaultConfig returns a default TransformerConfig. -// This should never fail, hence the Fatal panic. -func (tf *Factory) DefaultConfig() *TransformerConfig { - c, err := makeTransformerConfigFromBytes( - defaultconfig.GetDefaultFieldSpecs()) - if err != nil { - log.Fatalf("Unable to make default transformconfig: %v", err) - } - return c -} diff --git a/pkg/transformers/config/factory_test.go b/pkg/transformers/config/factory_test.go index 81b79de06..a380af74c 100644 --- a/pkg/transformers/config/factory_test.go +++ b/pkg/transformers/config/factory_test.go @@ -25,21 +25,28 @@ import ( "testing" ) -func TestMakeDefaultTransformerConfig(t *testing.T) { +func TestMakeDefaultConfig(t *testing.T) { // Confirm default can be made without fatal error inside call. - l, _, _ := makeFakeLoaderAndOutput() - _ = NewFactory(l).DefaultConfig() + _ = MakeDefaultConfig() } -func makeFakeLoaderAndOutput() (ifc.Loader, *TransformerConfig, *TransformerConfig) { - transformerConfig := ` +func makeTestLoader(path, content string) ifc.Loader { + fs := fs.MakeFakeFS() + fs.WriteFile(path, []byte(content)) + return loader.NewFileLoaderAtRoot(fs) +} + +func TestFromFiles(t *testing.T) { + path := "/transformerconfig/test/config.yaml" + ldr := makeTestLoader(path, ` namePrefix: - path: nameprefix/path kind: SomeKind -` - fakeFS := fs.MakeFakeFS() - fakeFS.WriteFile("/transformerconfig/test/config.yaml", []byte(transformerConfig)) - ldr := loader.NewFileLoaderAtRoot(fakeFS) +`) + tcfg, err := NewFactory(ldr).FromFiles([]string{"transformerconfig/test/config.yaml"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } expected := &TransformerConfig{ NamePrefix: []FieldSpec{ { @@ -48,16 +55,6 @@ namePrefix: }, }, } - return ldr, expected, NewFactory(ldr).DefaultConfig() -} - -func TestMakeTransformerConfigFromFiles(t *testing.T) { - ldr, expected, _ := makeFakeLoaderAndOutput() - tcfg, err := NewFactory(ldr).FromFiles([]string{"transformerconfig/test/config.yaml"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !reflect.DeepEqual(tcfg, expected) { t.Fatalf("expected %v\n but go6t %v\n", expected, tcfg) } diff --git a/pkg/transformers/config/factorycrd.go b/pkg/transformers/config/factorycrd.go index 24e0d5614..8aaa8ee64 100644 --- a/pkg/transformers/config/factorycrd.go +++ b/pkg/transformers/config/factorycrd.go @@ -21,15 +21,29 @@ import ( "strings" "github.com/ghodss/yaml" + "github.com/go-openapi/spec" "k8s.io/kube-openapi/pkg/common" "sigs.k8s.io/kustomize/pkg/gvk" + "sigs.k8s.io/kustomize/pkg/ifc" ) -// LoadCRDs parse CRD schemas from paths into a TransformerConfig -func (tf *Factory) LoadCRDs(paths []string) (*TransformerConfig, error) { - tc := tf.EmptyConfig() +type myProperties map[string]spec.Schema +type nameToApiMap map[string]common.OpenAPIDefinition + +// LoadConfigFromCRDs parse CRD schemas from paths into a TransformerConfig +func LoadConfigFromCRDs( + ldr ifc.Loader, paths []string) (*TransformerConfig, error) { + tc := MakeEmptyConfig() for _, path := range paths { - otherTc, err := tf.loadCRD(path) + content, err := ldr.Load(path) + if err != nil { + return nil, err + } + m, err := makeNameToApiMap(content) + if err != nil { + return nil, err + } + otherTc, err := makeConfigFromApiMap(m) if err != nil { return nil, err } @@ -41,28 +55,24 @@ func (tf *Factory) LoadCRDs(paths []string) (*TransformerConfig, error) { return tc, nil } -func (tf *Factory) loadCRD(path string) (*TransformerConfig, error) { - result := tf.EmptyConfig() - content, err := tf.loader().Load(path) - if err != nil { - return result, err - } - - var types map[string]common.OpenAPIDefinition +func makeNameToApiMap(content []byte) (result nameToApiMap, err error) { if content[0] == '{' { - err = json.Unmarshal(content, &types) + err = json.Unmarshal(content, &result) } else { - err = yaml.Unmarshal(content, &types) - } - if err != nil { - return nil, err + err = yaml.Unmarshal(content, &result) } + return +} - crds := getCRDs(types) - for crd, k := range crds { - tc := tf.EmptyConfig() - err = loadCrdIntoConfig( - types, crd, crd, k, []string{}, tc) +func makeConfigFromApiMap(m nameToApiMap) (*TransformerConfig, error) { + result := MakeEmptyConfig() + for name, api := range m { + if !looksLikeAk8sType(api.Schema.SchemaProps.Properties) { + continue + } + tc := MakeEmptyConfig() + err := loadCrdIntoConfig( + tc, makeGvkFromTypeName(name), m, name, []string{}) if err != nil { return result, err } @@ -71,107 +81,120 @@ func (tf *Factory) loadCRD(path string) (*TransformerConfig, error) { return result, err } } - return result, nil } -// getCRDs get all CRD types -func getCRDs(types map[string]common.OpenAPIDefinition) map[string]gvk.Gvk { - crds := map[string]gvk.Gvk{} - - for typename, t := range types { - properties := t.Schema.SchemaProps.Properties - _, foundKind := properties["kind"] - _, foundAPIVersion := properties["apiVersion"] - _, foundMetadata := properties["metadata"] - if foundKind && foundAPIVersion && foundMetadata { - // TODO: Get Group and Version for CRD from the openAPI definition once - // "x-kubernetes-group-version-kind" is available in CRD - kind := strings.Split(typename, ".")[len(strings.Split(typename, "."))-1] - crds[typename] = gvk.Gvk{Kind: kind} - } - } - return crds +// TODO: Get Group and Version for CRD from the +// openAPI definition once +// "x-kubernetes-group-version-kind" is available in CRD +func makeGvkFromTypeName(n string) gvk.Gvk { + names := strings.Split(n, ".") + kind := names[len(names)-1] + return gvk.Gvk{Kind: kind} } +func looksLikeAk8sType(properties myProperties) bool { + _, ok := properties["kind"] + if !ok { + return false + } + _, ok = properties["apiVersion"] + if !ok { + return false + } + _, ok = properties["metadata"] + if !ok { + return false + } + return true +} + +const ( + // "x-kubernetes-annotation": "" + xAnnotation = "x-kubernetes-annotation" + + // "x-kubernetes-label-selector": "" + xLabelSelector = "x-kubernetes-label-selector" + + // "x-kubernetes-identity": "" + xIdentity = "x-kubernetes-identity" + + // "x-kubernetes-object-ref-api-version": + xVersion = "x-kubernetes-object-ref-api-version" + + // "x-kubernetes-object-ref-kind": + xKind = "x-kubernetes-object-ref-kind" + + // "x-kubernetes-object-ref-name-key": "name" + // default is "name" + xNameKey = "x-kubernetes-object-ref-name-key" +) + // loadCrdIntoConfig loads a CRD spec into a TransformerConfig func loadCrdIntoConfig( - types map[string]common.OpenAPIDefinition, - atype string, crd string, in gvk.Gvk, - path []string, config *TransformerConfig) (err error) { - if _, ok := types[crd]; !ok { + theConfig *TransformerConfig, theGvk gvk.Gvk, theMap nameToApiMap, + typeName string, path []string) (err error) { + api, ok := theMap[typeName] + if !ok { return nil } - - for propname, property := range types[atype].Schema.SchemaProps.Properties { - _, annotate := property.Extensions.GetString(Annotation) + for propName, property := range api.Schema.SchemaProps.Properties { + _, annotate := property.Extensions.GetString(xAnnotation) if annotate { - err = config.AddAnnotationFieldSpec( - FieldSpec{ - CreateIfNotPresent: false, - Gvk: in, - Path: strings.Join(append(path, propname), "/"), - }, - ) + err = theConfig.AddAnnotationFieldSpec( + makeFs(theGvk, append(path, propName))) if err != nil { - return err + return } } - _, label := property.Extensions.GetString(LabelSelector) + _, label := property.Extensions.GetString(xLabelSelector) if label { - err = config.AddLabelFieldSpec( - FieldSpec{ - CreateIfNotPresent: false, - Gvk: in, - Path: strings.Join(append(path, propname), "/"), - }, - ) + err = theConfig.AddLabelFieldSpec( + makeFs(theGvk, append(path, propName))) if err != nil { - return err + return } } - _, identity := property.Extensions.GetString(Identity) + _, identity := property.Extensions.GetString(xIdentity) if identity { - err = config.AddPrefixFieldSpec( - FieldSpec{ - CreateIfNotPresent: false, - Gvk: in, - Path: strings.Join(append(path, propname), "/"), - }, - ) + err = theConfig.AddPrefixFieldSpec( + makeFs(theGvk, append(path, propName))) if err != nil { - return err + return } } - version, ok := property.Extensions.GetString(Version) + version, ok := property.Extensions.GetString(xVersion) if ok { - kind, ok := property.Extensions.GetString(Kind) + kind, ok := property.Extensions.GetString(xKind) if ok { - nameKey, ok := property.Extensions.GetString(NameKey) + nameKey, ok := property.Extensions.GetString(xNameKey) if !ok { nameKey = "name" } - err = config.AddNamereferenceFieldSpec(NameBackReferences{ - Gvk: gvk.Gvk{Kind: kind, Version: version}, - FieldSpecs: []FieldSpec{ - { - CreateIfNotPresent: false, - Gvk: in, - Path: strings.Join(append(path, propname, nameKey), "/"), - }, - }, - }) + err = theConfig.AddNamereferenceFieldSpec( + NameBackReferences{ + Gvk: gvk.Gvk{Kind: kind, Version: version}, + FieldSpecs: []FieldSpec{ + makeFs(theGvk, append(path, propName, nameKey))}, + }) if err != nil { - return err + return } } } - if property.Ref.GetURL() != nil { loadCrdIntoConfig( - types, property.Ref.String(), crd, in, - append(path, propname), config) + theConfig, theGvk, theMap, + property.Ref.String(), append(path, propName)) } } return nil } + +func makeFs(in gvk.Gvk, path []string) FieldSpec { + return FieldSpec{ + CreateIfNotPresent: false, + Gvk: in, + Path: strings.Join(path, "/"), + } +} diff --git a/pkg/transformers/config/factorycrd_test.go b/pkg/transformers/config/factorycrd_test.go index 0065533f4..18ed5d7f3 100644 --- a/pkg/transformers/config/factorycrd_test.go +++ b/pkg/transformers/config/factorycrd_test.go @@ -182,8 +182,7 @@ func TestLoadCRDs(t *testing.T) { NameReference: nbrs, } - actualTc, err := NewFactory(makeLoader(t)).LoadCRDs( - []string{"crd.json"}) + actualTc, err := LoadConfigFromCRDs(makeLoader(t), []string{"crd.json"}) if err != nil { t.Fatalf("unexpected error:%v", err) } diff --git a/pkg/transformers/config/transformerconfig.go b/pkg/transformers/config/transformerconfig.go index 641e9301c..556f0b814 100644 --- a/pkg/transformers/config/transformerconfig.go +++ b/pkg/transformers/config/transformerconfig.go @@ -19,7 +19,10 @@ limitations under the License. package config import ( + "log" "sort" + + "sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig" ) // TransformerConfig holds the data needed to perform transformations. @@ -33,6 +36,21 @@ type TransformerConfig struct { VarReference fsSlice `json:"varReference,omitempty" yaml:"varReference,omitempty"` } +// MakeEmptyConfig returns an empty TransformerConfig object +func MakeEmptyConfig() *TransformerConfig { + return &TransformerConfig{} +} + +// MakeDefaultConfig returns a default TransformerConfig. +func MakeDefaultConfig() *TransformerConfig { + c, err := makeTransformerConfigFromBytes( + defaultconfig.GetDefaultFieldSpecs()) + if err != nil { + log.Fatalf("Unable to make default transformconfig: %v", err) + } + return c +} + // sortFields provides determinism in logging, tests, etc. func (t *TransformerConfig) sortFields() { sort.Sort(t.NamePrefix) diff --git a/pkg/transformers/labelsandannotations_test.go b/pkg/transformers/labelsandannotations_test.go index 5bbaa70b6..5e88be13f 100644 --- a/pkg/transformers/labelsandannotations_test.go +++ b/pkg/transformers/labelsandannotations_test.go @@ -44,7 +44,7 @@ var crb = gvk.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "Clus var sa = gvk.Gvk{Version: "v1", Kind: "ServiceAccount"} var ingress = gvk.Gvk{Kind: "Ingress"} var rf = resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()) -var defaultTransformerConfig = config.NewFactory(nil).DefaultConfig() +var defaultTransformerConfig = config.MakeDefaultConfig() func TestLabelsRun(t *testing.T) { m := resmap.ResMap{