fix(manualapprovalgate): propagate TLS profile hash via TektonConfig#3657
Open
jkhelil wants to merge 1 commit into
Open
fix(manualapprovalgate): propagate TLS profile hash via TektonConfig#3657jkhelil wants to merge 1 commit into
jkhelil wants to merge 1 commit into
Conversation
tekton-robot
added
the
do-not-merge/work-in-progress
tekton-robot
added
do-not-merge/release-note-label-needed
Member
Author
|
/release-note-none |
tekton-robot
added
release-note-none
tekton-robot
removed
the
do-not-merge/work-in-progress
Contributor
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enarha The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
ManualApprovalGate is a standalone CR (not created by TektonConfig), so it was never part of the platform-data-hash propagation chain. When the cluster TLS profile changed, all other components updated their webhook deployments automatically — but the MAG webhook stayed stale, still showing the old TLS version and cipher suites. Root cause: the InstallerSet client detects TLS changes via the platform-data-hash annotation on the component CR. TektonConfig writes this annotation onto every child CR it owns (TektonPipeline, TektonChain, etc.) during PostReconcile. MAG was never wired in. Fix: add propagateMAGPlatformData() called from TektonConfig's OpenShiftExtension.PostReconcile(). It lists existing MAG CRs and writes the current TLS profile hash into their platform-data-hash annotation. The existing MAG controller informer then fires, triggers a reconcile, and the webhook deployment is re-applied with the correct TLS env vars. This is best-effort and safe when MAG is not installed: - no MAG CR present → list returns empty, loop is a no-op - MAG CRD absent → list error is logged as a warning, PostReconcile continues normally and TektonConfig reconciliation is unaffected The proper long-term fix (integrate MAG as a full TektonConfig child with ownerRef and spec field) is tracked in: tektoncd#3656 Relates-To: SRVKP-9613 Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
jkhelil
force-pushed
the
fix/mag-tls-propagation-via-tektonconfig
branch
from
29cd1d7 to
df1d8b9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ManualApprovalGateis a standalone CR (not created or owned by TektonConfig). As a result it was never part of theplatform-data-hashpropagation chain, so when the cluster TLS profile changed (e.g. Intermediate → Modern), the MAG webhook deployment stayed stale — still showingWEBHOOK_TLS_MIN_VERSION: 1.2with 9 ciphers instead of TLS 1.3 with 3 ciphers.Root cause
The
InstallerSetClientdetects TLS changes by includingplatform-data-hashin the InstallerSet hash computation. TektonConfig writes this annotation onto every CR it owns (TektonPipeline,TektonChain,TektonResult, etc.) duringPostReconcile. MAG was never wired into this flow because it was introduced as a standalone CRD without the TektonConfig integration (see #3656).Fix
Added
propagateMAGPlatformData()called fromOpenShiftExtension.PostReconcile(). It:GetPlatformData()ManualApprovalGateCRsoperator.tekton.dev/platform-data-hashon each CR (skipping if already up to date)The existing MAG controller informer fires on the annotation change, triggers a reconcile, and the webhook deployment is re-applied with the correct TLS env vars. No changes to the MAG controller or reconciler are needed.
Safety
Listreturns empty — loop is a no-opListreturns error — logged as warning,PostReconcilecontinuesLong-term fix
Proper integration of MAG as a full TektonConfig child (ownerRef, spec field, shared package) is tracked in #3656. This PR is the minimal correct stopgap that follows the same architectural pattern as PAC propagation in
PostReconcile.Relates-To: SRVKP-9613
Closes #3656 (partially — the immediate TLS symptom; full child integration deferred)
Made with Cursor