diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go index d1f0764aa..6e6e6374c 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go @@ -67,7 +67,8 @@ func upgradePipelineProperties(ctx context.Context, logger *zap.SugaredLogger, k return err } - if !*tcCR.Spec.Pipeline.EnableStepActions { + // For historical reasons, if it is upgraded from a historical version, this field may be nil + if tcCR.Spec.Pipeline.EnableStepActions == nil || !*tcCR.Spec.Pipeline.EnableStepActions { // update enable-step-actions to true from false which is default. tcCR.Spec.Pipeline.EnableStepActions = ptr.Bool(true) _, err = operatorClient.OperatorV1alpha1().TektonConfigs().Update(ctx, tcCR, metav1.UpdateOptions{}) diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go index 6666d8613..ed055765d 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go @@ -66,36 +66,63 @@ func TestResetTektonConfigConditions(t *testing.T) { } func TestUpgradePipelineProperties(t *testing.T) { - ctx := context.TODO() - operatorClient := operatorFake.NewSimpleClientset() - logger := logging.FromContext(ctx).Named("unit-test") - - // there is no tektonConfig CR, returns no error - err := upgradePipelineProperties(ctx, logger, nil, operatorClient, nil) - assert.NoError(t, err) - - // create tekconConfig CR with initial conditions - tc := &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: v1alpha1.ConfigResourceName, + tests := []struct { + name string + tc *v1alpha1.TektonConfig + expected bool + }{ + { + name: "with explicit false enable step actions", + tc: &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + Spec: v1alpha1.TektonConfigSpec{ + Pipeline: v1alpha1.Pipeline{ + PipelineProperties: v1alpha1.PipelineProperties{ + EnableStepActions: ptr.Bool(false), + }, + }, + }, + }, + expected: true, }, - Spec: v1alpha1.TektonConfigSpec{ - Pipeline: v1alpha1.Pipeline{ - PipelineProperties: v1alpha1.PipelineProperties{ - EnableStepActions: ptr.Bool(false), + { + name: "with default pipeline properties", + tc: &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, }, }, + expected: true, }, } - _, err = operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) - assert.NoError(t, err) - // update enable-step-actions to true - err = upgradePipelineProperties(ctx, logger, nil, operatorClient, nil) - assert.NoError(t, err) + ctx := context.TODO() + logger := logging.FromContext(ctx).Named("unit-test") - // verify the pipeline property enable-step-actions to true - tcData, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) - assert.NoError(t, err) - assert.Equal(t, tcData.Spec.Pipeline.EnableStepActions, ptr.Bool(true)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + + // test when no tektonConfig CR exists + err := upgradePipelineProperties(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + + // create tektonConfig CR + if tt.tc != nil { + _, err = operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tt.tc, metav1.CreateOptions{}) + assert.NoError(t, err) + } + + // update enable-step-actions to true + err = upgradePipelineProperties(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + + // verify the pipeline property enable-step-actions is set to true + tcData, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, *tcData.Spec.Pipeline.EnableStepActions, tt.expected) + }) + } }