Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/internal/generators/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func MakeConfigMap(
if err != nil {
return nil, err
}
if err = rn.LoadMapIntoConfigMapData(m); err != nil {
if err = rn.LoadMapIntoConfigMapData(m.values); err != nil {
Copy link

@jpfourny jpfourny Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't passing the raw map here throw away the sorting effort?
Also, it looks like sorting is done in LoadMapIntoConfigMapData already:

for _, k := range SortedMapKeys(m) {

I am starting to wonder if the problem happens somewhere else. Could the resource be decoded and re-encoded somewhere else (transformer?).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I suspected that as welll.
My idea was to return that order map first and set the values accordingly to the process. And then see if we can check the order after is loaded. But if is in other place that , this implementation is not enough, or it should be moved.

return nil, err
}
err = copyLabelsAndAnnotations(rn, args.Options)
Expand Down
2 changes: 1 addition & 1 deletion api/internal/generators/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func MakeSecret(
if err != nil {
return nil, err
}
if err = rn.LoadMapIntoSecretData(m); err != nil {
if err = rn.LoadMapIntoSecretData(m.values); err != nil {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

return nil, err
}
copyLabelsAndAnnotations(rn, args.Options)
Expand Down
30 changes: 21 additions & 9 deletions api/internal/generators/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,37 @@ kind: %s
return rn, nil
}

type orderedMap struct {
keys []string
values map[string]string
}

func makeValidatedDataMap(
ldr ifc.KvLoader, name string, sources types.KvPairSources) (map[string]string, error) {
ldr ifc.KvLoader, name string, sources types.KvPairSources,
) (orderedMap, error) {
pairs, err := ldr.Load(sources)
if err != nil {
return nil, errors.WrapPrefix(err, "loading KV pairs", 0)
return orderedMap{}, errors.WrapPrefix(err, "loading KV pairs", 0)
}
knownKeys := make(map[string]string)

seen := make(map[string]struct{})
values := make(map[string]string, len(pairs))
keys := make([]string, len(pairs))

for _, p := range pairs {
// legal key: alphanumeric characters, '-', '_' or '.'
if err := ldr.Validator().ErrIfInvalidKey(p.Key); err != nil {
return nil, err
return orderedMap{}, errors.Wrap(err, 1)
}
if _, ok := knownKeys[p.Key]; ok {
return nil, errors.Errorf(
if _, dup := seen[p.Key]; dup {
return orderedMap{}, errors.Errorf(
"configmap %s illegally repeats the key `%s`", name, p.Key)
}
knownKeys[p.Key] = p.Value
seen[p.Key] = struct{}{}
keys = append(keys, p.Key)
values[p.Key] = p.Value
}
return knownKeys, nil

return orderedMap{keys: keys, values: values}, nil
}

// copyLabelsAndAnnotations copies labels and annotations from
Expand Down
104 changes: 102 additions & 2 deletions api/internal/generators/utils_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// Copyright 2020 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0

package generators_test
package generators

import (
"testing"

"github.com/stretchr/testify/require"
. "sigs.k8s.io/kustomize/api/internal/generators"
"sigs.k8s.io/kustomize/api/ifc"
valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
"sigs.k8s.io/kustomize/api/types"
)

func TestParseFileSource(t *testing.T) {
Expand Down Expand Up @@ -49,3 +51,101 @@ func TestParseFileSource(t *testing.T) {
})
}
}

func TestMakeValidateDataMapOrder(t *testing.T) {
tests := []struct {
name string
pairs []types.Pair
wantKeys []string
wantValues map[string]string
wantErr bool
validatorFn func() ifc.Validator
}{
{
name: "single pair",
pairs: []types.Pair{
{Key: "key", Value: "value"},
},
wantKeys: []string{"key"},
wantValues: map[string]string{"key": "value"},
},
{
name: "multiple pairs preserve order",
pairs: []types.Pair{
{Key: "A", Value: "1"},
{Key: "B", Value: "2"},
{Key: "C", Value: "3"},
},
wantKeys: []string{"A", "B", "C"},
wantValues: map[string]string{"A": "1", "B": "2", "C": "3"},
},
{
name: "multiple pairs out of order - loader order wins",
pairs: []types.Pair{
{Key: "third", Value: "3"},
{Key: "first", Value: "1"},
{Key: "second", Value: "2"},
},
wantKeys: []string{"third", "first", "second"},
wantValues: map[string]string{"third": "3", "first": "1", "second": "2"},
},
{
name: "duplicate key returns error",
pairs: []types.Pair{
{Key: "dup", Value: "1"},
{Key: "dup", Value: "2"},
},
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
loader := &fakeKvLoader{
Pairs: tt.pairs,
Val: func() ifc.Validator {
if tt.validatorFn != nil {
return tt.validatorFn()
}
return valtest_test.MakeFakeValidator()
}(),
}

args := types.ConfigMapArgs{
GeneratorArgs: types.GeneratorArgs{
Name: "envConfigMap",
KvPairSources: types.KvPairSources{
EnvSources: []string{"app.env"},
},
},
}

res, err := makeValidatedDataMap(loader, args.Name, args.KvPairSources)

if tt.wantErr {
require.Error(t, err)
return
}

require.NoError(t, err)
require.Exactly(t, tt.wantKeys, res.keys)
require.Exactly(t, tt.wantValues, res.values)
})
}
}

type fakeKvLoader struct {
Pairs []types.Pair
Val ifc.Validator
}

func (f *fakeKvLoader) Load(_ types.KvPairSources) ([]types.Pair, error) {
return f.Pairs, nil
}

func (f *fakeKvLoader) Validator() ifc.Validator {
if f.Val != nil {
return f.Val
}
return valtest_test.MakeFakeValidator()
}