Skip to content

Commit

Permalink
Handle errors
Browse files Browse the repository at this point in the history
  • Loading branch information
ash2k committed Jun 4, 2021
1 parent 0305860 commit a3ed120
Show file tree
Hide file tree
Showing 25 changed files with 232 additions and 126 deletions.
10 changes: 8 additions & 2 deletions api/filesys/fsnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func isLegalFileNameForCreation(n string) bool {
func (n *fsNode) RegExpGlob(pattern string) ([]string, error) {
var result []string
var expression = regexp.MustCompile(pattern)
n.WalkMe(func(path string, info os.FileInfo, err error) error {
err := n.WalkMe(func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
Expand All @@ -571,6 +571,9 @@ func (n *fsNode) RegExpGlob(pattern string) ([]string, error) {
}
return nil
})
if err != nil {
return nil, err
}
sort.Strings(result)
return result, nil
}
Expand All @@ -582,7 +585,7 @@ func (n *fsNode) RegExpGlob(pattern string) ([]string, error) {
// This is how /bin/ls behaves.
func (n *fsNode) Glob(pattern string) ([]string, error) {
var result []string
n.WalkMe(func(path string, info os.FileInfo, err error) error {
err := n.WalkMe(func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
Expand All @@ -597,6 +600,9 @@ func (n *fsNode) Glob(pattern string) ([]string, error) {
}
return nil
})
if err != nil {
return nil, err
}
sort.Strings(result)
return result, nil
}
5 changes: 4 additions & 1 deletion api/internal/accumulator/loadconfigfromcrds.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,12 @@ func loadCrdIntoConfig(
}
}
if property.Ref.GetURL() != nil {
loadCrdIntoConfig(
err = loadCrdIntoConfig(
theConfig, theGvk, theMap,
property.Ref.String(), append(path, propName))
if err != nil {
return
}
}
}
return nil
Expand Down
12 changes: 5 additions & 7 deletions api/internal/accumulator/loadconfigfromcrds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"reflect"
"testing"

"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/filesys"
. "sigs.k8s.io/kustomize/api/internal/accumulator"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
Expand Down Expand Up @@ -162,16 +163,13 @@ func TestLoadCRDs(t *testing.T) {
}

fSys := filesys.MakeFsInMemory()
fSys.WriteFile("/testpath/crd.json", []byte(crdContent))
err := fSys.WriteFile("/testpath/crd.json", []byte(crdContent))
require.NoError(t, err)
ldr, err := loader.NewLoader(loader.RestrictionRootOnly, "/testpath", fSys)
if err != nil {
t.Fatalf("unexpected error:%v", err)
}
require.NoError(t, err)

actualTc, err := LoadConfigFromCRDs(ldr, []string{"crd.json"})
if err != nil {
t.Fatalf("unexpected error:%v", err)
}
require.NoError(t, err)
if !reflect.DeepEqual(actualTc, expectedTc) {
t.Fatalf("expected\n %v\n but got\n %v\n", expectedTc, actualTc)
}
Expand Down
30 changes: 20 additions & 10 deletions api/internal/accumulator/resaccumulator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/require"
. "sigs.k8s.io/kustomize/api/internal/accumulator"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
"sigs.k8s.io/kustomize/api/provider"
Expand Down Expand Up @@ -224,20 +225,26 @@ func TestResolveVarConflicts(t *testing.T) {
// create accumulators holding apparently conflicting vars that are not
// actually in conflict because they point to the same concrete value.
rm0 := resmap.New()
rm0.Append(rf.FromMap(fooAws))
err := rm0.Append(rf.FromMap(fooAws))
require.NoError(t, err)
ac0 := MakeEmptyAccumulator()
ac0.AppendAll(rm0)
ac0.MergeVars([]types.Var{varFoo})
err = ac0.AppendAll(rm0)
require.NoError(t, err)
err = ac0.MergeVars([]types.Var{varFoo})
require.NoError(t, err)

rm1 := resmap.New()
rm1.Append(rf.FromMap(barAws))
err = rm1.Append(rf.FromMap(barAws))
require.NoError(t, err)
ac1 := MakeEmptyAccumulator()
ac1.AppendAll(rm1)
ac1.MergeVars([]types.Var{varBar})
err = ac1.AppendAll(rm1)
require.NoError(t, err)
err = ac1.MergeVars([]types.Var{varBar})
require.NoError(t, err)

// validate that two vars of the same name which reference the same concrete
// value do not produce a conflict.
err := ac0.MergeAccumulator(ac1)
err = ac0.MergeAccumulator(ac1)
if err == nil {
t.Fatalf("see bug gh-1600")
}
Expand All @@ -246,10 +253,13 @@ func TestResolveVarConflicts(t *testing.T) {
// two above (because it contains a variable whose name is used in the other
// accumulators AND whose concrete values are different).
rm2 := resmap.New()
rm2.Append(rf.FromMap(barGcp))
err = rm2.Append(rf.FromMap(barGcp))
require.NoError(t, err)
ac2 := MakeEmptyAccumulator()
ac2.AppendAll(rm2)
ac2.MergeVars([]types.Var{varBar})
err = ac2.AppendAll(rm2)
require.NoError(t, err)
err = ac2.MergeVars([]types.Var{varBar})
require.NoError(t, err)
err = ac1.MergeAccumulator(ac2)
if err == nil {
t.Fatalf("dupe vars w/ different concrete values should conflict")
Expand Down
10 changes: 8 additions & 2 deletions api/internal/generators/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ func MakeConfigMap(
if err = rn.LoadMapIntoConfigMapData(m); err != nil {
return nil, err
}
copyLabelsAndAnnotations(rn, args.Options)
setImmutable(rn, args.Options)
err = copyLabelsAndAnnotations(rn, args.Options)
if err != nil {
return nil, err
}
err = setImmutable(rn, args.Options)
if err != nil {
return nil, err
}
return rn, nil
}
5 changes: 4 additions & 1 deletion api/internal/plugins/execplugin/execplugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ type argsConfig struct {

func (p *ExecPlugin) processOptionalArgsFields() error {
var c argsConfig
yaml.Unmarshal(p.cfg, &c)
err := yaml.Unmarshal(p.cfg, &c)
if err != nil {
return err
}
if c.ArgsOneLiner != "" {
p.args, _ = shlex.Split(c.ArgsOneLiner)
}
Expand Down
7 changes: 5 additions & 2 deletions api/internal/plugins/execplugin/execplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/filesys"
. "sigs.k8s.io/kustomize/api/internal/plugins/execplugin"
pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader"
Expand All @@ -21,11 +22,12 @@ import (

func TestExecPluginConfig(t *testing.T) {
fSys := filesys.MakeFsInMemory()
fSys.WriteFile("sed-input.txt", []byte(`
err := fSys.WriteFile("sed-input.txt", []byte(`
s/$FOO/foo/g
s/$BAR/bar baz/g
\ \ \
`))
require.NoError(t, err)
ldr, err := fLdr.NewLoader(
fLdr.RestrictionRootOnly, filesys.Separator, fSys)
if err != nil {
Expand Down Expand Up @@ -62,9 +64,10 @@ s/$BAR/bar baz/g
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
p.Config(
err = p.Config(
resmap.NewPluginHelpers(ldr, pvd.GetFieldValidator(), rf, pc),
yaml)
require.NoError(t, err)

expected := "someteam.example.com/v1/sedtransformer/SedTransformer"
if !strings.HasSuffix(p.Path(), expected) {
Expand Down
4 changes: 3 additions & 1 deletion api/internal/plugins/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,9 @@ func UpdateResMapValues(pluginName string, h *resmap.PluginHelpers, output []byt
for _, id := range rm.AllIds() {
newIdx, _ := newMap.GetIndexOfCurrentId(id)
if newIdx == -1 {
rm.Remove(id)
if err = rm.Remove(id); err != nil {
return err
}
}
}

Expand Down
16 changes: 9 additions & 7 deletions api/internal/plugins/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/filesys"
"sigs.k8s.io/kustomize/api/konfig"
"sigs.k8s.io/kustomize/api/provider"
Expand Down Expand Up @@ -86,8 +87,10 @@ func TestUpdateResourceOptions(t *testing.T) {
}
for i, c := range cases {
name := fmt.Sprintf("test%d", i)
in.Append(makeConfigMap(rf, name, c.behavior, c.hashValue))
expected.Append(makeConfigMapOptions(rf, name, c.behavior, !c.needsHash))
err := in.Append(makeConfigMap(rf, name, c.behavior, c.hashValue))
require.NoError(t, err)
err = expected.Append(makeConfigMapOptions(rf, name, c.behavior, !c.needsHash))
require.NoError(t, err)
}
actual, err := UpdateResourceOptions(in)
assert.NoError(t, err)
Expand All @@ -105,10 +108,9 @@ func TestUpdateResourceOptionsWithInvalidHashAnnotationValues(t *testing.T) {
for i, c := range cases {
name := fmt.Sprintf("test%d", i)
in := resmap.New()
in.Append(makeConfigMap(rf, name, "", &c))
_, err := UpdateResourceOptions(in)
if err == nil {
t.Errorf("expected error from value %q", c)
}
err := in.Append(makeConfigMap(rf, name, "", &c))
require.NoError(t, err)
_, err = UpdateResourceOptions(in)
require.Error(t, err)
}
}
Loading

0 comments on commit a3ed120

Please sign in to comment.