Skip to content
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

Prometheus 0.18.0 fails to start with short version of Alertmanger URL #1579

Closed
xbglowx opened this Issue Apr 23, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@xbglowx
Copy link

xbglowx commented Apr 23, 2016

I am trying to upgrade from Prometheus 0.17.0 to 0.18.0 and noticed that Prometheus is failing to start due to:

2016-04-22_23:43:36.52662 time="2016-04-22T23:43:36Z" level=error msg="invalid Alertmanager URL: http://prometheus-alertmanager.service.q:9093" source="main.go:65"

I first thought Prometheus couldn't resolve the URL, but after doing some straces, I didn't even see Prometheus trying to resolve the name. Prometheus 0.18.0 currently doesn't like it when there is a single character after the last . of the URL. Maybe there is regex in Prometheus which is assuming that all URLs will be FQDN and there are no single character TLDs?

Works:
http://prometheus-alertmanager.t.te

Doesn't work:
http://prometheus-alertmanager.t.t

@xbglowx xbglowx changed the title Prometheus 0.18.0 can't resolve short version of Alertmanger URL Prometheus 0.18.0 fails to start with short version of Alertmanger URL Apr 23, 2016

@grobie grobie added the bug label Apr 23, 2016

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 23, 2016

0.18.0 introduced #1456, which uses govalidator.IsURL() to validate the Alertmanager URL. And this is the regex it uses: https://github.com/asaskevich/govalidator/blob/master/patterns.go#L33

The {2,} in the end seems to point to the fact that it requires at least two letters in the TLD, if there is one. Not sure if some standard somewhere says that it has to be at least 2 chars (couldn't find it immediately), or if it should be fixed in govalidator. Certainly there is no public 1-letter TLD in existence today.

@matthiasr

This comment has been minimized.

Copy link
Contributor

matthiasr commented Apr 23, 2016

But requiring a TLD at all implies that relative domains can't be used. This would make e.g. using it in Kubernetes unnecessarily hard, so I think this should be relaxed a bit.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 23, 2016

@matthiasr So it allows having no TLD at all (just http://f validates fine), but if the last component is dot-something, it needs to be at least 2 chars long. Of course, you could still have relative URLs where there are more than 2 name components, but the last one should not be the TLD yet.

All for relaxing this.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 23, 2016

I filed an issue with govalidator, let's see: asaskevich/govalidator#125

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 23, 2016

It's fixed upstream.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.