Skip to content

Commit

Permalink
Alerting: Drop NamespaceID from responses on unstable ngalert API end…
Browse files Browse the repository at this point in the history
…points in favor of NamespaceUID (#79359)

* Drop from API response

* Drop from swagger docs

* Drop from integration tests

* regenerate public swagger docs

* Drop from frontend

* Drop asserts for namespaceID field
  • Loading branch information
alexweav committed Dec 15, 2023
1 parent f8a1e7d commit cf8e885
Show file tree
Hide file tree
Showing 19 changed files with 10 additions and 61 deletions.
13 changes: 6 additions & 7 deletions pkg/services/ngalert/api/api_ruler.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *contextmodel.ReqContext, nam

for groupKey, rules := range ruleGroups {
// nolint:staticcheck
result[namespaceTitle] = append(result[namespaceTitle], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, namespace.ID, provenanceRecords))
result[namespaceTitle] = append(result[namespaceTitle], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords))
}

return response.JSON(http.StatusAccepted, result)
Expand Down Expand Up @@ -193,7 +193,7 @@ func (srv RulerSrv) RouteGetRulesGroupConfig(c *contextmodel.ReqContext, namespa

result := apimodels.RuleGroupConfigResponse{
// nolint:staticcheck
GettableRuleGroupConfig: toGettableRuleGroupConfig(ruleGroup, rules, namespace.ID, provenanceRecords),
GettableRuleGroupConfig: toGettableRuleGroupConfig(ruleGroup, rules, provenanceRecords),
}
return response.JSON(http.StatusAccepted, result)
}
Expand Down Expand Up @@ -243,7 +243,7 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res
}
namespace := folder.Title
// nolint:staticcheck
result[namespace] = append(result[namespace], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, folder.ID, provenanceRecords))
result[namespace] = append(result[namespace], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords))
}
return response.JSON(http.StatusOK, result)
}
Expand Down Expand Up @@ -405,15 +405,15 @@ func changesToResponse(finalChanges *store.GroupDelta) response.Response {
return response.JSON(http.StatusAccepted, body)
}

func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, namespaceID int64, provenanceRecords map[string]ngmodels.Provenance) apimodels.GettableRuleGroupConfig {
func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, provenanceRecords map[string]ngmodels.Provenance) apimodels.GettableRuleGroupConfig {
rules.SortByGroupIndex()
ruleNodes := make([]apimodels.GettableExtendedRuleNode, 0, len(rules))
var interval time.Duration
if len(rules) > 0 {
interval = time.Duration(rules[0].IntervalSeconds) * time.Second
}
for _, r := range rules {
ruleNodes = append(ruleNodes, toGettableExtendedRuleNode(*r, namespaceID, provenanceRecords))
ruleNodes = append(ruleNodes, toGettableExtendedRuleNode(*r, provenanceRecords))
}
return apimodels.GettableRuleGroupConfig{
Name: groupName,
Expand All @@ -422,7 +422,7 @@ func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, name
}
}

