Skip to content

Commit

Permalink
config: Use HookWeakDecodeFromSlice in place of PatchSliceOfMaps
Browse files Browse the repository at this point in the history
  • Loading branch information
dnephin committed May 29, 2020
1 parent 5d09fe5 commit 4ec6036
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 358 deletions.
72 changes: 7 additions & 65 deletions agent/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"strings"

"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/lib/decode"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/hcl"
Expand Down Expand Up @@ -49,76 +48,19 @@ func Parse(data string, format string) (c Config, keys []string, err error) {
return Config{}, nil, err
}

// We want to be able to report fields which we cannot map as an
// error so that users find typos in their configuration quickly. To
// achieve this we use the mapstructure library which maps a a raw
// map[string]interface{} to a nested structure and reports unused
// fields. The input for a mapstructure.Decode expects a
// map[string]interface{} as produced by encoding/json.
//
// The HCL language allows to repeat map keys which forces it to
// store nested structs as []map[string]interface{} instead of
// map[string]interface{}. This is an ambiguity which makes the
// generated structures incompatible with a corresponding JSON
// struct. It also does not work well with the mapstructure library.
//
// In order to still use the mapstructure library to find unused
// fields we patch instances of []map[string]interface{} to a
// map[string]interface{} before we decode that into a Config
// struct.
//
// However, Config has some fields which are either
// []map[string]interface{} or are arrays of structs which
// encoding/json will decode to []map[string]interface{}. Therefore,
// we need to be able to specify exceptions for this mapping. The
// PatchSliceOfMaps() implements that mapping. All fields of type
// []map[string]interface{} are mapped to map[string]interface{} if
// it contains at most one value. If there is more than one value it
// panics. To define exceptions one can specify the nested field
// names in dot notation.
//
// todo(fs): There might be an easier way to achieve the same thing
// todo(fs): but this approach works for now.
m := lib.PatchSliceOfMaps(raw, []string{
"checks",
"segments",
"service.checks",
"services",
"services.checks",
"watches",
"service.connect.proxy.config.upstreams", // Deprecated
"services.connect.proxy.config.upstreams", // Deprecated
"service.connect.proxy.upstreams",
"services.connect.proxy.upstreams",
"service.connect.proxy.expose.paths",
"services.connect.proxy.expose.paths",
"service.proxy.upstreams",
"services.proxy.upstreams",
"service.proxy.expose.paths",
"services.proxy.expose.paths",
"acl.tokens.managed_service_provider",

// Need all the service(s) exceptions also for nested sidecar service.
"service.connect.sidecar_service.checks",
"services.connect.sidecar_service.checks",
"service.connect.sidecar_service.proxy.upstreams",
"services.connect.sidecar_service.proxy.upstreams",
"service.connect.sidecar_service.proxy.expose.paths",
"services.connect.sidecar_service.proxy.expose.paths",
}, []string{
"config_entries.bootstrap", // completely ignore this tree (fixed elsewhere)
})

var md mapstructure.Metadata
d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: decode.HookTranslateKeys,
Metadata: &md,
Result: &c,
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice, // TODO: only apply when format is hcl
decode.HookTranslateKeys,
),
Metadata: &md,
Result: &c,
})
if err != nil {
return Config{}, nil, err
}
if err := d.Decode(m); err != nil {
if err := d.Decode(raw); err != nil {
return Config{}, nil, err
}

Expand Down
7 changes: 6 additions & 1 deletion agent/connect/ca/provider_aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/acmpca"
"github.com/hashicorp/consul/lib/decode"
"github.com/mitchellh/mapstructure"

"github.com/hashicorp/consul/agent/connect"
Expand Down Expand Up @@ -690,7 +691,11 @@ func ParseAWSCAConfig(raw map[string]interface{}) (*structs.AWSCAProviderConfig,
}

decodeConf := &mapstructure.DecoderConfig{
DecodeHook: structs.ParseDurationFunc(),
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
structs.ParseDurationFunc(),
),
Result: &config,
WeaklyTypedInput: true,
}
Expand Down
7 changes: 6 additions & 1 deletion agent/connect/ca/provider_consul_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ import (

"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib/decode"
"github.com/mitchellh/mapstructure"
)

func ParseConsulCAConfig(raw map[string]interface{}) (*structs.ConsulCAProviderConfig, error) {
config := defaultConsulCAProviderConfig()
decodeConf := &mapstructure.DecoderConfig{
DecodeHook: structs.ParseDurationFunc(),
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
structs.ParseDurationFunc(),
),
Result: &config,
WeaklyTypedInput: true,
}
Expand Down
7 changes: 6 additions & 1 deletion agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib/decode"
vaultapi "github.com/hashicorp/vault/api"
"github.com/mitchellh/mapstructure"
)
Expand Down Expand Up @@ -412,7 +413,11 @@ func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderCon
}

decodeConf := &mapstructure.DecoderConfig{
DecodeHook: structs.ParseDurationFunc(),
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
structs.ParseDurationFunc(),
),
Result: &config,
WeaklyTypedInput: true,
}
Expand Down
6 changes: 1 addition & 5 deletions agent/discovery_chain_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

