-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Add externl references to NAP #1100
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small typos/doc fixes:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rafwegv
Just a few improvements suggestions
If the references are not available during ingress controller startup the pod will fail to startup. In case the resources are not available during a reload, the reload will fail, and ingress controller will process traffic with the previous correct configuration. | ||
|
||
### Check the HTTP response time to resources used in App Protect policy external references. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved under the Troubleshooting Methods section, as it is a troubleshooting method.
I suggest two small enhancements:
- For brevity, renaming it to "Check the Availability of APPolicy External References"
- Adding the disclaimer like this:
This method only applies if you're using external references in NGINX App Protect policies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
please replace 2 occurrences "Check the HTTP response time to resources used in App Protect policy external references" with "Check the Availability of APPolicy External References" before merging
Proposed changes
This change adds external references feature to ingress controller + App Protect, and also adds some content to troubleshooting docs.
Checklist
Before creating a PR, run through this checklist and mark each as complete.