diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_default_test.go b/pkg/apis/operator/v1alpha1/tektonconfig_default_test.go index e93723fea3..a0f2a27c87 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_default_test.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_default_test.go @@ -40,16 +40,16 @@ func Test_SetDefaults_OpenShift_MigratesKubernetesPipelinesAsCode(t *testing.T) Spec: TektonConfigSpec{ CommonSpec: CommonSpec{TargetNamespace: "ns"}, Platforms: Platforms{ - Kubernetes: Kubernetes{PipelinesAsCode: kpac}, + Kubernetes: &Kubernetes{PipelinesAsCode: kpac}, }, }, } tc.SetDefaults(context.TODO()) - if tc.Spec.Platforms.Kubernetes.PipelinesAsCode != nil { - t.Fatalf("expected kubernetes.pipelinesAsCode cleared after migration, got %+v", tc.Spec.Platforms.Kubernetes.PipelinesAsCode) + if tc.Spec.Platforms.Kubernetes != nil { + t.Fatalf("expected kubernetes platform nil after migration, got %+v", tc.Spec.Platforms.Kubernetes) } - if tc.Spec.Platforms.OpenShift.PipelinesAsCode == nil || tc.Spec.Platforms.OpenShift.PipelinesAsCode.PACSettings.Settings["application-name"] != "test" { - t.Fatalf("expected PAC migrated to openshift, got %+v", tc.Spec.Platforms.OpenShift.PipelinesAsCode) + if tc.Spec.Platforms.OpenShift == nil || tc.Spec.Platforms.OpenShift.PipelinesAsCode == nil || tc.Spec.Platforms.OpenShift.PipelinesAsCode.PACSettings.Settings["application-name"] != "test" { + t.Fatalf("expected PAC migrated to openshift, got %+v", tc.Spec.Platforms.OpenShift) } } @@ -211,7 +211,7 @@ func Test_SetDefaults_PipelineAsCode(t *testing.T) { Spec: TektonConfigSpec{ Addon: Addon{EnablePAC: ptr.Bool(false)}, Platforms: Platforms{ - OpenShift: OpenShift{ + OpenShift: &OpenShift{ PipelinesAsCode: &PipelinesAsCode{Enable: ptr.Bool(true)}, }, }, @@ -227,7 +227,7 @@ func Test_SetDefaults_PipelineAsCode(t *testing.T) { Spec: TektonConfigSpec{ Addon: Addon{EnablePAC: ptr.Bool(false)}, Platforms: Platforms{ - Kubernetes: Kubernetes{ + Kubernetes: &Kubernetes{ PipelinesAsCode: &PipelinesAsCode{Enable: ptr.Bool(true)}, }, }, @@ -319,7 +319,7 @@ func Test_SetDefaults_SCC(t *testing.T) { tektonConfig := TektonConfig{ Spec: TektonConfigSpec{ Platforms: Platforms{ - OpenShift: OpenShift{ + OpenShift: &OpenShift{ SCC: test.inputSCC, }, }, diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go index 6dcc879221..8083e4ee70 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go @@ -36,16 +36,20 @@ func (tc *TektonConfig) SetDefaults(ctx context.Context) { tc.Spec.Scheduler.SetDefaults() if IsOpenShiftPlatform() { + if tc.Spec.Platforms.OpenShift == nil { + tc.Spec.Platforms.OpenShift = &OpenShift{} + } + // PAC may appear under spec.platforms.kubernetes if the mutating webhook ran without // PLATFORM=openshift (e.g. wrong image/order) or from older releases. Move it to // spec.platforms.openshift so the stored TektonConfig matches the OpenShift operator. - if tc.Spec.Platforms.Kubernetes.PipelinesAsCode != nil { + if tc.Spec.Platforms.Kubernetes != nil && tc.Spec.Platforms.Kubernetes.PipelinesAsCode != nil { if tc.Spec.Platforms.OpenShift.PipelinesAsCode == nil { p := *tc.Spec.Platforms.Kubernetes.PipelinesAsCode tc.Spec.Platforms.OpenShift.PipelinesAsCode = &p } - tc.Spec.Platforms.Kubernetes.PipelinesAsCode = nil } + tc.Spec.Platforms.Kubernetes = nil if tc.Spec.Platforms.OpenShift.PipelinesAsCode != nil { tc.Spec.Addon.EnablePAC = nil @@ -88,6 +92,11 @@ func (tc *TektonConfig) SetDefaults(ctx context.Context) { setAddonDefaults(&tc.Spec.Addon) } else { // Kubernetes Platform + if tc.Spec.Platforms.Kubernetes == nil { + tc.Spec.Platforms.Kubernetes = &Kubernetes{} + } + tc.Spec.Platforms.OpenShift = nil + if tc.Spec.Platforms.Kubernetes.PipelinesAsCode == nil { tc.Spec.Platforms.Kubernetes.PipelinesAsCode = &PipelinesAsCode{ Enable: ptr.Bool(true), diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_types.go b/pkg/apis/operator/v1alpha1/tektonconfig_types.go index 03f12ddc5a..c669daa58a 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_types.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_types.go @@ -148,9 +148,15 @@ type TektonConfigSpec struct { // platform's spec.platforms subtree. func (s *TektonConfigSpec) PipelinesAsCodeForCurrentPlatform() *PipelinesAsCode { if IsOpenShiftPlatform() { - return s.Platforms.OpenShift.PipelinesAsCode + if s.Platforms.OpenShift != nil { + return s.Platforms.OpenShift.PipelinesAsCode + } + return nil } - return s.Platforms.Kubernetes.PipelinesAsCode + if s.Platforms.Kubernetes != nil { + return s.Platforms.Kubernetes.PipelinesAsCode + } + return nil } // TektonConfigStatus defines the observed state of TektonConfig @@ -215,10 +221,10 @@ type Config struct { type Platforms struct { // OpenShift allows configuring openshift specific components and configurations // +optional - OpenShift OpenShift `json:"openshift,omitempty"` + OpenShift *OpenShift `json:"openshift,omitempty"` // Kubernetes allows configuring kubernetes specific components and configurations // +optional - Kubernetes Kubernetes `json:"kubernetes,omitempty"` + Kubernetes *Kubernetes `json:"kubernetes,omitempty"` } type Hub struct { diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_types_test.go b/pkg/apis/operator/v1alpha1/tektonconfig_types_test.go index 0043fc6161..3e1ee2a2d8 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_types_test.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_types_test.go @@ -17,7 +17,9 @@ limitations under the License. package v1alpha1 import ( + "context" "encoding/json" + "strings" "testing" "gotest.tools/v3/assert" @@ -47,3 +49,43 @@ func TestPrune_PrunePerResourceJSONRoundTrip(t *testing.T) { _, present := roundTripped["prune-per-resource"] assert.Assert(t, present, "prune-per-resource key was dropped from marshaled JSON when false: %s", string(out)) } + +func TestPlatforms_OmitEmptyOppositeplatform(t *testing.T) { + tests := []struct { + name string + platform string + absentKey string + presentKey string + }{ + { + name: "OpenShift should omit kubernetes from serialized JSON", + platform: "openshift", + absentKey: "kubernetes", + presentKey: "openshift", + }, + { + name: "Kubernetes should omit openshift from serialized JSON", + platform: "kubernetes", + absentKey: "openshift", + presentKey: "kubernetes", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("PLATFORM", tt.platform) + + tc := &TektonConfig{} + tc.SetDefaults(context.TODO()) + + out, err := json.Marshal(tc.Spec.Platforms) + assert.NilError(t, err) + + serialized := string(out) + assert.Assert(t, !strings.Contains(serialized, tt.absentKey), + "expected %q to be absent from serialized platforms JSON, got: %s", tt.absentKey, serialized) + assert.Assert(t, strings.Contains(serialized, tt.presentKey), + "expected %q to be present in serialized platforms JSON, got: %s", tt.presentKey, serialized) + }) + } +} diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go index 5151a63082..c877f571a6 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go @@ -75,14 +75,14 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) { } logger := logging.FromContext(ctx) - if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift.PipelinesAsCode != nil { + if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift != nil && tc.Spec.Platforms.OpenShift.PipelinesAsCode != nil { errs = errs.Also(tc.Spec.Platforms.OpenShift.PipelinesAsCode.PACSettings.validate(logger, "spec.platforms.openshift.pipelinesAsCode")) - } else if !IsOpenShiftPlatform() && tc.Spec.Platforms.Kubernetes.PipelinesAsCode != nil { + } else if !IsOpenShiftPlatform() && tc.Spec.Platforms.Kubernetes != nil && tc.Spec.Platforms.Kubernetes.PipelinesAsCode != nil { errs = errs.Also(tc.Spec.Platforms.Kubernetes.PipelinesAsCode.PACSettings.validate(logger, "spec.platforms.kubernetes.pipelinesAsCode")) } // validate SCC config - if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift.SCC != nil { + if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift != nil && tc.Spec.Platforms.OpenShift.SCC != nil { defaultSCC := PipelinesSCC if tc.Spec.Platforms.OpenShift.SCC.Default != "" { defaultSCC = tc.Spec.Platforms.OpenShift.SCC.Default @@ -195,12 +195,12 @@ func isValueInArray(arr []string, key string) bool { return false } -func isOpenShiftPlatformsSectionSet(o OpenShift) bool { - return o.PipelinesAsCode != nil || o.SCC != nil +func isOpenShiftPlatformsSectionSet(o *OpenShift) bool { + return o != nil && (o.PipelinesAsCode != nil || o.SCC != nil) } -func isKubernetesPlatformsSectionSet(k Kubernetes) bool { - return k.PipelinesAsCode != nil +func isKubernetesPlatformsSectionSet(k *Kubernetes) bool { + return k != nil && k.PipelinesAsCode != nil } func verifySCCExists(ctx context.Context, sccName string) error { diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go b/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go index 6eaba1206c..5edcc8a0c1 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go @@ -97,7 +97,7 @@ func Test_ValidateTektonConfig_OpenShiftPlatformsOnKubernetes(t *testing.T) { }, Pruner: Prune{Disabled: true}, Platforms: Platforms{ - OpenShift: OpenShift{ + OpenShift: &OpenShift{ PipelinesAsCode: &PipelinesAsCode{Enable: ptr.Bool(true)}, }, }, @@ -120,7 +120,7 @@ func Test_ValidateTektonConfig_KubernetesPlatformsOnOpenShift(t *testing.T) { }, Pruner: Prune{Disabled: true}, Platforms: Platforms{ - Kubernetes: Kubernetes{ + Kubernetes: &Kubernetes{ PipelinesAsCode: &PipelinesAsCode{Enable: ptr.Bool(true)}, }, }, diff --git a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go index 1c72e6f82b..079c0ea647 100644 --- a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go @@ -1054,8 +1054,16 @@ func (in *PipelinesAsCode) DeepCopy() *PipelinesAsCode { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Platforms) DeepCopyInto(out *Platforms) { *out = *in - in.OpenShift.DeepCopyInto(&out.OpenShift) - in.Kubernetes.DeepCopyInto(&out.Kubernetes) + if in.OpenShift != nil { + in, out := &in.OpenShift, &out.OpenShift + *out = new(OpenShift) + (*in).DeepCopyInto(*out) + } + if in.Kubernetes != nil { + in, out := &in.Kubernetes, &out.Kubernetes + *out = new(Kubernetes) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/dummy.yaml b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/dummy.yaml deleted file mode 100644 index 9517f36448..0000000000 --- a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/dummy.yaml +++ /dev/null @@ -1,4 +0,0 @@ -apiVersion: v1 -kind: Namespace -metadata: - name: pac-test-010 diff --git a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/release.yaml b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/release.yaml new file mode 100644 index 0000000000..d9c09103b5 --- /dev/null +++ b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.1.0/release.yaml @@ -0,0 +1,86 @@ +# Copyright 2024 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +--- +apiVersion: v1 +kind: Namespace +metadata: + name: pipelines-as-code + labels: + app.kubernetes.io/version: "0.1.0" + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: pipelines-as-code-controller + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.1.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: pipelines-as-code + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.1.0" + app.kubernetes.io/part-of: pipelines-as-code +data: + application-name: "Pipelines as Code CI" +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: pipelines-as-code-controller + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.1.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code + template: + metadata: + labels: + app.kubernetes.io/version: "0.1.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code + spec: + serviceAccountName: pipelines-as-code-controller + containers: + - name: pac-controller + image: ghcr.io/openshift-pipelines/pipelines-as-code-controller:0.1.0 + ports: + - name: api + containerPort: 8080 + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + capabilities: + drop: + - "ALL" + seccompProfile: + type: RuntimeDefault diff --git a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/dummy.yaml b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/dummy.yaml deleted file mode 100644 index b77e5d1610..0000000000 --- a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/dummy.yaml +++ /dev/null @@ -1,4 +0,0 @@ -apiVersion: v1 -kind: Namespace -metadata: - name: pac-test-020 diff --git a/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/release.yaml b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/release.yaml new file mode 100644 index 0000000000..e5a95e387c --- /dev/null +++ b/pkg/reconciler/common/testdata/kodata/pipelines-as-code/0.2.0/release.yaml @@ -0,0 +1,86 @@ +# Copyright 2024 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +--- +apiVersion: v1 +kind: Namespace +metadata: + name: pipelines-as-code + labels: + app.kubernetes.io/version: "0.2.0" + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: pipelines-as-code-controller + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.2.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: pipelines-as-code + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.2.0" + app.kubernetes.io/part-of: pipelines-as-code +data: + application-name: "Pipelines as Code CI" +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: pipelines-as-code-controller + namespace: pipelines-as-code + labels: + app.kubernetes.io/version: "0.2.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code + template: + metadata: + labels: + app.kubernetes.io/version: "0.2.0" + app.kubernetes.io/component: controller + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: pipelines-as-code + spec: + serviceAccountName: pipelines-as-code-controller + containers: + - name: pac-controller + image: ghcr.io/openshift-pipelines/pipelines-as-code-controller:0.2.0 + ports: + - name: api + containerPort: 8080 + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + capabilities: + drop: + - "ALL" + seccompProfile: + type: RuntimeDefault diff --git a/pkg/reconciler/openshift/common/metricsca.go b/pkg/reconciler/openshift/common/metricsca.go index e50abcef90..a9e0ec7031 100644 --- a/pkg/reconciler/openshift/common/metricsca.go +++ b/pkg/reconciler/openshift/common/metricsca.go @@ -67,7 +67,7 @@ func ResolveMetricsMTLS(ctx context.Context, operatorClient versioned.Interface, } return false, fmt.Errorf("reading TektonConfig: %w", err) } - if tc.Spec.Platforms.OpenShift.EnableMetricsMTLS == nil || !*tc.Spec.Platforms.OpenShift.EnableMetricsMTLS { + if tc.Spec.Platforms.OpenShift == nil || tc.Spec.Platforms.OpenShift.EnableMetricsMTLS == nil || !*tc.Spec.Platforms.OpenShift.EnableMetricsMTLS { return false, nil } @@ -90,7 +90,8 @@ func IsMetricsMTLSEnabled(lister TektonConfigLister) bool { if err != nil { return false } - return tc.Spec.Platforms.OpenShift.EnableMetricsMTLS != nil && + return tc.Spec.Platforms.OpenShift != nil && + tc.Spec.Platforms.OpenShift.EnableMetricsMTLS != nil && *tc.Spec.Platforms.OpenShift.EnableMetricsMTLS } diff --git a/pkg/reconciler/openshift/common/metricsca_test.go b/pkg/reconciler/openshift/common/metricsca_test.go index 71b691d520..86cd4d5d90 100644 --- a/pkg/reconciler/openshift/common/metricsca_test.go +++ b/pkg/reconciler/openshift/common/metricsca_test.go @@ -150,7 +150,7 @@ func newTektonConfigWithMTLS(enabled *bool) *v1alpha1.TektonConfig { return &v1alpha1.TektonConfig{ Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ EnableMetricsMTLS: enabled, }, }, diff --git a/pkg/reconciler/openshift/common/tlsprofile.go b/pkg/reconciler/openshift/common/tlsprofile.go index 81cb7a46a8..613b980ca7 100644 --- a/pkg/reconciler/openshift/common/tlsprofile.go +++ b/pkg/reconciler/openshift/common/tlsprofile.go @@ -221,7 +221,8 @@ func ResolveCentralTLSToEnvVars(ctx context.Context, lister TektonConfigLister) // nil means the field was not set → treat as true (default-on after SetDefaults). // Explicitly false means the user opted out. - if tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && + if tc.Spec.Platforms.OpenShift != nil && + tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && !*tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig { return nil, nil } diff --git a/pkg/reconciler/openshift/common/tlsprofile_test.go b/pkg/reconciler/openshift/common/tlsprofile_test.go index 3b87222431..dbcdcc58bd 100644 --- a/pkg/reconciler/openshift/common/tlsprofile_test.go +++ b/pkg/reconciler/openshift/common/tlsprofile_test.go @@ -149,6 +149,7 @@ func TestResolveCentralTLSToEnvVars_TektonConfigNotFound(t *testing.T) { func TestResolveCentralTLSToEnvVars_NilTreatedAsEnabled(t *testing.T) { // nil means the field was never set → default-on behaviour; should NOT return nil early. tc := &v1alpha1.TektonConfig{} + tc.Spec.Platforms.OpenShift = &v1alpha1.OpenShift{} tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig = nil lister := &fakeTektonConfigLister{tc: tc} @@ -166,6 +167,7 @@ func TestResolveCentralTLSToEnvVars_NilTreatedAsEnabled(t *testing.T) { func TestResolveCentralTLSToEnvVars_Disabled(t *testing.T) { tc := &v1alpha1.TektonConfig{} disabled := false + tc.Spec.Platforms.OpenShift = &v1alpha1.OpenShift{} tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig = &disabled lister := &fakeTektonConfigLister{tc: tc} result, err := ResolveCentralTLSToEnvVars(context.Background(), lister) @@ -180,6 +182,7 @@ func TestResolveCentralTLSToEnvVars_Disabled(t *testing.T) { func TestResolveCentralTLSToEnvVars_EnabledButNoLister(t *testing.T) { tc := &v1alpha1.TektonConfig{} enabled := true + tc.Spec.Platforms.OpenShift = &v1alpha1.OpenShift{} tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig = &enabled lister := &fakeTektonConfigLister{tc: tc} diff --git a/pkg/reconciler/openshift/namespace/namespace.go b/pkg/reconciler/openshift/namespace/namespace.go index ee8a84d70f..2034efbff7 100644 --- a/pkg/reconciler/openshift/namespace/namespace.go +++ b/pkg/reconciler/openshift/namespace/namespace.go @@ -190,6 +190,9 @@ func (ac *reconciler) admissionAllowed(ctx context.Context, req *admissionv1.Adm } // Check if the SCC requested in namespace is in line with the maxAllowed SCC in TektonConfig + if tc.Spec.Platforms.OpenShift == nil || tc.Spec.Platforms.OpenShift.SCC == nil { + return true, nil, nil + } maxAllowedSCC := tc.Spec.Platforms.OpenShift.SCC.MaxAllowed // If no maxAllowed is set, no problem diff --git a/pkg/reconciler/openshift/tektonconfig/extension.go b/pkg/reconciler/openshift/tektonconfig/extension.go index 497bbae7c9..3f91a8dcdf 100644 --- a/pkg/reconciler/openshift/tektonconfig/extension.go +++ b/pkg/reconciler/openshift/tektonconfig/extension.go @@ -175,7 +175,8 @@ func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.Tekto // console plugin reconciler. PostReconcile consumes the cached value without // re-reading the APIServer. The APIServer watch in controller.go ensures that // a TLS profile change triggers a new reconcile, so the cache is always fresh. - if config.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && + if config.Spec.Platforms.OpenShift != nil && + config.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && *config.Spec.Platforms.OpenShift.EnableCentralTLSConfig { tlsConfig, err := occommon.ResolveCentralTLSToEnvVars(ctx, oe.tektonConfigLister) if err != nil { @@ -277,7 +278,8 @@ func (oe openshiftExtension) GetPlatformData() string { // Collect the TLS profile only when central TLS config is not explicitly disabled. var tlsProfile interface{} - tlsDisabled := tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && + tlsDisabled := tc.Spec.Platforms.OpenShift != nil && + tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig != nil && !*tc.Spec.Platforms.OpenShift.EnableCentralTLSConfig if !tlsDisabled { if profile, err := occommon.GetTLSProfileFromAPIServer(ctx); err == nil { @@ -289,7 +291,8 @@ func (oe openshiftExtension) GetPlatformData() string { // metrics-client-ca the hash changes and all component CRs get their // annotation bumped, immediately triggering their reconcilers. var metricsCABundle string - if tc.Spec.Platforms.OpenShift.EnableMetricsMTLS != nil && *tc.Spec.Platforms.OpenShift.EnableMetricsMTLS { + if tc.Spec.Platforms.OpenShift != nil && + tc.Spec.Platforms.OpenShift.EnableMetricsMTLS != nil && *tc.Spec.Platforms.OpenShift.EnableMetricsMTLS { if cm, err := oe.kubeClientSet.CoreV1().ConfigMaps(tc.GetSpec().GetTargetNamespace()).Get( ctx, occommon.MetricsClientCAConfigMap, metav1.GetOptions{}); err == nil { metricsCABundle = cm.Data[occommon.MetricsClientCAKey] diff --git a/pkg/reconciler/openshift/tektonconfig/rbac.go b/pkg/reconciler/openshift/tektonconfig/rbac.go index 739eb95ea6..ed3fa1c0ab 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac.go @@ -223,6 +223,9 @@ func (r *rbac) ensurePreRequisites(ctx context.Context) error { r.ownerRef = configOwnerRef(*rbacISet) // make sure default SCC is in place + if r.tektonConfig.Spec.Platforms.OpenShift == nil || r.tektonConfig.Spec.Platforms.OpenShift.SCC == nil { + return fmt.Errorf("tektonConfig.Spec.Platforms.OpenShift.SCC cannot be nil") + } defaultSCC := r.tektonConfig.Spec.Platforms.OpenShift.SCC.Default if defaultSCC == "" { // Should not really happen due to defaulting, but okay... diff --git a/pkg/reconciler/openshift/tektonconfig/rbac_test.go b/pkg/reconciler/openshift/tektonconfig/rbac_test.go index 612468b733..fe5c646d30 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac_test.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac_test.go @@ -64,7 +64,7 @@ func TestCreateResources(t *testing.T) { {Name: "createCABundleConfigMaps", Value: "false"}, }, Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ SCC: &v1alpha1.SCC{ Default: "pipelines-scc", }, @@ -98,7 +98,7 @@ func TestCreateResources(t *testing.T) { {Name: "createCABundleConfigMaps", Value: "false"}, }, Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ SCC: &v1alpha1.SCC{ Default: "pipelines-scc", }, @@ -167,7 +167,7 @@ func TestCreateResources(t *testing.T) { {Name: "createCABundleConfigMaps", Value: "true"}, }, Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ SCC: &v1alpha1.SCC{ Default: "pipelines-scc", }, @@ -201,7 +201,7 @@ func TestCreateResources(t *testing.T) { TargetNamespace: "test-ns", }, Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ SCC: &v1alpha1.SCC{ Default: "pipelines-scc", }, @@ -235,7 +235,7 @@ func TestCreateResources(t *testing.T) { TargetNamespace: "test-ns", }, Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ SCC: &v1alpha1.SCC{ Default: "pipelines-scc", }, diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go index 06c173b29f..45b6517cd2 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go @@ -156,16 +156,10 @@ func preUpgradeTektonPruner(ctx context.Context, logger *zap.SugaredLogger, k8sC // preUpgradePipelinesAsCodeArtifacts checks if Pipelines as Code is installed and updates // the hub catalog settings to use the artifact hub URL. It cleans up hub-catalog-name from: -// 1. TektonConfig CR settings -// 2. OpenShiftPipelinesAsCode CR settings +// 1. TektonConfig CR settings (both OpenShift and Kubernetes platforms) +// 2. OpenShiftPipelinesAsCode CR settings (OpenShift only) // 3. pipelines-as-code config map func preUpgradePipelinesAsCodeArtifacts(ctx context.Context, logger *zap.SugaredLogger, k8sClient kubernetes.Interface, operatorClient versioned.Interface, restConfig *rest.Config) error { - // Only run on OpenShift platform - if !v1alpha1.IsOpenShiftPlatform() { - logger.Infof("Not on OpenShift platform, skipping Pipelines as Code artifact upgrade") - return nil - } - // Get TektonConfig CR logger.Infof("Performing preupgrade for Pipelines as Code artifact settings") tc, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) @@ -178,7 +172,18 @@ func preUpgradePipelinesAsCodeArtifacts(ctx context.Context, logger *zap.Sugared return err } - pacSpec := tc.Spec.Platforms.OpenShift.PipelinesAsCode + // Get PaC spec for the current platform + var pacSpec *v1alpha1.PipelinesAsCode + if v1alpha1.IsOpenShiftPlatform() { + if tc.Spec.Platforms.OpenShift != nil { + pacSpec = tc.Spec.Platforms.OpenShift.PipelinesAsCode + } + } else { + if tc.Spec.Platforms.Kubernetes != nil { + pacSpec = tc.Spec.Platforms.Kubernetes.PipelinesAsCode + } + } + // Check if Pipelines as Code is enabled if pacSpec == nil || pacSpec.Enable == nil || !*pacSpec.Enable { logger.Infof("Pipelines as Code is not enabled, skipping artifact upgrade") @@ -218,11 +223,13 @@ func preUpgradePipelinesAsCodeArtifacts(ctx context.Context, logger *zap.Sugared return err } - // Also check and update the OpenShiftPipelinesAsCode CR if it exists - err = updateOpenShiftPipelinesAsCodeCR(ctx, logger, operatorClient) - if err != nil { - logger.Errorw("error updating OpenShiftPipelinesAsCode CR", err) - return err + // Also check and update the OpenShiftPipelinesAsCode CR if it exists (OpenShift only) + if v1alpha1.IsOpenShiftPlatform() { + err = updateOpenShiftPipelinesAsCodeCR(ctx, logger, operatorClient) + if err != nil { + logger.Errorw("error updating OpenShiftPipelinesAsCode CR", err) + return err + } } // Also check and update the deployed pipelines-as-code config map if it exists @@ -232,7 +239,7 @@ func preUpgradePipelinesAsCodeArtifacts(ctx context.Context, logger *zap.Sugared return err } - logger.Infof("Successfully updated Pipelines as Code artifact settings in TektonConfig CR, OpenShiftPipelinesAsCode CR, and config map") + logger.Infof("Successfully updated Pipelines as Code artifact settings") return nil } diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go index 408cd5e5f1..2e5f413be2 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go @@ -213,7 +213,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), }, @@ -233,7 +233,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), PACSettings: v1alpha1.PACSettings{ @@ -259,7 +259,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), PACSettings: v1alpha1.PACSettings{ @@ -285,7 +285,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), PACSettings: v1alpha1.PACSettings{ @@ -311,7 +311,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), PACSettings: v1alpha1.PACSettings{ @@ -338,7 +338,7 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { }, Spec: v1alpha1.TektonConfigSpec{ Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ + OpenShift: &v1alpha1.OpenShift{ PipelinesAsCode: &v1alpha1.PipelinesAsCode{ Enable: ptr.Bool(true), PACSettings: v1alpha1.PACSettings{ @@ -399,7 +399,6 @@ func TestPreUpgradePipelinesAsCodeArtifacts(t *testing.T) { } func TestPreUpgradePipelinesAsCodeArtifacts_NonOpenShift(t *testing.T) { - // Test on non-OpenShift platform t.Setenv("PLATFORM", "kubernetes") ctx := context.TODO() @@ -407,9 +406,43 @@ func TestPreUpgradePipelinesAsCodeArtifacts_NonOpenShift(t *testing.T) { operatorClient := operatorFake.NewSimpleClientset() k8sClient := k8sFake.NewSimpleClientset() - // Should return early without error on non-OpenShift platform - err := preUpgradePipelinesAsCodeArtifacts(ctx, logger, k8sClient, operatorClient, nil) + // Create a TektonConfig CR with Kubernetes platform PipelinesAsCode with tektonhub settings + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + Spec: v1alpha1.TektonConfigSpec{ + Platforms: v1alpha1.Platforms{ + Kubernetes: &v1alpha1.Kubernetes{ + PipelinesAsCode: &v1alpha1.PipelinesAsCode{ + Enable: ptr.Bool(true), + PACSettings: v1alpha1.PACSettings{ + Settings: map[string]string{ + "hub-catalog-type": "tektonhub", + "hub-url": "https://api.hub.tekton.dev/v1", + }, + }, + }, + }, + }, + }, + } + _, err := operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) + assert.NoError(t, err) + + // Run the upgrade function + err = preUpgradePipelinesAsCodeArtifacts(ctx, logger, k8sClient, operatorClient, nil) assert.NoError(t, err) + + // Verify the settings were updated to artifacthub + tcData, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.NotNil(t, tcData.Spec.Platforms.Kubernetes.PipelinesAsCode) + assert.NotNil(t, tcData.Spec.Platforms.Kubernetes.PipelinesAsCode.PACSettings.Settings) + + settings := tcData.Spec.Platforms.Kubernetes.PipelinesAsCode.PACSettings.Settings + assert.Equal(t, "artifacthub", settings["hub-catalog-type"]) + assert.Equal(t, "https://artifacthub.io", settings["hub-url"]) } func TestPreUpgradePipelinesAsCodeArtifacts_NoTektonConfig(t *testing.T) { diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go b/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go index 496365c5b9..38da9f1cc7 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go @@ -40,6 +40,7 @@ var ( preUpgradeTektonPruner, // upgrade #5: pre upgrade tekton pruner removeDeprecatedDisableAffinityAssistant, // upgrade #6: remove deprecated DisableAffinityAssistant field from pipeline config removeHubFromTektonConfig, // upgrade #7: clear deprecated hub field + preUpgradePipelinesAsCodeArtifacts, // upgrade #8: migrate PaC hub catalog settings to artifacthub } // post upgrade functions