Empty map deserialized as string

[ezracelli]

I think this is the same issue as traefik/traefik#8772, which was unfortunately closed without resolution.

The following traefik dynamic configuration object:

http:
  // -- snip --
  middlewares:
    testPluginMiddleware:
      plugin:
        testPlugin:
          variantA: {}

Is deserialized as (copied from traefik's --log.level=DEBUG logs):

DEBU[XXXX-XX-XXTXX:XX:XX-XX:XX] Configuration received: {"http":{"routers":"/* -- snip -- */","services":"/* -- snip -- */","middlewares":{"testPluginMiddleware":{"plugin":{"testPlugin":{"variantA":""}}}}},"tcp":{},"udp":{},"tls":{}}  providerName=file

• variantA sub-option is "" (empty string) instead of {} (empty map) in the JSON serialization
• This error happens regardless of the type of config file used (JSON, TOML, and YAML are all affected)

paerser_test.go

package traefik_plugin_thumbor_test

import (
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/traefik/paerser/file"
	"github.com/traefik/traefik/v2/pkg/config/dynamic"
	"github.com/traefik/traefik/v2/pkg/tls"
)

func newConfiguration() *dynamic.Configuration {
	return &dynamic.Configuration{
		HTTP: &dynamic.HTTPConfiguration{
			Routers:           make(map[string]*dynamic.Router),
			Middlewares:       make(map[string]*dynamic.Middleware),
			Services:          make(map[string]*dynamic.Service),
			ServersTransports: make(map[string]*dynamic.ServersTransport),
		},
		TCP: &dynamic.TCPConfiguration{
			Routers:     make(map[string]*dynamic.TCPRouter),
			Services:    make(map[string]*dynamic.TCPService),
			Middlewares: make(map[string]*dynamic.TCPMiddleware),
		},
		TLS: &dynamic.TLSConfiguration{
			Stores:  make(map[string]tls.Store),
			Options: make(map[string]tls.Options),
		},
		UDP: &dynamic.UDPConfiguration{
			Routers:  make(map[string]*dynamic.UDPRouter),
			Services: make(map[string]*dynamic.UDPService),
		},
	}
}

const yamlContent = `http:
  middlewares:
    testPluginMiddleware:
      plugin:
        testPlugin:
          variantA: {}
`

func TestYaml(t *testing.T) {
	configuration := newConfiguration()

	err := file.DecodeContent(yamlContent, ".yaml", configuration)
	if err != nil {
		t.Fatal(err)
	}

	assertEmptyMap(t, configuration)
}

func assertEmptyMap(t *testing.T, configuration *dynamic.Configuration) {
	t.Helper()

	value := configuration.HTTP.Middlewares["testPluginMiddleware"].Plugin["testPlugin"]["variantA"]
	assert.IsType(t, "", value) // PASS
	assert.IsType(t, make(map[string]interface{}), value) // FAIL
}

Context

I'm writing a traefik plugin which accepts one of a few possible configuration variants, which I was planning on implementing in the "traefik way" by using the pattern of one of many map fields.

For example, traefik dynamic config accepts:

http:
  middlewares:
    MIDDLEWARE_NAME:
      MIDDLEWARE_VARIANT:
        MIDDLEWARE_VARIANT_OPTION: MIDDLEWARE_VARIANT_OPTION_VALUE

...where "variant" might be addPrefix (the type of the middleware); exactly one variant is accepted.

In the same vein, I was structuring my plugin's config to be something like:

http:
  middlewares:
    MIDDLEWARE_NAME:
      plugin:
        PLUGIN_NAME:
          PLUGIN_VARIANT:
            PLUGIN_VARIANT_OPTION: PLUGIN_VARIANT_OPTION_VALUE

...where "variant" is one of the few possibilities mentioned above; exactly one variant is accepted. However, I ran into this issue because one of my "variants" doesn't have any sub-options, but the key is still required to be present. I believe this does not affect traefik because all its "variants" have sub-options.

plugin.go

package plugin

import (
	"context"
	"http"
)

type Config struct {
	VariantA *ConfigVariantA `json:"variantA,omitempty" toml:"variantA,omitempty" yaml:"variantA,omitempty"`
	VariantB *ConfigVariantB `json:"variantB,omitempty" toml:"variantB,omitempty" yaml:"variantB,omitempty"`
}

type ConfigVariantA struct {}

type ConfigVariantB struct {
 	Option int `json:"option,omitempty" toml:"option,omitempty" yaml:"option,omitempty"`
}

// -- SNIP --

With the following dynamic config (same as above):

http:
  // -- snip --
  middlewares:
    testPluginMiddleware:
      plugin:
        testPlugin:
          variantA: {}

...I get the following error message from traefik:

ERRO[XXXX-XX-XXTXX:XX:XX-XX:XX] plugin: failed to decode configuration: 1 error(s) decoding:

* 'VariantA' expected a map, got 'string'  entryPointName=web routerName=router@file

[ldez]

Hello,

I confirm the bug, as a workaround, you have to define explicitly the default value of at least one of the fields.

Paerser handles different sources of data (mainly labels, but also files and env vars), and this diversity of sources induces complexity of the side effects.

[ezracelli]

No doubt parsing is a complex task! Would you accept a PR to fix this specifically for the file parser?

[ezracelli]

Ahh, after testing a bit further, I think I see why this is an issue specifically for plugins.

Emtpy maps are only allowed via the "emptyMap" label, which is fine for traefik's typed config. For example: https://github.com/traefik/traefik/blob/d97d3a6726ad6e50110dd2ff2b552a8b17eeed55/pkg/config/dynamic/http_config.go#L50

type Router struct {
        // -- snip --
	TLS         *RouterTLSConfig `json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" kv:"allowEmpty" export:"true"`
}

But since a plugin's config is deserialized as a PluginConf (an untyped map[string]interface{}) (https://github.com/traefik/traefik/blob/v2.9.6/pkg/config/dynamic/plugins.go#L8), paerser don't have any "hints" as to what type it's supposed to be, and for empty maps falls back to an empty string.

type Configuration struct {
	Key map[string]interface{} `json:"key" toml:"key" yaml:"key"`
}

const yamlContent = `key:
  emptyMap: {}
`

func TestYaml(t *testing.T) {
	configuration := &Configuration{}

	err := file.DecodeContent(yamlContent, ".yaml", configuration)
	if err != nil {
		t.Fatal(err)
	}

	assertEmptyMap(t, configuration)
}

func assertEmptyMap(t *testing.T, configuration *Configuration) {
	t.Helper()

	value := configuration.Key["emptyMap"]
	assert.IsType(t, "", value) // PASS
	assert.IsType(t, make(map[string]interface{}), value) // FAIL
}

So, I think this only affects deserialization of map[string]interface{} types.

[nmengin]

Hello @ezracelli,

No doubt parsing is a complex task! Would you accept a PR to fix this specifically for the file parser?

This feature would not make it to our roadmap as we are focused elsewhere. If you or a community member would like to build it, let us know, and we will work with you to ensure you have all the information needed to merge it.

We prefer to work with our community members at the beginning of the design process to ensure that we are aligned and can move quickly with the review and merge process. Let us know here or create a PR before you start, and we will work with you there.

Don’t forget to check out the contributor docs and link the PR to this issue.