Repository navigation
Conversation
|
📄 Docs preview for |
3 of 18 tasks
dcmcand
added a commit
that referenced
this pull request
Oct 1, 2026
…41) #44 rewrites the same Helm section in both copies of the CRD reference. Use its text, with the review suggestions posted on #44 applied (fetch step, accurate toJson rule, routing block), so both PRs make identical changes there and merge in either order. Drops two sentences #45 had added that #44 does not carry: the link to the nebari-app chart directory and the note that required fields are checked after rendering.
The template developed, tested and documented against v0.1.0-alpha.19, while NIC v0.14.0 deploys v0.1.0-alpha.20 and the latest release is v0.1.1. - dev/Makefile: default OPERATOR_REF to v0.1.1 (overridable with ?=) and clone operator scripts into a ref-specific cache directory, so changing the ref never reuses scripts cloned for another version - test-integration.yaml: run the job for v0.1.1 and v0.1.0-alpha.20 - CRD reference: link the v0.1.1 types and state which release NIC deploys - README: describe the two CI operator versions
The Auth Flow page told developers to disable NebariApp locally, while the Makefile claimed the full stack works. Neither was right: the release install.yaml leaves the operator's in-cluster issuer at port 8080 with no /auth path, so Envoy Gateway rejected every SecurityPolicy and auth-enabled apps returned 500. The browser was also sent to in-cluster Keycloak URLs it cannot reach, and no per-app certificates were issued. - dev/configure-operator.sh sets KEYCLOAK_ISSUER_SERVICE_PORT, KEYCLOAK_ISSUER_CONTEXT_PATH, KEYCLOAK_EXTERNAL_URL and TLS_CLUSTER_ISSUER_NAME, as NIC does, and applies dev/keycloak-route.yaml to expose Keycloak at keycloak.nebari.local (with a Forwarded header so Keycloak builds https login URLs) - Makefile: run it during cluster setup, build chart dependencies before installing, add a keycloak hosts entry and a login-test target - dev/login-test.sh drives the OIDC login with curl and checks the app - CI configures the operator the same way - Docs: replace the local-development limitation, list up-podinfo and login-test, and add a troubleshooting entry for the 500 caused by a rejected SecurityPolicy
With no spec.routing the operator creates no HTTPRoute and no TLS, yet the NebariApp reports Ready=True. basic-nginx and wrap-existing-chart shipped without routing, so they deployed cleanly and returned 404, and CI passed because it only waited for Ready. - basic-nginx and wrap-existing-chart values: add routing.routes and routing.tls (chart versions bumped) - Add routing to every NebariApp snippet in the README and docs, and say what happens without it - dev/verify-nebariapp.sh waits for RoutingReady and TLSReady (and AuthReady with --auth), checks the HTTPRoute and SecurityPolicy exist, and sends an HTTPS request through the Gateway - Makefile and integration CI use it instead of waiting on Ready. The Makefile waited on nebariapp/my-pack-my-pack, which never exists: the release name already contains the chart name, so the NebariApp is my-pack - lint.yaml fails if a chart renders a NebariApp without routing - wrap-existing-chart uses helm dependency build against its committed lock
NIC locks ArgoCD's default AppProject to deny-all and expects packs in nebari-apps. Every Application example used project: default, and their CreateNamespace=true namespaces were never labeled for the operator. - All Application examples: project: nebari-apps, plus managedNamespaceMetadata labeling the namespace nebari.dev/managed=true - build-your-own: a complete Application example, the NIC requirements, how to check the pack is reachable, and the repository Secret needed for private sources - CRD reference: label the namespace through ArgoCD - lint.yaml fails if an example uses project: default
Operator v0.1.1 creates the groups in Keycloak and lists them for the landing page, but the SecurityPolicy it generates has no authorization rule, so any user in the realm gets through (nebari-dev/nebari-operator#153). - CRD reference, README and example READMEs say groups is not enforced and point to checking the groups claim in the app - Drop "restricted to groups" wording from the kustomize production overlay docs and comment the overlay's groups list
The docs and auth-fastapi decoded the IdToken cookie without checking its signature, saying Envoy Gateway had verified it. Envoy's OAuth2 filter does not check JWT signatures, and requests can reach the app without passing it: any pod can call the Service, and publicRoutes have no SecurityPolicy. An unsigned token sent straight to the Service was shown as the logged-in user. - app: verify signature (JWKS), iss, aud and exp with PyJWT before using any claim; reject requests carrying more than one IdToken-* cookie; fail closed when verification is not configured - chart: pass client-id and issuer-url from the operator's OIDC Secret, with oidc.issuerURL and oidc.jwksURL overrides; add an optional NetworkPolicy that only admits the Envoy proxies (chart version bumped) - tests/: table-driven pytest suite, run in lint.yaml - integration CI: log in through Keycloak, and check forged tokens are rejected with and without the NetworkPolicy - Docs: auth-flow, README and the example README describe verification
…docs (#38) - Auth Flow: SecurityPolicy is <name>-security targeting <name>-route, HTTPRoutes are <name>-route and <name>-public-route, and Certificates are <name>-<namespace>-cert in envoy-gateway-system; show the endpoints and logoutPath the operator actually sets - README troubleshooting: look for Certificates in envoy-gateway-system, the operator in nebari-operator-system, and Envoy Gateway logs via its Deployment
The CRD reference said only the app's pods can read the OIDC client Secret. The operator's Role grants one ServiceAccount API access; RBAC is additive, and any pod in the namespace can mount the Secret through the kubelet. - CRD reference: reword serviceAccountName and add "Who can read the OIDC Secret" - Auth Flow: secretKeyRef works without the Role; the Role only matters for API reads
- Add routing.tls.secretName and landingPage.iconLight/iconDark, marked with the release that introduced them - Replace the condition reason table with every reason the v0.1.1 controllers set, grouped by condition, and note the defined-but-unused ones - Explain that Ready does not wait for RoutingReady, TLSReady or AuthReady - Mark clientSecretRef, status.gatewayRef and status.clientSecretRef as not implemented, and fix the displayName and externalUrl descriptions - Replace the stale hand-written Helm template with the nebari-app library chart pattern - README: explain TLSReady=False/ClusterIssuerNotConfigured
…41) #44 rewrites the same Helm section in both copies of the CRD reference. Use its text, with the review suggestions posted on #44 applied (fetch step, accurate toJson rule, routing block), so both PRs make identical changes there and merge in either order. Drops two sentences #45 had added that #44 does not carry: the link to the nebari-app chart directory and the note that required fields are checked after rendering.
Deleting the vanilla-yaml Deployment with the default background cascade returned before its pods were gone, so the kustomize step's 'kubectl wait -l app=my-pack' matched the terminating pod and failed with NotFound. Use foreground cascading so cleanup blocks until pods are deleted.
dcmcand
force-pushed
the
fix/pack-guide-accuracy
branch
from
October 7, 2026 10:08
e9e1c55 to
9589ce9
Compare
Both copies of the CRD reference carried a hand-maintained field reference that duplicates the operator's generated api-reference.md. Replace the field, status and condition tables with a pointer to the generated reference, the reconciler docs and the nebari-app chart guide, all pinned to v0.1.1, plus an index of the examples. Keep what only this repository documents: namespace opt-in, who can read the OIDC Secret, and the plain YAML, Kustomize and Helm deployment patterns. Keep the two caveats the generated reference gets wrong or omits: auth.groups is not enforced in v0.1.1 (nebari-operator#153), and iconLight/iconDark are not in the alpha.20 operator NIC v0.14.0 deploys. Repoint the nine references that promised a complete field reference on this page (README, both auth-flow copies, index, build-your-own, what-is-a-software-pack) to the generated reference. Links to the Pack Specification are left out until nebari-operator#187 publishes it. Supersedes #30. Refs nebari-dev/governance#59
Retiring the field tables dropped three corrections the table rows carried and the generated reference does not: spec.auth.clientSecretRef is ignored and the Secret is always <nebariapp-name>-oidc-client (nebari-operator#193), Ready=True does not wait for RoutingReady, TLSReady or AuthReady, and landingPage.displayName is not validated. Add them to "What the generated reference does not tell you" in both copies. Each was checked against the v0.1.1 source.
Docs:
- README's "key code" snippet now matches main.py, including
options={"require": ["exp", "iss", "aud"]}; without it PyJWT accepts
a token with no exp.
- Raise the documented PyJWT floor to 2.10.1. 2.10.0 does a partial
match on the issuer (CVE-2024-53861).
- README no longer says NIC sets KEYCLOAK_ISSUER_SERVICE_PORT. NIC sets
the context path and serves Keycloak on the default port, 8080.
- CRD reference: a hand-managed client must be named
<namespace>-<nebariapp-name>, because the SecurityPolicy always uses
that client ID. Link the Ready and displayName caveats to
nebari-operator#195 and #196.
- auth-flow: serviceAccountName defaults to the NebariApp name;
issuer-url is always written but may be empty; local login needs
make update-hosts for keycloak.nebari.local; link the trust-model
question to nebari-operator#194. The snippet drops cache_keys and
logs JWKS outages, matching main.py.
- Stop naming the stale operator version in the CRD page's history.
Example:
- Drop cache_keys=True, whose per-kid cache never expires, so a key
removed from Keycloak's JWKS stops being trusted within the 5 minute
JWK-set cache.
- Log a JWKS connection failure at warning instead of as a rejected
token. Requests still fail closed.
- Test alg=none and HS256-keyed-with-the-public-key tokens, and the
JWKS outage path. Widening algorithms now fails the HS256 case.
- NetworkPolicy comment says the proxy selector is platform-owned and
links nebari-operator#197.
CI:
- Lint fails when an operator docs or release link differs from
OPERATOR_REF, or when prose names an operator version the integration
matrix does not test.
- Pin kubeconform to v0.8.0 and check its SHA-256.
- Retry the forged-token probe while the deleted NetworkPolicy is still
being enforced.
- Assert directly that TokenVerifier.verify passes algorithms=["RS256"]. The HS256 case only failed under a widened pin because PyJWT raised a TypeError the handler doesn't catch; adding "none" or ES256 passed. Note that the alg=none and HS256 cases document PyJWT's own refusal. - Move the operator version guard to dev/check-operator-refs.sh and check every match rather than every line. It now also covers releases/tag links, the "Operator version this page tracks" pins, and prose such as "not enforced in v0.1.1". tree/main stays allowed for the separately versioned nebari-app chart links. Keep the kustomize comment's version on one line with its context so the guard sees it. - Say "NetworkPolicy still blocks the Service" when the forged-token probe never gets through, instead of "forged token was not rejected". - The issuer-url comment now says the operator writes it only when it provisions the client.
Contributor
Author
|
@viniciusdc this is ready for review when you have a chance. It makes the pack guide and template accurate for third-party pack authors and folds in #30. The field tables now point at the operator's generated API reference (pinned to v0.1.1), with notes on its known gaps. The auth-fastapi example now verifies the IdToken signature itself. A new lint check fails CI when docs name an operator version that the integration matrix doesn't test. It pairs with the pack spec in nebari-operator#187. |
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
1 participant
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.
Fixes everything in #43: following packs.nebari.dev and the examples now produces a pack that deploys on a NIC cluster, is reachable over HTTPS, and doesn't trust identities it hasn't verified. One commit per issue.
Closes #34, closes #35, closes #36, closes #37, closes #38, closes #39, closes #40, closes #41, closes #42
Fixes #46
Supersedes #30.
What was wrong, briefly
routing, so the operator made no HTTPRoute or TLS while reportingReady. CI passed because it only waited forReady.project: default, which NIC locks to deny-all.auth.groupsrestricts access (it doesn't, security: auth.groups is documented as access control but is not enforced at the gateway nebari-operator#153) and that apps can decode the IdToken without verifying it. An unsigned token sent straight to the auth-fastapi Service was shown as the logged-in user.v0.1.0-alpha.19. CRD reference is missing newer fields and condition reasons, and documents fields that do nothing #41 is resolved by retiring its field and status tables in favour of the generatedv0.1.1reference, rolling in Retire the hand-maintained NebariApp field reference #30. The page keeps what only this repo documents (namespace opt-in, OIDC Secret access, the YAML/Kustomize/Helm patterns) and theauth.groupscaveat the generated reference gets wrong. Pack Specification links wait for Publish the Nebari Pack Specification nebari-operator#187.How it was checked
On a kind cluster built from scratch with operator v0.1.1:
make up-*target deploys and gets a real HTTPS response through the Gateway, not justReady.make login-testlogs in through Keycloak, and the app shows the verified user.:latestdisplays as "mallory" is rejected by the new image, and withnetworkPolicy.enabledthe request doesn't connect at all.project: defaultgetsInvalidSpecError.CI now runs the integration job for v0.1.1 and for v0.1.0-alpha.20, which NIC v0.14.0 deploys. The alpha.20 run has only been checked from source here.
Heads-up for merge
my-pack-0.1.3release.make up-fastapiuses the published:latestimage, which picks up the token verification once the merge builds a new one.build.test.ts, Nebari branding/footer) already fails onmain, and CI doesn't run it. It's untouched here.