From 7fde1e6b59dafb6a53c7db2f865034782a19e3fb Mon Sep 17 00:00:00 2001 From: manosf Date: Mon, 2 Apr 2018 17:54:42 +0300 Subject: [PATCH 1/2] Removed yaml workaround and replaced with UnmarshalStrict Updated expected test message. Added comment. Signed-off-by: manosf --- config/config.go | 19 +++---------------- config/http_config.go | 29 ++++------------------------- config/http_config_test.go | 2 +- config/tls_config_test.go | 4 ++-- 4 files changed, 10 insertions(+), 44 deletions(-) diff --git a/config/config.go b/config/config.go index 9195c34bf..30719d838 100644 --- a/config/config.go +++ b/config/config.go @@ -11,23 +11,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package config - -import ( - "fmt" - "strings" -) +// This package no longer handles safe yaml parsing. In order to +// ensure correct yaml unmarshalling, use "yaml.UnmarshalStrict()". -func checkOverflow(m map[string]interface{}, ctx string) error { - if len(m) > 0 { - var keys []string - for k := range m { - keys = append(keys, k) - } - return fmt.Errorf("unknown fields in %s: %s", ctx, strings.Join(keys, ", ")) - } - return nil -} +package config // Secret special type for storing secrets. type Secret string diff --git a/config/http_config.go b/config/http_config.go index b404df3a3..59a12e953 100644 --- a/config/http_config.go +++ b/config/http_config.go @@ -32,9 +32,6 @@ type BasicAuth struct { Username string `yaml:"username"` Password Secret `yaml:"password,omitempty"` PasswordFile string `yaml:"password_file,omitempty"` - - // Catches all undefined fields and must be empty after parsing. - XXX map[string]interface{} `yaml:",inline"` } // URL is a custom URL type that allows validation at configuration load time. @@ -77,9 +74,6 @@ type HTTPClientConfig struct { ProxyURL URL `yaml:"proxy_url,omitempty"` // TLSConfig to use to connect to the targets. TLSConfig TLSConfig `yaml:"tls_config,omitempty"` - - // Catches all undefined fields and must be empty after parsing. - XXX map[string]interface{} `yaml:",inline"` } // Validate validates the HTTPClientConfig to check only one of BearerToken, @@ -103,25 +97,16 @@ func (c *HTTPClientConfig) Validate() error { // UnmarshalYAML implements the yaml.Unmarshaler interface func (c *HTTPClientConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { type plain HTTPClientConfig - err := unmarshal((*plain)(c)) - if err != nil { + if err := unmarshal((*plain)(c)); err != nil { return err } - err = c.Validate() - if err != nil { - return c.Validate() - } - return checkOverflow(c.XXX, "http_client_config") + return c.Validate() } // UnmarshalYAML implements the yaml.Unmarshaler interface. func (a *BasicAuth) UnmarshalYAML(unmarshal func(interface{}) error) error { type plain BasicAuth - err := unmarshal((*plain)(a)) - if err != nil { - return err - } - return checkOverflow(a.XXX, "basic_auth") + return unmarshal((*plain)(a)) } // NewClient returns a http.Client using the specified http.RoundTripper. @@ -318,18 +303,12 @@ type TLSConfig struct { ServerName string `yaml:"server_name,omitempty"` // Disable target certificate validation. InsecureSkipVerify bool `yaml:"insecure_skip_verify"` - - // Catches all undefined fields and must be empty after parsing. - XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. func (c *TLSConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { type plain TLSConfig - if err := unmarshal((*plain)(c)); err != nil { - return err - } - return checkOverflow(c.XXX, "TLS config") + return unmarshal((*plain)(c)) } func (c HTTPClientConfig) String() string { diff --git a/config/http_config_test.go b/config/http_config_test.go index 251cb30c1..bd3771854 100644 --- a/config/http_config_test.go +++ b/config/http_config_test.go @@ -542,7 +542,7 @@ func TestInvalidHTTPConfigs(t *testing.T) { // LoadHTTPConfig parses the YAML input s into a HTTPClientConfig. func LoadHTTPConfig(s string) (*HTTPClientConfig, error) { cfg := &HTTPClientConfig{} - err := yaml.Unmarshal([]byte(s), cfg) + err := yaml.UnmarshalStrict([]byte(s), cfg) if err != nil { return nil, err } diff --git a/config/tls_config_test.go b/config/tls_config_test.go index 4e5244017..b8d5d34b5 100644 --- a/config/tls_config_test.go +++ b/config/tls_config_test.go @@ -28,8 +28,8 @@ func LoadTLSConfig(filename string) (*tls.Config, error) { if err != nil { return nil, err } - cfg := TLSConfig{} - if err = yaml.Unmarshal(content, &cfg); err != nil { + cfg := &TLSConfig{} + if err = yaml.UnmarshalStrict(content, cfg); err != nil { return nil, err } return NewTLSConfig(&cfg) From 44c20fc5c7c8e21902882d33ab677ece3fe4dbb3 Mon Sep 17 00:00:00 2001 From: manosf Date: Thu, 26 Apr 2018 16:44:43 +0300 Subject: [PATCH 2/2] Minor fix. Signed-off-by: manosf --- config/tls_config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/tls_config_test.go b/config/tls_config_test.go index b8d5d34b5..31ddb6e9a 100644 --- a/config/tls_config_test.go +++ b/config/tls_config_test.go @@ -28,8 +28,8 @@ func LoadTLSConfig(filename string) (*tls.Config, error) { if err != nil { return nil, err } - cfg := &TLSConfig{} - if err = yaml.UnmarshalStrict(content, cfg); err != nil { + cfg := TLSConfig{} + if err = yaml.UnmarshalStrict(content, &cfg); err != nil { return nil, err } return NewTLSConfig(&cfg)