Repository: VictoriaMetrics/VictoriaMetrics
Pull request #4344 (2023-05-25 11:52:43)

vmalert: do not return nil `rules` for /api/v1/rules
The fix addresses a case when vmalert is configured with a group which has `name`, but doesn't have `rules` configured. In this case it still returns a `nil` instead of `[]` slice.

Fixing this via current commit.

See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4221

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.02`** :warning:
> Comparison is base [(`76f7e66`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/76f7e66d8e7d0bb05b3577466227651ce8d779ff?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 61.20% compared to head [(`44ff3b2`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 61.19%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4344      +/-   ##
==========================================
- Coverage   61.20%   61.19%   -0.02%     
==========================================
  Files         330      330              
  Lines       58997    58998       +1     
==========================================
- Hits        36112    36106       -6     
- Misses      20914    20922       +8     
+ Partials     1971     1970       -1     
```


| [Impacted Files](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) | Coverage Δ | |
|---|---|---|
| [app/vmalert/manager.go](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-YXBwL3ZtYWxlcnQvbWFuYWdlci5nbw==) | `79.64% <100.00%> (+0.12%)` | :arrow_up: |

... and [6 files with indirect coverage changes](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4344?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/app/vmalert/manager.go b/app/vmalert/manager.go
index fa04f778a..25fdfbe92 100644
--- a/app/vmalert/manager.go
+++ b/app/vmalert/manager.go
@@ -188,6 +188,7 @@ func (g *Group) toAPI() APIGroup {
 
 		Labels: g.Labels,
 	}
+	ag.Rules = make([]APIRule, 0)
 	for _, r := range g.Rules {
 		ag.Rules = append(ag.Rules, r.ToAPI())
 	}


Test patch:
diff --git a/app/vmalert/web_test.go b/app/vmalert/web_test.go
index 3a70f6fef..a036a586f 100644
--- a/app/vmalert/web_test.go
+++ b/app/vmalert/web_test.go
@@ -165,8 +165,8 @@ func TestHandler(t *testing.T) {
 }
 
 func TestEmptyResponse(t *testing.T) {
-	rh := &requestHandler{m: &manager{groups: make(map[uint64]*Group)}}
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rh.handler(w, r) }))
+	rhWithNoGroups := &requestHandler{m: &manager{groups: make(map[uint64]*Group)}}
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rhWithNoGroups.handler(w, r) }))
 	defer ts.Close()
 
 	getResp := func(url string, to interface{}, code int) {
@@ -190,7 +190,7 @@ func TestEmptyResponse(t *testing.T) {
 		}
 	}
 
-	t.Run("/api/v1/alerts", func(t *testing.T) {
+	t.Run("no groups /api/v1/alerts", func(t *testing.T) {
 		lr := listAlertsResponse{}
 		getResp(ts.URL+"/api/v1/alerts", &lr, 200)
 		if lr.Data.Alerts == nil {
@@ -204,7 +204,7 @@ func TestEmptyResponse(t *testing.T) {
 		}
 	})
 
-	t.Run("/api/v1/rules", func(t *testing.T) {
+	t.Run("no groups /api/v1/rules", func(t *testing.T) {
 		lr := listGroupsResponse{}
 		getResp(ts.URL+"/api/v1/rules", &lr, 200)
 		if lr.Data.Groups == nil {
@@ -217,4 +217,26 @@ func TestEmptyResponse(t *testing.T) {
 			t.Errorf("expected /api/v1/rules response to have non-nil data")
 		}
 	})
+
+	rhWithEmptyGroup := &requestHandler{m: &manager{groups: map[uint64]*Group{0: {Name: "test"}}}}
+	ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rhWithEmptyGroup.handler(w, r) })
+
+	t.Run("empty group /api/v1/rules", func(t *testing.T) {
+		lr := listGroupsResponse{}
+		getResp(ts.URL+"/api/v1/rules", &lr, 200)
+		if lr.Data.Groups == nil {
+			t.Fatalf("expected /api/v1/rules response to have non-nil data")
+		}
+
+		lr = listGroupsResponse{}
+		getResp(ts.URL+"/vmalert/api/v1/rules", &lr, 200)
+		if lr.Data.Groups == nil {
+			t.Fatalf("expected /api/v1/rules response to have non-nil data")
+		}
+
+		group := lr.Data.Groups[0]
+		if group.Rules == nil {
+			t.Fatalf("expected /api/v1/rules response to have non-nil rules for group")
+		}
+	})
 }