diff --git a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go index 525275bac0..8b787ceb56 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation.go @@ -54,8 +54,16 @@ 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. 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 1e13553412..4d6f20e3fe 100644 --- a/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go +++ b/pkg/apis/operator/v1alpha1/openshiftpipelinesascode_validation_test.go @@ -194,3 +194,27 @@ 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) + assert.Assert(t, opacCR.Spec.PACSettings.Settings == nil, "Validate must not mutate the CR") +}