feat: remove RekorSearchUI and migrate to Console CR - #2170
Merged
Conversation
PR Summary by QodoRemove RekorSearchUI and migrate v1alpha1 config to Console CR
AI Description
Diagram
High-Level Assessment
Files changed (40)
|
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
from
July 29, 2026 12:56
16e6158 to
e63a37c
Compare
Code Review by Qodo
1. Paused reconcile drops migration
|
![qodo-for-securesign[bot]](https://avatars.githubusercontent.com/in/3596402?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2170 +/- ##
==========================================
- Coverage 56.95% 56.77% -0.19%
==========================================
Files 287 284 -3
Lines 16251 16153 -98
==========================================
- Hits 9256 9171 -85
+ Misses 6043 6032 -11
+ Partials 952 950 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
from
July 29, 2026 13:28
e63a37c to
5324f1c
Compare
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
5 times, most recently
from
July 29, 2026 20:18
a21102b to
c9799f5
Compare
Member
|
Will need to do org wide cleanup of Rekor Search UI for >= 1.5.0 (e.g. sigstore-e2e, pipelines, quickstarts?) |
Collaborator
Author
Yes I will create a jira to request change in sigstore-e2e for Console. I prepared securesign/pipelines#569 to exclude these tests for deployments without Search UI. |
osmman
marked this pull request as draft
July 30, 2026 09:12
Collaborator
Author
|
Depends on #2149 to correctly configure Rekor ServiceReference during migration |
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
from
August 4, 2026 07:54
c9799f5 to
3f12100
Compare
osmman
marked this pull request as ready for review
August 4, 2026 07:54
Comment on lines
+77
to
+82
| if migration.Has(instance, v1alpha1.MigrationSearchUIData) { | ||
| before := instance.DeepCopy() | ||
| migration.Remove(instance, v1alpha1.MigrationSearchUIData) | ||
| if err := i.Client.Patch(ctx, instance, client.MergeFrom(before)); err != nil { | ||
| return i.Error(ctx, fmt.Errorf("could not remove migration annotation from Securesign: %w", err), instance) | ||
| } |
There was a problem hiding this comment.
1. Paused reconcile drops migration 🐞 Bug ≡ Correctness
In ensure_rekor.Handle, the SearchUI migration annotation is removed from Securesign even when Rekor updates are skipped due to pause-reconciliation, so the migration payload may never be propagated to the Rekor CR. This can permanently lose RekorSearchUI configuration (and block Console CR creation/cleanup) during upgrade for paused Rekor instances.
Agent Prompt
## Issue description
`ensure_rekor` removes the migration annotation from the parent `Securesign` even in cases where the migration annotation was not successfully propagated to the child `Rekor` (e.g., because `CreateOrUpdate` skips all mutate/ensure functions when the target object is paused). This can drop the only remaining copy of the migration payload and prevent Rekor-side migration from ever running.
## Issue Context
- `kubernetes.CreateOrUpdate` exits early (skipping all provided ensure/mutate functions) when the managed object has `rhtas.redhat.com/pause-reconciliation=true`.
- `ensure_rekor` currently always removes the migration annotation from `Securesign` if present, regardless of whether it was actually copied onto `Rekor`.
## Fix Focus Areas
- internal/controller/securesign/actions/ensure_rekor.go[55-83]
### Suggested implementation direction
- Only remove `v1alpha1.MigrationSearchUIData` from `Securesign` after confirming the child `Rekor` has the annotation (either because it was just propagated, or it already existed).
- If the child `Rekor` is paused (or otherwise did not receive the annotation), leave the parent annotation intact and let a future reconcile attempt propagate it once unpaused.
- Add/extend a unit test to cover the paused-child scenario (Rekor has pause annotation; ensure_rekor should not remove migration annotation from Securesign unless propagation is confirmed).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 3f12100 |
fghanmi
requested changes
Aug 4, 2026
| Namespace: instance.Namespace, | ||
| }, | ||
| }, | ||
| } |
Member
There was a problem hiding this comment.
could you please add TUF ref here as well ?
object.Spec.Api.Tuf = rhtasv1.ServiceReference{
Ref: tufRef,
}
It's always better to have the TUF public public URL (if possible) instead of the internal one.
Collaborator
Author
There was a problem hiding this comment.
I let to resolved it by autodiscovery (ResolveExternalServiceUrl) in Console controller. Rekor have not got any usefull data from which I can extract value.
Collaborator
Author
There was a problem hiding this comment.
@fghanmi is TUF url mandatory for API server or optional. Because by enabling autodiscovery and removing your old fallback I make it mandatory.
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
from
August 5, 2026 09:11
3f12100 to
69a62c7
Compare
fghanmi
reviewed
Aug 5, 2026
| } | ||
| } else { | ||
| tufURL = fmt.Sprintf("http://tuf.%s.svc", instance.Namespace) | ||
| tufURL, err := utils.ResolveInternalServiceUrl(ctx, i.Client, instance.Spec.Api.Tuf, instance.Namespace, &rhtasv1.Tuf{}) |
Member
There was a problem hiding this comment.
Is there any reason for this change ?
Same as rekor url here, we prefer that we get TUF public url (if not, fallback to local url)
Collaborator
Author
There was a problem hiding this comment.
Console API is an in-cluster service, so using the internal service URL is simpler than routing through ingress. I switched it to use ResolveInternalServiceUrl with a new serviceresolver registration for TUF (same pattern as Trillian), which also handles user-configured URL/Ref overrides.
Are there plans for the Console dashboard to show the TUF URL to users? If so, I'll change it back to use the ingress endpoint.
Member
There was a problem hiding this comment.
Are there plans for the Console dashboard to show the TUF URL to users?
Yes, this is what we do currently and this is why we prefer the route URL.
Example:
Collaborator
Author
There was a problem hiding this comment.
Thank you, I will use External url by default.
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
2 times, most recently
from
August 5, 2026 13:35
5a26448 to
4229470
Compare
fghanmi
reviewed
Aug 5, 2026
Remove the deprecated RekorSearchUI component from the v1 API and introduce a migration path that creates a Console CR for v1alpha1 users who had SearchUI enabled. Add cleanup action to remove orphaned SearchUI resources (Deployment, Service, Ingress, ServiceAccount, RoleBinding) and the UiAvailable status condition. Introduce a shared internal/migration package providing annotation- based helpers (Set, Read, Pop, Propagate, Remove) for preserving v1alpha1-only fields through the conversion webhook. Refs: SECURESIGN-5066 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Tomas Turek <tturek@redhat.com>
osmman
force-pushed
the
tturek/SECURESIGN-5066
branch
from
August 5, 2026 14:01
4229470 to
d769fe5
Compare
fghanmi
approved these changes
Aug 5, 2026
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
4 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.
Why
The
RekorSearchUIcomponent was embedded directly in the Rekor CR spec (v1alpha1 only). With the introduction of the standaloneConsoleCR, SearchUI functionality is superseded. Existing v1alpha1 users need a migration path that preserves their SearchUI configuration as a Console CR, and orphaned SearchUI resources must be cleaned up.What
Migration path — a shared
internal/migrationpackage provides annotation-based helpers (Set,Read,Pop,Propagate,Remove) for preserving v1alpha1-only fields through the conversion webhook. The v1alpha1→v1 converter (ConvertTo) serializesRekorSearchUIinto a migration annotation; the Rekor controller's newmigrationActionreads it, creates aConsoleCR whenEnabled=true, and removes the annotation.Cleanup — a new
cleanupActiondeletes orphaned SearchUI resources (Deployment, Service, Ingress, ServiceAccount, RoleBinding) owned by the Rekor CR and removes the staleUiAvailablestatus condition. Transient API errors cause a requeue rather than best-effort continuation.v1 API removal —
RekorSearchUIfield, image constant (RekorSearchUi), kustomize replacement block, and all SearchUI controller actions are removed from the v1 API surface.Key design decisions:
migration.Setalways fires) — it faithfully preserves v1alpha1 data without content-aware logic; the migration action decides behavior based onEnabledensure_rekoruses JSON Patch to surgically remove the migration annotation from Securesign, avoiding full-object update conflictskubernetes.CreateOrUpdatefor Console CR creation, which handles retry on Conflict/AlreadyExistsTest coverage:
HubAfterMutationBreaking changes
The
spec.rekorSearchUIfield is removed from the v1RekorCRD. Users on v1alpha1 with SearchUI enabled will have aConsoleCR automatically created on upgrade. Users on v1 are unaffected — SearchUI was never part of the v1 API.Refs: SECURESIGN-5066