Deprecate and remove api/v1/ #2970
Conversation
gotjosh
force-pushed
the
remove-api-v1
branch
2 times, most recently
from
e0d9502
to
cac28d3
Compare
Signed-off-by: gotjosh <josue.abreu@gmail.com>
gotjosh
force-pushed
the
remove-api-v1
branch
from
cac28d3
to
a0d920f
Compare
| uptime: time.Now(), | ||
| peer: peer, | ||
| logger: l, | ||
| m: metrics.NewAlerts("v1", r), |
There was a problem hiding this comment.
Technically, we can remove this distinction now but it would be a rather critical breaking change - I would rather not risk it.
There was a problem hiding this comment.
nvm, I made it permament.
Signed-off-by: gotjosh <josue.abreu@gmail.com>
|
@simonpasquier I think this ready for review - this is currently blocking the progress on #3567 as making API v1 compatible with that work is a large endeavour for something we don't need. Would appreciate a speedy review - I think the changes to the deprecation router are good enough to let users know that something that we deprecated back in 0.18 is now gone. |
api/v1_deprecation_router.go
Outdated
| Error string `json:"error"` | ||
| }{ | ||
| "deprecated", | ||
| "The Alertmanager v1 API was deprecated in version 0.16.0 and entirely removed since version 0.28.0 - please use the equivalent route in the v2 API", |
There was a problem hiding this comment.
Small nit, feel free to ignore 😄
| "The Alertmanager v1 API was deprecated in version 0.16.0 and entirely removed since version 0.28.0 - please use the equivalent route in the v2 API", | |
| "The Alertmanager v1 API was deprecated in version 0.16.0 and is removed as of version 0.28.0 - please use the equivalent route in the v2 API", |
| } | ||
|
|
||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(410) |
api/v1_deprecation_router.go
Outdated
| w.WriteHeader(410) | ||
|
|
||
| // We don't care about errors for this route. | ||
| b, _ := json.Marshal(resp) |
There was a problem hiding this comment.
You don't need to change this, but you can also use:
if err := json.NewEncoder(w).Encode(resp); err != nil {
level.Error(dr.logger).Log("msg", "failed to write response", "err", err)
}
|
Looks great in general, but you are so deep into the weeds here that I don't feel qualified to evaluate the details. :) |
Signed-off-by: gotjosh <josue.abreu@gmail.com>
gotjosh
force-pushed
the
remove-api-v1
branch
from
93c6ea4
to
0cf6797
Compare
|
Thanks @beorn7 - With the amount of context you have, you'll always be overqualified to review anything we throw at you. |
|
Should we follow up with a change on prometheus to now allow configuring v1 endpoints anymore? |
Yes please Alan! Would you be happy to open a PR for that? |
This is intentionally incomplete and is open mostly as a guideline to see what breaks.
The description will be updated once the work is finished and serves as a checklist to give visibility of a plan for reviewers.
fixes #2469
The steps to take are:
/api/v1/alertsto behave like/api/v2/alerts.api/v1Edit:
api/v1/*redirect to the/api/v2I opted for adding a deprecation notice in JSON to all of the v1 routes. On top of that, each time a v1 route is called it'll log a warning letting the operator know that something is not going right.