diff --git a/api/v2beta1/helmrelease_types.go b/api/v2beta1/helmrelease_types.go index 24d0e1d0e..925994083 100644 --- a/api/v2beta1/helmrelease_types.go +++ b/api/v2beta1/helmrelease_types.go @@ -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 || @@ -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 diff --git a/go.mod b/go.mod index 3035a66fa..b29c07f61 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 diff --git a/internal/controllers/helmrelease_controller.go b/internal/controllers/helmrelease_controller.go index fb3dc3ada..0ce6c1413 100644 --- a/internal/controllers/helmrelease_controller.go +++ b/internal/controllers/helmrelease_controller.go @@ -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 { diff --git a/internal/util/util.go b/internal/util/util.go index 5c2247b3d..8c43d53b0 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -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 @@ -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 { diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 1dabf4fe4..04826f642 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -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" ) @@ -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 { @@ -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) + } + }) + } +}