Gate the HTTPS URL and redirect on real certificate issuance - #59
Conversation
Adds a pure classifier over the cert-manager Certificate that ingress-shim creates for a StackResource's Ingress, plus the lookup and the reverse name mapping used to watch it. The stage is one of issuing, ready, or unavailable. Explicit failure and a grace-period timeout collapse into the same unavailable stage because every consumer treats them identically; the reason on the result distinguishes them for the UI. Failure is detected from Status.FailedIssuanceAttempts rather than the Issuing condition's reason, since that condition is removed outright once issuance completes. The grace clock reads TLSConfigured's LastTransitionTime, which stays pinned at the start of the pending window because meta.SetStatusCondition only moves it when the status changes. Nothing consumes the classifier yet.
…e the cert A 301 is cached by browsers indefinitely. If a certificate later expires or a renewal breaks, that cached redirect keeps forcing users onto the dead HTTPS endpoint with no way back to a working HTTP one. The Middleware CR is namespace-scoped and shared, so this also affects ObjectStorage ingresses in the same namespace. That is intended. Costs one extra redirect hop per fresh visit.
…ate issuance A release went live with a clickable https:// link before any certificate existed. Traefik served its self-signed default, the redirect pushed every visitor onto it, and a user who clicked through the browser warning had that origin's security state pinned for their whole profile — the site still read as insecure long after the real certificate arrived. The TLS block still goes on immediately, since that is what makes ingress-shim create the Certificate at all. The Traefik redirect annotation now waits for the ready stage, so nothing steers a user toward HTTPS until it works. The key is in ManagedAnnotations, so SyncManagedAnnotations also strips it from an existing Ingress that regresses out of ready. ExternalAddress follows the same stage: nothing published while issuing, https:// when ready, and http:// once no certificate is coming, so the site stays reachable rather than becoming permanently unlinkable. TLSConfigured now reports actual issuance instead of flipping True as soon as a ClusterIssuer was found, and carries cert-manager's own failure text. Available and Ready are untouched — release latency does not change. A Certificate watch drives the upgrade the moment issuance lands. A deferred requeue covers the silent-stall case, which produces no Certificate event at all.
Two specs against a real cert-manager. The first uses a self-signed ClusterIssuer and asserts the resource reaches TLSConfigured=True with an https:// address and the redirect annotation attached. The second points at a ClusterIssuer that does not exist and asserts the resource still becomes Available, publishes http://, and carries no redirect. That is the guarantee that matters: a certificate that is never coming must not hold a release hostage or make the site unreachable. Registers cmv1 in the test client's scheme so the ClusterIssuer can be created.
Integration resultFull suite on this branch: 150 passed · 1 failed · 8 skipped (159 specs, 59 min). Both new specs passed inside that full run:
The one failure is pre-existing and unrelated
Attribution, run in isolation both ways:
It passes on this branch when run on its own, so it is not a deterministic regression from these changes. Root cause — a race in the test, not in the operator. The failing assertion is "old credentials no longer work", and the diagnostics captured two That spec reaches the pod by port-forward over plain HTTP — no Ingress, no Traefik middleware, no certificate. None of the surfaces this PR touches are in that path. Suggested follow-up (not in this PR): have the rotation spec wait for the rollout to complete before port-forwarding, or make Verification summary
The integration gate flagged when this PR was opened is now closed. |
1 participant
The problem
A release would go live, the dashboard showed it ready with a clickable
https://link, and the first person to click it landed on a browser security warning. Worse, it stayed broken — once the certificate finally issued, that user still saw an insecure site. Only a fresh incognito window worked.This is a sequencing bug, not a reporting one. The Ingress goes out with a TLS block naming a secret that does not exist yet, so Traefik falls back to its built-in self-signed certificate. The HTTP→HTTPS redirect was attached in the same pass, so every visitor was pushed onto that broken endpoint. A user who clicked through the warning had Chrome record a per-origin security decision, and TLS session resumption kept it pinned for the whole profile.
Compounding it,
TLSConfiguredreportedTruethe moment a ClusterIssuer was found — a statement about configuration, not about any certificate existing.The fix
https://once ready, and falls back tohttp://if no certificate is ever coming — the site stays reachable instead of becoming permanently unlinkable.AvailableandReadyare untouched. Release latency does not change. A release goes live on workload facts alone; only the link waits.Behaviour
TLSConfiguredreasonCertificateIssuinghttps://<fqdn>TLSReadyhttp://<fqdn>CertificateFailedhttp://<fqdn>CertificateTimedOuthttp://<fqdn>ClusterIssuerNotConfigured/NotFoundhttp://immediatelyIn every terminal state the site is reachable, and in no state is a user routed to the self-signed endpoint. A late-issuing certificate still upgrades to
https://— the Ready check runs before any timing logic, so the timeout can never mask it.Design notes
Certificate.Status.FailedIssuanceAttempts, not theIssuingcondition's reason — that condition is removed outright once issuance completes.TLSConfigured's ownLastTransitionTime.meta.SetStatusConditiononly moves it when the status changes, so it stays pinned across reason rewrites. No extra fetch, no new status field.Accepted tradeoffs
ClusterIssuerNotFoundand the issuer is added later, the grace is measured from that earlier failure. Result ishttp://published early — the safe direction, and issuance still upgrades it.https://during the pending window still reaches the self-signed endpoint. Unavoidable while the TLS block is present, and that block is what triggers issuance.ReadytoFalsereverts the address tohttp://. An expired certificate is genuinely broken, so that is the honest signal.internal/controller/objectstorage/ingress_reconciler.goattaches its redirect just as eagerly. Deliberately out of scope; worth a follow-up.Verification
go build ./...,go vet ./...,gofmt— clean.buildExternalAddresseschanged for every exposed port). Please treat that as an open gate — I will report the result on this PR.No RBAC change needed:
config/deploy/01-rbac.yaml:321already grants get/list/watch oncertificates.