feat(control-plane): issue a certificate for the gateway console Route by rh-amarin · Pull Request #268 · openshift-online/hypershell

rh-amarin · 2026-09-10T18:52:33Z

Problem

The gateway console Route is built with edge termination and no annotations, so it always rides the OpenShift router's default certificate. That certificate only covers the cluster ingress domain, so on any cluster whose gateway hosts live under a custom domain the console serves a name-mismatched certificate and every browser refuses it.

The gateway Route next to it does not have this problem: GATEWAY_ROUTE_TLS_ISSUER already gives it a per-host certificate.

Why this has not been seen before

On the Gateway API clusters a DNS-01 wildcard certificate on the shared Gateway listener (*.gwlb.<instance>.<zone>) covers every console host, so no per-host issuance is needed.

Route-mode clusters have no such wildcard. Where DNS-01 is unavailable, HTTP-01 cannot issue a wildcard, so the console needs a per-host certificate -- exactly what the gateway Route already gets. The route-mode clusters in the fleet today keep their gateway hosts under the cluster's own *.apps domain, where the router default certificate happens to match, which is why the gap has stayed invisible.

Change

When GATEWAY_ROUTE_TLS_ISSUER names a cert-manager ClusterIssuer, annotate the console Route so the openshift-routes controller mints a certificate for it.

The injected certificate/key are carried forward on every reconcile. This is the subtle part: the built spec deliberately omits them because openshift-routes co-owns them, and reconcileResource replaces the whole Route -- so without carry-forward each reconcile would strip the certificate and the console would flap between a valid and a default one. The gateway path already does this; the console path now mirrors it.

readInjectedRouteCert takes a route name (it was hardcoded to the gateway Route).

No behaviour change when GATEWAY_ROUTE_TLS_ISSUER is unset, and none at all for Gateway API mode.

Verification

Unit tests added for the three cases: issuer annotates the Route, no issuer leaves it untouched, and an injected certificate survives a reconcile.

Verified on a live IBM ROKS cluster in route ingress mode with gateway hosts under *.gwlb.<instance>.<zone>:

Before: the console host served the router default certificate; curl failed certificate validation.
After: creating a gateway produced an automatically annotated console Route, cert-manager issued within ~40s, and the host serves a Let's Encrypt certificate whose SAN matches it (HTTP 302 to the OIDC login).

No manual step per gateway.

coderabbitai · 2026-09-10T18:52:42Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: f911d40e-2cd6-4948-bb04-e6bb1c8d3e98

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

_{Comment @coderabbitai help to get the list of available commands.}

amber-review-bot · 2026-09-10T18:53:15Z

Amber review

Status: Complete

View the submitted review.

amber-review-bot

Verdict

This change cleanly extends the existing gateway-Route certificate pattern to the console Route, correctly carrying forward the openshift-routes-injected certificate/key so reconciles do not strip the cert and flap the console. The implementation is well-documented and well-tested; my notes are minor, plus one cross-PR coordination item that needs a maintainer decision.

What works well

Mirrors the proven gateway-Route carry-forward mechanism rather than inventing a new one; readInjectedRouteCert is generalized cleanly by taking a routeName.
No behavior change when GATEWAY_ROUTE_TLS_ISSUER is unset, and none for Gateway API mode - the blast radius is contained to route-mode clusters.
Tests are additive (issuer annotates, no-issuer leaves untouched, injected cert survives a reconcile). No pre-existing assertions were flipped, so no removed guarantees.
Conventional commit message and no secrets logged (the private key is placed into the Route spec but never emitted to logs).

Minor findings

[Minor] readInjectedRouteCert collapses every Get error (including transient API errors) into ("", ""), so a transient failure during reconcile skips the carry-forward and lets reconcileResource replace the Route without the injected cert - briefly re-introducing the exact flap this PR guards against. This mirrors the pre-existing gateway-Route behavior, so it is not a regression, but distinguishing IsNotFound (expected, first reconcile) from transient errors would make both call sites more robust.
[Minor] The routeTLSIssuer doc comment still describes it as issuing "a per-gateway public certificate for a reencrypt Route" and "Only meaningful with reencrypt termination". This PR now also uses it to drive an edge-terminated console Route, so the comment is narrower than the current behavior. Consider updating it.

