Skip to content

Commit 3f2a125

Browse files
hiddecohiddeco
authored
Merge pull request #684 from longquan0104/bugfix/checksum-chart-values-order-on-key-value
Stable sort release values by key
2 parents 2d1dbc1 + 30b131a commit 3f2a125

File tree

5 files changed

+277
-4
lines changed

5 files changed

+277
-4
lines changed

‎api/v2beta1/helmrelease_types.go

+27
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,8 @@ func HelmReleaseReady(hr HelmRelease) HelmRelease {
945945

946946
// HelmReleaseAttempted registers an attempt of the given HelmRelease with the given state.
947947
// and returns the modified HelmRelease and a boolean indicating a state change.
948+
//
949+
// Deprecated: in favor of HelmReleaseChanged and HelmReleaseRecordAttempt.
948950
func HelmReleaseAttempted(hr HelmRelease, revision string, releaseRevision int, valuesChecksum string) (HelmRelease, bool) {
949951
changed := hr.Status.LastAttemptedRevision != revision ||
950952
hr.Status.LastReleaseRevision != releaseRevision ||
@@ -956,6 +958,31 @@ func HelmReleaseAttempted(hr HelmRelease, revision string, releaseRevision int,
956958
return hr, changed
957959
}
958960

961+
// HelmReleaseChanged returns if the HelmRelease has changed compared to the
962+
// provided values.
963+
func HelmReleaseChanged(hr HelmRelease, revision string, releaseRevision int, valuesChecksums ...string) bool {
964+
return hr.Status.LastAttemptedRevision != revision ||
965+
hr.Status.LastReleaseRevision != releaseRevision ||
966+
!inStringSlice(hr.Status.LastAttemptedValuesChecksum, valuesChecksums)
967+
}
968+
969+
// HelmReleaseRecordAttempt returns an attempt of the given HelmRelease with the
970+
// given state in the Status of the provided object.
971+
func HelmReleaseRecordAttempt(hr *HelmRelease, revision string, releaseRevision int, valuesChecksum string) {
972+
hr.Status.LastAttemptedRevision = revision
973+
hr.Status.LastReleaseRevision = releaseRevision
974+
hr.Status.LastAttemptedValuesChecksum = valuesChecksum
975+
}
976+
977+
func inStringSlice(str string, s []string) bool {
978+
for _, v := range s {
979+
if str == v {
980+
return true
981+
}
982+
}
983+
return false
984+
}
985+
959986
func resetFailureCounts(hr *HelmRelease) {
960987
hr.Status.Failures = 0
961988
hr.Status.InstallFailures = 0

‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ require (
2020
github.com/opencontainers/go-digest v1.0.0
2121
github.com/opencontainers/go-digest/blake3 v0.0.0-20220411205349-bde1400a84be
2222
github.com/spf13/pflag v1.0.5
23+
gopkg.in/yaml.v2 v2.4.0
2324
helm.sh/helm/v3 v3.11.3
2425
k8s.io/api v0.26.3
2526
k8s.io/apiextensions-apiserver v0.26.3
@@ -159,7 +160,6 @@ require (
159160
google.golang.org/grpc v1.53.0 // indirect
160161
google.golang.org/protobuf v1.28.1 // indirect
161162
gopkg.in/inf.v0 v0.9.1 // indirect
162-
gopkg.in/yaml.v2 v2.4.0 // indirect
163163
gopkg.in/yaml.v3 v3.0.1 // indirect
164164
k8s.io/apiserver v0.26.3 // indirect
165165
k8s.io/component-base v0.26.3 // indirect

‎internal/controllers/helmrelease_controller.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,15 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context,
322322
return v2.HelmReleaseNotReady(hr, v2.GetLastReleaseFailedReason, "failed to get last release revision"), err
323323
}
324324

325-
// Register the current release attempt.
325+
// Detect divergence between release in storage and HelmRelease spec.
326326
revision := chart.Metadata.Version
327327
releaseRevision := util.ReleaseRevision(rel)
328-
valuesChecksum := util.ValuesChecksum(values)
329-
hr, hasNewState := v2.HelmReleaseAttempted(hr, revision, releaseRevision, valuesChecksum)
328+
// TODO: deprecate "unordered" checksum.
329+
valuesChecksum := util.OrderedValuesChecksum(values)
330+
hasNewState := v2.HelmReleaseChanged(hr, revision, releaseRevision, util.ValuesChecksum(values), valuesChecksum)
331+
332+
// Register the current release attempt.
333+
v2.HelmReleaseRecordAttempt(&hr, revision, releaseRevision, valuesChecksum)
330334

331335
// Run diff against current cluster state.
332336
if !hasNewState && rel != nil {

‎internal/util/util.go

+78
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ package util
1919
import (
2020
"crypto/sha1"
2121
"fmt"
22+
"sort"
2223

24+
goyaml "gopkg.in/yaml.v2"
2325
"helm.sh/helm/v3/pkg/chartutil"
2426
"helm.sh/helm/v3/pkg/release"
27+
"sigs.k8s.io/yaml"
2528
)
2629

2730
// ValuesChecksum calculates and returns the SHA1 checksum for the
@@ -34,6 +37,81 @@ func ValuesChecksum(values chartutil.Values) string {
3437
return fmt.Sprintf("%x", sha1.Sum([]byte(s)))
3538
}
3639

40+
// OrderedValuesChecksum sort the chartutil.Values then calculates
41+
// and returns the SHA1 checksum for the sorted values.
42+
func OrderedValuesChecksum(values chartutil.Values) string {
43+
var s []byte
44+
if len(values) != 0 {
45+
msValues := yaml.JSONObjectToYAMLObject(copyValues(values))
46+
SortMapSlice(msValues)
47+
s, _ = goyaml.Marshal(msValues)
48+
}
49+
return fmt.Sprintf("%x", sha1.Sum(s))
50+
}
51+
52+
// SortMapSlice recursively sorts the given goyaml.MapSlice by key.
53+
// This is used to ensure that the values checksum is the same regardless
54+
// of the order of the keys in the values file.
55+
func SortMapSlice(ms goyaml.MapSlice) {
56+
sort.Slice(ms, func(i, j int) bool {
57+
return fmt.Sprint(ms[i].Key) < fmt.Sprint(ms[j].Key)
58+
})
59+
for _, item := range ms {
60+
if nestedMS, ok := item.Value.(goyaml.MapSlice); ok {
61+
SortMapSlice(nestedMS)
62+
} else if _, ok := item.Value.([]interface{}); ok {
63+
for _, vItem := range item.Value.([]interface{}) {
64+
if itemMS, ok := vItem.(goyaml.MapSlice); ok {
65+
SortMapSlice(itemMS)
66+
}
67+
}
68+
}
69+
}
70+
}
71+
72+
// cleanUpMapValue changes all instances of
73+
// map[interface{}]interface{} to map[string]interface{}.
74+
// This is for handling the mismatch when unmarshaling
75+
// reference to the issue: https://github.com/go-yaml/yaml/issues/139
76+
func cleanUpMapValue(v interface{}) interface{} {
77+
switch v := v.(type) {
78+
case []interface{}:
79+
return cleanUpInterfaceArray(v)
80+
case map[interface{}]interface{}:
81+
return cleanUpInterfaceMap(v)
82+
default:
83+
return v
84+
}
85+
}
86+
87+
func cleanUpInterfaceMap(in map[interface{}]interface{}) map[string]interface{} {
88+
result := make(map[string]interface{})
89+
for k, v := range in {
90+
result[fmt.Sprintf("%v", k)] = cleanUpMapValue(v)
91+
}
92+
return result
93+
}
94+
95+
func cleanUpInterfaceArray(in []interface{}) []interface{} {
96+
result := make([]interface{}, len(in))
97+
for i, v := range in {
98+
result[i] = cleanUpMapValue(v)
99+
}
100+
return result
101+
}
102+
103+
func copyValues(in map[string]interface{}) map[string]interface{} {
104+
copiedValues, _ := goyaml.Marshal(in)
105+
newValues := make(map[string]interface{})
106+
107+
_ = goyaml.Unmarshal(copiedValues, newValues)
108+
for i, value := range newValues {
109+
newValues[i] = cleanUpMapValue(value)
110+
}
111+
112+
return newValues
113+
}
114+
37115
// ReleaseRevision returns the revision of the given release.Release.
38116
func ReleaseRevision(rel *release.Release) int {
39117
if rel == nil {

‎internal/util/util_test.go

+164
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ limitations under the License.
1717
package util
1818

1919
import (
20+
"reflect"
2021
"testing"
2122

23+
goyaml "gopkg.in/yaml.v2"
2224
"helm.sh/helm/v3/pkg/chartutil"
2325
"helm.sh/helm/v3/pkg/release"
2426
)
@@ -54,6 +56,38 @@ func TestValuesChecksum(t *testing.T) {
5456
}
5557
}
5658

59+
func TestOrderedValuesChecksum(t *testing.T) {
60+
tests := []struct {
61+
name string
62+
values chartutil.Values
63+
want string
64+
}{
65+
{
66+
name: "empty",
67+
values: chartutil.Values{},
68+
want: "da39a3ee5e6b4b0d3255bfef95601890afd80709",
69+
},
70+
{
71+
name: "value map",
72+
values: chartutil.Values{
73+
"foo": "bar",
74+
"baz": map[string]string{
75+
"fruit": "apple",
76+
"cool": "stuff",
77+
},
78+
},
79+
want: "dfd6589332e4d2da5df7bcbf5885f406f08b58ee",
80+
},
81+
}
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
if got := OrderedValuesChecksum(tt.values); got != tt.want {
85+
t.Errorf("OrderedValuesChecksum() = %v, want %v", got, tt.want)
86+
}
87+
})
88+
}
89+
}
90+
5791
func TestReleaseRevision(t *testing.T) {
5892
var rel *release.Release
5993
if rev := ReleaseRevision(rel); rev != 0 {
@@ -64,3 +98,133 @@ func TestReleaseRevision(t *testing.T) {
6498
t.Fatalf("ReleaseRevision() = %v, want %v", rev, 1)
6599
}
66100
}
101+
102+
func TestSortMapSlice(t *testing.T) {
103+
tests := []struct {
104+
name string
105+
input goyaml.MapSlice
106+
expected goyaml.MapSlice
107+
}{
108+
{
109+
name: "Simple case",
110+
input: goyaml.MapSlice{
111+
{Key: "b", Value: 2},
112+
{Key: "a", Value: 1},
113+
},
114+
expected: goyaml.MapSlice{
115+
{Key: "a", Value: 1},
116+
{Key: "b", Value: 2},
117+
},
118+
},
119+
{
120+
name: "Nested MapSlice",
121+
input: goyaml.MapSlice{
122+
{Key: "b", Value: 2},
123+
{Key: "a", Value: 1},
124+
{Key: "c", Value: goyaml.MapSlice{
125+
{Key: "d", Value: 4},
126+
{Key: "e", Value: 5},
127+
}},
128+
},
129+
expected: goyaml.MapSlice{
130+
{Key: "a", Value: 1},
131+
{Key: "b", Value: 2},
132+
{Key: "c", Value: goyaml.MapSlice{
133+
{Key: "d", Value: 4},
134+
{Key: "e", Value: 5},
135+
}},
136+
},
137+
},
138+
{
139+
name: "Empty MapSlice",
140+
input: goyaml.MapSlice{},
141+
expected: goyaml.MapSlice{},
142+
},
143+
{
144+
name: "Single element",
145+
input: goyaml.MapSlice{
146+
{Key: "a", Value: 1},
147+
},
148+
expected: goyaml.MapSlice{
149+
{Key: "a", Value: 1},
150+
},
151+
},
152+
{
153+
name: "Already sorted",
154+
input: goyaml.MapSlice{
155+
{Key: "a", Value: 1},
156+
{Key: "b", Value: 2},
157+
{Key: "c", Value: 3},
158+
},
159+
expected: goyaml.MapSlice{
160+
{Key: "a", Value: 1},
161+
{Key: "b", Value: 2},
162+
{Key: "c", Value: 3},
163+
},
164+
},
165+
166+
{
167+
name: "Complex Case",
168+
input: goyaml.MapSlice{
169+
{Key: "b", Value: 2},
170+
{Key: "a", Value: map[interface{}]interface{}{
171+
"d": []interface{}{4, 5},
172+
"c": 3,
173+
}},
174+
{Key: "c", Value: goyaml.MapSlice{
175+
{Key: "f", Value: 6},
176+
{Key: "e", Value: goyaml.MapSlice{
177+
{Key: "h", Value: 8},
178+
{Key: "g", Value: 7},
179+
}},
180+
}},
181+
},
182+
expected: goyaml.MapSlice{
183+
{Key: "a", Value: map[interface{}]interface{}{
184+
"c": 3,
185+
"d": []interface{}{4, 5},
186+
}},
187+
{Key: "b", Value: 2},
188+
{Key: "c", Value: goyaml.MapSlice{
189+
{Key: "e", Value: goyaml.MapSlice{
190+
{Key: "g", Value: 7},
191+
{Key: "h", Value: 8},
192+
}},
193+
{Key: "f", Value: 6},
194+
}},
195+
},
196+
},
197+
{
198+
name: "Map slice in slice",
199+
input: goyaml.MapSlice{
200+
{Key: "b", Value: 2},
201+
{Key: "a", Value: []interface{}{
202+
map[interface{}]interface{}{
203+
"d": 4,
204+
"c": 3,
205+
},
206+
1,
207+
}},
208+
},
209+
expected: goyaml.MapSlice{
210+
{Key: "a", Value: []interface{}{
211+
map[interface{}]interface{}{
212+
"c": 3,
213+
"d": 4,
214+
},
215+
1,
216+
}},
217+
{Key: "b", Value: 2},
218+
},
219+
},
220+
}
221+
222+
for _, test := range tests {
223+
t.Run(test.name, func(t *testing.T) {
224+
SortMapSlice(test.input)
225+
if !reflect.DeepEqual(test.input, test.expected) {
226+
t.Errorf("Expected %v, got %v", test.expected, test.input)
227+
}
228+
})
229+
}
230+
}

0 commit comments

Comments
 (0)