Skip to content

Feat nxe phase - #17

Merged
kjdev merged 2 commits into
mainfrom
feat/nxe-phase
Sep 2, 2026
Merged

kjdev merged 2 commits into
mainfrom
feat/nxe-phase

Conversation

@kjdev

@kjdev kjdev commented Sep 2, 2026 •

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Improvements

    • Improved request-processing consistency by ensuring access-control checks run at a predictable point, regardless of module load order.
    • Reduced configuration sensitivity when multiple modules participate in the same request-processing phase.
  • Build & Packaging

    • Added the required phase-ordering component to source and container builds.
    • Builds now provide a clear error when the required component is unavailable.

The auth_rbac handler was registered on NGX_HTTP_PRECONTENT_PHASE with
a plain ngx_array_push(), so its execution order relative to other
same-phase modules (auth-gate, auth-cedar, internal-redirect) was
whatever the reverse of --add-module/load_module ordering happened to
produce, not something a config author could control.

Vendor the shared nxe-phase submodule and register the handler with
nxe_phase_add_handler(cf, NGX_HTTP_PRECONTENT_PHASE, NXE_PHASE_PRIO_RBAC,
ngx_http_auth_rbac_handler, "auth_rbac"), matching the same migration
already done in nginx-auth-cedar. Priority 750 places it after
auth-gate (600) and auth-cedar (700), before internal-redirect (900).
No return-value change is needed: the handler already returns
NGX_DECLINED on both allow and deny paths.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: a0f2ea0c-9649-42c2-b06b-936228fadd35

📥 Commits

Reviewing files that changed from the base of the PR and between 56ab9d6 and 645af0f.

📒 Files selected for processing (6)
  • .gitmodules
  • CHANGELOG.md
  • Dockerfile
  • config
  • nxe-phase
  • src/ngx_http_auth_rbac_module.c

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds the nxe-phase submodule, integrates its build configuration, and registers the RBAC PRECONTENT handler through nxe_phase_add_handler at NXE_PHASE_PRIO_RBAC.

Changes

PRECONTENT Handler Ordering

Layer / File(s) Summary
Add and package nxe-phase
.gitmodules, nxe-phase, Dockerfile
The repository pins the nxe-phase submodule and copies it into the Docker build context.
Integrate nxe-phase build configuration
config
The build requires nxe-phase/config.ngx and merges its module metadata into the RBAC module configuration.
Register the RBAC handler at a fixed priority
src/ngx_http_auth_rbac_module.c, CHANGELOG.md
The module registers ngx_http_auth_rbac_handler with nxe_phase_add_handler and NXE_PHASE_PRIO_RBAC. The changelog records the ordering change and dependency.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 645af

The current changes leave no actionable merge-blocking risk beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant ngx_http_auth_rbac_init
  participant nxe_phase_add_handler
  participant NGX_HTTP_PRECONTENT_PHASE
  ngx_http_auth_rbac_init->>nxe_phase_add_handler: Register auth_rbac with NXE_PHASE_PRIO_RBAC
  nxe_phase_add_handler->>NGX_HTTP_PRECONTENT_PHASE: Add ngx_http_auth_rbac_handler at fixed priority
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the main change: adding nxe-phase integration. It is concise and related to the pull request, although the wording is abbreviated.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/nxe-phase

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@kjdev
kjdev merged commit 645af0f into main Sep 2, 2026
7 checks passed
@kjdev
kjdev deleted the feat/nxe-phase branch September 2, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant