Navigation Menu

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

Alerting: Modify alertmanager endpoints for proxying using the datasource UID #47978

Merged
merged 10 commits into from Apr 29, 2022

Conversation

papagian
Copy link
Contributor

@papagian papagian commented Apr 20, 2022

What this PR does / why we need it:
follow-up of #47829
This is part of work of removing the sequential IDs from the API and store layers. For more details you can refer to this doc.

More specifically, it:

  • modifies ProxyDatasourceRequestWithUID() so that now optionally expects a string for passing the datasource UID to look for; if it's not provided then the uid path parameter value is used.
  • modifies the alertmanager endpoints so that now expect a DatasourceUID path parameter instead of DatasourceID

Example of a successful request to a known datasource UID:

curl -X 'GET' \
  'http://localhost:3000/api/alertmanager/Y4t6ca_Mz/api/v2/silences' \
  -H 'accept: application/json' \
  -H 'authorization: Basic YWRtaW46YWRtaW4='
[{"id":"9b652196-711f-47a9-8032-d7913fdce8a7","status":{"state":"expired"},"updatedAt":"2022-04-15T15:21:40.231Z","comment":"a comment","createdBy":"spapagian","endsAt":"2022-04-15T15:21:40.231Z","matchers":[{"isEqual":true,"isRegex":true,"name":"foo","value":"bar"}],"startsAt":"2022-04-15T15:21:40.231Z"}]

Example of requesting an unknown datasource UID:

curl -X 'GET'   'http://localhost:3000/api/alertmanager/unknown/api/v2/silences'   -H 'accept: application/json'   -H 'authorization: Basic YWRtaW46YWRtaW4='
{"error":"unexpected backend type (unknown)","message":"unexpected backend type (unknown)","traceID":"00000000000000000000000000000000"}

Release notice breaking change

Grafana alerting endpoints prefixed with api/alertmanager/ that proxy requests to an Alertmanager now expect the data source UID as a path parameter instead of the data source numeric identifier.

@papagian papagian added the no-backport Skip backport of PR label Apr 20, 2022
@papagian papagian added this to the 9.0.0 milestone Apr 20, 2022
@papagian papagian requested review from a team as code owners April 20, 2022 15:16
@papagian papagian requested review from wbrowne, marefr, JacobsonMT, santihernandezc, zserge, yangkb09 and ying-jeanne and removed request for a team April 20, 2022 15:16
pkg/services/ngalert/api/authorization.go Outdated Show resolved Hide resolved
case http.MethodPost + "/api/alertmanager/{DatasourceID}/api/v2/silences":
eval = ac.EvalPermission(ac.ActionAlertingInstancesExternalWrite, datasources.ScopeProvider.GetResourceScope(ac.Parameter(":DatasourceID")))
case http.MethodPost + "/api/alertmanager/uid/{DatasourceUID}/api/v2/silences":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're up for it, it would be great to make a path function that at least provides the prefix:

AMV2Path(pathSuffix) string {
  return path.Join("/api/alertmanager/datasource/{DatasourceUID}/api/v2", pathSuffix)
}

...

AMV2Path("silence/{SIlenceID")

that makes it easier to scan which API is being evaluated.

If not, I'll create an issue and I can tackle it later.

@papagian papagian requested a review from a team as a code owner April 28, 2022 10:45
@papagian papagian requested review from peterholmberg and removed request for a team April 28, 2022 10:45
@papagian papagian requested a review from konrad147 April 28, 2022 10:45
@papagian papagian changed the title Alerting: introduce alertmanager endpoints for proxying using the datasource UID Alerting: modify alertmanager endpoints for proxying using the datasource UID Apr 28, 2022
Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@papagian papagian self-assigned this Apr 29, 2022
@papagian papagian merged commit 3e752a0 into main Apr 29, 2022
@papagian papagian deleted the papagian/ngalert-api-am-uids branch April 29, 2022 07:25
@papagian papagian added add to changelog breaking change Relevant for changelog generation labels Apr 29, 2022
@papagian papagian changed the title Alerting: modify alertmanager endpoints for proxying using the datasource UID Alerting: Modify alertmanager endpoints for proxying using the datasource UID May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants