ruler: ignore non-config values when checking defaults (grafana#2894)

* ruler: ignore non-config values when checking defaults

The idea of `rulestore.IsDefaults()` is to check whether the ruler has
been configured by the user. However `reflect.DeepEqual()` checks all
data within the Config struct, including things which are not actually
config.

This change skips any fields within the config marked as `yaml:"-"`,
which are not settable from yaml and therefore not config.

Using package `github.com/google/go-cmp/cmp`, which is documented as
"not for use in production" since it panics when it cannot compare
things. However we should catch that in unit testing, so should be ok.
(Note gRPC also uses this package).

Added a test which (artificially) induces the problem.

* Fix up go.mod

Author: bboreham

go.mod

@@ -53,6 +53,7 @@ require (
 
 require (
 	github.com/alecthomas/chroma v0.10.0
+	github.com/google/go-cmp v0.5.8
 	github.com/google/go-github/v32 v32.1.0
 	github.com/google/uuid v1.3.0
 	github.com/grafana-tools/sdk v0.0.0-20211220201350-966b3088eec9
@@ -135,7 +136,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/go-cmp v0.5.8 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/google/pprof v0.0.0-20220608213341-c488b8fa1db3 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.1.0 // indirect

pkg/ruler/rulestore/config.go

@@ -9,6 +9,7 @@ import (
 	"flag"
 	"reflect"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/grafana/dskit/flagext"
 
 	"github.com/grafana/mimir/pkg/ruler/rulestore/local"
@@ -35,5 +36,28 @@ func (cfg *Config) IsDefaults() bool {
 	defaults := Config{}
 	flagext.DefaultValues(&defaults)
 
-	return reflect.DeepEqual(*cfg, defaults)
+	// Note: cmp.Equal will panic if it encounters anything it cannot handle.
+	return cmp.Equal(*cfg, defaults, cmp.FilterPath(filterNonYaml, cmp.Ignore()), cmp.Comparer(equalSecrets))
+}
+
+// Return true if the path contains a struct field with tag `yaml:"-"`.
+func filterNonYaml(path cmp.Path) bool {
+	for i, step := range path {
+		// If we're not looking at a struct, or next step not available, skip.
+		if step.Type().Kind() != reflect.Struct || i >= len(path)-1 {
+			continue
+		}
+		field := step.Type().Field((path[i+1].(cmp.StructField)).Index())
+		if tag, ok := field.Tag.Lookup("yaml"); ok {
+			if tag == "-" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// Helper for cmp.Equal to compare Secret values for equality, since it has unexported fields.
+func equalSecrets(a, b flagext.Secret) bool {
+	return a == b
 }

pkg/ruler/rulestore/config_test.go

@@ -34,6 +34,13 @@ func TestIsDefaults(t *testing.T) {
 			},
 			expected: false,
 		},
+		"should return true if only a non-config field has changed": {
+			setup: func(cfg *Config) {
+				flagext.DefaultValues(cfg)
+				cfg.Middlewares = append(cfg.Middlewares, nil)
+			},
+			expected: true,
+		},
 	}
 
 	for testName, testData := range tests {