Skip to content

Commit

Permalink
Merge pull request #684 from longquan0104/bugfix/checksum-chart-value…
Browse files Browse the repository at this point in the history 
…s-order-on-key-value

Stable sort release values by key
  • Loading branch information
@hiddeco
hiddeco authored May 11, 2023
2 parents 2d1dbc1 + 30b131a commit 3f2a125
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 4 deletions.
27 changes: 27 additions & 0 deletions api/v2beta1/helmrelease_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,8 @@ func HelmReleaseReady(hr HelmRelease) HelmRelease {

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

// HelmReleaseChanged returns if the HelmRelease has changed compared to the
// provided values.
func HelmReleaseChanged(hr HelmRelease, revision string, releaseRevision int, valuesChecksums ...string) bool {
return hr.Status.LastAttemptedRevision != revision ||
hr.Status.LastReleaseRevision != releaseRevision ||
!inStringSlice(hr.Status.LastAttemptedValuesChecksum, valuesChecksums)
}

// HelmReleaseRecordAttempt returns an attempt of the given HelmRelease with the
// given state in the Status of the provided object.
func HelmReleaseRecordAttempt(hr *HelmRelease, revision string, releaseRevision int, valuesChecksum string) {
hr.Status.LastAttemptedRevision = revision
hr.Status.LastReleaseRevision = releaseRevision
hr.Status.LastAttemptedValuesChecksum = valuesChecksum
}

func inStringSlice(str string, s []string) bool {
for _, v := range s {
if str == v {
return true
}
}
return false
}

func resetFailureCounts(hr *HelmRelease) {
hr.Status.Failures = 0
hr.Status.InstallFailures = 0
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ require (
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/go-digest/blake3 v0.0.0-20220411205349-bde1400a84be
github.com/spf13/pflag v1.0.5
gopkg.in/yaml.v2 v2.4.0
helm.sh/helm/v3 v3.11.3
k8s.io/api v0.26.3
k8s.io/apiextensions-apiserver v0.26.3
Expand Down Expand Up @@ -159,7 +160,6 @@ require (
google.golang.org/grpc v1.53.0 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiserver v0.26.3 // indirect
k8s.io/component-base v0.26.3 // indirect
Expand Down
10 changes: 7 additions & 3 deletions internal/controllers/helmrelease_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,15 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context,
return v2.HelmReleaseNotReady(hr, v2.GetLastReleaseFailedReason, "failed to get last release revision"), err
}

// Register the current release attempt.
// Detect divergence between release in storage and HelmRelease spec.
revision := chart.Metadata.Version
releaseRevision := util.ReleaseRevision(rel)
valuesChecksum := util.ValuesChecksum(values)
hr, hasNewState := v2.HelmReleaseAttempted(hr, revision, releaseRevision, valuesChecksum)
// TODO: deprecate "unordered" checksum.
valuesChecksum := util.OrderedValuesChecksum(values)
hasNewState := v2.HelmReleaseChanged(hr, revision, releaseRevision, util.ValuesChecksum(values), valuesChecksum)

// Register the current release attempt.
v2.HelmReleaseRecordAttempt(&hr, revision, releaseRevision, valuesChecksum)

// Run diff against current cluster state.
if !hasNewState && rel != nil {
Expand Down
78 changes: 78 additions & 0 deletions internal/util/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@ package util
import (
"crypto/sha1"
"fmt"
"sort"

goyaml "gopkg.in/yaml.v2"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/release"
"sigs.k8s.io/yaml"
)

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

// OrderedValuesChecksum sort the chartutil.Values then calculates
// and returns the SHA1 checksum for the sorted values.
func OrderedValuesChecksum(values chartutil.Values) string {
var s []byte
if len(values) != 0 {
msValues := yaml.JSONObjectToYAMLObject(copyValues(values))
SortMapSlice(msValues)
s, _ = goyaml.Marshal(msValues)
}
return fmt.Sprintf("%x", sha1.Sum(s))
}

// SortMapSlice recursively sorts the given goyaml.MapSlice by key.
// This is used to ensure that the values checksum is the same regardless
// of the order of the keys in the values file.
func SortMapSlice(ms goyaml.MapSlice) {
sort.Slice(ms, func(i, j int) bool {
return fmt.Sprint(ms[i].Key) < fmt.Sprint(ms[j].Key)
})
for _, item := range ms {
if nestedMS, ok := item.Value.(goyaml.MapSlice); ok {
SortMapSlice(nestedMS)
} else if _, ok := item.Value.([]interface{}); ok {
for _, vItem := range item.Value.([]interface{}) {
if itemMS, ok := vItem.(goyaml.MapSlice); ok {
SortMapSlice(itemMS)
}
}
}
}
}

// cleanUpMapValue changes all instances of
// map[interface{}]interface{} to map[string]interface{}.
// This is for handling the mismatch when unmarshaling
// reference to the issue: https://github.com/go-yaml/yaml/issues/139
func cleanUpMapValue(v interface{}) interface{} {
switch v := v.(type) {
case []interface{}:
return cleanUpInterfaceArray(v)
case map[interface{}]interface{}:
return cleanUpInterfaceMap(v)
default:
return v
}
}

func cleanUpInterfaceMap(in map[interface{}]interface{}) map[string]interface{} {
result := make(map[string]interface{})
for k, v := range in {
result[fmt.Sprintf("%v", k)] = cleanUpMapValue(v)
}
return result
}

func cleanUpInterfaceArray(in []interface{}) []interface{} {
result := make([]interface{}, len(in))
for i, v := range in {
result[i] = cleanUpMapValue(v)
}
return result
}

func copyValues(in map[string]interface{}) map[string]interface{} {
copiedValues, _ := goyaml.Marshal(in)
newValues := make(map[string]interface{})

_ = goyaml.Unmarshal(copiedValues, newValues)
for i, value := range newValues {
newValues[i] = cleanUpMapValue(value)
}

return newValues
}

// ReleaseRevision returns the revision of the given release.Release.
func ReleaseRevision(rel *release.Release) int {
if rel == nil {
Expand Down
164 changes: 164 additions & 0 deletions internal/util/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package util

import (
"reflect"
"testing"

goyaml "gopkg.in/yaml.v2"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/release"
)
Expand Down Expand Up @@ -54,6 +56,38 @@ func TestValuesChecksum(t *testing.T) {
}
}

func TestOrderedValuesChecksum(t *testing.T) {
tests := []struct {
name string
values chartutil.Values
want string
}{
{
name: "empty",
values: chartutil.Values{},
want: "da39a3ee5e6b4b0d3255bfef95601890afd80709",
},
{
name: "value map",
values: chartutil.Values{
"foo": "bar",
"baz": map[string]string{
"fruit": "apple",
"cool": "stuff",
},
},
want: "dfd6589332e4d2da5df7bcbf5885f406f08b58ee",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := OrderedValuesChecksum(tt.values); got != tt.want {
t.Errorf("OrderedValuesChecksum() = %v, want %v", got, tt.want)
}
})
}
}

func TestReleaseRevision(t *testing.T) {
var rel *release.Release
if rev := ReleaseRevision(rel); rev != 0 {
Expand All @@ -64,3 +98,133 @@ func TestReleaseRevision(t *testing.T) {
t.Fatalf("ReleaseRevision() = %v, want %v", rev, 1)
}
}

func TestSortMapSlice(t *testing.T) {
tests := []struct {
name string
input goyaml.MapSlice
expected goyaml.MapSlice
}{
{
name: "Simple case",
input: goyaml.MapSlice{
{Key: "b", Value: 2},
{Key: "a", Value: 1},
},
expected: goyaml.MapSlice{
{Key: "a", Value: 1},
{Key: "b", Value: 2},
},
},
{
name: "Nested MapSlice",
input: goyaml.MapSlice{
{Key: "b", Value: 2},
{Key: "a", Value: 1},
{Key: "c", Value: goyaml.MapSlice{
{Key: "d", Value: 4},
{Key: "e", Value: 5},
}},
},
expected: goyaml.MapSlice{
{Key: "a", Value: 1},
{Key: "b", Value: 2},
{Key: "c", Value: goyaml.MapSlice{
{Key: "d", Value: 4},
{Key: "e", Value: 5},
}},
},
},
{
name: "Empty MapSlice",
input: goyaml.MapSlice{},
expected: goyaml.MapSlice{},
},
{
name: "Single element",
input: goyaml.MapSlice{
{Key: "a", Value: 1},
},
expected: goyaml.MapSlice{
{Key: "a", Value: 1},
},
},
{
name: "Already sorted",
input: goyaml.MapSlice{
{Key: "a", Value: 1},
{Key: "b", Value: 2},
{Key: "c", Value: 3},
},
expected: goyaml.MapSlice{
{Key: "a", Value: 1},
{Key: "b", Value: 2},
{Key: "c", Value: 3},
},
},

{
name: "Complex Case",
input: goyaml.MapSlice{
{Key: "b", Value: 2},
{Key: "a", Value: map[interface{}]interface{}{
"d": []interface{}{4, 5},
"c": 3,
}},
{Key: "c", Value: goyaml.MapSlice{
{Key: "f", Value: 6},
{Key: "e", Value: goyaml.MapSlice{
{Key: "h", Value: 8},
{Key: "g", Value: 7},
}},
}},
},
expected: goyaml.MapSlice{
{Key: "a", Value: map[interface{}]interface{}{
"c": 3,
"d": []interface{}{4, 5},
}},
{Key: "b", Value: 2},
{Key: "c", Value: goyaml.MapSlice{
{Key: "e", Value: goyaml.MapSlice{
{Key: "g", Value: 7},
{Key: "h", Value: 8},
}},
{Key: "f", Value: 6},
}},
},
},
{
name: "Map slice in slice",
input: goyaml.MapSlice{
{Key: "b", Value: 2},
{Key: "a", Value: []interface{}{
map[interface{}]interface{}{
"d": 4,
"c": 3,
},
1,
}},
},
expected: goyaml.MapSlice{
{Key: "a", Value: []interface{}{
map[interface{}]interface{}{
"c": 3,
"d": 4,
},
1,
}},
{Key: "b", Value: 2},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
SortMapSlice(test.input)
if !reflect.DeepEqual(test.input, test.expected) {
t.Errorf("Expected %v, got %v", test.expected, test.input)
}
})
}
}

0 comments on commit 3f2a125

Please sign in to comment.