Add platform outputs - #26
Conversation
viniciusdc
left a comment
viniciusdc
left a comment
There was a problem hiding this comment.
Hey @marcelo, re-reviewed this now that nebari-dev/nebari-infrastructure-core#609 (the new nic outputs) is on the table, and it shifts how I would read this a bit.
No blockers, and the module is honest that it is scaffolding until nic outputs lands (closes #606), so I would not over-invest here - once #609 is in, this whole extraction becomes the thin passthrough it already calls itself.
Two things I would fix before merge though, and #609 sharpens both - left them inline. The gateway lookup and the credential polling are the ones that matter; the rest are small.
| '-l', | ||
| GATEWAY_SVC_SELECTOR, | ||
| '-o', | ||
| `jsonpath={.items[?(@.spec.type=="LoadBalancer")].status.loadBalancer.ingress[0].${field}}` |
There was a problem hiding this comment.
This jsonpath emits one value per matched item with no separator, so two LoadBalancer services matching the selector give you a fused 10.0.0.410.0.0.5. #609 does this same lookup in pkg/endpoint (owning-gateway-name, single-shot, hostname-or-IP) - worth matching that shape, or at least scoping the selector with owning-gateway-namespace.
| // runs longer can read the secret themselves once it materializes. | ||
| setSecretOutput( | ||
| 'keycloak-realm-admin-password', | ||
| poll('secret keycloak/nebari-realm-admin-credentials', 6, 5, () => |
There was a problem hiding this comment.
#609 corrected something we had backwards: nebari-realm-admin-credentials is created eagerly in createKeycloakSecrets, not async by the PostSync hook, so this poll is not buying us anything. The one that is actually async is argocd-initial-admin-secret (Argo CD writes it on first start), and we read that one single-shot at :191 - that is the one that comes back empty on a fast or wait: false run. I would flip it: poll the argocd secret, drop the poll here.
| let issuerUrl: string | ||
| if (issuerHost) { | ||
| domain = issuerHost.replace(/^keycloak\./, '') | ||
| issuerUrl = `https://${issuerHost}` |
There was a problem hiding this comment.
https://${issuerHost} misses KeycloakBasePath - #609 pulls the issuer from NewTemplateData for exactly that reason. Empty on default deploys so not urgent, but wrong once a base path is set.
| readSecretKey('argocd', 'argocd-initial-admin-secret', 'password', env) | ||
| ) | ||
|
|
||
| const gatewayIp = poll('gateway LoadBalancer address', 12, 5, () => |
There was a problem hiding this comment.
The sandbox just hit a metallb-speaker stall where the LB IP was not assigned for over 300s on cold kind, and a 60s poll would have come back empty there. Worth bumping, or leaning on nic outputs --wait once #609 lands.
2 participants
Adds the outputs consumers need to talk to the platform this action just deployed: the Keycloak and ArgoCD admin credentials (masked in logs), the gateway address, the domain, and the Keycloak issuer URL.
Implementation notes:
nic outputscommand exposing the deployed platform's entry points nebari-infrastructure-core#606 gives usnic outputs.Related to #12. This covers the outputs gap only, pack registration and in-cluster DNS are still open.