cachetype "github.com/hashicorp/consul/agent/cache-types"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/lib/decode"
"github.com/mitchellh/mapstructure"
)
Expand Down Expand Up @@ -102,13 +101,10 @@ type discoveryChainReadResponse struct {
}

func decodeDiscoveryChainReadRequest(raw map[string]interface{}) (*discoveryChainReadRequest, error) {
// lib.TranslateKeys doesn't understand []map[string]interface{} so we have
// to do this part first.
raw = lib.PatchSliceOfMaps(raw, nil, nil)

var apiReq discoveryChainReadRequest
decodeConf := &mapstructure.DecoderConfig{
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
mapstructure.StringToTimeDurationHookFunc(),
),
Expand Down
58 changes: 1 addition & 57 deletions agent/structs/config_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,10 @@ func DecodeConfigEntry(raw map[string]interface{}) (ConfigEntry, error) {
return nil, fmt.Errorf("Kind value in payload is not a string")
}

skipWhenPatching, err := ConfigEntryDecodeRulesForKind(entry.GetKind())
if err != nil {
return nil, err
}

// lib.TranslateKeys doesn't understand []map[string]interface{} so we have
// to do this part first.
raw = lib.PatchSliceOfMaps(raw, skipWhenPatching, nil)

var md mapstructure.Metadata
decodeConf := &mapstructure.DecoderConfig{
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
mapstructure.StringToTimeDurationHookFunc(),
),
Expand Down Expand Up @@ -326,54 +318,6 @@ func DecodeConfigEntry(raw map[string]interface{}) (ConfigEntry, error) {
return entry, nil
}

// ConfigEntryDecodeRulesForKind returns rules for 'fixing' config entry key
// formats by kind. This is shared between the 'structs' and 'api' variations
// of config entries.
func ConfigEntryDecodeRulesForKind(kind string) (skipWhenPatching []string, err error) {
switch kind {
case ProxyDefaults:
return []string{
"expose.paths",
"Expose.Paths",
}, nil
case ServiceDefaults:
return []string{
"expose.paths",
"Expose.Paths",
}, nil
case ServiceRouter:
return []string{
"routes",
"Routes",
"routes.match.http.header",
"Routes.Match.HTTP.Header",
"routes.match.http.query_param",
"Routes.Match.HTTP.QueryParam",
}, nil
case ServiceSplitter:
return []string{
"splits",
"Splits",
}, nil
case ServiceResolver:
return nil, nil
case IngressGateway:
return []string{
"listeners",
"Listeners",
"listeners.services",
"Listeners.Services",
}, nil
case TerminatingGateway:
return []string{
"services",
"Services",
}, nil
default:
return nil, fmt.Errorf("kind %q should be explicitly handled here", kind)
}
}

type ConfigEntryOp string

const (
Expand Down
7 changes: 6 additions & 1 deletion agent/structs/connect_ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/lib/decode"
"github.com/mitchellh/mapstructure"
)

Expand Down Expand Up @@ -290,7 +291,11 @@ func (c *CAConfiguration) GetCommonConfig() (*CommonCAProviderConfig, error) {
config.CSRMaxPerSecond = 50 // See doc comment for rationale here.

decodeConf := &mapstructure.DecoderConfig{
DecodeHook: ParseDurationFunc(),
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
ParseDurationFunc(),
),
Result: &config,
WeaklyTypedInput: true,
}
Expand Down
11 changes: 9 additions & 2 deletions agent/xds/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ type GatewayConfig struct {
func ParseGatewayConfig(m map[string]interface{}) (GatewayConfig, error) {
var cfg GatewayConfig
d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: decode.HookTranslateKeys,
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
),
Result: &cfg,
WeaklyTypedInput: true,
})
Expand Down Expand Up @@ -212,7 +215,11 @@ func (p PassiveHealthCheck) AsOutlierDetection() *envoycluster.OutlierDetection
func ParseUpstreamConfigNoDefaults(m map[string]interface{}) (UpstreamConfig, error) {
var cfg UpstreamConfig
config := &mapstructure.DecoderConfig{
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
mapstructure.StringToTimeDurationHookFunc(),
),
Result: &cfg,
WeaklyTypedInput: true,
}
Expand Down
12 changes: 1 addition & 11 deletions command/config/write/config_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import (
"fmt"
"io"

"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
"github.com/hashicorp/consul/command/helpers"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/lib/decode"
"github.com/hashicorp/go-multierror"
"github.com/mitchellh/cli"
Expand Down Expand Up @@ -133,18 +131,10 @@ func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) {
return nil, fmt.Errorf("Kind value in payload is not a string")
}

skipWhenPatching, err := structs.ConfigEntryDecodeRulesForKind(entry.GetKind())
if err != nil {
return nil, err
}

// lib.TranslateKeys doesn't understand []map[string]interface{} so we have
// to do this part first.
raw = lib.PatchSliceOfMaps(raw, skipWhenPatching, nil)

var md mapstructure.Metadata
decodeConf := &mapstructure.DecoderConfig{
DecodeHook: mapstructure.ComposeDecodeHookFunc(
decode.HookWeakDecodeFromSlice,
decode.HookTranslateKeys,
mapstructure.StringToTimeDurationHookFunc(),
),
Expand Down
Loading

0 comments on commit 4ec6036

Please sign in to comment.