func toGettableExtendedRuleNode(r ngmodels.AlertRule, namespaceID int64, provenanceRecords map[string]ngmodels.Provenance) apimodels.GettableExtendedRuleNode {
func toGettableExtendedRuleNode(r ngmodels.AlertRule, provenanceRecords map[string]ngmodels.Provenance) apimodels.GettableExtendedRuleNode {
provenance := ngmodels.ProvenanceNone
if prov, exists := provenanceRecords[r.ResourceID()]; exists {
provenance = prov
Expand All @@ -439,7 +439,6 @@ func toGettableExtendedRuleNode(r ngmodels.AlertRule, namespaceID int64, provena
Version: r.Version,
UID: r.UID,
NamespaceUID: r.NamespaceUID,
NamespaceID: namespaceID,
RuleGroup: r.RuleGroup,
NoDataState: apimodels.NoDataState(r.NoDataState),
ExecErrState: apimodels.ExecutionErrorState(r.ExecErrState),
Expand Down
5 changes: 0 additions & 5 deletions pkg/services/ngalert/api/tooling/api.json
Original file line number Diff line number Diff line change
Expand Up @@ -1431,10 +1431,6 @@
"is_paused": {
"type": "boolean"
},
"namespace_id": {
"format": "int64",
"type": "integer"
},
"namespace_uid": {
"type": "string"
},
Expand Down Expand Up @@ -4472,7 +4468,6 @@
"type": "array"
},
"integration": {
"description": "Integration integration",
"properties": {
"lastNotifyAttempt": {
"description": "A timestamp indicating the last attempt to deliver a notification regardless of the outcome.\nFormat: date-time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ type GettableGrafanaRule struct {
Version int64 `json:"version" yaml:"version"`
UID string `json:"uid" yaml:"uid"`
NamespaceUID string `json:"namespace_uid" yaml:"namespace_uid"`
NamespaceID int64 `json:"namespace_id" yaml:"namespace_id"`
RuleGroup string `json:"rule_group" yaml:"rule_group"`
NoDataState NoDataState `json:"no_data_state" yaml:"no_data_state"`
ExecErrState ExecutionErrorState `json:"exec_err_state" yaml:"exec_err_state"`
Expand Down
9 changes: 2 additions & 7 deletions pkg/services/ngalert/api/tooling/post.json
Original file line number Diff line number Diff line change
Expand Up @@ -1431,10 +1431,6 @@
"is_paused": {
"type": "boolean"
},
"namespace_id": {
"format": "int64",
"type": "integer"
},
"namespace_uid": {
"type": "string"
},
Expand Down Expand Up @@ -3988,6 +3984,7 @@
"type": "object"
},
"URL": {
"description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use the EscapedPath method, which preserves\nthe original encoding of Path.\n\nThe RawPath field is an optional field which is only set when the default\nencoding of Path is different from the escaped path. See the EscapedPath method\nfor more details.\n\nURL's String method uses the EscapedPath method to obtain the path.",
"properties": {
"ForceQuery": {
"type": "boolean"
Expand Down Expand Up @@ -4023,7 +4020,7 @@
"$ref": "#/definitions/Userinfo"
}
},
"title": "URL is a custom URL type that allows validation at configuration load time.",
"title": "A URL represents a parsed URL (technically, a URI reference).",
"type": "object"
},
"UpdateRuleGroupResponse": {
Expand Down Expand Up @@ -4412,7 +4409,6 @@
"type": "object"
},
"gettableAlerts": {
"description": "GettableAlerts gettable alerts",
"items": {
"$ref": "#/definitions/gettableAlert"
},
Expand Down Expand Up @@ -4655,7 +4651,6 @@
"type": "object"
},
"receiver": {
"description": "Receiver receiver",
"properties": {
"active": {
"description": "active",
Expand Down
9 changes: 2 additions & 7 deletions pkg/services/ngalert/api/tooling/spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -4624,10 +4624,6 @@
"is_paused": {
"type": "boolean"
},
"namespace_id": {
"type": "integer",
"format": "int64"
},
"namespace_uid": {
"type": "string"
},
Expand Down Expand Up @@ -7181,8 +7177,9 @@
}
},
"URL": {
"description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use the EscapedPath method, which preserves\nthe original encoding of Path.\n\nThe RawPath field is an optional field which is only set when the default\nencoding of Path is different from the escaped path. See the EscapedPath method\nfor more details.\n\nURL's String method uses the EscapedPath method to obtain the path.",
"type": "object",
"title": "URL is a custom URL type that allows validation at configuration load time.",
"title": "A URL represents a parsed URL (technically, a URI reference).",
"properties": {
"ForceQuery": {
"type": "boolean"
Expand Down Expand Up @@ -7608,7 +7605,6 @@
"$ref": "#/definitions/gettableAlert"
},
"gettableAlerts": {
"description": "GettableAlerts gettable alerts",
"type": "array",
"items": {
"$ref": "#/definitions/gettableAlert"
Expand Down Expand Up @@ -7856,7 +7852,6 @@
"$ref": "#/definitions/postableSilence"
},
"receiver": {
"description": "Receiver receiver",
"type": "object",
"required": [
"active",
Expand Down
1 change: 0 additions & 1 deletion pkg/services/ngalert/store/alert_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ func (st DBstore) GetUserVisibleNamespaces(ctx context.Context, orgID int64, use
continue
}
namespaceMap[hit.UID] = &folder.Folder{
ID: hit.ID, // nolint:staticcheck
UID: hit.UID,
Title: hit.Title,
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/tests/api/alerting/api_alertmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,6 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
"version": 1,
"uid": "",
"namespace_uid": %q,
"namespace_id": 1,
"rule_group": "arulegroup",
"no_data_state": "NoData",
"exec_err_state": "Alerting"
Expand Down Expand Up @@ -1250,7 +1249,6 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
"version":1,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"NoData",
"exec_err_state":"Alerting"
Expand Down Expand Up @@ -1287,7 +1285,6 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
"version":1,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"Alerting",
"exec_err_state":"Alerting"
Expand Down Expand Up @@ -1596,7 +1593,6 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
"version":2,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"Alerting",
"exec_err_state":"Alerting"
Expand Down Expand Up @@ -1706,7 +1702,6 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
"version":3,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"Alerting",
"exec_err_state":"Alerting"
Expand Down Expand Up @@ -1795,7 +1790,6 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
"version":3,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"Alerting",
"exec_err_state":"Alerting"
Expand Down Expand Up @@ -2102,7 +2096,6 @@ func TestIntegrationQuota(t *testing.T) {
"version":2,
"uid":"uid",
"namespace_uid":"nsuid",
"namespace_id":1,
"rule_group":"arulegroup",
"no_data_state":"NoData",
"exec_err_state":"Alerting"
Expand Down
5 changes: 0 additions & 5 deletions pkg/tests/api/alerting/api_ruler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,9 @@ func TestIntegrationAlertRulePermissions(t *testing.T) {

for _, rule := range allRules["folder1"][0].Rules {
assert.Equal(t, "folder1", rule.GrafanaManagedAlert.NamespaceUID)
assert.Equal(t, int64(1), rule.GrafanaManagedAlert.NamespaceID)
}
for _, rule := range allRules["folder2"][0].Rules {
assert.Equal(t, "folder2", rule.GrafanaManagedAlert.NamespaceUID)
assert.Equal(t, int64(2), rule.GrafanaManagedAlert.NamespaceID)
}
})

Expand Down Expand Up @@ -664,7 +662,6 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
"version": 1,
"uid": "uid",
"namespace_uid": "nsuid",
"namespace_id": 1,
"rule_group": "anotherrulegroup",
"no_data_state": "NoData",
"exec_err_state": "Alerting"
Expand Down Expand Up @@ -698,7 +695,6 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
"version": 1,
"uid": "uid",
"namespace_uid": "nsuid",
"namespace_id": 1,
"rule_group": "anotherrulegroup",
"no_data_state": "Alerting",
"exec_err_state": "Alerting"
Expand Down Expand Up @@ -744,7 +740,6 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
"version": 1,
"uid": "uid",
"namespace_uid": "nsuid",
"namespace_id": 1,
"rule_group": "anotherrulegroup",
"no_data_state": "NoData",
"exec_err_state": "Alerting"
Expand Down
2 changes: 0 additions & 2 deletions pkg/tests/api/alerting/test-data/rulegroup-1-get.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"version": 1,
"uid": "<dynamic>",
"namespace_uid": "<dynamic>",
"namespace_id": 0,
"rule_group": "Group1",
"no_data_state": "NoData",
"exec_err_state": "Alerting",
Expand Down Expand Up @@ -81,7 +80,6 @@
"version": 1,
"uid": "<dynamic>",
"namespace_uid": "<dynamic>",
"namespace_id": 0,
"rule_group": "Group1",
"no_data_state": "NoData",
"exec_err_state": "Alerting",
Expand Down
1 change: 0 additions & 1 deletion pkg/tests/api/alerting/test-data/rulegroup-2-get.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"version": 1,
"uid": "<dynamic>",
"namespace_uid": "<dynamic>",
"namespace_id": 0,
"rule_group": "Group2",
"no_data_state": "NoData",
"exec_err_state": "Error",
Expand Down
5 changes: 0 additions & 5 deletions public/api-merged.json
Original file line number Diff line number Diff line change
Expand Up @@ -14733,10 +14733,6 @@
"is_paused": {
"type": "boolean"
},
"namespace_id": {
"type": "integer",
"format": "int64"
},
"namespace_uid": {
"type": "string"
},
Expand Down Expand Up @@ -20962,7 +20958,6 @@
}
},
"integration": {
"description": "Integration integration",
"type": "object",
"required": [
"name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ describe('RuleEditor grafana managed rules', () => {
grafana_alert: {
uid,
namespace_uid: 'abcd',
namespace_id: 1,
condition: 'B',
data: getDefaultQueries(),
exec_err_state: GrafanaAlertStateDecision.Error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ describe('GrafanaModifyExport', () => {
uid: 'test-rule-uid',
title: 'cpu-usage',
namespace_uid: 'folder-test-uid',
namespace_id: 1,
data: [
{
refId: 'A',
Expand Down
2 changes: 0 additions & 2 deletions public/app/features/alerting/unified/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ export const mockRulerGrafanaRule = (
uid: '123',
title: 'myalert',
namespace_uid: '123',
namespace_id: 1,
condition: 'A',
no_data_state: GrafanaAlertStateDecision.Alerting,
exec_err_state: GrafanaAlertStateDecision.Alerting,
Expand Down Expand Up @@ -212,7 +211,6 @@ export const mockGrafanaRulerRule = (partial: Partial<GrafanaRuleDefinition> = {
uid: '',
title: 'my rule',
namespace_uid: 'NAMESPACE_UID',
namespace_id: 0,
condition: '',
no_data_state: GrafanaAlertStateDecision.NoData,
exec_err_state: GrafanaAlertStateDecision.Error,
Expand Down
1 change: 0 additions & 1 deletion public/app/features/alerting/unified/utils/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ describe('alertRuleToQueries', () => {
const grafanaAlert = {
condition: 'B',
exec_err_state: GrafanaAlertStateDecision.Alerting,
namespace_id: 11,
namespace_uid: 'namespaceuid123',
no_data_state: GrafanaAlertStateDecision.NoData,
title: 'Test alert',
Expand Down
1 change: 0 additions & 1 deletion public/app/features/alerting/unified/utils/rule-id.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ describe('hashRulerRule', () => {
const grafanaAlertDefinition: GrafanaRuleDefinition = {
uid: RULE_UID,
namespace_uid: 'namespace',
namespace_id: 0,
title: 'my rule',
condition: '',
data: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export function getRulerRulesResponse(folderName: string, folderUid: string, see
],
uid: random.guid(),
namespace_uid: folderUid,
namespace_id: 0,
no_data_state: GrafanaAlertStateDecision.NoData,
exec_err_state: GrafanaAlertStateDecision.Error,
is_paused: false,
Expand Down
1 change: 0 additions & 1 deletion public/app/types/unified-alerting-dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ export interface GrafanaRuleDefinition extends PostableGrafanaRuleDefinition {
id?: string;
uid: string;
namespace_uid: string;
namespace_id: number;
provenance?: string;
}

Expand Down
5 changes: 0 additions & 5 deletions public/openapi3.json
Original file line number Diff line number Diff line change
Expand Up @@ -5625,10 +5625,6 @@
"is_paused": {
"type": "boolean"
},
"namespace_id": {
"format": "int64",
"type": "integer"
},
"namespace_uid": {
"type": "string"
},
Expand Down Expand Up @@ -11853,7 +11849,6 @@
"type": "array"
},
"integration": {
"description": "Integration integration",
"properties": {
"lastNotifyAttempt": {
"description": "A timestamp indicating the last attempt to deliver a notification regardless of the outcome.\nFormat: date-time",
Expand Down

0 comments on commit cf8e885

Please sign in to comment.