*: fix JSON marshaling for nil slices in rules (#2888)

Go's `encoding/json` turns nil slices into `null` instead of `[]`. This
is problematic for consumers expecting that the `alerts` field is a JSON
array. The same is true for empty rule groups.

The solution is to enforce that nil slices are converted to zero-length
slices when encoding.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

pkg/query/api/v1_test.go

@@ -1359,6 +1359,7 @@ func TestRulesHandler(t *testing.T) {
 			EvaluationTime: all[3].GetAlert().EvaluationDurationSeconds,
 			Duration:       all[3].GetAlert().DurationSeconds,
 			Annotations:    nil,
+			Alerts:         []*testpromcompatibility.Alert{},
 			Type:           "alerting",
 		},
 	}

pkg/rules/rulespb/custom.go

@@ -177,6 +177,15 @@ func (r1 *Rule) Compare(r2 *Rule) int {
 	return 0
 }
 
+func (r *RuleGroups) MarshalJSON() ([]byte, error) {
+	if r.Groups == nil {
+		// Ensure that empty slices are marshaled as '[]' and not 'null'.
+		return []byte(`{"groups":[]}`), nil
+	}
+	type plain RuleGroups
+	return json.Marshal((*plain)(r))
+}
+
 func (m *Rule) UnmarshalJSON(entry []byte) error {
 	decider := struct {
 		Type string `json:"type"`
@@ -219,6 +228,10 @@ func (m *Rule) MarshalJSON() ([]byte, error) {
 		})
 	}
 	a := m.GetAlert()
+	if a.Alerts == nil {
+		// Ensure that empty slices are marshaled as '[]' and not 'null'.
+		a.Alerts = make([]*AlertInstance, 0)
+	}
 	return json.Marshal(struct {
 		*Alert
 		Type string `json:"type"`

pkg/rules/rulespb/custom_test.go

@@ -29,9 +29,10 @@ func TestJSONUnmarshalMarshal(t *testing.T) {
 		expectedJSONOutput string // If empty, expected same one as marshaled input.
 	}{
 		{
-			name:          "Empty JSON",
-			input:         &testpromcompatibility.RuleDiscovery{},
-			expectedProto: &RuleGroups{},
+			name:               "Empty JSON",
+			input:              &testpromcompatibility.RuleDiscovery{},
+			expectedProto:      &RuleGroups{},
+			expectedJSONOutput: `{"groups":[]}`,
 		},
 		{
 			name: "one empty group",
@@ -165,6 +166,78 @@ func TestJSONUnmarshalMarshal(t *testing.T) {
 			},
 			expectedErr: errors.New("failed to unmarshal \"asdfsdfsdfsd\" as 'partial_response_strategy'. Possible values are ABORT,WARN"),
 		},
+		{
+			name: "one valid group with 1 alerting rule containing no alerts.",
+			input: &testpromcompatibility.RuleDiscovery{
+				RuleGroups: []*testpromcompatibility.RuleGroup{
+					{
+						Name: "group1",
+						Rules: []testpromcompatibility.Rule{
+							testpromcompatibility.AlertingRule{
+								Type:  RuleAlertingType,
+								Name:  "alert1",
+								Query: "up == 0",
+								Labels: labels.Labels{
+									{Name: "a2", Value: "b2"},
+									{Name: "c2", Value: "d2"},
+								},
+								Annotations: labels.Labels{
+									{Name: "ann1", Value: "ann44"},
+									{Name: "ann2", Value: "ann33"},
+								},
+								Health:         "health2",
+								LastError:      "1",
+								Duration:       60,
+								State:          "pending",
+								EvaluationTime: 1.1,
+							},
+						},
+						File:                              "file1.yml",
+						Interval:                          2442,
+						EvaluationTime:                    2.1,
+						DeprecatedPartialResponseStrategy: "WARN",
+						PartialResponseStrategy:           "ABORT",
+					},
+				},
+			},
+			expectedProto: &RuleGroups{
+				Groups: []*RuleGroup{
+					{
+						Name: "group1",
+						Rules: []*Rule{
+							NewAlertingRule(&Alert{
+								Name:  "alert1",
+								Query: "up == 0",
+								Labels: PromLabels{
+									Labels: []storepb.Label{
+										{Name: "a2", Value: "b2"},
+										{Name: "c2", Value: "d2"},
+									},
+								},
+								Annotations: PromLabels{
+									Labels: []storepb.Label{
+										{Name: "ann1", Value: "ann44"},
+										{Name: "ann2", Value: "ann33"},
+									},
+								},
+								DurationSeconds:           60,
+								State:                     AlertState_PENDING,
+								LastError:                 "1",
+								Health:                    "health2",
+								EvaluationDurationSeconds: 1.1,
+							}),
+						},
+						File:                              "file1.yml",
+						Interval:                          2442,
+						EvaluationDurationSeconds:         2.1,
+						DeprecatedPartialResponseStrategy: storepb.PartialResponseStrategy_WARN,
+						PartialResponseStrategy:           storepb.PartialResponseStrategy_ABORT,
+					},
+				},
+			},
+			// Different than input due to the alerts slice being initialized to a zero-length slice instead of nil.
+			expectedJSONOutput: `{"groups":[{"name":"group1","file":"file1.yml","rules":[{"state":"pending","name":"alert1","query":"up == 0","duration":60,"labels":{"a2":"b2","c2":"d2"},"annotations":{"ann1":"ann44","ann2":"ann33"},"alerts":[],"health":"health2","lastError":"1","evaluationTime":1.1,"lastEvaluation":"0001-01-01T00:00:00Z","type":"alerting"}],"interval":2442,"evaluationTime":2.1,"lastEvaluation":"0001-01-01T00:00:00Z","partial_response_strategy":"WARN","partialResponseStrategy":"ABORT"}]}`,
+		},
 		{
 			name: "one valid group, with 1 rule and alert each and second empty group.",
 			input: &testpromcompatibility.RuleDiscovery{
@@ -353,7 +426,7 @@ func TestJSONUnmarshalMarshal(t *testing.T) {
 				testutil.Equals(t, tcase.expectedJSONOutput, string(jsonProto))
 				return
 			}
-			testutil.Equals(t, jsonInput, jsonProto)
+			testutil.Equals(t, string(jsonInput), string(jsonProto))
 		})
 	}
 }