Add SnippetsFilter guide #2721
Add SnippetsFilter guide #2721
Conversation
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
LGTM generally: I'd consider using the ghcode shortcode to explicitly show the YAML contents of files, but it might lead to reading fatigue.
Using it for the code that includes the actual snippets could show the reader what to expect if they decide to configure NGF with them, at least.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2721 +/- ##
=======================================
Coverage ? 89.38%
=======================================
Files ? 110
Lines ? 10913
Branches ? 50
=======================================
Hits ? 9755
Misses ? 1100
Partials ? 58 ☔ View full report in Codecov by Sentry. |
|
|
||
| ## Overview | ||
|
|
||
| Snippets allow Cluster Operators to insert NGINX configuration into different contexts of the |
There was a problem hiding this comment.
I think we need to be clear calling them SnippetsFilters, not just Snippets. And also make it clear that these attach to an HTTP/GRPCRoute rule and apply to that rule specifically.
There was a problem hiding this comment.
I think we need to be clear calling them SnippetsFilters, not just Snippets.
I followed Ingress controller's precedence here: https://docs.nginx.com/nginx-ingress-controller/configuration/ingress-resources/advanced-configuration-with-snippets/
And think that its good to stick with Snippets because it seems like that's already an established NGINX term? At least with the NIC crowd it will strike a familiar bell if we stick with saying Snippets are configured through SnippetsFilters.
And also make it clear that these attach to an HTTP/GRPCRoute rule and apply to that rule specifically.
Though, if a SF sets things in the main, http, or http.server contexts, then the SF doesn't apply to that rule specifically right? I was a little confused on this behavior as well.
There was a problem hiding this comment.
Fair; after re-reading, I think it makes sense to talk about the general idea of snippets, but then describe specifically what a SnippetsFilter is (because in the future we may also have SnippetsPolicy). I'd also rename the doc to snippets.md.
Though, if a SF sets things in the main, http, or http.server contexts, then the SF doesn't apply to that rule specifically right? I was a little confused on this behavior as well.
True, but I think we should make it clear what the intention is. Maybe it can be phrased better. If someone puts something in main, it really should be because it's required for a configuration for a rule. Technically a user could put anything in there, but that's not what it's for.
There was a problem hiding this comment.
I've added some information on this, please let me know if what I have is correct and/or if there is more stuff I should add.
There was a problem hiding this comment.
Great work on this @bjee19. Sorry to jump in late with comments.
| NGINX configurations that NGINX Gateway Fabric generates. | ||
|
|
||
| Snippets are for advanced NGINX users who need more control over the generated NGINX configuration, | ||
| and can be used in cases where Gateway API resources or NGINX extension policies don't apply. |
There was a problem hiding this comment.
continuation of the suggestion above:
| and can be used in cases where Gateway API resources or NGINX extension policies don't apply. | |
| and only in cases where Gateway API resources or NGINX extension policies don't apply. |
There was a problem hiding this comment.
same comment as above with copying NIC's precedence
| --- | ||
|
|
||
| ## Further reading | ||
|
|
There was a problem hiding this comment.
I think we need a section here or in our troubleshooting guide about how to diagnose issues with SnippetsFilters. Most importantly, we need to mention somewhere (probably here) that if the SnippetsFilter cannot be resolved, the route will return a 500 HTTP error code response.
There was a problem hiding this comment.
Added a troubleshooting section in the document, let me know if you think there's anything that needs to be adjusted or added/removed, thanks!
There was a problem hiding this comment.
I think we should move this to troubleshooting guide . Its better consolidated in one place and we don't generally follow the practice of having troubleshooting sections with our traffic-management guides
There was a problem hiding this comment.
True, though I did see https://docs.nginx.com/nginx-ingress-controller/configuration/ingress-resources/advanced-configuration-with-snippets/ and how they put a troubleshooting guide in with the snippets that is separate from all of their other troubleshooting guides. this also breaks their practices too it seems (I just checked some of their guides and they don't have troubleshooting guides in them)
@ADubhlaoich thoughts?
There was a problem hiding this comment.
Really great troubleshooting section!
I personally like the troubleshooting info to be close to the documentation on the feature, but I'll defer to @ADubhlaoich.
| --- | ||
|
|
||
| ## Further reading | ||
|
|
There was a problem hiding this comment.
I think we should move this to troubleshooting guide . Its better consolidated in one place and we don't generally follow the practice of having troubleshooting sections with our traffic-management guides
| --- | ||
|
|
||
| ## Further reading | ||
|
|
There was a problem hiding this comment.
Really great troubleshooting section!
I personally like the troubleshooting info to be close to the documentation on the feature, but I'll defer to @ADubhlaoich.
Add SnippetsFilters guide.
Problem: We want a guide for using SnippetsFilters.
Solution: Add a SnippetsFilters guide.
Testing: Verified using
make watchHugo command.Closes #2377
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.