Skip to content

Commit 104bf58

Browse files
deepsm007deepsm007
committed
Order branches and commits where loaded, remove marshal overrides
1 parent 7c717b8 commit 104bf58

File tree

8 files changed

+348
-191
lines changed

8 files changed

+348
-191
lines changed

‎cmd/ci-secret-bootstrap/main_test.go‎

Lines changed: 71 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -176,36 +176,61 @@ var (
176176
}
177177

178178
defaultConfig = secretbootstrap.Config{
179+
ClusterGroups: map[string][]string{},
179180
Secrets: []secretbootstrap.SecretConfig{
181+
{
182+
From: map[string]secretbootstrap.ItemContext{
183+
".dockerconfigjson": {
184+
Item: "quay.io",
185+
Field: "pull-credentials",
186+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
187+
},
188+
},
189+
To: []secretbootstrap.SecretContext{
190+
{
191+
Cluster: "default",
192+
Namespace: "ci",
193+
Name: "ci-pull-credentials",
194+
Type: "kubernetes.io/dockerconfigjson",
195+
},
196+
},
197+
},
180198
{
181199
From: map[string]secretbootstrap.ItemContext{
182200
"key-name-1": {
183-
Item: "item-name-1",
184-
Field: "field-name-1",
201+
Item: "item-name-1",
202+
Field: "field-name-1",
203+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
185204
},
186205
"key-name-2": {
187-
Item: "item-name-1",
188-
Field: "field-name-2",
206+
Item: "item-name-1",
207+
Field: "field-name-2",
208+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
189209
},
190210
"key-name-3": {
191-
Item: "item-name-1",
192-
Field: "field-name-3",
211+
Item: "item-name-1",
212+
Field: "field-name-3",
213+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
193214
},
194215
"key-name-4": {
195-
Item: "item-name-2",
196-
Field: "field-name-1",
216+
Item: "item-name-2",
217+
Field: "field-name-1",
218+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
197219
},
198220
"key-name-5": {
199-
Item: "item-name-2",
200-
Field: "field-name-2",
221+
Item: "item-name-2",
222+
Field: "field-name-2",
223+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
201224
},
202225
"key-name-6": {
203-
Item: "item-name-3",
204-
Field: "field-name-1",
226+
Item: "item-name-3",
227+
Field: "field-name-1",
228+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
205229
},
206230
"key-name-7": {
207-
Item: "item-name-2",
208-
Field: "field-name-2",
231+
Item: "item-name-2",
232+
Field: "field-name-2",
233+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
209234
},
210235
},
211236
To: []secretbootstrap.SecretContext{
@@ -221,55 +246,47 @@ var (
221246
},
222247
},
223248
},
224-
{
225-
From: map[string]secretbootstrap.ItemContext{
226-
".dockerconfigjson": {
227-
Item: "quay.io",
228-
Field: "pull-credentials",
229-
},
230-
},
231-
To: []secretbootstrap.SecretContext{
232-
{
233-
Cluster: "default",
234-
Namespace: "ci",
235-
Name: "ci-pull-credentials",
236-
Type: "kubernetes.io/dockerconfigjson",
237-
},
238-
},
239-
},
240249
},
241250
}
242251
defaultConfigWithoutDefaultCluster = secretbootstrap.Config{
252+
ClusterGroups: map[string][]string{},
243253
Secrets: []secretbootstrap.SecretConfig{
244254
{
245255
From: map[string]secretbootstrap.ItemContext{
246256
"key-name-1": {
247-
Item: "item-name-1",
248-
Field: "field-name-1",
257+
Item: "item-name-1",
258+
Field: "field-name-1",
259+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
249260
},
250261
"key-name-2": {
251-
Item: "item-name-1",
252-
Field: "field-name-2",
262+
Item: "item-name-1",
263+
Field: "field-name-2",
264+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
253265
},
254266
"key-name-3": {
255-
Item: "item-name-1",
256-
Field: "field-name-3",
267+
Item: "item-name-1",
268+
Field: "field-name-3",
269+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
257270
},
258271
"key-name-4": {
259-
Item: "item-name-2",
260-
Field: "field-name-1",
272+
Item: "item-name-2",
273+
Field: "field-name-1",
274+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
261275
},
262276
"key-name-5": {
263-
Item: "item-name-2",
264-
Field: "field-name-2",
277+
Item: "item-name-2",
278+
Field: "field-name-2",
279+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
265280
},
266281
"key-name-6": {
267-
Item: "item-name-3",
268-
Field: "field-name-1",
282+
Item: "item-name-3",
283+
Field: "field-name-1",
284+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
269285
},
270286
"key-name-7": {
271-
Item: "item-name-2",
272-
Field: "field-name-2",
287+
Item: "item-name-2",
288+
Field: "field-name-2",
289+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
273290
},
274291
},
275292
To: []secretbootstrap.SecretContext{
@@ -375,8 +392,14 @@ func TestCompleteOptions(t *testing.T) {
375392
expectedConfig: secretbootstrap.Config{
376393
ClusterGroups: map[string][]string{"group-a": {"default"}},
377394
Secrets: []secretbootstrap.SecretConfig{{
378-
From: map[string]secretbootstrap.ItemContext{"key-name-1": {Item: "item-name-1", Field: "field-name-1"}},
379-
To: []secretbootstrap.SecretContext{{ClusterGroups: []string{"group-a"}, Cluster: "default", Namespace: "ns", Name: "name"}},
395+
From: map[string]secretbootstrap.ItemContext{
396+
"key-name-1": {
397+
Item: "item-name-1",
398+
Field: "field-name-1",
399+
DockerConfigJSONData: []secretbootstrap.DockerConfigJSONData{},
400+
},
401+
},
402+
To: []secretbootstrap.SecretContext{{ClusterGroups: []string{"group-a"}, Cluster: "default", Namespace: "ns", Name: "name"}},
380403
}},
381404
},
382405
expectedClusters: []string{"default"},
@@ -997,12 +1020,12 @@ func TestConstructSecrets(t *testing.T) {
9971020
},
9981021
},
9991022
},
1000-
expectedError: `[config.0."key-name-1": item at path "prefix/item-name-1" has no key "field-name-1", config.1.".dockerconfigjson": Error making API request.
1023+
expectedError: `[config.0.".dockerconfigjson": Error making API request.
10011024
10021025
URL: GET fakeVaultClient.GetKV
10031026
Code: 404. Errors:
10041027
1005-
* no data at path prefix/quay.io]`,
1028+
* no data at path prefix/quay.io, config.1."key-name-1": item at path "prefix/item-name-1" has no key "field-name-1"]`,
10061029
expected: map[string][]*coreapi.Secret{},
10071030
},
10081031
{

‎pkg/api/secretbootstrap/secretboostrap.go‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"reflect"
8+
"sort"
89
"strings"
910

1011
"github.com/getlantern/deepcopy"
@@ -61,6 +62,42 @@ type SecretConfig struct {
6162
To []SecretContext `json:"to"`
6263
}
6364

65+
// orderSecretConfig sorts a copy of SecretConfig data structures for deterministic ordering
66+
// This mutates the SecretConfig in place, so it should only be called on copies
67+
func (s *SecretConfig) orderSecretConfig() {
68+
// Sort From map keys and DockerConfigJSONData slices
69+
sortedFrom := make(map[string]ItemContext)
70+
keys := make([]string, 0, len(s.From))
71+
for k := range s.From {
72+
keys = append(keys, k)
73+
}
74+
sort.Strings(keys)
75+
76+
for _, k := range keys {
77+
itemCtx := s.From[k]
78+
// Sort DockerConfigJSONData slice
79+
dockerData := make([]DockerConfigJSONData, len(itemCtx.DockerConfigJSONData))
80+
copy(dockerData, itemCtx.DockerConfigJSONData)
81+
sort.Slice(dockerData, func(i, j int) bool {
82+
if dockerData[i].RegistryURL != dockerData[j].RegistryURL {
83+
return dockerData[i].RegistryURL < dockerData[j].RegistryURL
84+
}
85+
if dockerData[i].Item != dockerData[j].Item {
86+
return dockerData[i].Item < dockerData[j].Item
87+
}
88+
return dockerData[i].AuthField < dockerData[j].AuthField
89+
})
90+
91+
sortedFrom[k] = ItemContext{
92+
Item: itemCtx.Item,
93+
Field: itemCtx.Field,
94+
DockerConfigJSONData: dockerData,
95+
Base64Decode: itemCtx.Base64Decode,
96+
}
97+
}
98+
s.From = sortedFrom
99+
}
100+
64101
// LoadConfigFromFile renders a Config object loaded from the given file
65102
func LoadConfigFromFile(file string, config *Config) error {
66103
bytes, err := gzip.ReadFileMaybeGZIP(file)
@@ -71,14 +108,72 @@ func LoadConfigFromFile(file string, config *Config) error {
71108
}
72109

73110
// SaveConfigToFile serializes a Config object to the given file
111+
// It orders a copy of the config for deterministic output before saving
74112
func SaveConfigToFile(file string, config *Config) error {
75-
bytes, err := yaml.Marshal(config)
113+
// Create a deep copy to avoid mutating the original config
114+
var configCopy Config
115+
if err := deepcopy.Copy(&configCopy, config); err != nil {
116+
return fmt.Errorf("failed to copy config: %w", err)
117+
}
118+
// Order the copy for deterministic output
119+
configCopy.orderConfig()
120+
bytes, err := yaml.Marshal(&configCopy)
76121
if err != nil {
77122
return err
78123
}
79124
return os.WriteFile(file, bytes, 0644)
80125
}
81126

127+
// orderConfig sorts the Config data structures for deterministic ordering
128+
func (c *Config) orderConfig() {
129+
// Sort ClusterGroups map keys
130+
sortedClusterGroups := make(map[string][]string)
131+
clusterGroupKeys := make([]string, 0, len(c.ClusterGroups))
132+
for k := range c.ClusterGroups {
133+
clusterGroupKeys = append(clusterGroupKeys, k)
134+
}
135+
sort.Strings(clusterGroupKeys)
136+
for _, k := range clusterGroupKeys {
137+
clusters := make([]string, len(c.ClusterGroups[k]))
138+
copy(clusters, c.ClusterGroups[k])
139+
sort.Strings(clusters)
140+
sortedClusterGroups[k] = clusters
141+
}
142+
c.ClusterGroups = sortedClusterGroups
143+
144+
// Sort Secrets slice
145+
sort.Slice(c.Secrets, func(i, j int) bool {
146+
// Sort by first To entry's Cluster, then Namespace, then Name
147+
// This groups secrets by cluster, making the output more organized
148+
if len(c.Secrets[i].To) == 0 && len(c.Secrets[j].To) == 0 {
149+
return false
150+
}
151+
if len(c.Secrets[i].To) == 0 {
152+
return true
153+
}
154+
if len(c.Secrets[j].To) == 0 {
155+
return false
156+
}
157+
toI := c.Secrets[i].To[0]
158+
toJ := c.Secrets[j].To[0]
159+
if toI.Cluster != toJ.Cluster {
160+
return toI.Cluster < toJ.Cluster
161+
}
162+
if toI.Namespace != toJ.Namespace {
163+
return toI.Namespace < toJ.Namespace
164+
}
165+
return toI.Name < toJ.Name
166+
})
167+
168+
// Sort UserSecretsTargetClusters
169+
sort.Strings(c.UserSecretsTargetClusters)
170+
171+
// Order each SecretConfig
172+
for i := range c.Secrets {
173+
c.Secrets[i].orderSecretConfig()
174+
}
175+
}
176+
82177
// Config is what we version in our repository
83178
type Config struct {
84179
VaultDPTPPrefix string `json:"vault_dptp_prefix,omitempty"`

‎pkg/api/secretbootstrap/secretboostrap_test.go‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func TestResolving(t *testing.T) {
4949
"group-b": {"b"},
5050
},
5151
Secrets: []SecretConfig{{
52+
From: nil,
5253
To: []SecretContext{
5354
{
5455
ClusterGroups: []string{"group-a", "group-b"},
@@ -89,8 +90,13 @@ func TestResolving(t *testing.T) {
8990
},
9091
expectedConfig: Config{
9192
VaultDPTPPrefix: "prefix",
93+
ClusterGroups: nil,
9294
Secrets: []SecretConfig{{
93-
From: map[string]ItemContext{"...": {Item: "prefix/foo", Field: "bar"}},
95+
From: map[string]ItemContext{"...": {
96+
Item: "prefix/foo",
97+
Field: "bar",
98+
DockerConfigJSONData: nil,
99+
}},
94100
To: []SecretContext{{
95101
Cluster: "foo",
96102
Namespace: "namspace",
@@ -116,6 +122,7 @@ func TestResolving(t *testing.T) {
116122
},
117123
expectedConfig: Config{
118124
VaultDPTPPrefix: "prefix",
125+
ClusterGroups: nil,
119126
Secrets: []SecretConfig{{
120127
From: map[string]ItemContext{"...": {DockerConfigJSONData: []DockerConfigJSONData{{Item: "prefix/foo", AuthField: "bar"}}}},
121128
To: []SecretContext{{
@@ -168,8 +175,16 @@ func TestLoadConfigFromFile(t *testing.T) {
168175
Secrets: []SecretConfig{
169176
{
170177
From: map[string]ItemContext{
171-
"ops-mirror.pem": {Item: "dptp/mirror.openshift.com", Field: "cert-key.pem"},
172-
"rh-cdn.pem": {Item: "dptp/rh-cdn", Field: "rh-cdn.pem"},
178+
"ops-mirror.pem": {
179+
Item: "dptp/mirror.openshift.com",
180+
Field: "cert-key.pem",
181+
DockerConfigJSONData: nil,
182+
},
183+
"rh-cdn.pem": {
184+
Item: "dptp/rh-cdn",
185+
Field: "rh-cdn.pem",
186+
DockerConfigJSONData: nil,
187+
},
173188
},
174189
To: []SecretContext{{
175190
ClusterGroups: []string{"build_farm"},
@@ -241,10 +256,22 @@ func TestRoundtripConfig(t *testing.T) {
241256
t.Fatalf("error saving config file: %v", err)
242257
}
243258

259+
// Load both input and output files into Config structs and normalize them
260+
// This allows comparison regardless of the input file's ordering
244261
inFile := filepath.Join("testdata", "zz_fixture_TestRoundtripConfig_basic_base.yaml")
245-
in, _ := os.ReadFile(inFile)
246-
out, _ := os.ReadFile(outFile)
247-
if diff := cmp.Diff(in, out); diff != "" {
262+
var inputConfig, outputConfig Config
263+
if err := LoadConfigFromFile(inFile, &inputConfig); err != nil {
264+
t.Fatalf("error loading input config file: %v", err)
265+
}
266+
if err := LoadConfigFromFile(outFile, &outputConfig); err != nil {
267+
t.Fatalf("error loading output config file: %v", err)
268+
}
269+
270+
// Normalize both configs for comparison
271+
inputConfig.orderConfig()
272+
outputConfig.orderConfig()
273+
274+
if diff := cmp.Diff(inputConfig, outputConfig); diff != "" {
248275
t.Fatalf("input and output configs are not equal. %s", diff)
249276
}
250277
})

0 commit comments

Comments
 (0)