From fb1e6bc76d59d0d3f85988e70ab198de00e33538 Mon Sep 17 00:00:00 2001 From: Jesse Suen Date: Fri, 14 Jun 2019 15:11:55 -0700 Subject: [PATCH] Change step hashing function to derive hash from json representation of steps --- utils/conditions/conditions.go | 14 ++++++++++---- utils/conditions/conditions_test.go | 8 +++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 70e41228ee..dac3d81707 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -1,6 +1,7 @@ package conditions import ( + "encoding/json" "fmt" "hash/fnv" "math" @@ -236,12 +237,17 @@ func RolloutComplete(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatu // ComputeStepHash returns a hash value calculated from the Rollout's steps. The hash will // be safe encoded to avoid bad words. func ComputeStepHash(rollout *v1alpha1.Rollout) string { - rolloutStepHasher := fnv.New32a() - if rollout.Spec.Strategy.BlueGreenStrategy != nil { + if rollout.Spec.Strategy.BlueGreenStrategy != nil || rollout.Spec.Strategy.CanaryStrategy == nil { return "" } - if rollout.Spec.Strategy.CanaryStrategy != nil { - hashutil.DeepHashObject(rolloutStepHasher, rollout.Spec.Strategy.CanaryStrategy.Steps) + rolloutStepHasher := fnv.New32a() + stepsBytes, err := json.Marshal(rollout.Spec.Strategy.CanaryStrategy.Steps) + if err != nil { + panic(err) + } + _, err = rolloutStepHasher.Write(stepsBytes) + if err != nil { + panic(err) } return rand.SafeEncodeString(fmt.Sprint(rolloutStepHasher.Sum32())) } diff --git a/utils/conditions/conditions_test.go b/utils/conditions/conditions_test.go index 2e88b238d2..00233488bc 100644 --- a/utils/conditions/conditions_test.go +++ b/utils/conditions/conditions_test.go @@ -763,6 +763,9 @@ func TestComputeGenerationHash(t *testing.T) { assert.NotEqual(t, baseline, roPausedHash) } +// TestComputeStableStepHash verifies we generate different hashes for various step definitions. +// Also verifies we do not unintentionally break our ComputeStepHash function somehow (e.g. by +// modifying types or change libraries) func TestComputeStepHash(t *testing.T) { ro := &v1alpha1.Rollout{ Spec: v1alpha1.RolloutSpec{ @@ -788,23 +791,26 @@ func TestComputeStepHash(t *testing.T) { }, } roWithDiffStepsHash := ComputeStepHash(roWithDiffSteps) + assert.Equal(t, "79c9b9f6bf", roWithDiffStepsHash) roWithSameSteps := ro.DeepCopy() roWithSameSteps.Status.CurrentPodHash = "Test" roWithSameSteps.Spec.Replicas = pointer.Int32Ptr(1) roWithSameStepsHash := ComputeStepHash(roWithSameSteps) + assert.Equal(t, "6b9b86fbd5", roWithSameStepsHash) roNoSteps := ro.DeepCopy() roNoSteps.Spec.Strategy.CanaryStrategy.Steps = nil roNoStepsHash := ComputeStepHash(roNoSteps) + assert.Equal(t, "5ffbfbbd64", roNoStepsHash) roBlueGreen := ro.DeepCopy() roBlueGreen.Spec.Strategy.CanaryStrategy = nil roBlueGreen.Spec.Strategy.BlueGreenStrategy = &v1alpha1.BlueGreenStrategy{} roBlueGreenHash := ComputeStepHash(roBlueGreen) + assert.Equal(t, "", roBlueGreenHash) assert.NotEqual(t, baseline, roWithDiffStepsHash) assert.Equal(t, baseline, roWithSameStepsHash) assert.NotEqual(t, baseline, roNoStepsHash) - assert.Equal(t, "", roBlueGreenHash) }