Cross-PR coordination

Another open pull request adopts the upstream OpenShell Helm chart for gateway deployments and, in doing so, removes the control plane's static-manifest / SSA route and cert-manager reconciliation (it deletes manifests/gateway/route.yaml, drops the IngressModeRoute -> reconcileRouteResources path, and delegates Deployment, Services, RBAC, cert-manager, and routing to the chart). That is a direct design conflict with this PR: this PR places ownership of per-host console-Route TLS issuance in the control plane (cert-manager annotations plus injected-cert carry-forward), while that PR moves Route and cert-manager ownership into the Helm chart. Maintainers need to decide which layer owns console/gateway Route certificate issuance and in what order these land - if the Helm adoption merges first, the console per-host certificate behavior added here must be re-expressed as chart values rather than control-plane annotation logic.

Findings Summary (ordered by severity, highest first)

[Minor] readInjectedRouteCert swallows transient Get errors and can momentarily strip the injected cert on reconcile - Reconciliation Robustness (reconciler.go L1700-1704)
[Minor] routeTLSIssuer doc comment is stale for the new edge-terminated console use - Docs (reconciler.go L1685-1690)

Convention Checklist

Convention	Result
No `panic()` in production code	Pass
Errors wrapped with `fmt.Errorf` context	Pass
`errors.IsNotFound` handled for 404 scenarios	Pass (delete path); partial for `readInjectedRouteCert`
No secrets in logs or responses	Pass
Reconcile pattern used (not create-or-skip)	Pass
Conventional commit message	Pass
Test Diff Scrutiny (no flipped assertions)	Pass

amber-review-bot · 2026-09-10T18:56:26Z

