Skip to content

Add SnippetsPolicy for Gateway level #4461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
fabian4 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add SnippetsPolicy for Gateway level #4461

fabian4 wants to merge 1 commit into from

Conversation

@fabian4
Copy link
Contributor

@fabian4 fabian4 commented Dec 13, 2025 •
edited
Loading

Problem: Users need a mechanism to inject raw NGINX configuration snippets into the generated configuration to leverage advanced NGINX features or directives that are not yet exposed via the Gateway API or existing NGINX Gateway Fabric policies.

Solution: Implemented the SnippetsPolicy Custom Resource Definition (CRD).

  • API: The policy targets a Gateway and supports main, http, and server contexts.
  • Validation: Added validation logic to enforce a maximum snippet size (2KB), ensure unique contexts per policy, and disallow unsupported contexts (e.g., http.server.location).
  • Generation: The generator creates dedicated include files for each policy (e.g., includes/policy/<gateway>/SnippetsPolicy_main_...conf), ensuring clean separation of injected config.
  • Integration: Wired the new policy into the ChangeProcessor, Controller Manager, and Helm charts.

Testing:

  • Added unit tests for the Validator, Generator, and ChangeProcessor logic.
  • Added a new integration test suite (tests/suite/snippets_policy_test.go) covering:
    • Valid snippets in all supported contexts.
    • Invalid context validation.
    • Duplicate context validation.
  • Verified Helm chart generation.

Please focus on (optional): The validation logic in internal/controller/nginx/config/policies/snippetspolicy/validator.go, specifically the size limits and context checks.

Closes #4071

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

 Added support for `SnippetsPolicy`, allowing users to inject custom NGINX configuration snippets into the `main`, `http`, and `server` contexts.

@nginx-bot
Copy link

nginx-bot bot commented Dec 13, 2025

Hi @fabian4!

Thanks for opening this pull request!
Be sure to check out our Contributing Guidelines while you wait for someone on the team to review this.

@nginx-bot nginx-bot bot added the community label Dec 13, 2025
@github-actions github-actions bot added helm-chart Relates to helm chart documentation Improvements or additions to documentation tests Pull requests that update tests labels Dec 14, 2025
@fabian4 fabian4 force-pushed the add_snippestspolicy branch from ebe3381 to 8e5feb3 Compare December 14, 2025 13:37
@github-actions github-actions bot removed the tests Pull requests that update tests label Dec 14, 2025
@fabian4 fabian4 force-pushed the add_snippestspolicy branch from 8e5feb3 to 63897e7 Compare December 14, 2025 13:52
@github-actions github-actions bot added the tests Pull requests that update tests label Dec 14, 2025
@fabian4 fabian4 marked this pull request as ready for review December 14, 2025 14:14
@fabian4 fabian4 requested a review from a team as a code owner December 14, 2025 14:14
sjberman
sjberman reviewed Dec 15, 2025
View reviewed changes
Copy link
Collaborator

@sjberman sjberman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on! It's not easy work.

@fabian4 fabian4 requested a review from sjberman December 16, 2025 13:30
@fabian4 fabian4 force-pushed the add_snippestspolicy branch from e11ed2f to a155e01 Compare December 18, 2025 13:19
sjberman
sjberman reviewed Dec 18, 2025
View reviewed changes
Copy link
Collaborator

@sjberman sjberman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When submitting changes, if you could avoid squashing, it makes it easier to review what changed between commits. TIA

internal/controller/nginx/config/policies/snippetspolicy/generator.go
content = fmt.Sprintf(mainTemplate, policyNsName, gwNsName, snippet.Value)
filename = filepath.Join(
"includes", "policy", gwNsName,
fmt.Sprintf("SnippetsPolicy_main_%s.conf", sp.GetName()),
Copy link
Collaborator

@sjberman sjberman Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just name the file SnippetsPolicy_main_namespace_name.conf and then createIncludesFromPolicyGenerateResult will generate the full path with directory?

bjee19
bjee19 reviewed Dec 19, 2025
View reviewed changes
Copy link
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work and thanks for taking this on!

If you could also rebase your fork/branch off of upstream main, it will fix some issues we were having from the pipeline not working on forks. We'll then be able to run the pipeline and have the functional test run.

tests/suite/snippets_policy_test.go
Comment on lines +49 to +52
Expect(err).ToNot(HaveOccurred())
Expect(nginxPodNames).To(HaveLen(1))

nginxPodName = nginxPodNames[0]
Copy link
Contributor

@bjee19 bjee19 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm pretty sure because we apply two separate gateways, this'll fail since two nginx pods are created.

@fabian4 fabian4 force-pushed the add_snippestspolicy branch from a155e01 to 4e1423e Compare December 19, 2025 02:51
sjberman
sjberman reviewed Dec 19, 2025
View reviewed changes
tests/suite/snippets_policy_test.go
Comment on lines +119 to +122
// gwNsName := fmt.Sprintf("%s-%s", namespace, "gateway")
// mainContext := fmt.Sprintf("%s%s/SnippetsPolicy_main_", snippetsPolicyFilePrefix, gwNsName)
// httpContext := fmt.Sprintf("%s%s/SnippetsPolicy_http_", snippetsPolicyFilePrefix, gwNsName)
// serverContext := fmt.Sprintf("%s%s/http:80/SnippetsPolicy_server_", snippetsPolicyFilePrefix, gwNsName)
Copy link
Collaborator

@sjberman sjberman Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjberman sjberman sjberman left review comments

@bjee19 bjee19 bjee19 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

@salonichf5 salonichf5

Labels

community documentation Improvements or additions to documentation helm-chart Relates to helm chart tests Pull requests that update tests

Projects

NGINX Gateway Fabric
Status: 🆕 New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for Snippets at the Gateway level

4 participants

@fabian4 @sjberman @bjee19 @salonichf5