From 4572a2c3957463961f947e1d64953a2fbe2ce399 Mon Sep 17 00:00:00 2001 From: Shiv Verma Date: Mon, 20 Jul 2026 11:38:46 +0530 Subject: [PATCH] feat(manualapprovalgate): integrate MAG as TektonConfig child component Add ManualApproval field to TektonConfig spec so that TektonConfig manages the ManualApprovalGate CR lifecycle including ownerRef and platform-data-hash propagation for TLS profile changes. - Disabled by default on fresh installs - On upgrade, standalone MAG CRs (no ownerRef) are adopted under TektonConfig instead of being deleted - Add validation for spec.manualApproval.options - Add unit tests for create, delete, ownerRef migration, and platform-data-hash annotation sync Fixes #3656 Signed-off-by: Shiv Verma Assisted-by: Claude Opus 4.6 (via Claude Code) --- ...erator.tekton.dev_manualapprovalgates.yaml | 493 +----------------- .../operator.tekton.dev_tektonconfigs.yaml | 43 ++ docs/TektonConfig.md | 20 + .../v1alpha1/manualapprovalgate_defaults.go | 34 ++ .../manualapprovalgate_defaults_test.go | 109 ++++ .../v1alpha1/manualapprovalgate_types.go | 3 + .../v1alpha1/manualapprovalgate_validation.go | 2 +- .../manualapprovalgate_validation_test.go | 85 +++ .../v1alpha1/tektonconfig_defaults.go | 1 + .../operator/v1alpha1/tektonconfig_types.go | 3 + .../v1alpha1/tektonconfig_validation.go | 1 + .../v1alpha1/tektonconfig_validation_test.go | 31 ++ .../v1alpha1/zz_generated.deepcopy.go | 6 + .../shared/tektonconfig/controller.go | 7 + .../manualapprovalgate/manualapprovalgate.go | 160 ++++++ .../manualapprovalgate_test.go | 239 +++++++++ .../shared/tektonconfig/tektonconfig.go | 48 ++ .../tektonconfig/upgrade/pre_upgrade.go | 39 ++ .../tektonconfig/upgrade/pre_upgrade_test.go | 129 +++++ .../shared/tektonconfig/upgrade/upgrade.go | 1 + pkg/webhook/webhook.go | 13 +- 21 files changed, 970 insertions(+), 497 deletions(-) create mode 100644 pkg/apis/operator/v1alpha1/manualapprovalgate_defaults.go create mode 100644 pkg/apis/operator/v1alpha1/manualapprovalgate_defaults_test.go create mode 100644 pkg/apis/operator/v1alpha1/manualapprovalgate_validation_test.go create mode 100644 pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate.go create mode 100644 pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate_test.go diff --git a/config/base/generated-crds/operator.tekton.dev_manualapprovalgates.yaml b/config/base/generated-crds/operator.tekton.dev_manualapprovalgates.yaml index 3dcf235d6e..eb923f7267 100644 --- a/config/base/generated-crds/operator.tekton.dev_manualapprovalgates.yaml +++ b/config/base/generated-crds/operator.tekton.dev_manualapprovalgates.yaml @@ -50,496 +50,9 @@ spec: type: object spec: properties: - networkPolicy: - description: |- - NetworkPolicy configures NetworkPolicy creation for the controller - and webhook workloads deployed by ManualApprovalGate. - properties: - disabled: - description: |- - Disabled disables all NetworkPolicy creation for this component. - Existing policies are removed on the next reconcile. - type: boolean - policies: - additionalProperties: - description: NetworkPolicySpec provides the specification of - a NetworkPolicy - properties: - egress: - description: |- - egress is a list of egress rules to be applied to the selected pods. Outgoing traffic - is allowed if there are no NetworkPolicies selecting the pod (and cluster policy - otherwise allows the traffic), OR if the traffic matches at least one egress rule - across all of the NetworkPolicy objects whose podSelector matches the pod. If - this field is empty then this NetworkPolicy limits all outgoing traffic (and serves - solely to ensure that the pods it selects are isolated by default). - This field is beta-level in 1.8 - items: - description: |- - NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. - This type is beta-level in 1.8 - properties: - ports: - description: |- - ports is a list of destination ports for outgoing traffic. - Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - to: - description: |- - to is a list of destinations for outgoing traffic of pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all destinations (traffic not restricted by - destination). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the to list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The requirements - are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The requirements - are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - ingress: - description: |- - ingress is a list of ingress rules to be applied to the selected pods. - Traffic is allowed to a pod if there are no NetworkPolicies selecting the pod - (and cluster policy otherwise allows the traffic), OR if the traffic source is - the pod's local node, OR if the traffic matches at least one ingress rule - across all of the NetworkPolicy objects whose podSelector matches the pod. If - this field is empty then this NetworkPolicy does not allow any traffic (and serves - solely to ensure that the pods it selects are isolated by default) - items: - description: |- - NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. - properties: - from: - description: |- - from is a list of sources which should be able to access the pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all sources (traffic not restricted by - source). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the from list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The requirements - are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The requirements - are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - ports: - description: |- - ports is a list of ports which should be made accessible on the pods selected for - this rule. Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - podSelector: - description: |- - podSelector selects the pods to which this NetworkPolicy object applies. - The array of rules is applied to any pods selected by this field. An empty - selector matches all pods in the policy's namespace. - Multiple network policies can select the same set of pods. In this case, - the ingress rules for each are combined additively. - This field is optional. If it is not specified, it defaults to an empty selector. - properties: - matchExpressions: - description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key that the selector - applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - policyTypes: - description: |- - policyTypes is a list of rule types that the NetworkPolicy relates to. - Valid options are ["Ingress"], ["Egress"], or ["Ingress", "Egress"]. - If this field is not specified, it will default based on the existence of ingress or egress rules; - policies that contain an egress section are assumed to affect egress, and all policies - (whether or not they contain an ingress section) are assumed to affect ingress. - If you want to write an egress-only policy, you must explicitly specify policyTypes [ "Egress" ]. - Likewise, if you want to write a policy that specifies that no egress is allowed, - you must specify a policyTypes value that include "Egress" (since such a policy would not include - an egress section and would otherwise default to just [ "Ingress" ]). - This field is beta-level in 1.8 - items: - description: |- - PolicyType string describes the NetworkPolicy type - This type is beta-level in 1.8 - type: string - type: array - x-kubernetes-list-type: atomic - type: object - description: |- - Policies merges with the operator's default NetworkPolicies by name. - A key matching a default policy name replaces that default entirely. - A key not matching any default is added alongside the defaults. - If nil or empty, all operator defaults are applied unchanged. - type: object - x-kubernetes-preserve-unknown-fields: true - type: object + disabled: + description: enable or disable manual approval gate feature + type: boolean options: description: options holds additions fields and these fields will be updated on the manifests diff --git a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml index 87eec9f4f5..66ec3282d6 100644 --- a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml +++ b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml @@ -533,6 +533,49 @@ spec: type: string type: object type: array + required: + - options + type: object + manualApproval: + description: ManualApproval holds the customizable options for the + ManualApprovalGate component + properties: + disabled: + description: enable or disable manual approval gate feature + type: boolean + options: + description: options holds additions fields and these fields will + be updated on the manifests + properties: + configMaps: + x-kubernetes-preserve-unknown-fields: true + deployments: + x-kubernetes-preserve-unknown-fields: true + disabled: + type: boolean + horizontalPodAutoscalers: + x-kubernetes-preserve-unknown-fields: true + statefulSets: + x-kubernetes-preserve-unknown-fields: true + webhookConfigurationOptions: + additionalProperties: + description: WebhookOptions defines options for webhooks + properties: + failurePolicy: + description: FailurePolicyType specifies a failure policy + that defines how unrecognized errors from the admission + endpoint are handled. + type: string + sideEffects: + description: SideEffectClass specifies the types of + side effects a webhook may have. + type: string + timeoutSeconds: + format: int32 + type: integer + type: object + type: object + type: object type: object multiclusterProxyAAE: description: MulticlusterProxyAAE holds the customizable options for diff --git a/docs/TektonConfig.md b/docs/TektonConfig.md index 35fd1389dc..a992cb045d 100644 --- a/docs/TektonConfig.md +++ b/docs/TektonConfig.md @@ -19,6 +19,7 @@ Other than the above components depending on the platform operator also provides - On both Kubernetes and OpenShift - [TektonChain](./TektonChain.md) - [TektonResult](./TektonResult.md) + - [ManualApprovalGate](./ManualApprovalGate.md) - On Kubernetes - [TektonDashboard](./TektonDashboard.md) - [OpenShiftPipelinesAsCode](./OpenShiftPipelinesAsCode.md) (installed via `spec.platforms.kubernetes.pipelinesAsCode`; same CRD/kind as on OpenShift) @@ -47,6 +48,8 @@ spec: priorityClassName: system-cluster-critical chain: disabled: false + manualApproval: + disabled: true pipeline: await-sidecar-readiness: true coschedule: workspaces @@ -306,6 +309,23 @@ chain: transparency.url: #value ``` +### Manual Approval Gate + +Manual Approval Gate section allows user to enable or disable the [ManualApprovalGate](./ManualApprovalGate.md) component through TektonConfig. When enabled, the operator installs and manages the ManualApprovalGate CR automatically. + +Example: + +```yaml +manualApproval: + disabled: true # - `disabled` : if the value set as `true`, ManualApprovalGate will not be installed (default: `true`) + options: + disabled: false + deployments: {} +``` + +- `disabled`: if set to `true`, the ManualApprovalGate component will not be installed. Default is `true` (disabled). +- `options`: allows customizing the ManualApprovalGate deployments and configmaps. See [Additional fields as options](#additional-fields-as-options) for details. + ### Result Result section allows user to customize the Tekton Result component, Refer to [Result Spec](https://github.com/tektoncd/operator/blob/main/docs/TektonResult.md#spec) section in TektonResult for available options. diff --git a/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults.go b/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults.go new file mode 100644 index 0000000000..93664def94 --- /dev/null +++ b/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults.go @@ -0,0 +1,34 @@ +/* +Copyright 2026 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. +*/ + +package v1alpha1 + +import "context" + +func (mag *ManualApprovalGate) SetDefaults(_ context.Context) { + mag.Spec.ManualApproval.setDefaults() +} + +func (m *ManualApproval) setDefaults() { + if m.Disabled == nil { + disabled := true + m.Disabled = &disabled + } +} + +func (m *ManualApproval) IsDisabled() bool { + return m.Disabled == nil || *m.Disabled +} diff --git a/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults_test.go b/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults_test.go new file mode 100644 index 0000000000..22243706db --- /dev/null +++ b/pkg/apis/operator/v1alpha1/manualapprovalgate_defaults_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2026 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. +*/ + +package v1alpha1 + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestSetDefaultsManualApprovalGate(t *testing.T) { + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ManualApprovalGates, + }, + Spec: ManualApprovalGateSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } + + mag.SetDefaults(context.TODO()) + + if mag.Spec.ManualApproval.Disabled == nil { + t.Error("expected Disabled to be set, got nil") + } + if !*mag.Spec.ManualApproval.Disabled { + t.Error("expected Disabled to default to true, got false") + } +} + +func TestSetDefaultsManualApprovalGate_DisabledAlreadySet(t *testing.T) { + disabled := false + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ManualApprovalGates, + }, + Spec: ManualApprovalGateSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + ManualApproval: ManualApproval{ + Disabled: &disabled, + }, + }, + } + + mag.SetDefaults(context.TODO()) + + if mag.Spec.ManualApproval.Disabled == nil { + t.Error("expected Disabled to remain set, got nil") + } + if *mag.Spec.ManualApproval.Disabled { + t.Error("expected Disabled to remain false, got true") + } +} + +func TestManualApprovalIsDisabled(t *testing.T) { + tests := []struct { + name string + disabled *bool + want bool + }{ + { + name: "nil defaults to disabled", + disabled: nil, + want: true, + }, + { + name: "explicitly disabled", + disabled: boolPtr(true), + want: true, + }, + { + name: "explicitly enabled", + disabled: boolPtr(false), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &ManualApproval{Disabled: tt.disabled} + if got := m.IsDisabled(); got != tt.want { + t.Errorf("IsDisabled() = %v, want %v", got, tt.want) + } + }) + } +} + +func boolPtr(b bool) *bool { + return &b +} diff --git a/pkg/apis/operator/v1alpha1/manualapprovalgate_types.go b/pkg/apis/operator/v1alpha1/manualapprovalgate_types.go index 3f209e1693..4b3268571c 100644 --- a/pkg/apis/operator/v1alpha1/manualapprovalgate_types.go +++ b/pkg/apis/operator/v1alpha1/manualapprovalgate_types.go @@ -55,6 +55,9 @@ type ManualApprovalGateSpec struct { } type ManualApproval struct { + // enable or disable manual approval gate feature + // +optional + Disabled *bool `json:"disabled,omitempty"` // options holds additions fields and these fields will be updated on the manifests // +optional Options AdditionalOptions `json:"options"` diff --git a/pkg/apis/operator/v1alpha1/manualapprovalgate_validation.go b/pkg/apis/operator/v1alpha1/manualapprovalgate_validation.go index 85fddd1743..d01ab308c7 100644 --- a/pkg/apis/operator/v1alpha1/manualapprovalgate_validation.go +++ b/pkg/apis/operator/v1alpha1/manualapprovalgate_validation.go @@ -1,5 +1,5 @@ /* -Copyright 2024 The Tekton Authors +Copyright 2026 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. diff --git a/pkg/apis/operator/v1alpha1/manualapprovalgate_validation_test.go b/pkg/apis/operator/v1alpha1/manualapprovalgate_validation_test.go new file mode 100644 index 0000000000..5469cc7ec9 --- /dev/null +++ b/pkg/apis/operator/v1alpha1/manualapprovalgate_validation_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2026 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. +*/ + +package v1alpha1 + +import ( + "context" + "testing" + + "gotest.tools/v3/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" +) + +func TestValidateManualApprovalGate_ValidConfig(t *testing.T) { + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ManualApprovalGates, + }, + Spec: ManualApprovalGateSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } + + err := mag.Validate(context.TODO()) + if err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + +func TestValidateManualApprovalGate_InvalidResourceName(t *testing.T) { + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-name", + }, + Spec: ManualApprovalGateSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } + + err := mag.Validate(context.TODO()) + assert.ErrorContains(t, err, "Only one instance of ManualApprovalGate is allowed") +} + +func TestValidateManualApprovalGate_MissingTargetNamespace(t *testing.T) { + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ManualApprovalGates, + }, + Spec: ManualApprovalGateSpec{}, + } + + err := mag.Validate(context.TODO()) + assert.ErrorContains(t, err, "missing field(s): spec.targetNamespace") +} + +func TestValidateManualApprovalGate_SkipOnDelete(t *testing.T) { + mag := &ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-name", + }, + } + + err := mag.Validate(apis.WithinDelete(context.TODO())) + if err != nil { + t.Errorf("expected no error on delete, got: %v", err) + } +} diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go index 6dcc879221..563272b105 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go @@ -34,6 +34,7 @@ func (tc *TektonConfig) SetDefaults(ctx context.Context) { tc.Spec.Result.setDefaults() tc.Spec.TektonPruner.SetDefaults() tc.Spec.Scheduler.SetDefaults() + tc.Spec.ManualApproval.setDefaults() if IsOpenShiftPlatform() { // PAC may appear under spec.platforms.kubernetes if the mutating webhook ran without diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_types.go b/pkg/apis/operator/v1alpha1/tektonconfig_types.go index a225e7132a..ec5fe79e7d 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_types.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_types.go @@ -116,6 +116,9 @@ type TektonConfigSpec struct { // Chain holds the customizable option for chains component // +optional Chain Chain `json:"chain,omitempty"` + // ManualApproval holds the customizable options for the ManualApprovalGate component + // +optional + ManualApproval ManualApproval `json:"manualApproval,omitempty"` // Result holds the customize option for results component // +optional Result Result `json:"result,omitempty"` diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go index 5151a63082..66504c053a 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go @@ -143,6 +143,7 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(tc.Spec.Result.Options.validate("spec.result.options")) errs = errs.Also(tc.Spec.Result.Watcher.Validate("spec.result.watcher")) errs = errs.Also(tc.Spec.MulticlusterProxyAAE.Options.validate("spec.multiclusterProxyAAE.options")) + errs = errs.Also(tc.Spec.ManualApproval.Options.validate("spec.manualApproval.options")) return errs.Also(tc.Spec.Trigger.TriggersProperties.validate("spec.trigger")) } diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go b/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go index 6eaba1206c..47a827a21f 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go @@ -287,6 +287,37 @@ func Test_ValidateTektonConfig_InvalidPipelineOptions(t *testing.T) { assert.Equal(t, "invalid value: InvalidPolicy: spec.pipeline.options.webhookconfigurationoptions.failurePolicy", err.Error()) } +func Test_ValidateTektonConfig_InvalidManualApprovalOptions(t *testing.T) { + invalidPolicy := admissionregistrationv1.FailurePolicyType("InvalidPolicy") + sideEffectUnknown := admissionregistrationv1.SideEffectClassUnknown + tc := &TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + Namespace: "namespace", + }, + Spec: TektonConfigSpec{ + CommonSpec: CommonSpec{ + TargetNamespace: "namespace", + }, + Profile: "all", + ManualApproval: ManualApproval{ + Options: AdditionalOptions{ + WebhookConfigurationOptions: map[string]WebhookConfigurationOptions{ + "validation.webhook.manualapproval.dev": { + FailurePolicy: &invalidPolicy, + SideEffects: &sideEffectUnknown, + }, + }, + }, + }, + Pruner: Prune{Disabled: true}, + }, + } + + err := tc.Validate(context.TODO()) + assert.Equal(t, "invalid value: InvalidPolicy: spec.manualApproval.options.webhookconfigurationoptions.failurePolicy", err.Error()) +} + func Test_ValidateTektonConfig_InvalidTriggerProperties(t *testing.T) { tc := &TektonConfig{ diff --git a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go index 1c72e6f82b..07cf51965c 100644 --- a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go @@ -382,6 +382,11 @@ func (in *LokiStackProperties) DeepCopy() *LokiStackProperties { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ManualApproval) DeepCopyInto(out *ManualApproval) { *out = *in + if in.Disabled != nil { + in, out := &in.Disabled, &out.Disabled + *out = new(bool) + **out = **in + } in.Options.DeepCopyInto(&out.Options) return } @@ -1845,6 +1850,7 @@ func (in *TektonConfigSpec) DeepCopyInto(out *TektonConfigSpec) { in.Pipeline.DeepCopyInto(&out.Pipeline) in.Trigger.DeepCopyInto(&out.Trigger) in.Chain.DeepCopyInto(&out.Chain) + in.ManualApproval.DeepCopyInto(&out.ManualApproval) in.Result.DeepCopyInto(&out.Result) in.Dashboard.DeepCopyInto(&out.Dashboard) in.MulticlusterProxyAAE.DeepCopyInto(&out.MulticlusterProxyAAE) diff --git a/pkg/reconciler/shared/tektonconfig/controller.go b/pkg/reconciler/shared/tektonconfig/controller.go index a2b740c7e1..ea5e0a5f6b 100644 --- a/pkg/reconciler/shared/tektonconfig/controller.go +++ b/pkg/reconciler/shared/tektonconfig/controller.go @@ -26,6 +26,7 @@ import ( mf "github.com/manifestival/manifestival" "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" operatorclient "github.com/tektoncd/operator/pkg/client/injection/client" + tektonManualApprovalGateinformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/manualapprovalgate" tektonChaininformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektonchain" tektonConfiginformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektonconfig" tektonInstallerinformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektoninstallerset" @@ -112,6 +113,12 @@ func NewExtensibleController(generator common.ExtensionGenerator) injection.Cont }); err != nil { logger.Panicf("Couldn't register TektonResult informer event handler: %w", err) } + if _, err := tektonManualApprovalGateinformer.Get(ctx).Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterController(&v1alpha1.TektonConfig{}), + Handler: controller.HandleAll(impl.EnqueueControllerOf), + }); err != nil { + logger.Panicf("Couldn't register TektonManualApprovalGate informer event handler: %w", err) + } if _, err := tektonInstallerinformer.Get(ctx).Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: controller.FilterController(&v1alpha1.TektonConfig{}), diff --git a/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate.go b/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate.go new file mode 100644 index 0000000000..c22cd73daa --- /dev/null +++ b/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate.go @@ -0,0 +1,160 @@ +/* +Copyright 2026 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. +*/ + +package manualapprovalgate + +import ( + "context" + "fmt" + "reflect" + "strings" + + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" + op "github.com/tektoncd/operator/pkg/client/clientset/versioned/typed/operator/v1alpha1" + apierrs "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" +) + +func EnsureManualApprovalGateExists(ctx context.Context, clients op.ManualApprovalGateInterface, mag *v1alpha1.ManualApprovalGate) (*v1alpha1.ManualApprovalGate, error) { + magCR, err := GetManualApprovalGate(ctx, clients, v1alpha1.ManualApprovalGates) + if err != nil { + if !apierrs.IsNotFound(err) { + return nil, err + } + if err := CreateManualApprovalGate(ctx, clients, mag); err != nil { + return nil, err + } + return nil, v1alpha1.RECONCILE_AGAIN_ERR + } + + magCR, err = UpdateManualApprovalGate(ctx, magCR, mag, clients) + if err != nil { + return nil, err + } + + ready, err := isManualApprovalGateReady(magCR) + if err != nil { + return nil, err + } + if !ready { + return nil, v1alpha1.RECONCILE_AGAIN_ERR + } + + return magCR, err +} + +func EnsureManualApprovalGateCRNotExists(ctx context.Context, clients op.ManualApprovalGateInterface) error { + if _, err := GetManualApprovalGate(ctx, clients, v1alpha1.ManualApprovalGates); err != nil { + if apierrs.IsNotFound(err) { + return nil + } + return err + } + if err := clients.Delete(ctx, v1alpha1.ManualApprovalGates, metav1.DeleteOptions{}); err != nil { + if apierrs.IsNotFound(err) { + return nil + } + return fmt.Errorf("ManualApprovalGate %q failed to delete: %v", v1alpha1.ManualApprovalGates, err) + } + return v1alpha1.RECONCILE_AGAIN_ERR +} + +func GetManualApprovalGate(ctx context.Context, clients op.ManualApprovalGateInterface, name string) (*v1alpha1.ManualApprovalGate, error) { + return clients.Get(ctx, name, metav1.GetOptions{}) +} + +func CreateManualApprovalGate(ctx context.Context, clients op.ManualApprovalGateInterface, mag *v1alpha1.ManualApprovalGate) error { + _, err := clients.Create(ctx, mag, metav1.CreateOptions{}) + return err +} + +func UpdateManualApprovalGate(ctx context.Context, old *v1alpha1.ManualApprovalGate, new *v1alpha1.ManualApprovalGate, clients op.ManualApprovalGateInterface) (*v1alpha1.ManualApprovalGate, error) { + updated := false + + if old.ObjectMeta.Labels == nil { + old.ObjectMeta.Labels = map[string]string{} + } + + if new.Spec.TargetNamespace != old.Spec.TargetNamespace { + old.Spec.TargetNamespace = new.Spec.TargetNamespace + updated = true + } + + if !reflect.DeepEqual(old.Spec.ManualApproval, new.Spec.ManualApproval) { + old.Spec.ManualApproval = new.Spec.ManualApproval + updated = true + } + + if len(old.ObjectMeta.OwnerReferences) == 0 { + old.ObjectMeta.OwnerReferences = new.ObjectMeta.OwnerReferences + updated = true + } + + oldLabels, oldHasLabels := old.ObjectMeta.Labels[v1alpha1.ReleaseVersionKey] + newLabels, newHasLabels := new.ObjectMeta.Labels[v1alpha1.ReleaseVersionKey] + if !oldHasLabels || (newHasLabels && oldLabels != newLabels) { + old.ObjectMeta.Labels[v1alpha1.ReleaseVersionKey] = newLabels + updated = true + } + + oldPlatformData := old.ObjectMeta.Annotations[v1alpha1.PlatformDataHashKey] + newPlatformData := new.ObjectMeta.Annotations[v1alpha1.PlatformDataHashKey] + if oldPlatformData != newPlatformData { + if old.ObjectMeta.Annotations == nil { + old.ObjectMeta.Annotations = map[string]string{} + } + old.ObjectMeta.Annotations[v1alpha1.PlatformDataHashKey] = newPlatformData + updated = true + } + + if updated { + _, err := clients.Update(ctx, old, metav1.UpdateOptions{}) + if err != nil { + return nil, err + } + return nil, v1alpha1.RECONCILE_AGAIN_ERR + } + return old, nil +} + +func isManualApprovalGateReady(mag *v1alpha1.ManualApprovalGate) (bool, error) { + if mag.GetStatus() != nil && mag.GetStatus().GetCondition(apis.ConditionReady) != nil { + if strings.Contains(mag.GetStatus().GetCondition(apis.ConditionReady).Message, v1alpha1.UpgradePending) { + return false, v1alpha1.DEPENDENCY_UPGRADE_PENDING_ERR + } + } + return mag.Status.IsReady(), nil +} + +func GetManualApprovalGateCR(config *v1alpha1.TektonConfig, operatorVersion string) *v1alpha1.ManualApprovalGate { + ownerRef := *metav1.NewControllerRef(config, config.GroupVersionKind()) + return &v1alpha1.ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ManualApprovalGates, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + Labels: map[string]string{ + v1alpha1.ReleaseVersionKey: operatorVersion, + }, + }, + Spec: v1alpha1.ManualApprovalGateSpec{ + CommonSpec: v1alpha1.CommonSpec{ + TargetNamespace: config.Spec.TargetNamespace, + }, + ManualApproval: config.Spec.ManualApproval, + }, + } +} diff --git a/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate_test.go b/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate_test.go new file mode 100644 index 0000000000..6b88a27888 --- /dev/null +++ b/pkg/reconciler/shared/tektonconfig/manualapprovalgate/manualapprovalgate_test.go @@ -0,0 +1,239 @@ +/* +Copyright 2026 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. +*/ + +package manualapprovalgate + +import ( + "context" + "testing" + + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" + op "github.com/tektoncd/operator/pkg/client/clientset/versioned/typed/operator/v1alpha1" + "github.com/tektoncd/operator/pkg/client/injection/client/fake" + util "github.com/tektoncd/operator/pkg/reconciler/common/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ts "knative.dev/pkg/reconciler/testing" +) + +func TestEnsureManualApprovalGateExists(t *testing.T) { + ctx, _, _ := ts.SetupFakeContextWithCancel(t) + c := fake.Get(ctx) + mag := GetManualApprovalGateCR(getTektonConfig(), "v0.80.0") + + // first invocation should create instance as it is non-existent and return RECONCILE_AGAIN_ERR + _, err := EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // during second invocation instance exists but waiting on dependencies + // hence returns RECONCILE_AGAIN_ERR + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // make upgrade checks pass + makeUpgradeCheckPass(t, ctx, c.OperatorV1alpha1().ManualApprovalGates()) + + // next invocation should return RECONCILE_AGAIN_ERR as MAG is waiting for installation + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // mark the instance ready + markMAGReady(t, ctx, c.OperatorV1alpha1().ManualApprovalGates()) + + // next invocation should return nil error as the instance is ready + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, nil) + + // test update propagation from tektonConfig + mag.Spec.TargetNamespace = "foobar" + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, nil) +} + +func TestEnsureManualApprovalGateCRNotExists(t *testing.T) { + ctx, _, _ := ts.SetupFakeContextWithCancel(t) + c := fake.Get(ctx) + + // when no instance exists, nil error is returned immediately + err := EnsureManualApprovalGateCRNotExists(ctx, c.OperatorV1alpha1().ManualApprovalGates()) + util.AssertEqual(t, err, nil) + + // create an instance for testing other cases + mag := GetManualApprovalGateCR(getTektonConfig(), "v0.80.0") + _, err = EnsureManualApprovalGateExists(ctx, c.OperatorV1alpha1().ManualApprovalGates(), mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // when an instance exists the first invocation should make the delete API call and + // return RECONCILE_AGAIN_ERR. So that the deletion can be confirmed in a subsequent invocation + err = EnsureManualApprovalGateCRNotExists(ctx, c.OperatorV1alpha1().ManualApprovalGates()) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // when the instance is completely removed from a cluster, the function should return nil error + err = EnsureManualApprovalGateCRNotExists(ctx, c.OperatorV1alpha1().ManualApprovalGates()) + util.AssertEqual(t, err, nil) +} + +func TestEnsureManualApprovalGateExists_MigratesOwnerRef(t *testing.T) { + ctx, _, _ := ts.SetupFakeContextWithCancel(t) + c := fake.Get(ctx) + clients := c.OperatorV1alpha1().ManualApprovalGates() + + // simulate a standalone MAG CR created directly by the user (no ownerRef) + standalone := &v1alpha1.ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ManualApprovalGates, + }, + Spec: v1alpha1.ManualApprovalGateSpec{ + CommonSpec: v1alpha1.CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } + _, err := clients.Create(ctx, standalone, metav1.CreateOptions{}) + util.AssertEqual(t, err, nil) + + // verify the standalone CR has no ownerReferences + existing, err := clients.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + util.AssertEqual(t, len(existing.OwnerReferences), 0) + + // build the desired CR (with ownerRef from TektonConfig) + desired := GetManualApprovalGateCR(getTektonConfig(), "v0.80.0") + + // EnsureExists should adopt the standalone CR by adding the ownerRef + _, err = EnsureManualApprovalGateExists(ctx, clients, desired) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // verify ownerRef was added + migrated, err := clients.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + util.AssertEqual(t, len(migrated.OwnerReferences), 1) + util.AssertEqual(t, migrated.OwnerReferences[0].Name, v1alpha1.ConfigResourceName) +} + +func TestEnsureManualApprovalGateExists_PropagatesPlatformDataHash(t *testing.T) { + ctx, _, _ := ts.SetupFakeContextWithCancel(t) + c := fake.Get(ctx) + clients := c.OperatorV1alpha1().ManualApprovalGates() + + // create an initial MAG CR without platform-data-hash + mag := GetManualApprovalGateCR(getTektonConfig(), "v0.80.0") + _, err := EnsureManualApprovalGateExists(ctx, clients, mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + makeUpgradeCheckPass(t, ctx, clients) + + // reconcile again after upgrade check updated labels + _, err = EnsureManualApprovalGateExists(ctx, clients, mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + markMAGReady(t, ctx, clients) + + _, err = EnsureManualApprovalGateExists(ctx, clients, mag) + util.AssertEqual(t, err, nil) + + // verify no platform-data-hash annotation exists yet + existing, err := clients.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + util.AssertEqual(t, existing.Annotations[v1alpha1.PlatformDataHashKey], "") + + // simulate TektonConfig setting platform-data-hash (TLS profile change) + mag.Annotations = map[string]string{ + v1alpha1.PlatformDataHashKey: "abc123", + } + _, err = EnsureManualApprovalGateExists(ctx, clients, mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // verify annotation was propagated + updated, err := clients.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + util.AssertEqual(t, updated.Annotations[v1alpha1.PlatformDataHashKey], "abc123") + + markMAGReady(t, ctx, clients) + + // simulate a TLS profile change (hash changes) + mag.Annotations[v1alpha1.PlatformDataHashKey] = "def456" + _, err = EnsureManualApprovalGateExists(ctx, clients, mag) + util.AssertEqual(t, err, v1alpha1.RECONCILE_AGAIN_ERR) + + // verify the new hash was propagated + updated, err = clients.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + util.AssertEqual(t, updated.Annotations[v1alpha1.PlatformDataHashKey], "def456") +} + +func TestGetManualApprovalGateCR(t *testing.T) { + config := getTektonConfig() + mag := GetManualApprovalGateCR(config, "v0.80.0") + + util.AssertEqual(t, mag.Name, v1alpha1.ManualApprovalGates) + util.AssertEqual(t, mag.Spec.TargetNamespace, "tekton-pipelines") + util.AssertEqual(t, len(mag.OwnerReferences), 1) + util.AssertEqual(t, mag.OwnerReferences[0].Name, v1alpha1.ConfigResourceName) + util.AssertEqual(t, mag.Labels[v1alpha1.ReleaseVersionKey], "v0.80.0") +} + +func markMAGReady(t *testing.T, ctx context.Context, c op.ManualApprovalGateInterface) { + t.Helper() + mag, err := c.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + mag.Status.MarkDependenciesInstalled() + mag.Status.MarkPreReconcilerComplete() + mag.Status.MarkInstallerSetAvailable() + mag.Status.MarkInstallerSetReady() + mag.Status.MarkPostReconcilerComplete() + _, err = c.UpdateStatus(ctx, mag, metav1.UpdateOptions{}) + util.AssertEqual(t, err, nil) +} + +func makeUpgradeCheckPass(t *testing.T, ctx context.Context, c op.ManualApprovalGateInterface) { + t.Helper() + mag, err := c.Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + util.AssertEqual(t, err, nil) + setDummyVersionLabel(t, mag) + _, err = c.Update(ctx, mag, metav1.UpdateOptions{}) + util.AssertEqual(t, err, nil) +} + +func setDummyVersionLabel(t *testing.T, mag *v1alpha1.ManualApprovalGate) { + t.Helper() + + oprVersion := "v1.2.3" + t.Setenv(v1alpha1.VersionEnvKey, oprVersion) + + labels := mag.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[v1alpha1.ReleaseVersionKey] = oprVersion + mag.SetLabels(labels) +} + +func getTektonConfig() *v1alpha1.TektonConfig { + return &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + Spec: v1alpha1.TektonConfigSpec{ + Profile: v1alpha1.ProfileAll, + CommonSpec: v1alpha1.CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } +} diff --git a/pkg/reconciler/shared/tektonconfig/tektonconfig.go b/pkg/reconciler/shared/tektonconfig/tektonconfig.go index d246d3fe5d..6e966d7f10 100644 --- a/pkg/reconciler/shared/tektonconfig/tektonconfig.go +++ b/pkg/reconciler/shared/tektonconfig/tektonconfig.go @@ -26,6 +26,7 @@ import ( tektonConfigreconciler "github.com/tektoncd/operator/pkg/client/injection/reconciler/operator/v1alpha1/tektonconfig" "github.com/tektoncd/operator/pkg/reconciler/common" "github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/chain" + "github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/manualapprovalgate" "github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/multiclusterproxyaae" "github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/pipeline" "github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/pruner" @@ -38,6 +39,7 @@ import ( "k8s.io/client-go/kubernetes" "knative.dev/pkg/apis" "knative.dev/pkg/logging" + "knative.dev/pkg/ptr" pkgreconciler "knative.dev/pkg/reconciler" ) @@ -80,6 +82,9 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1alpha1.Tekton if err := chain.EnsureTektonChainCRNotExists(ctx, r.operatorClientSet.OperatorV1alpha1().TektonChains()); err != nil { return err } + if err := manualapprovalgate.EnsureManualApprovalGateCRNotExists(ctx, r.operatorClientSet.OperatorV1alpha1().ManualApprovalGates()); err != nil { + return err + } if err := result.EnsureTektonResultCRNotExists(ctx, r.operatorClientSet.OperatorV1alpha1().TektonResults()); err != nil { return err } @@ -326,6 +331,49 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonConfi logger.Debug("TektonChain CR removal reconciled successfully") } + // Ensure ManualApprovalGate CR + // If a standalone MAG CR exists (no ownerRef, from a previous version), + // adopt its config into TektonConfig and enable it. + if tc.Spec.ManualApproval.IsDisabled() { + existingMAG, err := manualapprovalgate.GetManualApprovalGate(ctx, r.operatorClientSet.OperatorV1alpha1().ManualApprovalGates(), v1alpha1.ManualApprovalGates) + if err == nil && len(existingMAG.OwnerReferences) == 0 { + logger.Infow("Found standalone ManualApprovalGate CR from previous version, adopting into TektonConfig") + tc.Spec.ManualApproval = existingMAG.Spec.ManualApproval + tc.Spec.ManualApproval.Disabled = ptr.Bool(false) + if _, err := r.operatorClientSet.OperatorV1alpha1().TektonConfigs().Update(ctx, tc, metav1.UpdateOptions{}); err != nil { + logger.Errorw("Failed to adopt standalone MAG into TektonConfig", "error", err) + return v1alpha1.REQUEUE_EVENT_AFTER + } + return v1alpha1.REQUEUE_EVENT_AFTER + } + } + if !tc.Spec.ManualApproval.IsDisabled() { + magCR := manualapprovalgate.GetManualApprovalGateCR(tc, r.operatorVersion) + if platformData := r.extension.GetPlatformData(); platformData != "" { + if magCR.Annotations == nil { + magCR.Annotations = map[string]string{} + } + magCR.Annotations[v1alpha1.PlatformDataHashKey] = platformData + } + logger.Debug("Ensuring ManualApprovalGate CR exists") + if _, err := manualapprovalgate.EnsureManualApprovalGateExists(ctx, r.operatorClientSet.OperatorV1alpha1().ManualApprovalGates(), magCR); err != nil { + errMsg := fmt.Sprintf("ManualApprovalGate: %s", err.Error()) + logger.Errorw("Failed to ensure ManualApprovalGate exists", "error", err) + tc.Status.MarkComponentNotReady(errMsg) + return v1alpha1.REQUEUE_EVENT_AFTER + } + logger.Debug("ManualApprovalGate CR reconciled successfully") + } else { + logger.Debugw("Ensuring ManualApprovalGate CR doesn't exist", "manualApprovalDisabled", tc.Spec.ManualApproval.IsDisabled()) + if err := manualapprovalgate.EnsureManualApprovalGateCRNotExists(ctx, r.operatorClientSet.OperatorV1alpha1().ManualApprovalGates()); err != nil { + errMsg := fmt.Sprintf("ManualApprovalGate: %s", err.Error()) + logger.Errorw("Failed to ensure ManualApprovalGate has been deleted", "error", err) + tc.Status.MarkComponentNotReady(errMsg) + return v1alpha1.REQUEUE_EVENT_AFTER + } + logger.Debug("ManualApprovalGate CR removal reconciled successfully") + } + // Ensure Result CR if !tc.Spec.Result.Disabled && (tc.Spec.Profile == v1alpha1.ProfileAll || tc.Spec.Profile == v1alpha1.ProfileBasic) { tektonresult := result.GetTektonResultCR(tc, r.operatorVersion) diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go index 06c173b29f..2726508e8b 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go @@ -350,6 +350,45 @@ func updateOpenShiftPipelinesAsCodeCR(ctx context.Context, logger *zap.SugaredLo return nil } +// preUpgradeManualApprovalGate adopts a standalone ManualApprovalGate CR into TektonConfig. +// If a MAG CR exists without ownerReferences (from a previous version where MAG was installed +// independently), copy its config into TektonConfig.Spec.ManualApproval and enable it. +// This ensures the user's existing MAG configuration (Options, etc.) is preserved across the upgrade. +func preUpgradeManualApprovalGate(ctx context.Context, logger *zap.SugaredLogger, k8sClient kubernetes.Interface, operatorClient versioned.Interface, restConfig *rest.Config) error { + tc, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + if err != nil { + if apierrs.IsNotFound(err) { + return nil + } + return err + } + + if !tc.Spec.ManualApproval.IsDisabled() { + logger.Infof("ManualApprovalGate already enabled in TektonConfig, skipping pre-upgrade adoption") + return nil + } + + magCR, err := operatorClient.OperatorV1alpha1().ManualApprovalGates().Get(ctx, v1alpha1.ManualApprovalGates, metav1.GetOptions{}) + if err != nil { + if apierrs.IsNotFound(err) { + logger.Infof("No standalone ManualApprovalGate CR found, skipping pre-upgrade adoption") + return nil + } + return err + } + + if len(magCR.OwnerReferences) > 0 { + logger.Infof("ManualApprovalGate CR already has ownerReferences, skipping pre-upgrade adoption") + return nil + } + + logger.Infof("Found standalone ManualApprovalGate CR, adopting config into TektonConfig") + tc.Spec.ManualApproval = magCR.Spec.ManualApproval + tc.Spec.ManualApproval.Disabled = ptr.Bool(false) + _, err = operatorClient.OperatorV1alpha1().TektonConfigs().Update(ctx, tc, metav1.UpdateOptions{}) + return err +} + // removeDeprecatedDisableAffinityAssistant removes the deprecated DisableAffinityAssistant field from tektonConfig CR spec during pre upgrade // TODO: Remove this upgrade function in the release-v0.80.x func removeDeprecatedDisableAffinityAssistant(ctx context.Context, logger *zap.SugaredLogger, k8sClient kubernetes.Interface, operatorClient versioned.Interface, restConfig *rest.Config) error { diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go index 408cd5e5f1..c3c331869c 100644 --- a/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go +++ b/pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go @@ -760,6 +760,135 @@ func TestUpdateOpenShiftPipelinesAsCodeCR_NotFound(t *testing.T) { assert.NoError(t, err) } +func TestPreUpgradeManualApprovalGate(t *testing.T) { + ctx := context.TODO() + logger := logging.FromContext(ctx).Named("unit-test") + + t.Run("no TektonConfig CR", func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + err := preUpgradeManualApprovalGate(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + }) + + t.Run("MAG already enabled in TektonConfig", func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + Spec: v1alpha1.TektonConfigSpec{ + ManualApproval: v1alpha1.ManualApproval{ + Disabled: ptr.Bool(false), + }, + }, + } + _, err := operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = preUpgradeManualApprovalGate(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + }) + + t.Run("no standalone MAG CR exists", func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + } + _, err := operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = preUpgradeManualApprovalGate(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + + // verify MAG is still disabled + updated, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.True(t, updated.Spec.ManualApproval.IsDisabled()) + }) + + t.Run("MAG CR exists with ownerRef - skip adoption", func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + } + _, err := operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) + assert.NoError(t, err) + + mag := &v1alpha1.ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ManualApprovalGates, + OwnerReferences: []metav1.OwnerReference{ + {Name: v1alpha1.ConfigResourceName, Kind: "TektonConfig"}, + }, + }, + } + _, err = operatorClient.OperatorV1alpha1().ManualApprovalGates().Create(ctx, mag, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = preUpgradeManualApprovalGate(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + + // verify MAG is still disabled in TektonConfig + updated, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.True(t, updated.Spec.ManualApproval.IsDisabled()) + }) + + t.Run("standalone MAG CR adopted with config preserved", func(t *testing.T) { + operatorClient := operatorFake.NewSimpleClientset() + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ConfigResourceName, + }, + Spec: v1alpha1.TektonConfigSpec{ + CommonSpec: v1alpha1.CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + }, + } + _, err := operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{}) + assert.NoError(t, err) + + // create standalone MAG CR with user config (Options with configMaps) + mag := &v1alpha1.ManualApprovalGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1alpha1.ManualApprovalGates, + }, + Spec: v1alpha1.ManualApprovalGateSpec{ + CommonSpec: v1alpha1.CommonSpec{ + TargetNamespace: "tekton-pipelines", + }, + ManualApproval: v1alpha1.ManualApproval{ + Options: v1alpha1.AdditionalOptions{ + ConfigMaps: map[string]v1.ConfigMap{ + "manual-approval-gate-config": { + Data: map[string]string{ + "custom-key": "custom-value", + }, + }, + }, + }, + }, + }, + } + _, err = operatorClient.OperatorV1alpha1().ManualApprovalGates().Create(ctx, mag, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = preUpgradeManualApprovalGate(ctx, logger, nil, operatorClient, nil) + assert.NoError(t, err) + + // verify TektonConfig now has MAG enabled with user's config preserved + updated, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.False(t, updated.Spec.ManualApproval.IsDisabled()) + assert.Equal(t, "custom-value", updated.Spec.ManualApproval.Options.ConfigMaps["manual-approval-gate-config"].Data["custom-key"]) + }) +} + func TestRemoveDeprecatedDisableAffinityAssistant(t *testing.T) { ctx := context.TODO() logger := logging.FromContext(ctx).Named("unit-test") diff --git a/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go b/pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go index 496365c5b9..23ca548ab2 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 + preUpgradeManualApprovalGate, // upgrade #8: adopt standalone MAG config into TektonConfig } // post upgrade functions diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index e5944a9a55..7308bf428d 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -31,12 +31,13 @@ import ( ) var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{ - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonConfig): &v1alpha1.TektonConfig{}, - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonPipeline): &v1alpha1.TektonPipeline{}, - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonTrigger): &v1alpha1.TektonTrigger{}, - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonResult): &v1alpha1.TektonResult{}, - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonChain): &v1alpha1.TektonChain{}, - v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonPruner): &v1alpha1.TektonPruner{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonConfig): &v1alpha1.TektonConfig{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonPipeline): &v1alpha1.TektonPipeline{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonTrigger): &v1alpha1.TektonTrigger{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonResult): &v1alpha1.TektonResult{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonChain): &v1alpha1.TektonChain{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindTektonPruner): &v1alpha1.TektonPruner{}, + v1alpha1.SchemeGroupVersion.WithKind(v1alpha1.KindManualApprovalGate): &v1alpha1.ManualApprovalGate{}, } func SetTypes(platform string) {