+func readInjectedRouteCert(ctx context.Context, dynamicClient dynamic.Interface, namespace, routeName string) (string, string) {
 	gvr := schema.GroupVersionResource{Group: "route.openshift.io", Version: "v1", Resource: "routes"}
-	existing, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(ctx, "openshell-gateway", metav1.GetOptions{})
+	existing, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(ctx, routeName, metav1.GetOptions{})


[Minor] The if err != nil below returns ("", "") for any Get error, so a transient API error during reconcile (not just a genuinely absent Route) skips the carry-forward and lets reconcileResource replace the Route without the injected cert - briefly re-introducing the flap this mechanism exists to prevent. Consider distinguishing k8serrors.IsNotFound (expected on first reconcile) from transient errors so the carry-forward is only skipped when the Route truly has no injected cert. This mirrors the pre-existing gateway-Route behavior, so it is not a regression from this PR.

amber-review-bot · 2026-09-10T18:56:26Z

+			// intentionally omits certificate/key: openshift-routes co-owns this
+			// Route and injects them. Carry any injection forward, otherwise every
+			// reconcile strips the certificate and the console flaps.
+			if cert, key := readInjectedRouteCert(ctx, dynamicClient, namespace, consoleName); cert != "" {


Nice reuse of the gateway-Route carry-forward pattern. One caveat: readInjectedRouteCert treats a missing/absent Route the same as a transient Get failure (both yield empty strings), so on a transient error the freshly built spec (which intentionally omits certificate/key) is applied and the injected cert is dropped for that reconcile. Fine as the common case; see the note on readInjectedRouteCert.

amber-review-bot · 2026-09-11T06:00:56Z

Amber review

Status: Complete

View the submitted review.

amber-review-bot

Verdict

This is a tight, well-scoped fix: it gives the console Route its own per-host certificate exactly where the gateway Route already does, and the injected-cert carry-forward correctly mirrors the established pattern so reconciles do not strip the openshift-routes edge cert. The change is additive, covered by three focused new tests, and I found no blocking or critical issues; the notes below are minor consistency and robustness points, plus one cross-PR coordination item for the maintainers.

The code is honest about the subtle part (full-replace reconcile + co-owned Route), the comments are accurate, and no secrets are logged even though certificate/key material is carried forward in memory.

Findings

[Minor] Annotation set diverges between the gateway and console paths. annotateRouteForIssuer (console.go) writes three annotations - cert-manager.io/issuer-name, -kind, and -group - while the gateway path in reconcileRouteResources (reconciler.go:616-617) writes only the first two. The behavior is equivalent (issuer-group defaults to cert-manager.io), but having two spellings of "annotate this Route for the issuer" invites drift. Consider having the gateway path call the new shared helper so there is a single source of truth. Confidence: High.

[Minor] readInjectedRouteCert treats a transient Get error as "no injected cert". reconciler.go:1703 returns "", "" for any error, so a transient API failure during reconcile drops the carry-forward and lets the full-replace strip the edge certificate - the exact flap this logic guards against. This is pre-existing behavior for the gateway Route, but this PR now extends the same helper to the console path, so it is worth distinguishing IsNotFound (genuinely absent, safe to skip) from transient errors (where skipping the reconcile or returning an error would be safer than silently stripping the cert). Confidence: Medium.

[Minor] Route name literal vs. constant. The new gatewayRouteName = "openshell-gateway" constant is used by readInjectedRouteCert, but the gateway Route builder still hardcodes "openshell-gateway" in metadata.name (reconciler.go, route object). Using the constant in both places keeps the read path and the build path from drifting. Confidence: High.

Cross-PR coordination

A separate open pull request replaces the control plane's Go-based gateway deployment (including the OpenShift Route) with a runtime Helm-chart install, removing the IngressModeRoute branch that invokes reconcileRouteResources and moving Route rendering into the chart. This PR extends that same Go path: it changes the readInjectedRouteCert signature, adds the gatewayRouteName constant, and mirrors the gateway Route's cert-issuer annotation and injected-cert carry-forward onto the console Route. These two designs cannot both own gateway Route generation, and the per-host cert-issuer annotation plus carry-forward behavior this PR relies on would have to be re-expressed in the chart under the other design. Maintainers should decide the merge order and where the Route cert-issuer logic lives (Go reconcile vs. chart template) so the console-certificate behavior added here is not lost when the Helm migration lands. I have not commented on that pull request.

Findings Summary (ordered by severity, highest first)

[Minor] Annotation set diverges between gateway and console issuer paths - Spec Consistency (console.go:1041-1048, reconciler.go:616-617)
[Minor] readInjectedRouteCert swallows transient Get errors and can strip the injected cert - Robustness (reconciler.go:1700-1706)
[Minor] Route name literal duplicated instead of using the new gatewayRouteName constant - Spec Consistency (reconciler.go)

Convention Checklist

Convention	Result
No `panic()` in production code	Pass
Errors wrapped with `fmt.Errorf` context	Pass
`errors.IsNotFound` handled for 404 scenarios	Pass (with Minor note on transient errors)
No secrets in logs or responses	Pass
Reconcile pattern used (not create-or-skip)	Pass
Test diff scrutiny (additive, no flipped assertions)	Pass
Conventional commit message	Pass
No em dashes	Pass

amber-review-bot · 2026-09-11T06:09:01Z

+	}
+	annotations["cert-manager.io/issuer-name"] = issuer
+	annotations["cert-manager.io/issuer-kind"] = "ClusterIssuer"
+	annotations["cert-manager.io/issuer-group"] = "cert-manager.io"


Minor: annotateRouteForIssuer writes three annotations (issuer-name, issuer-kind, issuer-group), while the gateway path in reconcileRouteResources (reconciler.go:616-617) writes only issuer-name and issuer-kind. Behavior is equivalent since issuer-group defaults to cert-manager.io, but two spellings of the same intent will drift. Consider having the gateway path call this shared helper so there is one source of truth.

amber-review-bot · 2026-09-11T06:09:01Z

 	gvr := schema.GroupVersionResource{Group: "route.openshift.io", Version: "v1", Resource: "routes"}
-	existing, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(ctx, "openshell-gateway", metav1.GetOptions{})
+	existing, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(ctx, routeName, metav1.GetOptions{})
 	if err != nil {


Minor/robustness: this returns "", "" for any Get error, so a transient API failure drops the carry-forward and the full-replace reconcile then strips the injected edge certificate - the exact flap this logic guards against. Pre-existing for the gateway Route, but this PR now routes the console path through the same helper. Consider distinguishing IsNotFound (safe to skip) from transient errors (where returning an error / skipping the replace is safer than silently stripping the cert).

The gateway console Route is built with edge termination and no annotations, so it always rides the OpenShift router's default certificate. That certificate only covers the cluster ingress domain, so on any cluster whose gateway hosts live under a custom domain the console serves a name-mismatched certificate and browsers refuse it. This is invisible on the Gateway API clusters, where a DNS-01 WILDCARD certificate on the shared Gateway listener already covers every console host. Route-mode clusters have no such wildcard -- and where DNS-01 is unavailable HTTP-01 cannot issue one -- so the console needs a per-host certificate, the same way the gateway Route already gets one via GATEWAY_ROUTE_TLS_ISSUER. When GATEWAY_ROUTE_TLS_ISSUER names a cert-manager ClusterIssuer, annotate the console Route so the openshift-routes controller mints a certificate for it. The injected certificate/key are carried forward on each reconcile: the built spec deliberately omits them because openshift-routes co-owns them, and reconcileResource replaces the whole Route, so without this the console would flap between a valid and a default certificate. readInjectedRouteCert now takes a route name; it was hardcoded to the gateway Route. Verified on an IBM ROKS cluster (route ingress mode, gateway hosts under *.gwlb.<instance>.<zone>): a newly created gateway's console Route is annotated automatically, cert-manager issues within a minute, and the console serves a Let's Encrypt certificate whose SAN matches the host. Before the change the same request failed certificate validation.

Multi-part fix for the e2e test suite to work seamlessly with podman: 1. Auto-detect container engine (podman/docker) - Tries podman first (more common in Linux environments) - Falls back to docker if podman unavailable - Respects explicit CONTAINER_ENGINE override 2. Smart HTTPRoute access with fallback port-remapping - Try direct HTTPRoute access first (works in most setups with docker, podman, or iptables-forwarded 443) - Falls back to ephemeral port remapping only if direct connection fails (curl exit code 7: connection refused/timeout) - Provides best compatibility: works out-of-the-box while supporting edge cases where HTTPRoutes aren't directly accessible Fixes: e2e test failing with "Could not discover HyperShell API host" and "Token error: no response" in podman environments. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

amber-review-bot · 2026-09-11T08:01:06Z

Amber review

Status: Complete

View the submitted review.

amber-review-bot

Verdict

This change correctly extends the existing per-host Route certificate pattern (issuer annotation + injected-cert carry-forward) from the gateway Route to the console Route, and the added unit tests cover the three meaningful cases. I found no blockers in the change itself; the notes below are minor polish plus one cross-PR coordination item that needs a maintainer decision on ownership and merge order.

Confidence: High on the code analysis; Medium on the e2e-driver behavior notes (could not run the suite in this environment - Go/bash tooling unavailable).

Findings

Minor - annotation set diverges from the gateway path it claims to mirror (components/control-plane/internal/gateway/console.go:1048)
The PR body says the console "mirrors" the gateway path, but annotateRouteForIssuer writes three annotations (issuer-name, issuer-kind, issuer-group) while the gateway path in reconciler.go:616-617 writes only issuer-name and issuer-kind. issuer-group: cert-manager.io is the cert-manager default, so this is harmless, but the two Route-issuance paths should annotate identically or the difference should be intentional and documented.

Minor - stale doc comments after the semantic/signature widening

reconciler.go routeTLSIssuer doc still says it is "Only meaningful with reencrypt termination; reconcileRouteResources ignores it otherwise", but the console path now consumes the same GATEWAY_ROUTE_TLS_ISSUER with edge termination. Update the comment so the reuse is not surprising to the next reader.
readInjectedRouteCert doc still describes reading "the existing openshell-gateway Route", but the function is now parameterized by routeName. Reword to reflect the generalization.

Minor - e2e _driver_curl fallback and output handling (tests/e2e/drivers/kind.sh:93,97)

The fallback only triggers on curl exit 7 (connection refused), but the comment says "connection refused/timeout"; a genuine timeout (exit 28) will not fall back to port-remapping. Either handle 28 too or drop "timeout" from the comment.
The direct attempt now captures with 2>&1, folding curl's stderr into the returned body. With -s this is usually empty, but on failure the error text is echoed as the "response" and can corrupt callers that JSON-parse the output. Consider keeping stderr separate.

Test Diff Scrutiny

The test changes are purely additive (a new TestReconcileConsoleExposureTLSIssuer block); no pre-existing assertion was flipped or weakened. No optional -> required tightening, so no backfill/fallback concern applies.

Cross-PR coordination

There is a competing design for issuing the gateway Route's TLS certificate that needs a maintainer decision with #194. This PR extends the in-process Go route reconciliation - routeTLSIssuer (GATEWAY_ROUTE_TLS_ISSUER), readInjectedRouteCert, and the openshift-routes annotation/carry-forward flow in reconcileRouteResources - and reuses that machinery for the console Route. #194 moves gateway deployment (including the Route and cert-manager wiring) to the upstream Helm chart, removes the inline reconcileRouteResources call, and introduces a different configuration surface for external Route certificates (EXTERNAL_CA_ISSUER_NAME / EXTERNAL_CA_ISSUER_KIND). These are incompatible ownership models and competing config interfaces for the same concern, and this PR's console change depends on readInjectedRouteCert living in reconciler.go. Maintainers should decide which model is canonical and the merge order: if #194 lands first, the console certificate logic here must be re-expressed in the Helm-based model and reconciled against the EXTERNAL_CA_ISSUER_* surface; if this lands first, #194 must preserve or migrate the console Route TLS behavior.

Findings Summary (ordered by severity, highest first)

[Minor] Console Route annotation set (issuer-group added) diverges from the gateway path it mirrors - Spec Consistency (console.go L1048, reconciler.go L616)
[Minor] Stale doc comments on routeTLSIssuer (edge reuse) and readInjectedRouteCert (now parameterized) - Spec Completeness (reconciler.go L1690, L1694)
[Minor] e2e _driver_curl: fallback only on exit 7 (not timeout) and 2>&1 folds stderr into the returned body - Test Infrastructure (kind.sh L93, L97)

Convention Checklist

Convention	Result
No `panic()` in production code	Pass
Errors wrapped with `fmt.Errorf` context	Pass
`errors.IsNotFound` handled for 404 scenarios	Pass
No secrets in logs or responses	Pass
Reconcile pattern used (not create-or-skip)	Pass
Secret references, not inline secrets	Pass
Test diff additive (no flipped assertions)	Pass
Conventional commit messages	Pass

amber-review-bot · 2026-09-11T08:05:31Z

+	}
+	annotations["cert-manager.io/issuer-name"] = issuer
+	annotations["cert-manager.io/issuer-kind"] = "ClusterIssuer"
+	annotations["cert-manager.io/issuer-group"] = "cert-manager.io"


annotateRouteForIssuer writes cert-manager.io/issuer-group in addition to issuer-name/issuer-kind, but the gateway Route path (reconciler.go:616-617) sets only the latter two. The PR describes the console path as mirroring the gateway path. issuer-group: cert-manager.io is the cert-manager default so this is harmless, but consider aligning the two Route-issuance paths (either add the group to the gateway path or drop it here) so the annotation contract is identical.

amber-review-bot · 2026-09-11T08:05:31Z

+		// certificate on the shared Gateway listener already covers the host, and
+		// HTTP-01 cannot issue wildcards -- so a per-host certificate is the only
+		// option where DNS-01 is unavailable.
+		if issuer := routeTLSIssuer(); issuer != "" {


The console Route reuses GATEWAY_ROUTE_TLS_ISSUER (via routeTLSIssuer()) with edge termination, whereas that env var's doc comment states it is "only meaningful with reencrypt termination." The reuse is reasonable, but please update the routeTLSIssuer doc so the next reader is not misled about its scope.

amber-review-bot · 2026-09-11T08:05:31Z

 // these forward to avoid clobbering the injected edge certificate (which would
 // flap ALPN h2, and hence gRPC, on every reconcile).
-func readInjectedRouteCert(ctx context.Context, dynamicClient dynamic.Interface, namespace string) (string, string) {
+func readInjectedRouteCert(ctx context.Context, dynamicClient dynamic.Interface, namespace, routeName string) (string, string) {


Now that readInjectedRouteCert is parameterized by routeName, the doc comment above it (still describing "the existing openshell-gateway Route") is stale. Reword it to reflect that it reads the named Route's injected spec.tls.{certificate,key}.

amber-review-bot · 2026-09-11T08:05:31Z

+  status=$?
+
+  # If direct access failed due to connection error, try port-remapping
+  if [[ $status -eq 7 ]]; then


The port-remap fallback only triggers on curl exit code 7 (connection refused), but the comment says "connection refused/timeout." A real timeout is exit 28 and will not fall back. Either also handle 28 or drop "timeout" from the comment to avoid a false expectation.

amber-review-bot · 2026-09-11T08:05:31Z

+  # forwarded 443). If direct access fails (connection refused/timeout), fall
+  # back to port-remapping via the ephemeral kindccm-gw port.
+  local output status
+  output=$(curl -sk --ipv4 "$@" 2>&1)


Capturing with 2>&1 folds curl's stderr into output, which is then echoed as the response. With -s this is normally empty, but on failure curl's error text becomes the returned "body" and can corrupt callers that JSON-parse it. Consider keeping stderr separate from the captured body.

amber-review-bot reviewed Sep 10, 2026

View reviewed changes

amber-review-bot reviewed Sep 11, 2026

View reviewed changes

Angel Marin and others added 2 commits September 11, 2026 09:47

rh-amarin force-pushed the feat/console-route-tls-issuer branch from 5b1f6b2 to fe23564 Compare September 11, 2026 07:58

amber-review-bot reviewed Sep 11, 2026

View reviewed changes

jhjaggars added this pull request to the merge queue Sep 11, 2026

Merged via the queue into openshift-online:main with commit 289c9c5 Sep 11, 2026
26 checks passed

rh-amarin deleted the feat/console-route-tls-issuer branch October 19, 2026 12:56

Conversation

rh-amarin commented Sep 10, 2026

Problem

Why this has not been seen before

Change

Verification

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

amber-review-bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Amber review

Uh oh!

amber-review-bot left a comment

Choose a reason for hiding this comment

Verdict

What works well

Minor findings

Cross-PR coordination

Findings Summary (ordered by severity, highest first)

Convention Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amber-review-bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Amber review

Uh oh!

amber-review-bot left a comment

Choose a reason for hiding this comment

Verdict

Findings

Cross-PR coordination

Findings Summary (ordered by severity, highest first)

Convention Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amber-review-bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Amber review

Uh oh!

amber-review-bot left a comment

Choose a reason for hiding this comment

Verdict

Findings

Test Diff Scrutiny

Cross-PR coordination

Findings Summary (ordered by severity, highest first)

Convention Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

amber-review-bot commented Sep 10, 2026 •

edited

Loading

amber-review-bot commented Sep 11, 2026 •

edited

Loading

amber-review-bot commented Sep 11, 2026 •

edited

Loading