diff --git a/api/internal/plugins/utils/utils.go b/api/internal/plugins/utils/utils.go index ffd10087a..821b3c859 100644 --- a/api/internal/plugins/utils/utils.go +++ b/api/internal/plugins/utils/utils.go @@ -4,6 +4,7 @@ package utils import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -13,7 +14,6 @@ import ( "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" - "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -148,50 +148,52 @@ func GetResMapWithIDAnnotation(rm resmap.ResMap) (resmap.ResMap, error) { // UpdateResMapValues updates the Resource value in the given ResMap // with the emitted Resource values in output. func UpdateResMapValues(pluginName string, h *resmap.PluginHelpers, output []byte, rm resmap.ResMap) error { - resFactory := h.ResmapFactory().RF() + mapFactory := h.ResmapFactory() + resFactory := mapFactory.RF() resources, err := resFactory.SliceFromBytes(output) if err != nil { return err } + // Don't use resources here, or error message will be unfriendly to plugin builders + newMap, err := mapFactory.NewResMapFromBytes([]byte{}) + if err != nil { + return err + } + for _, r := range resources { - // for each emitted Resource, find the matching Resource in the original ResMap - // using its id - annotations := r.GetAnnotations() - idString, ok := annotations[idAnnotation] - // Transformer-generated resource--add to resMap - if !ok { - if err := rm.Append(r); err != nil { - return err + removeIDAnnotation(r) // stale--not manipulated by plugin transformers + + // Add to the new map, checking for duplicates + if err := newMap.Append(r); err != nil { + prettyID, err := json.Marshal(r.CurId()) + if err != nil { + prettyID = []byte(r.CurId().String()) } - continue + return fmt.Errorf("plugin %s generated duplicate resource: %s", pluginName, prettyID) } - id := resid.ResId{} - err = yaml.Unmarshal([]byte(idString), &id) + // Add to or update the old map + oldIdx, err := rm.GetIndexOfCurrentId(r.CurId()) if err != nil { return err } - - // Outdated ID is likely a duplicated resource--add to resMap - if !id.Equals(r.CurId()) { - removeIDAnnotation(r) + if oldIdx != -1 { + rm.GetByIndex(oldIdx).ResetPrimaryData(r) + } else { if err := rm.Append(r); err != nil { return err } - continue } - - // Existing resource--update in resMap - - res, err := rm.GetByCurrentId(id) - if err != nil { - return fmt.Errorf("unable to find unique match to %s", id.String()) - } - removeIDAnnotation(r) - - // update the resource value with the transformed object - res.ResetPrimaryData(r) } + + // Remove items the transformer deleted from the old map + for _, id := range rm.AllIds() { + newIdx, _ := newMap.GetIndexOfCurrentId(id) + if newIdx == -1 { + rm.Remove(id) + } + } + return nil } diff --git a/plugin/someteam.example.com/v1/starlarkmixer/StarlarkMixer_test.go b/plugin/someteam.example.com/v1/starlarkmixer/StarlarkMixer_test.go index 467179491..aa07ed1f6 100644 --- a/plugin/someteam.example.com/v1/starlarkmixer/StarlarkMixer_test.go +++ b/plugin/someteam.example.com/v1/starlarkmixer/StarlarkMixer_test.go @@ -4,6 +4,7 @@ package starlarkmixer_test import ( + "strings" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -29,6 +30,11 @@ metadata: name: some-cm data: foo: bar +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: delete-me `) th.AssertActualEqualsExpected(rm, ` @@ -59,3 +65,46 @@ metadata: name: net-new `) } + +func TestStarlarkMixerPlugin_duplicate_rejection(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t) + defer th.Reset() + + th.RunTransformerAndCheckError(` +apiVersion: someteam.example.com/v1 +kind: StarlarkMixer +metadata: + name: mixer-instance + annotations: + config.kubernetes.io/function: | + starlark: + path: mixer.star +`, + `apiVersion: v1 +kind: ConfigMap +metadata: + name: some-cm +data: + foo: bar +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-cm-copy +data: + foo: bar +`, + func(t *testing.T, err error) { + if err == nil { + t.Fatal("expected error") + } + expectedErr := "plugin api: someteam.example.com/v1, " + + "kind: StarlarkMixer, " + + "name: mixer-instance generated duplicate " + + "resource: {\"version\":\"v1\",\"kind\":\"ConfigMap\",\"name\":\"some-cm-copy\"}" + if !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("unexpected error: %s", err.Error()) + } + }, + ) +}