Repository navigation
feat: Route TLS - #11
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a new composite GitHub Action (tls-route-provisioner) to provision OpenShift TLS vanity routes, including local certificate/key validation and an archival backup of existing route TLS material, and wires in documentation plus a basic CI dry-run workflow.
Changes:
- Added
tls-route-provisionercomposite action to validate TLS inputs, optionally back up existing route certs/keys, and apply a generated Route manifest. - Added action-specific README plus linked the action from the repo root README.
- Added a PR/push workflow to exercise the action in
dry_runmode using generated dummy certs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| tls-route-provisioner/action.yml | New composite action implementing TLS validation, OpenShift login/backup, and Route YAML generation/apply. |
| tls-route-provisioner/README.md | New action documentation with usage snippet and inputs table. |
| README.md | Adds the new action to the repo’s top-level catalog (and shifts numbering). |
| .github/workflows/pr-open.yml | CI workflow that dry-runs the action using generated test certificates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a97ef75 to
7c05acc
Compare
Extract the composite action into provision.sh so real PEMs can be dry-run on a laptop without GitHub or a cluster. Fix caCertificate indent, backup app labels, and stop printing private keys.
CodeQL flags workflows with no permissions block; this job only needs checkout.
c82c3a5 to
0aab8eb
Compare
Deny GITHUB_TOKEN at workflow and job scope. Checkout only needs the public tree.
That runner is a login+checkout action and was invoked with no with: block, so oc_server was empty. Download the client from the OpenShift mirror instead; provision.sh already logs in.
There was a problem hiding this comment.
🟡 Changes recommended
Credential handling and archival behavior currently introduce security and backup-reliability risks.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 7
- Review effort level: Balanced
The action sets spec.host and spec.tls; a vanity DNS name is only one caller. hostname replaces vanity_url; backup Secrets use backup-type=route-tls.
Keep naming as Route TLS, drop frontend-specific Service examples, and treat the local CLI as secondary.
PR-close sweeps by app= and would delete a TLS Route if we labeled it. Leave the Route unlabeled and name it <repository>-vanity-url unless the caller overrides.
Without spec.tls.caCertificate the leaf cannot chain to the public roots.
Gold API login always skips API TLS verify. That is not a caller choice and is unrelated to Route certificates.
Shrink the bash ADR, require the CA to have issued the leaf, pin the oc client to 4.16.45, and snapshot ca.crt alongside cert and key so a bad apply can be undone.
Pass oc_namespace, oc_server, and oc_token through so the runner can log in. Client version stays the runner default.
There was a problem hiding this comment.
🟡 Changes recommended
TLS hostname handling, sensitive-file cleanup, error reporting, and cluster authentication have unresolved correctness and security issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
route-tls/provision.sh:124
- In GitHub jobs,
RUNNER_TEMPis set, so the generated Route manifest containing the private key is outsideWORKDIRand survives the EXIT cleanup. Keep non-dry-run output in the protected temporary directory so the key is deleted after apply.
ROUTE_OUT="${RUNNER_TEMP:-$WORKDIR}/route.yml"
- Files reviewed: 5/6 changed files
- Comments generated: 3
- Review effort level: Balanced
Skip oc login when already authenticated so GitHub does not re-send the long-lived token. Compare DNS names case-insensitively, and die with a specific error if openssl cannot parse the cert or key.
Adds the Route TLS composite action (
route-tls): GitHub secrets in, OpenShift Route with custom TLS out. openssl checks the PEMs; existing Route TLS is snapshotted before overwrite.See route-tls/README.md. Populate
TLS_CERTIFICATE,TLS_PRIVATE_KEY, andTLS_CA_CERTIFICATEon a prod GitHub Environment.