fix: promote nil pointer error when there are no steps (argoproj#1510)

Signed-off-by: Leonardo Luz Almeida <[email protected]>

Author: leoluz

pkg/kubectl-argo-rollouts/cmd/promote/promote.go

@@ -138,11 +138,14 @@ func getPatches(rollout *v1alpha1.Rollout, skipCurrentStep, skipAllStep, full bo
 		_, index := replicasetutil.GetCurrentCanaryStep(rollout)
 		// At this point, the controller knows that the rollout is a canary with steps and GetCurrentCanaryStep returns 0 if
 		// the index is not set in the rollout
-		if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
-			*index++
+		if index != nil {
+			if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
+				*index++
+			}
+			statusPatch = []byte(fmt.Sprintf(setCurrentStepIndex, *index))
+			unifiedPatch = statusPatch
 		}
-		statusPatch = []byte(fmt.Sprintf(setCurrentStepIndex, *index))
-		unifiedPatch = statusPatch
+
 	case skipAllStep:
 		statusPatch = []byte(fmt.Sprintf(setCurrentStepIndex, len(rollout.Spec.Strategy.Canary.Steps)))
 		unifiedPatch = statusPatch
@@ -165,11 +168,13 @@ func getPatches(rollout *v1alpha1.Rollout, skipCurrentStep, skipAllStep, full bo
 			_, index := replicasetutil.GetCurrentCanaryStep(rollout)
 			// At this point, the controller knows that the rollout is a canary with steps and GetCurrentCanaryStep returns 0 if
 			// the index is not set in the rollout
-			if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
-				*index++
+			if index != nil {
+				if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
+					*index++
+				}
+				statusPatch = []byte(fmt.Sprintf(clearPauseConditionsPatchWithStep, *index))
+				unifiedPatch = []byte(fmt.Sprintf(unpauseAndClearPauseConditionsPatchWithStep, *index))
 			}
-			statusPatch = []byte(fmt.Sprintf(clearPauseConditionsPatchWithStep, *index))
-			unifiedPatch = []byte(fmt.Sprintf(unpauseAndClearPauseConditionsPatchWithStep, *index))
 		}
 	}
 	return specPatch, statusPatch, unifiedPatch

pkg/kubectl-argo-rollouts/cmd/promote/promote_test.go

@@ -94,6 +94,31 @@ func TestPromoteSkipFlagOnNoStepCanaryError(t *testing.T) {
 	assert.Contains(t, stderr, skipFlagWithNoStepCanaryError)
 }
 
+func TestPromoteNoStepCanary(t *testing.T) {
+	ro := v1alpha1.Rollout{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "guestbook",
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: v1alpha1.RolloutSpec{
+			Strategy: v1alpha1.RolloutStrategy{
+				Canary: &v1alpha1.CanaryStrategy{},
+			},
+		},
+	}
+	tf, o := options.NewFakeArgoRolloutsOptions(&ro)
+	defer tf.Cleanup()
+	cmd := NewCmdPromote(o)
+	cmd.PersistentPreRunE = o.PersistentPreRunE
+	cmd.SetArgs([]string{"guestbook"})
+	err := cmd.Execute()
+	assert.NoError(t, err)
+	stdout := o.Out.(*bytes.Buffer).String()
+	assert.NotEmpty(t, stdout)
+	stderr := o.ErrOut.(*bytes.Buffer).String()
+	assert.Empty(t, stderr)
+}
+
 func TestPromoteCmdSuccesSkipAllSteps(t *testing.T) {
 	ro := v1alpha1.Rollout{
 		ObjectMeta: metav1.ObjectMeta{
@@ -144,7 +169,7 @@ func TestPromoteCmdSuccesSkipAllSteps(t *testing.T) {
 	assert.Empty(t, stderr)
 }
 
-func TestPromoteCmdSuccesFirstStep(t *testing.T) {
+func TestPromoteCmdSuccesFirstStepWithSkipFirstStep(t *testing.T) {
 	ro := v1alpha1.Rollout{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "guestbook",
@@ -194,6 +219,56 @@ func TestPromoteCmdSuccesFirstStep(t *testing.T) {
 	assert.Empty(t, stderr)
 }
 
+func TestPromoteCmdSuccesFirstStep(t *testing.T) {
+	ro := v1alpha1.Rollout{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "guestbook",
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: v1alpha1.RolloutSpec{
+			Strategy: v1alpha1.RolloutStrategy{
+				Canary: &v1alpha1.CanaryStrategy{
+					Steps: []v1alpha1.CanaryStep{
+						{
+							SetWeight: pointer.Int32Ptr(1),
+						},
+						{
+							SetWeight: pointer.Int32Ptr(2),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	tf, o := options.NewFakeArgoRolloutsOptions(&ro)
+	defer tf.Cleanup()
+	fakeClient := o.RolloutsClient.(*fakeroclient.Clientset)
+	fakeClient.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		if patchAction, ok := action.(kubetesting.PatchAction); ok {
+			patchRo := v1alpha1.Rollout{}
+			err := json.Unmarshal(patchAction.GetPatch(), &patchRo)
+			if err != nil {
+				panic(err)
+			}
+			ro.Status.CurrentStepIndex = patchRo.Status.CurrentStepIndex
+		}
+		return true, &ro, nil
+	})
+
+	cmd := NewCmdPromote(o)
+	cmd.PersistentPreRunE = o.PersistentPreRunE
+	cmd.SetArgs([]string{"guestbook"})
+
+	err := cmd.Execute()
+	assert.Nil(t, err)
+	assert.Equal(t, int32(1), *ro.Status.CurrentStepIndex)
+	stdout := o.Out.(*bytes.Buffer).String()
+	stderr := o.ErrOut.(*bytes.Buffer).String()
+	assert.Equal(t, stdout, "rollout 'guestbook' promoted\n")
+	assert.Empty(t, stderr)
+}
+
 func TestPromoteCmdSuccessDoNotGoPastLastStep(t *testing.T) {
 	ro := v1alpha1.Rollout{
 		ObjectMeta: metav1.ObjectMeta{