Migrate to gopkg.in/yaml.v3

Makefile

@@ -301,6 +301,11 @@ lint: check-makefiles
 	faillint -paths "github.com/NYTimes/gziphandler" \
 		./pkg/... ./cmd/... ./tools/... ./integration/...
 
+	# We don't want to use yaml.v2 anywhere, because we use yaml.v3 now,
+	# and UnamrshalYAML signature is not compatible between them.
+	faillint -paths "gopkg.in/yaml.v2" \
+		./pkg/... ./cmd/... ./tools/... ./integration/...
+
 	# Ensure packages we imported from Thanos are no longer used.
 	GOFLAGS="-tags=requires_docker" faillint -paths \
 		"github.com/thanos/thanos-io/pkg/store,\

cmd/mimir/main.go

@@ -6,6 +6,7 @@
 package main
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"flag"
 	"fmt"
@@ -22,7 +23,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/weaveworks/common/tracing"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/mimir"
 	util_log "github.com/grafana/mimir/pkg/util/log"
@@ -252,8 +253,9 @@ func LoadConfig(filename string, expandEnv bool, cfg *mimir.Config) error {
 		buf = expandEnvironmentVariables(buf)
 	}
 
-	err = yaml.UnmarshalStrict(buf, cfg)
-	if err != nil {
+	dec := yaml.NewDecoder(bytes.NewReader(buf))
+	dec.KnownFields(true)
+	if err := dec.Decode(cfg); err != nil {
 		return errors.Wrap(err, "Error parsing config file")
 	}
 

cmd/mimir/main_test.go

@@ -16,9 +16,10 @@ import (
 	"testing"
 
 	"github.com/go-kit/log"
+	"github.com/grafana/dskit/flagext"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/mimir"
 	"github.com/grafana/mimir/pkg/util/fieldcategory"
@@ -98,9 +99,21 @@ func TestFlagParsing(t *testing.T) {
 			stderrExcluded: "ingester\n",
 		},
 
-		"root level configuration option specified as an empty node in YAML": {
-			yaml:          "querier:",
-			stderrMessage: "the Querier configuration in YAML has been specified as an empty YAML node",
+		"root level configuration option specified as an empty node in YAML does not set entire config to zero value": {
+			yaml: "querier:",
+			assertConfig: func(t *testing.T, cfg *mimir.Config) {
+				defaults := mimir.Config{}
+				flagext.DefaultValues(&defaults)
+
+				require.NotZero(t, defaults.Querier.MaxQueryIntoFuture,
+					"This test asserts that mimir.Config.Querier.MaxQueryIntoFuture default value is not zero. "+
+						"If it's zero, this test is useless. Please change it to use a config value with a non-zero default.",
+				)
+
+				require.Equal(t, cfg.Querier.MaxQueryIntoFuture, defaults.Querier.MaxQueryIntoFuture,
+					"YAML parser has set the [entire] Querier config to zero values by specifying an empty node."+
+						"If this happens again, check git history on how this was checked with previous YAML parser implementation.")
+			},
 		},
 
 		"version": {

go.mod

@@ -47,7 +47,7 @@ require (
 	golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f
 	golang.org/x/time v0.0.0-20220609170525-579cf78fd858
 	google.golang.org/grpc v1.47.0
-	gopkg.in/yaml.v2 v2.4.0
+	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1
 )
 
@@ -254,3 +254,6 @@ replace github.com/vimeo/galaxycache => github.com/thanos-community/galaxycache
 
 // grpc v1.46.0 removed "WithBalancerName()" API, still in use by weaveworks/commons.
 replace google.golang.org/grpc => google.golang.org/grpc v1.45.0
+
+// gopkg.in/yaml.v3 + https://github.com/go-yaml/yaml/pull/691
+replace gopkg.in/yaml.v3 => github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22

go.sum

@@ -281,6 +281,8 @@ github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMe
 github.com/cockroachdb/cockroach-go v0.0.0-20181001143604-e0a95dfd547c/go.mod h1:XGLbWH/ujMcbPbhZq52Nv6UrCghb1yGn//133kEsvDk=
 github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
 github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI=
+github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22 h1:uVG5v+c6ndz9seCorYjQmlVlPbh3OMcMWJzAJZWdM/g=
+github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 github.com/containerd/containerd v1.2.7/go.mod h1:bC6axHOhabU15QhwfG7w5PipXdVtMXFTttgp+kVtyUA=
 github.com/coreos/etcd v3.3.25+incompatible h1:0GQEw6h3YnuOVdtwygkIfJ+Omx0tZ8/QkVyXI4LkbeY=
 github.com/coreos/etcd v3.3.25+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
@@ -2178,13 +2180,6 @@ gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
-gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
 gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=

pkg/alertmanager/api.go

@@ -22,7 +22,7 @@ import (
 	"github.com/prometheus/alertmanager/config"
 	"github.com/prometheus/alertmanager/template"
 	commoncfg "github.com/prometheus/common/config"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/dskit/tenant"
 

pkg/api/handlers_test.go

@@ -144,7 +144,7 @@ func TestConfigDiffHandler(t *testing.T) {
 			},
 			expectedStatusCode: 500,
 			expectedBody: "yaml: unmarshal errors:\n" +
-				"  line 1: cannot unmarshal !!str `x` into map[interface {}]interface {}\n",
+				"  line 1: cannot unmarshal !!str `x` into map[string]interface {}\n",
 		},
 	} {
 		defaultCfg := newDefaultDiffConfigMock()

pkg/compactor/compactor_test.go

@@ -42,7 +42,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/thanos-io/thanos/pkg/block/metadata"
 	"github.com/thanos-io/thanos/pkg/objstore"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/storage/bucket"
 	"github.com/grafana/mimir/pkg/storage/bucket/filesystem"

pkg/ingester/activeseries/custom_trackers_config.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	amlabels "github.com/prometheus/alertmanager/pkg/labels"
+	"gopkg.in/yaml.v3"
 )
 
 // CustomTrackersConfig configures active series custom trackers.
@@ -123,9 +124,9 @@ func customTrackerFlagValueToMap(s string) (map[string]string, error) {
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface.
 // CustomTrackersConfig are marshaled in yaml as a map[string]string, with matcher names as keys and strings as matchers definitions.
-func (c *CustomTrackersConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (c *CustomTrackersConfig) UnmarshalYAML(value *yaml.Node) error {
 	stringMap := map[string]string{}
-	err := unmarshal(&stringMap)
+	err := value.DecodeWithOptions(&stringMap, yaml.DecodeOptions{KnownFields: true})
 	if err != nil {
 		return err
 	}

pkg/ingester/activeseries/custom_trackers_config_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 func mustNewCustomTrackersConfigFromMap(t *testing.T, source map[string]string) CustomTrackersConfig {

pkg/ingester/instance_limits.go

@@ -9,6 +9,7 @@ import (
 	"flag"
 
 	"github.com/pkg/errors"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/util/globalerror"
 )
@@ -48,10 +49,10 @@ func (l *InstanceLimits) RegisterFlags(f *flag.FlagSet) {
 var defaultInstanceLimits *InstanceLimits = nil
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface. If give
-func (l *InstanceLimits) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (l *InstanceLimits) UnmarshalYAML(value *yaml.Node) error {
 	if defaultInstanceLimits != nil {
 		*l = *defaultInstanceLimits
 	}
 	type plain InstanceLimits // type indirection to make sure we don't go into recursive loop
-	return unmarshal((*plain)(l))
+	return value.DecodeWithOptions((*plain)(l), yaml.DecodeOptions{KnownFields: true})
 }

pkg/ingester/instance_limits_test.go

@@ -6,10 +6,11 @@
 package ingester
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 func TestInstanceLimitsUnmarshal(t *testing.T) {
@@ -25,8 +26,9 @@ func TestInstanceLimitsUnmarshal(t *testing.T) {
 max_ingestion_rate: 125.678
 max_tenants: 50000
 `
-
-	require.NoError(t, yaml.UnmarshalStrict([]byte(input), &l))
+	dec := yaml.NewDecoder(strings.NewReader(input))
+	dec.KnownFields(true)
+	require.NoError(t, dec.Decode(&l))
 	require.Equal(t, float64(125.678), l.MaxIngestionRate)
 	require.Equal(t, int64(50000), l.MaxInMemoryTenants)
 	require.Equal(t, int64(30), l.MaxInMemorySeries)       // default value

pkg/mimir/mimir.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"reflect"
 	"strconv"
 	"strings"
 
@@ -32,7 +31,7 @@ import (
 	"github.com/weaveworks/common/server"
 	"github.com/weaveworks/common/signals"
 	"google.golang.org/grpc/health/grpc_health_v1"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/dskit/tenant"
 
@@ -165,7 +164,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet, logger log.Logger) {
 	c.Common.RegisterFlags(f)
 }
 
-func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (c *Config) UnmarshalYAML(value *yaml.Node) error {
 	// First unmarshal common into the specific locations.
 	common := configWithCustomCommonUnmarshaler{
 		Common: &commonConfigUnmarshaler{
@@ -176,15 +175,15 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
 			},
 		},
 	}
-	if err := unmarshal(&common); err != nil {
+	if err := value.DecodeWithOptions(&common, yaml.DecodeOptions{KnownFields: true}); err != nil {
 		return fmt.Errorf("can't unmarshal common config: %w", err)
 	}
 
 	// Then unmarshal config in a standard way.
 	// This will override previously set common values by the specific ones, if they're provided.
 	// (YAML specific takes precedence over YAML common)
 	type plain Config
-	return unmarshal((*plain)(c))
+	return value.DecodeWithOptions((*plain)(c), yaml.DecodeOptions{KnownFields: true})
 }
 
 func (c *Config) InheritCommonFlagValues(log log.Logger, fs *flag.FlagSet) error {
@@ -232,10 +231,6 @@ func inheritFlags(log log.Logger, orig util.RegisteredFlags, dest util.Registere
 // Validate the mimir config and return an error if the validation
 // doesn't pass
 func (c *Config) Validate(log log.Logger) error {
-	if err := c.validateYAMLEmptyNodes(); err != nil {
-		return err
-	}
-
 	if err := c.validateBucketConfigs(); err != nil {
 		return fmt.Errorf("%w: %s", errInvalidBucketConfig, err)
 	}
@@ -294,33 +289,6 @@ func (c *Config) isAnyModuleEnabled(modules ...string) bool {
 	return false
 }
 
-// validateYAMLEmptyNodes ensure that no empty node has been specified in the YAML config file.
-// When an empty node is defined in YAML, the YAML parser sets the whole struct to its zero value
-// and so we loose all default values. It's very difficult to detect this case for the user, so we
-// try to prevent it (on the root level) with this custom validation.
-func (c *Config) validateYAMLEmptyNodes() error {
-	defaults := Config{}
-	flagext.DefaultValues(&defaults)
-
-	defStruct := reflect.ValueOf(defaults)
-	cfgStruct := reflect.ValueOf(*c)
-
-	// We expect all structs are the exact same. This check should never fail.
-	if cfgStruct.NumField() != defStruct.NumField() {
-		return errors.New("unable to validate configuration because of mismatching internal config data structure")
-	}
-
-	for i := 0; i < cfgStruct.NumField(); i++ {
-		// If the struct has been reset due to empty YAML value and the zero struct value
-		// doesn't match the default one, then we should warn the user about the issue.
-		if cfgStruct.Field(i).Kind() == reflect.Struct && cfgStruct.Field(i).IsZero() && !defStruct.Field(i).IsZero() {
-			return fmt.Errorf("the %s configuration in YAML has been specified as an empty YAML node", cfgStruct.Type().Field(i).Name)
-		}
-	}
-
-	return nil
-}
-
 func (c *Config) validateBucketConfigs() error {
 	errs := multierror.New()
 
@@ -428,9 +396,9 @@ type commonConfigUnmarshaler struct {
 // where this should be unmarshaled.
 type specificLocationsUnmarshaler map[string]interface{}
 
-func (m specificLocationsUnmarshaler) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (m specificLocationsUnmarshaler) UnmarshalYAML(value *yaml.Node) error {
 	for l, v := range m {
-		if err := unmarshal(v); err != nil {
+		if err := value.DecodeWithOptions(v, yaml.DecodeOptions{KnownFields: true}); err != nil {
 			return fmt.Errorf("key %q: %w", l, err)
 		}
 	}

pkg/mimir/runtime_config.go

@@ -12,7 +12,7 @@ import (
 
 	"github.com/grafana/dskit/kv"
 	"github.com/grafana/dskit/runtimeconfig"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/ingester"
 	"github.com/grafana/mimir/pkg/util"
@@ -67,7 +67,7 @@ func loadRuntimeConfig(r io.Reader) (interface{}, error) {
 	var overrides = &runtimeConfigValues{}
 
 	decoder := yaml.NewDecoder(r)
-	decoder.SetStrict(true)
+	decoder.KnownFields(true)
 
 	// Decode the first document. An empty document (EOF) is OK.
 	if err := decoder.Decode(&overrides); err != nil && !errors.Is(err, io.EOF) {

pkg/storage/bucket/azure/bucket_client.go

@@ -9,7 +9,7 @@ import (
 	"github.com/go-kit/log"
 	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/thanos-io/thanos/pkg/objstore/azure"
-	yaml "gopkg.in/yaml.v2"
+	yaml "gopkg.in/yaml.v3"
 )
 
 func NewBucketClient(cfg Config, name string, logger log.Logger) (objstore.Bucket, error) {

pkg/storage/bucket/client_test.go

@@ -17,7 +17,7 @@ import (
 	"github.com/grafana/dskit/flagext"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	yaml "gopkg.in/yaml.v2"
+	yaml "gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/storage/bucket/filesystem"
 	util_log "github.com/grafana/mimir/pkg/util/log"

pkg/storage/bucket/gcs/bucket_client.go

@@ -11,7 +11,7 @@ import (
 	"github.com/go-kit/log"
 	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/thanos-io/thanos/pkg/objstore/gcs"
-	yaml "gopkg.in/yaml.v2"
+	yaml "gopkg.in/yaml.v3"
 )
 
 // NewBucketClient creates a new GCS bucket client

pkg/storage/bucket/swift/bucket_client.go

@@ -10,7 +10,7 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/thanos-io/thanos/pkg/objstore/swift"
-	yaml "gopkg.in/yaml.v2"
+	yaml "gopkg.in/yaml.v3"
 )
 
 // NewBucketClient creates a new Swift bucket client

pkg/storegateway/indexcache/inmemory.go

@@ -20,7 +20,7 @@ import (
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/storage"
 	"github.com/thanos-io/thanos/pkg/model"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/mimir/pkg/storage/sharding"
 )