Skip to content

Commit

Permalink
api/: wrap error returned to external package,receive changes in inte…
Browse files Browse the repository at this point in the history
…rface Generate
  • Loading branch information
charles-chenzz committed Sep 9, 2023
1 parent e0fc92c commit ce9f793
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 82 deletions.
4 changes: 3 additions & 1 deletion api/internal/generators/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package generators

import (
"fmt"

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/yaml"
Expand Down Expand Up @@ -38,7 +40,7 @@ func MakeConfigMap(
return nil, nil, err
}
if err = rn.LoadMapIntoConfigMapData(m); err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to LoadMapIntoConfigMapData, error:%w", err)
}
err = copyLabelsAndAnnotations(rn, args.Options)
if err != nil {
Expand Down
8 changes: 5 additions & 3 deletions api/internal/generators/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package generators

import (
"fmt"

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/yaml"
Expand Down Expand Up @@ -44,14 +46,14 @@ func MakeSecret(
yaml.FieldSetter{
Name: "type",
Value: yaml.NewStringRNode(t)}); err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to MakeSecret, error:%w", err)
}
m, orderKeys, err := makeValidatedDataMap(ldr, args.Name, args.KvPairSources)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to makeValidatedDataMap, error:%w", err)
}
if err = rn.LoadMapIntoSecretData(m); err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to LoadMapIntoSecretData, error:%w", err)
}
copyLabelsAndAnnotations(rn, args.Options)
setImmutable(rn, args.Options)
Expand Down
2 changes: 1 addition & 1 deletion api/internal/generators/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func makeValidatedDataMap(
for _, p := range pairs {
// legal key: alphanumeric characters, '-', '_' or '.'
if err := ldr.Validator().ErrIfInvalidKey(p.Key); err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to validate key, error:%w", err)
}
if _, ok := knownKeys[p.Key]; ok {
return nil, nil, errors.Errorf(
Expand Down
69 changes: 36 additions & 33 deletions api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,48 +122,48 @@ func LoadKustFile(ldr ifc.Loader) ([]byte, string, error) {

// MakeCustomizedResMap creates a fully customized ResMap
// per the instructions contained in its kustomization instance.
func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) {
func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, [][]string, error) {
return kt.makeCustomizedResMap()
}

func (kt *KustTarget) makeCustomizedResMap() (resmap.ResMap, error) {
func (kt *KustTarget) makeCustomizedResMap() (resmap.ResMap, [][]string, error) {
var origin *resource.Origin
if len(kt.kustomization.BuildMetadata) != 0 {
origin = &resource.Origin{}
}
kt.origin = origin
ra, err := kt.AccumulateTarget()
ra, keys, err := kt.AccumulateTarget()
if err != nil {
return nil, err
return nil, keys, err
}

// The following steps must be done last, not as part of
// the recursion implicit in AccumulateTarget.

err = kt.addHashesToNames(ra)
if err != nil {
return nil, err
return nil, nil, err
}

// Given that names have changed (prefixs/suffixes added),
// fix all the back references to those names.
err = ra.FixBackReferences()
if err != nil {
return nil, err
return nil, nil, fmt.Errorf("failed to FixBackReferences, error:%w", err)
}

// With all the back references fixed, it's OK to resolve Vars.
err = ra.ResolveVars()
if err != nil {
return nil, err
return nil, nil, fmt.Errorf("failed to ResolveVars, error:%w", err)
}

err = kt.IgnoreLocal(ra)
if err != nil {
return nil, err
return nil, nil, err
}

return ra.ResMap(), nil
return ra.ResMap(), keys, nil
}

func (kt *KustTarget) addHashesToNames(
Expand All @@ -190,64 +190,64 @@ func (kt *KustTarget) addHashesToNames(
// accordingly. When a remote base is found, it updates `origin.repo`
// and `origin.ref` accordingly.
func (kt *KustTarget) AccumulateTarget() (
ra *accumulator.ResAccumulator, err error) {
ra *accumulator.ResAccumulator, keys [][]string, err error) {
return kt.accumulateTarget(accumulator.MakeEmptyAccumulator())
}

// ra should be empty when this KustTarget is a Kustomization, or the ra of the parent if this KustTarget is a Component
// (or empty if the Component does not have a parent).
func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) (
resRa *accumulator.ResAccumulator, err error) {
resRa *accumulator.ResAccumulator, keys [][]string, err error) {
ra, err = kt.accumulateResources(ra, kt.kustomization.Resources)
if err != nil {
return nil, errors.WrapPrefixf(err, "accumulating resources")
return nil, nil, errors.WrapPrefixf(err, "accumulating resources")
}
tConfig, err := builtinconfig.MakeTransformerConfig(
kt.ldr, kt.kustomization.Configurations)
if err != nil {
return nil, err
return nil, nil, fmt.Errorf("failed to MakeTransformerConfig, error:%w", err)
}
err = ra.MergeConfig(tConfig)
if err != nil {
return nil, errors.WrapPrefixf(
return nil, nil, errors.WrapPrefixf(
err, "merging config %v", tConfig)
}
crdTc, err := accumulator.LoadConfigFromCRDs(kt.ldr, kt.kustomization.Crds)
if err != nil {
return nil, errors.WrapPrefixf(
return nil, nil, errors.WrapPrefixf(
err, "loading CRDs %v", kt.kustomization.Crds)
}
err = ra.MergeConfig(crdTc)
if err != nil {
return nil, errors.WrapPrefixf(
return nil, nil, errors.WrapPrefixf(
err, "merging CRDs %v", crdTc)
}
err = kt.runGenerators(ra)
keys, err = kt.runGenerators(ra)
if err != nil {
return nil, err
return nil, nil, err
}

// components are expected to execute after reading resources and adding generators ,before applying transformers and validation.
// https://github.com/kubernetes-sigs/kustomize/pull/5170#discussion_r1212101287
ra, err = kt.accumulateComponents(ra, kt.kustomization.Components)
if err != nil {
return nil, errors.WrapPrefixf(err, "accumulating components")
return nil, nil, errors.WrapPrefixf(err, "accumulating components")
}

err = kt.runTransformers(ra)
if err != nil {
return nil, err
return nil, nil, err
}
err = kt.runValidators(ra)
if err != nil {
return nil, err
return nil, nil, err
}
err = ra.MergeVars(kt.kustomization.Vars)
if err != nil {
return nil, errors.WrapPrefixf(
return nil, nil, errors.WrapPrefixf(
err, "merging vars %v", kt.kustomization.Vars)
}
return ra, nil
return ra, keys, nil
}

// IgnoreLocal drops the local resource by checking the annotation "config.kubernetes.io/local-config".
Expand All @@ -264,36 +264,39 @@ func (kt *KustTarget) IgnoreLocal(ra *accumulator.ResAccumulator) error {
}

func (kt *KustTarget) runGenerators(
ra *accumulator.ResAccumulator) error {
ra *accumulator.ResAccumulator) ([][]string, error) {
var generators []*resmap.GeneratorWithProperties

gs, err := kt.configureBuiltinGenerators()
if err != nil {
return err
return nil, err
}
generators = append(generators, gs...)

gs, err = kt.configureExternalGenerators()
if err != nil {
return errors.WrapPrefixf(err, "loading generator plugins")
return nil, errors.WrapPrefixf(err, "loading generator plugins")
}
generators = append(generators, gs...)
orderKeys := make([][]string, 0)
for i, g := range generators {
resMap, _, err := g.Generate()
resMap, keys, err := g.Generate()
orderKeys = append(orderKeys, keys)
if err != nil {
return err
return nil, fmt.Errorf("failed to generate, error:%w", err)
}
if resMap != nil {
err = resMap.AddOriginAnnotation(generators[i].Origin)
if err != nil {
return errors.WrapPrefixf(err, "adding origin annotations for generator %v", g)
return nil, errors.WrapPrefixf(err, "adding origin annotations for generator %v", g)
}
}
err = ra.AbsorbAll(resMap)
if err != nil {
return errors.WrapPrefixf(err, "merging from generator %v", g)
return nil, errors.WrapPrefixf(err, "merging from generator %v", g)
}
}
return nil
return orderKeys, nil
}

func (kt *KustTarget) configureExternalGenerators() (
Expand Down Expand Up @@ -514,12 +517,12 @@ func (kt *KustTarget) accumulateDirectory(
var subRa *accumulator.ResAccumulator
if isComponent {
// Components don't create a new accumulator: the kustomization directives are added to the current accumulator
subRa, err = subKt.accumulateTarget(ra)
subRa, _, err = subKt.accumulateTarget(ra)
ra = accumulator.MakeEmptyAccumulator()
} else {
// Child Kustomizations create a new accumulator which resolves their kustomization directives, which will later
// be merged into the current accumulator.
subRa, err = subKt.AccumulateTarget()
subRa, _, err = subKt.AccumulateTarget()
}
if err != nil {
return nil, errors.WrapPrefixf(
Expand Down
8 changes: 4 additions & 4 deletions api/internal/target/kusttarget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ metadata:

kt := makeKustTargetWithRf(t, th.GetFSys(), "/whatever", pvd)
assert.NoError(t, kt.Load())
actual, err := kt.MakeCustomizedResMap()
actual, _, err := kt.MakeCustomizedResMap()
assert.NoError(t, err)
actual.RemoveBuildAnnotations()
actYaml, err := actual.AsYaml()
Expand Down Expand Up @@ -386,7 +386,7 @@ metadata:

kt := makeKustTargetWithRf(t, th.GetFSys(), "/merge-config", pvd)
require.NoError(t, kt.Load())
actual, err := kt.MakeCustomizedResMap()
actual, _, err := kt.MakeCustomizedResMap()
require.NoError(t, err)
actual.RemoveBuildAnnotations()
actYaml, err := actual.AsYaml()
Expand Down Expand Up @@ -422,7 +422,7 @@ func TestDuplicateExternalGeneratorsForbidden(t *testing.T) {
image: 'someimage:12345'
configPath: another_config.json
`)
_, err := makeAndLoadKustTarget(t, th.GetFSys(), "/generator").AccumulateTarget()
_, _, err := makeAndLoadKustTarget(t, th.GetFSys(), "/generator").AccumulateTarget()
assert.Error(t, err)
assert.Contains(t, err.Error(), "may not add resource with an already registered id: ManifestGenerator.v1.generators.example/ManifestGenerator")
}
Expand Down Expand Up @@ -451,7 +451,7 @@ func TestDuplicateExternalTransformersForbidden(t *testing.T) {
image: example.docker.com/my-functions/valueannotator:1.0.0
value: 'fail'
`)
_, err := makeAndLoadKustTarget(t, th.GetFSys(), "/transformer").AccumulateTarget()
_, _, err := makeAndLoadKustTarget(t, th.GetFSys(), "/transformer").AccumulateTarget()
assert.Error(t, err)
assert.Contains(t, err.Error(), "may not add resource with an already registered id: ValueAnnotator.v1.transformers.example.co/notImportantHere")
}
6 changes: 3 additions & 3 deletions api/internal/target/vars_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ vars:
name: heron
apiVersion: v300
`)
ra, err := makeAndLoadKustTarget(
ra, _, err := makeAndLoadKustTarget(
t, th.GetFSys(), "/app").AccumulateTarget()
if err != nil {
t.Fatalf("Err: %v", err)
Expand Down Expand Up @@ -119,7 +119,7 @@ resources:
- ../o1
`)

ra, err := makeAndLoadKustTarget(
ra, _, err := makeAndLoadKustTarget(
t, th.GetFSys(), "/app/overlays/o2").AccumulateTarget()
if err != nil {
t.Fatalf("Err: %v", err)
Expand Down Expand Up @@ -176,7 +176,7 @@ vars:
resources:
- ../o1
`)
_, err := makeAndLoadKustTarget(
_, _, err := makeAndLoadKustTarget(
t, th.GetFSys(), "/app/overlays/o2").AccumulateTarget()
if err == nil {
t.Fatalf("expected var collision")
Expand Down
2 changes: 1 addition & 1 deletion api/krusty/accumulation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ resources:
}

b := krusty.MakeKustomizer(krusty.MakeDefaultOptions())
_, err := b.Run(fs, root)
_, _, err := b.Run(fs, root)
require.Regexp(t, buildError(test), err.Error())
})
}
Expand Down
10 changes: 5 additions & 5 deletions api/krusty/fnplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ data:
label_name: my-ns-name
label_value: function-test
`)))
m, err := b.Run(
m, _, err := b.Run(
fSys,
tmpDir.String())
assert.NoError(t, err)
Expand Down Expand Up @@ -703,7 +703,7 @@ data:
data:
value: '{{ env "TESTTEMPLATE" }}'
`)))
m, err := b.Run(
m, _, err := b.Run(
fSys,
tmpDir.String())
assert.NoError(t, err)
Expand Down Expand Up @@ -769,7 +769,7 @@ spec:
assert.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "charts", "helloworld-values", "values.yaml"), []byte(`
replicaCount: 5
`)))
m, err := b.Run(
m, _, err := b.Run(
fSys,
tmpDir.String())
assert.NoError(t, err)
Expand Down Expand Up @@ -808,7 +808,7 @@ generators:
src: "/tmp/dir"
dst: "/tmp/charts"
`)))
_, err = b.Run(
_, _, err = b.Run(
fSys,
tmpDir.String())
assert.Error(t, err)
Expand Down Expand Up @@ -841,7 +841,7 @@ generators:
src: "./tmp/../../dir"
dst: "/tmp/charts"
`)))
_, err = b.Run(
_, _, err = b.Run(
fSys,
tmpDir.String())
assert.Error(t, err)
Expand Down
Loading

0 comments on commit ce9f793

Please sign in to comment.