-
Notifications
You must be signed in to change notification settings - Fork 37
Add base alert management API #657
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
base: alerts-management-api
Are you sure you want to change the base?
Add base alert management API #657
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: machadovilaca The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cc @simonpasquier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a separate PR on main for the golangci-lint addition? It's very much appreciated but it'd be easier to review separately. Can we also have a minimal golangci-lint configuration? Once this is done we should also consider adding a job to openshift/release to enforce the verification.
Regarding goimports, I'd recommend using the gci linter which does the same IIUC.
cfc4748 to
dc61cb7
Compare
Signed-off-by: machadovilaca <[email protected]>
dc61cb7 to
5aad6ea
Compare
removed for now, will create a PR to add it separately |
Signed-off-by: machadovilaca <[email protected]>
Signed-off-by: machadovilaca <[email protected]>
| certArg = flag.String("cert", "", "cert file path to enable TLS (disabled by default)") | ||
| keyArg = flag.String("key", "", "private key file path to enable TLS (disabled by default)") | ||
| featuresArg = flag.String("features", "", "enabled features, comma separated.\noptions: ['acm-alerting', 'incidents', 'dev-config', 'perses-dashboards']") | ||
| featuresArg = flag.String("features", "", "enabled features, comma separated.\noptions: ['acm-alerting', 'incidents', 'dev-config', 'perses-dashboards', 'management-api']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| featuresArg = flag.String("features", "", "enabled features, comma separated.\noptions: ['acm-alerting', 'incidents', 'dev-config', 'perses-dashboards', 'management-api']") | |
| featuresArg = flag.String("features", "", "enabled features, comma separated.\noptions: ['acm-alerting', 'incidents', 'dev-config', 'perses-dashboards', 'alert-management-api']") |
|
|
||
| if managementClient != nil { | ||
| managementRouter := managementrouter.New(managementClient) | ||
| router.PathPrefix("/api/v1/alerting").Handler(managementRouter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we've discussed before that we wanted an API scheme similar to https://prometheus.io/docs/prometheus/latest/querying/api/#rules so it can be used as a drop-in replacement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| State string `form:"state"` | ||
| } | ||
|
|
||
| type GetAlertsResponse struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this response should match the alerting API response: https://github.com/prometheus/alertmanager/blob/main/api/v2/openapi.yaml#L423
No description provided.