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

loki.api.url should discard any provided /loki/api/v1/push #12271

Closed
simondeziel opened this issue Sep 18, 2023 · 2 comments · Fixed by #12273
Closed

loki.api.url should discard any provided /loki/api/v1/push #12271

simondeziel opened this issue Sep 18, 2023 · 2 comments · Fixed by #12273
Assignees

Comments

@simondeziel
Copy link
Member

When configuring integration with Loki, one provides LXD with the URL to Loki's API. Since a URL can include the path, a user may do lxc config set loki.api.url=http://10.76.66.141:3100/loki/api/v1/push. The problem is that LXD unconditionally appends /loki/api/v1/push to the provided URL resulting in bogus HTTP POSTs to http://10.76.66.141:3100/loki/api/v1/push/loki/api/v1/push.

The doc on loki.api.url is pretty succinct so I think it's a fair mistake for someone to do, at least I think I did when implementing the charm because the Loki integration lib returns http://${hostname|ip}:3100/loki/api/v1/push and I feed that directly to LXD. While I could fix the charm, I think the LXD side should be improved. Either by dropping any provided /loki/api/v1/push suffix or by rejecting the value as bogus.

@monstermunchkin
Copy link
Contributor

I prefer dropping the /loki/api/v1/push if provided by the user.

monstermunchkin added a commit to monstermunchkin/lxd that referenced this issue Sep 19, 2023
LXD adds the "/loki/api/v1/push" suffix to `loki.api.url` automatically.
There's no need for the user to provide it. Should it be provided
anyway, the suffix is stripped.

Fixes canonical#12271

Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
@tomponline
Copy link
Member

My preference would be to improve the docs and explain that LXD will add the /loki/api/v1/push to the URL, otherwise this url setting will be a special case compared to other ones (such as the ones we use for interacting with external auth providers, where we also append certain paths to the end).

Also if we start stripping stuff from the URL that the user provides we don't know if they do infact need that, perhaps to get through a reverse proxy to their loki server.

monstermunchkin added a commit to monstermunchkin/lxd that referenced this issue Sep 19, 2023
Mention that LXD adds the `/loki/api/v1/push` suffix to the provided
`loki.api.url` config value.

Fixes canonical#12271

Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
mihalicyn pushed a commit to mihalicyn/lxd that referenced this issue Sep 21, 2023
Mention that LXD adds the `/loki/api/v1/push` suffix to the provided
`loki.api.url` config value.

Fixes canonical#12271

Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants