-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[thog-1548] log verification errors #2107
Conversation
951b68e
to
709c480
Compare
Related to #1924. From my own experiences enabling this, it would be good to release this feature in tandem with an improvement to the list of known false-positives. For instance, e.g., this should never even be attempted to be verified:
The SQLServer pattern is also overly permissive and results in a lot of frequent (and large) verification errors.
|
i think placeholders make sense to filter out, but localhost/127 seem useful for internal testers, no? |
That makes sense for 172.x, 192.168.x, and 10.x (private IP ranges). Localhost/0.0.0.0/127.0.0.1 will never succeed unless you are already running the service on your local machine, otherwise you'll get a connection refused error. It's an exceptionally low signal/noise ratio that drowns out legitimate verification errors. Not to mention that "localhost" is often the default for incomplete or incorrect information. |
61f0b06
to
b2f328d
Compare
closed in favor of #2335 |
will make separate tickets for these since they are not blocking for the log output changes |
Description:
Update plaintext logging to include verification error
Current Output
Updated Output
Checklist:
make test-community
)?make lint
this requires golangci-lint)?