Repository: tektoncd/operator (go)

Issue:
Tekton operator >= 0.73 raise as error: panic: runtime error: invalid memory address or nil pointer dereference
# Expected Behavior

Deployment on kubernetes of the Tekton Operator (>= 0.73) should not raise an error: `panic: runtime error: invalid memory address or nil pointer dereference` but a more human readable.


# Actual Behavior

Tekton operator >= 0.73 raise as error: panic: runtime error: invalid memory address or nil pointer dereference
```
I1220 15:44:58.104509       1 leaderelection.go:250] attempting to acquire leader lease tekton-operator/tekton-operator-lifecycle.github.com.tektoncd.operator.pkg.reconciler.kubernetes.tektonresult.reconci
I1220 15:45:59.263201       1 leaderelection.go:260] successfully acquired lease tekton-operator/tekton-operator-lifecycle.github.com.tektoncd.operator.pkg.reconciler.shared.tektonconfig.reconciler.00-of-0
{"level":"info","timestamp":"2024-12-20T15:45:59.264Z","logger":"tekton-operator-lifecycle","caller":"leaderelection/context.go:158","msg":"\"tekton-operator-6f69d5b8c9-9rklm_d77cc153-cfe3-4667-9e02-593715
{"level":"info","timestamp":"2024-12-20T15:45:59.268Z","logger":"tekton-operator-lifecycle","caller":"tektonconfig/tektonconfig.go:105","msg":"Reconciling TektonConfig","commit":"dbf2fac-dirty","knative.de
{"level":"error","timestamp":"2024-12-20T15:45:59.330Z","logger":"tekton-operator-lifecycle","caller":"tektonconfig/reconciler.go:295","msg":"Returned an error","commit":"dbf2fac-dirty","knative.dev/pod":"
{"level":"error","timestamp":"2024-12-20T15:45:59.331Z","logger":"tekton-operator-lifecycle","caller":"controller/controller.go:566","msg":"Reconcile error","commit":"dbf2fac-dirty","knative.dev/pod":"tekt
{"level":"info","timestamp":"2024-12-20T15:45:59.331Z","logger":"tekton-operator-lifecycle","caller":"tektonconfig/tektonconfig.go:105","msg":"Reconciling TektonConfig","commit":"dbf2fac-dirty","knative.de
{"level":"info","timestamp":"2024-12-20T15:45:59.331Z","logger":"tekton-operator-lifecycle.event-broadcaster","caller":"record/event.go:364","msg":"Event(v1.ObjectReference{Kind:\"TektonConfig\", Namespace
{"level":"error","timestamp":"2024-12-20T15:45:59.365Z","logger":"tekton-operator-lifecycle","caller":"tektonconfig/reconciler.go:295","msg":"Returned an error","commit":"dbf2fac-dirty","knative.dev/pod":"
{"level":"error","timestamp":"2024-12-20T15:45:59.365Z","logger":"tekton-operator-lifecycle","caller":"controller/controller.go:566","msg":"Reconcile error","commit":"dbf2fac-dirty","knative.dev/pod":"tekt
{"level":"info","timestamp":"2024-12-20T15:45:59.365Z","logger":"tekton-operator-lifecycle","caller":"tektonconfig/tektonconfig.go:105","msg":"Reconciling TektonConfig","commit":"dbf2fac-dirty","knative.de
{"level":"info","timestamp":"2024-12-20T15:45:59.365Z","logger":"tekton-operator-lifecycle.event-broadcaster","caller":"record/event.go:364","msg":"Event(v1.ObjectReference{Kind:\"TektonConfig\", Namespace
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1436a0c]

goroutine 408 [running]:
github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade.upgradePipelineProperties({0x1d432e8, 0x4001264060}, 0x1a566ba?, {0x1f?, 0x0?}, {0xffff53a791b8, 0x40002ff350}, 0x40011475d8?)
    github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go:70 +0x8c
github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade.(*Upgrade).executeUpgrade(0x4000068200, {0x1d432e8, 0x4001264060}, {0x2ccefc0, 0x2, 0x0?}, 0x1)
    github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go:99 +0x2ec
github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade.(*Upgrade).RunPreUpgrade(...)
    github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go:65
github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig.(*Reconciler).ReconcileKind(0x40003ded80, {0x1d432e8, 0x4001264060}, 0x400122a408)
    github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/tektonconfig.go:117 +0x2ac
github.com/tektoncd/operator/pkg/client/injection/reconciler/operator/v1alpha1/tektonconfig.(*reconcilerImpl).Reconcile(0x40004e4820, {0x1d432e8, 0x4001264030}, {0x40005b31e6, 0x6})
    github.com/tektoncd/operator/pkg/client/injection/reconciler/operator/v1alpha1/tektonconfig/reconciler.go:236 +0x3d4
knative.dev/pkg/controller.(*Impl).processNextWorkItem(0x400070dc80)
    knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/controller/controller.go:542 +0x3b8
knative.dev/pkg/controller.(*Impl).RunContext.func3()
    knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/controller/controller.go:491 +0x5c
created by knative.dev/pkg/controller.(*Impl).RunContext in goroutine 392
    knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/controller/controller.go:489 +0x298
```

# Steps to Reproduce the Problem

1. Create a kind cluster
2. Deploy the Tekton Operator >= 0.73
3. Deploy the following Tekton Config: https://github.com/ch007m/fork-konflux-ci/blob/idpbuilder/idp/dependencies/tekton-config/tekton-config.yml
 

# Additional Info

- Kubernetes version:

  **Output of `kubectl version`:**

```
Client Version: v1.31.2
Kustomize Version: v5.4.2
Server Version: v1.30.3
```

- Tekton Pipeline version:

  **Output of `tkn version` or `kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'`**

```
❯ tkn version
Client version: 0.38.1
Pipeline version: unknown, pipeline controller may be installed in another namespace please use tkn version -n {namespace}
Operator version: v0.74.0
```


Pull request description:
fix: avoid panic when upgrading pipeline in tektonconfig
fix #2476

# Changes

<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

<!-- Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! -->

# Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

- [x] Run `make test lint` before submitting a PR
- [x] Includes [tests](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if functionality changed/added)
- [ ] Includes [docs](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if user facing)
- [ ] Commit messages follow [commit message best practices](https://github.com/tektoncd/community/blob/master/standards.md#commit-messages)

_See [the contribution guide](https://github.com/tektoncd/operator/blob/master/CONTRIBUTING.md) for more details._

# Release Notes

```release-note
fix: avoid panic when upgrading pipeline in tektonconfig
```


Patch:
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{})


Test patch:
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)
+		})
+	}
 }