From 0a295b20e87b1ff9355aec174dd1800932b903cd Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:20:21 -0700 Subject: [PATCH 1/2] fix(pipelinesascode): guard nil Settings in validate Disabling PipelinesAsCode in TektonConfig, via either platforms..pipelinesAsCode.enable: false or the deprecated addon.enablePipelinesAsCode: false, is rejected by the validating webhook, which panics with "assignment to entry in nil map" on every attempt. Because the whole object is rejected, this also blocks unrelated TektonConfig field updates while the disable is requested. SetDefaults nils out PACSettings.Settings on the disable path (both the Kubernetes and OpenShift branches in tektonconfig_defaults.go), but (*PACSettings).validate unconditionally forwards ps.Settings into the vendored pipelines-as-code SyncConfig, which writes into that map unconditionally in getHubCatalogs (default.go:21). A nil map panics on write. Guard ps.Settings to an empty map before calling SyncConfig, in (*PACSettings).validate. This is the single method used by both the OpenShiftPipelinesAsCode.Validate and TektonConfig.Validate (Kubernetes and OpenShift branches) entry points, so one guard covers all three call sites; no other caller of SyncConfig or PACSettings.validate was found unguarded. Added TestValidateNilSettings, which reproduces the exact panic reported in the issue (PACSettings{Settings: nil} through OpenShiftPipelinesAsCode.Validate) before the fix, and passes after it. Verified by temporarily reverting only the guard: the test panics with the same stack (getHubCatalogs -> SyncConfig -> PACSettings.validate -> OpenShiftPipelinesAsCode.Validate) reported in the issue; with the guard restored it passes. Validation: - go test ./pkg/apis/operator/v1alpha1/... (pass) - go test -race ./pkg/apis/operator/v1alpha1/... (pass) - go build ./pkg/apis/... (pass) - make lint-go PKG=./pkg/apis/operator/v1alpha1/... (0 issues) - gofmt -l on changed files (clean) - No API types changed, so codegen is unaffected and was not run. - A full-repo `go build ./...` could not be completed locally: this sandbox has very little free disk space, unrelated to this change, and the build exhausts it while compiling large unrelated dependency trees. The affected package builds, vets, and tests cleanly in isolation. Report: https://github.com/tektoncd/operator/issues/4057 Assisted-by: Claude Sonnet 5 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- .../openshiftpipelinesascode_validation.go | 6 +++++ ...penshiftpipelinesascode_validation_test.go | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go index 525275bac0..ba8eba9e4b 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go @@ -54,6 +54,12 @@ func (pac *OpenShiftPipelinesAsCode) Validate(ctx context.Context) *apis.FieldEr func (ps *PACSettings) validate(logger *zap.SugaredLogger, path string) *apis.FieldError { var errs *apis.FieldError + // Settings may be nil when PAC is disabled (SetDefaults nils it out), but + // SyncConfig writes into it unconditionally, so guard against a nil map here. + if ps.Settings == nil { + ps.Settings = map[string]string{} + } + defaultPacSettings := pacSettings.Settings{} if err := pacSettings.SyncConfig(logger, &defaultPacSettings, ps.Settings, pacSettings.DefaultValidators(), http.DefaultClient); err != nil { errs = errs.Also(apis.ErrInvalidValue(err, fmt.Sprintf("%s.settings", path))) diff --git a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go index 1e13553412..deeada7057 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go @@ -194,3 +194,26 @@ func TestValidateAddtionalPACControllerInvalidSetting(t *testing.T) { err := opacCR.Validate(context.TODO()) assert.Equal(t, "invalid value: invalid value: invalid value for URL, error: parse \"test/path\": invalid URI for request: validation failed for field custom-console-url: spec.additionalPACControllers.settings", err.Error()) } + +// TestValidateNilSettings guards against a regression where a nil +// PACSettings.Settings map (as left behind when PAC is disabled via +// SetDefaults) reaches SyncConfig and panics with "assignment to entry in +// nil map". +func TestValidateNilSettings(t *testing.T) { + opacCR := &OpenShiftPipelinesAsCode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "namespace", + }, + Spec: OpenShiftPipelinesAsCodeSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "openshift-pipelines", + }, + PACSettings: PACSettings{ + Settings: nil, + }, + }, + } + err := opacCR.Validate(context.TODO()) + assert.Assert(t, err == nil, "unexpected validation error: %v", err) +} From bd90612c9df637980bdfe2ab6af2c5a6c4bca41e Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:09:08 -0700 Subject: [PATCH 2/2] fix(pipelinesascode): don't mutate CR when guarding nil Settings Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- .../v1alpha1/openshiftpipelinesascode_validation.go | 10 ++++++---- .../openshiftpipelinesascode_validation_test.go | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go index ba8eba9e4b..8b787ceb56 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go @@ -55,13 +55,15 @@ func (ps *PACSettings) validate(logger *zap.SugaredLogger, path string) *apis.Fi var errs *apis.FieldError // Settings may be nil when PAC is disabled (SetDefaults nils it out), but - // SyncConfig writes into it unconditionally, so guard against a nil map here. - if ps.Settings == nil { - ps.Settings = map[string]string{} + // SyncConfig writes into it unconditionally. Use a local map so the CR + // being validated is not mutated. + settings := ps.Settings + if settings == nil { + settings = map[string]string{} } defaultPacSettings := pacSettings.Settings{} - if err := pacSettings.SyncConfig(logger, &defaultPacSettings, ps.Settings, pacSettings.DefaultValidators(), http.DefaultClient); err != nil { + if err := pacSettings.SyncConfig(logger, &defaultPacSettings, settings, pacSettings.DefaultValidators(), http.DefaultClient); err != nil { errs = errs.Also(apis.ErrInvalidValue(err, fmt.Sprintf("%s.settings", path))) } diff --git a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go index deeada7057..4d6f20e3fe 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go @@ -216,4 +216,5 @@ func TestValidateNilSettings(t *testing.T) { } err := opacCR.Validate(context.TODO()) assert.Assert(t, err == nil, "unexpected validation error: %v", err) + assert.Assert(t, opacCR.Spec.PACSettings.Settings == nil, "Validate must not mutate the CR") }