Adding /api/v2 prefix to all endpoints in OpenAPI specification

[dannykopping]

All API endpoints are prefixed with /api/v2

[roidelapluie]

openapi has dedicated prefix: setting for this.

[dannykopping]

openapi has dedicated prefix: setting for this.

I looked for it here, but couldn't find it; I'm not very familiar with OpenAPI.
Could you direct me to it pls?

[roidelapluie]

Look for basePath here: https://swagger.io/specification/v2/

[dannykopping]

Ah, thanks @roidelapluie. I'm curious - are many people using the client library?
If so, I'd imagine that someone must've come across this issue?

Some context:
I'm not using it, but this page directed me to the openapi spec. I wanted to send a test alert to AM via the API, and struggled to find the correct API path to use.

[roidelapluie]

tests are all broken now. I wonder if that is a valid change since the basePath must be at the root but you can change it with flags ..

[dannykopping]

Indeed. I think I'll close this, and we can revive if necessary.

[roidelapluie]

I don't think we should close this, we should fix the tests and see what the result is.

We better fix this now than after 1.0 is released.

[dannykopping]

Alright. What do you suggest we do regarding the tests?
I'm new to the codebase.

[roidelapluie]

We'd need to investigate why they fail

[JohnnyQQQQ]

When adding a basePath the generated server automatically has this path. But we add it already in api.go in line 190.

alertmanager/api/api.go

Lines 189 to 192 in fad7969

	mux.Handle(
	apiPrefix+"/api/v2/",
	api.limitHandler(http.StripPrefix(apiPrefix+"/api/v2", api.v2.Handler)),
	)

So we end up with a path like /api/v2/api/v2, you can check this locally starting an instance an opening the api/v2/api/v2/status endpoint.

It seems sensible to have the basePath set correctly to generate the clients and server with the correct path embedded. I'm not sure what the best solution would be here.

cc @roidelapluie

[JohnnyQQQQ]

I did dig a bit deeper. The problem is in line 191. We don't need StripePrefix anymore.

Let me explain in detail.

We serve the Handler under the /api/v2/ route. The Handler is expecting Requests with a /api/v2/ prefix. By removing the prefix, you require adding it again to make the request work. That's why /api/v2/api/v2/status works. As we strip /api/v2 one time with StripePrefix, making it correct.

By simply removing the /api/v2/ part from StripPrefix everything should work. I tested it locally and opened a PR to your branch (dannykopping#1).

[dannykopping]

I did dig a bit deeper. The problem is in line 191. We don't need StripePrefix anymore.

Let me explain in detail.

We serve the Handler under the /api/v2/ route. The Handler is expecting Requests with a /api/v2/ prefix. By removing the prefix, you require adding it again to make the request work. That's why /api/v2/api/v2/status works. As we strip /api/v2 one time with StripePrefix, making it correct.

By simply removing the /api/v2/ part from StripPrefix everything should work. I tested it locally and opened a PR to your branch (dannykopping#1).

Thanks for that @JohnnyQQQQ!

[dannykopping]

Hhmm, looks like the DCO build stage is failing now from your last commit

[JohnnyQQQQ]

@dannykopping Sorry yeah, you need to do the steps mentioned here https://github.com/prometheus/alertmanager/pull/2696/checks?check_run_id=4187219569

[dannykopping]

@dannykopping Sorry yeah, you need to do the steps mentioned here https://github.com/prometheus/alertmanager/pull/2696/checks?check_run_id=4187219569

You're right; i thought you'd have to sign your commit, but turns out i can sign your tweet and keep your authorship 👍

[roidelapluie]

Thanks!

[amitvermaa3101]

Please suggest if alertmanager UI is available to manage silence or other things