Don't allow calling QIDs with an empty IDs list (#4707)

* Don't allow calling QIDs with an empty IDs list

Calling QIDs with an empty list of IDs is meaningless, and the result
may be unexpected (at the moment it would return all silences, but one
might expect it to never return any). Make it an error.

Signed-off-by: Guido Trotter <[email protected]>

* Use errors.New instead of fmt.Errorf

Co-authored-by: Ben Kochie <[email protected]>
Signed-off-by: Guido Trotter <[email protected]>

---------

Signed-off-by: Guido Trotter <[email protected]>
Signed-off-by: Guido Trotter <[email protected]>
Co-authored-by: Guido Trotter <[email protected]>
Co-authored-by: Ben Kochie <[email protected]>

Author: 3 people

silence/silence.go

@@ -735,6 +735,9 @@ type silenceFilter func(*pb.Silence, *Silences, time.Time) (bool, error)
 // QIDs configures a query to select the given silence IDs.
 func QIDs(ids ...string) QueryParam {
 	return func(q *query) error {
+		if len(ids) == 0 {
+			return errors.New("QIDs filter must have at least one id")
+		}
 		q.ids = append(q.ids, ids...)
 		return nil
 	}

silence/silence_test.go

@@ -1152,6 +1152,44 @@ func TestSilencesQuery(t *testing.T) {
 	}
 }
 
+func TestQIDs(t *testing.T) {
+	s, err := New(Options{Metrics: prometheus.NewRegistry()})
+	require.NoError(t, err)
+
+	s.st = state{
+		"1": &pb.MeshSilence{Silence: &pb.Silence{Id: "1"}},
+		"2": &pb.MeshSilence{Silence: &pb.Silence{Id: "2"}},
+		"3": &pb.MeshSilence{Silence: &pb.Silence{Id: "3"}},
+		"4": &pb.MeshSilence{Silence: &pb.Silence{Id: "4"}},
+	}
+
+	// Test QIDs with empty arguments returns an error
+	_, _, err = s.Query(QIDs())
+	require.Error(t, err, "expected error when QIDs is called with no arguments")
+	require.Contains(t, err.Error(), "QIDs filter must have at least one id")
+
+	// Test QIDs with empty arguments returns an error via QueryOne
+	_, err = s.QueryOne(QIDs())
+	require.Error(t, err, "expected error when QIDs is called with no arguments")
+	require.Contains(t, err.Error(), "QIDs filter must have at least one id")
+
+	// Test QIDs with single ID works
+	res, _, err := s.Query(QIDs("1"))
+	require.NoError(t, err)
+	require.Len(t, res, 1)
+	require.Equal(t, "1", res[0].Id)
+
+	// Test QIDs with multiple IDs works
+	res, _, err = s.Query(QIDs("1", "2"))
+	require.NoError(t, err)
+	require.Len(t, res, 2)
+
+	// Test QueryOne with single ID works
+	sil, err := s.QueryOne(QIDs("1"))
+	require.NoError(t, err)
+	require.Equal(t, "1", sil.Id)
+}
+
 type silencesByID []*pb.Silence
 
 func (s silencesByID) Len() int           { return len(s) }