-
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
AP: enable FQDN as syslog destination #1939
Conversation
9f2980d
to
e245fd0
Compare
aa36fb6
to
3cd26ce
Compare
ca25f78
to
3203d01
Compare
013a8bc
to
0bdd080
Compare
df77910
to
d374f0e
Compare
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 The changes look good
092c0b3
to
666be2a
Compare
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
Please see my comments
Additionally:
(1) Shall we update the doc for the logDest
field of the WAF Policy? (https://docs.nginx.com/nginx-ingress-controller/configuration/policy-resource/#waf). the annotation doc seems fines.
(2) Would it be possible to update and simplify the AppProtect tests so that they no longer rely on fetching the svc IP address, but rather use the DNS name?
646789a
to
efbcccc
Compare
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.
@rafwegv lgtm! 👍 sorry for the delay
efbcccc
to
9987ee2
Compare
Proposed changes
Add checks that enable an FQDN be specified as the destination for security logs
Checklist
Before creating a PR, run through this checklist and mark each as complete.