-
Notifications
You must be signed in to change notification settings - Fork 3.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
ksonnet: Remove invalid hostname from default promtail configuration #3254
Conversation
@@ -10,7 +10,7 @@ | |||
username:: '', | |||
password:: '', | |||
scheme:: 'https', | |||
hostname:: 'logs-us-west1.grafana.net', | |||
hostname:: 'set valid hostname here', |
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 is what you are looking for. It could be a breaking change though for people updating their lib and promtails suddenly stopping to work.
hostname:: 'set valid hostname here', | |
hostname:: error 'must define a valid hostname', |
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.
Thanks @sandeepsukhani - that looks perfect!
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.
It could be a breaking change though for people updating their lib and promtails suddenly stopping to work.
@sandeepsukhani If people are using this default setting, all they're getting is 404
s, so it's not working for them anyway 😉
You'll need to sign the CLA, but this looks great. Thanks |
@owen-d is the CLA required for Grafana Labs employees? |
Signed-off-by: Dave Henderson <[email protected]>
89e54c8
to
a0f2e4d
Compare
I've signed the CLA anyway - the bot seems to have been a bit confused so I had to kick it 🤷♂️ |
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.
LGTM!
What this PR does / why we need it:
We're seeing traffic attempting to hit this host (and failing), and I'm guessing it's coming from misconfigured promtail instances that are using this config. Simply changing this won't help instances that already have this configuration, but in case they're updated at a future date they'll at least fail with a semi-recognizable error.
There may be a better way to declare that the value is required, but I am a jsonnet novice 😅.
Special notes for your reviewer:
Checklist
Signed-off-by: Dave Henderson [email protected]