From d11f03d96fbaad8e9f1d0e6b51cac0618815229b Mon Sep 17 00:00:00 2001
From: Julian Brost <julian.brost@icinga.com>
Date: Thu, 3 Aug 2023 17:26:40 +0200
Subject: [PATCH] cmd/icingadb: warn about unknown config options instead of
 rejecting them

This makes the change from #605 more suitable for the v1.1.1 bug fix release
because this way, no new fatal errors are added while keeping the helpful
information in a warning.
---
 cmd/icingadb/main.go      |  4 ++++
 pkg/config/config.go      | 21 ++++++++++++----
 pkg/config/config_test.go | 50 +++++++++++++++++++++++----------------
 3 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/cmd/icingadb/main.go b/cmd/icingadb/main.go
index 1c22afc79..77ce577b2 100644
--- a/cmd/icingadb/main.go
+++ b/cmd/icingadb/main.go
@@ -55,6 +55,10 @@ func run() int {
 	logger := logs.GetLogger()
 	defer logger.Sync()
 
+	if warn := cmd.Config.DecodeWarning; warn != nil {
+		logger.Warnf("ignoring unknown config option, this will become a fatal error in Icinga DB v1.2:\n\n%v", warn)
+	}
+
 	logger.Info("Starting Icinga DB")
 
 	db, err := cmd.Database(logs.GetChildLogger("database"))
diff --git a/pkg/config/config.go b/pkg/config/config.go
index db6662c3e..a68301493 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -1,8 +1,10 @@
 package config
 
 import (
+	"bytes"
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"github.com/creasty/defaults"
 	"github.com/goccy/go-yaml"
 	"github.com/jessevdk/go-flags"
@@ -17,6 +19,8 @@ type Config struct {
 	Redis     Redis     `yaml:"redis"`
 	Logging   Logging   `yaml:"logging"`
 	Retention Retention `yaml:"retention"`
+
+	DecodeWarning error `yaml:"-"`
 }
 
 // Validate checks constraints in the supplied configuration and returns an error if they are violated.
@@ -47,14 +51,13 @@ type Flags struct {
 
 // FromYAMLFile returns a new Config value created from the given YAML config file.
 func FromYAMLFile(name string) (*Config, error) {
-	f, err := os.Open(name)
+	f, err := os.ReadFile(name)
 	if err != nil {
-		return nil, errors.Wrap(err, "can't open YAML file "+name)
+		return nil, errors.Wrap(err, "can't read YAML file "+name)
 	}
-	defer f.Close()
 
 	c := &Config{}
-	d := yaml.NewDecoder(f, yaml.DisallowUnknownField())
+	d := yaml.NewDecoder(bytes.NewReader(f))
 
 	if err := defaults.Set(c); err != nil {
 		return nil, errors.Wrap(err, "can't set config defaults")
@@ -64,8 +67,16 @@ func FromYAMLFile(name string) (*Config, error) {
 		return nil, errors.Wrap(err, "can't parse YAML file "+name)
 	}
 
+	// Decode again with yaml.DisallowUnknownField() (like v1.2 will do) and issue a warning if it returns an error.
+	c.DecodeWarning = yaml.NewDecoder(bytes.NewReader(f), yaml.DisallowUnknownField()).Decode(&Config{})
+
 	if err := c.Validate(); err != nil {
-		return nil, errors.Wrap(err, "invalid configuration")
+		const msg = "invalid configuration"
+		if warn := c.DecodeWarning; warn != nil {
+			return nil, fmt.Errorf("%s: %w\n\nwarning: ignored unknown config option:\n\n%v", msg, err, warn)
+		} else {
+			return nil, errors.Wrap(err, msg)
+		}
 	}
 
 	return c, nil
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 94e377355..241809429 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -20,33 +20,34 @@ redis:
   host: 2001:db8::1
 `
 
+	miniOutput := &Config{}
+	_ = defaults.Set(miniOutput)
+
+	miniOutput.Database.Host = "192.0.2.1"
+	miniOutput.Database.Database = "icingadb"
+	miniOutput.Database.User = "icingadb"
+	miniOutput.Database.Password = "icingadb"
+
+	miniOutput.Redis.Host = "2001:db8::1"
+	miniOutput.Logging.Output = logging.CONSOLE
+
 	subtests := []struct {
 		name   string
 		input  string
 		output *Config
+		warn   bool
 	}{
 		{
-			name:  "mini",
-			input: miniConf,
-			output: func() *Config {
-				c := &Config{}
-				_ = defaults.Set(c)
-
-				c.Database.Host = "192.0.2.1"
-				c.Database.Database = "icingadb"
-				c.Database.User = "icingadb"
-				c.Database.Password = "icingadb"
-
-				c.Redis.Host = "2001:db8::1"
-				c.Logging.Output = logging.CONSOLE
-
-				return c
-			}(),
+			name:   "mini",
+			input:  miniConf,
+			output: miniOutput,
+			warn:   false,
 		},
 		{
 			name:   "mini-with-unknown",
 			input:  miniConf + "\nunknown: 42",
-			output: nil,
+			output: miniOutput,
+			warn:   true,
 		},
 	}
 
@@ -58,12 +59,19 @@ redis:
 
 			require.NoError(t, os.WriteFile(tempFile.Name(), []byte(st.input), 0o600))
 
-			if actual, err := FromYAMLFile(tempFile.Name()); st.output == nil {
-				require.Error(t, err)
+			actual, err := FromYAMLFile(tempFile.Name())
+			require.NoError(t, err)
+
+			if st.warn {
+				require.Error(t, actual.DecodeWarning, "reading config should produce a warning")
+
+				// Reset the warning so that the following require.Equal() doesn't try to compare it.
+				actual.DecodeWarning = nil
 			} else {
-				require.NoError(t, err)
-				require.Equal(t, st.output, actual)
+				require.NoError(t, actual.DecodeWarning, "reading config should not produce a warning")
 			}
+
+			require.Equal(t, st.output, actual)
 		})
 	}
 }