Merge pull request #10155 from hashicorp/dnephin/config-entry-remove-fields

config-entry: remove Kind and Name field from Mesh config entry
This commit is contained in:
Daniel Nephin 2021-05-04 17:27:56 -04:00 committed by GitHub
commit 55f620d636
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 190 additions and 68 deletions

View File

@ -4173,7 +4173,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
"bootstrap": [ "bootstrap": [
{ {
"kind": "mesh", "kind": "mesh",
"name": "mesh",
"meta" : { "meta" : {
"foo": "bar", "foo": "bar",
"gir": "zim" "gir": "zim"
@ -4190,7 +4189,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
config_entries { config_entries {
bootstrap { bootstrap {
kind = "mesh" kind = "mesh"
name = "mesh"
meta { meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -4206,8 +4204,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.DataDir = dataDir rt.DataDir = dataDir
rt.ConfigEntryBootstrap = []structs.ConfigEntry{ rt.ConfigEntryBootstrap = []structs.ConfigEntry{
&structs.MeshConfigEntry{ &structs.MeshConfigEntry{
Kind: structs.MeshConfig,
Name: structs.MeshConfigMesh,
Meta: map[string]string{ Meta: map[string]string{
"foo": "bar", "foo": "bar",
"gir": "zim", "gir": "zim",
@ -4228,7 +4224,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
"bootstrap": [ "bootstrap": [
{ {
"Kind": "mesh", "Kind": "mesh",
"Name": "mesh",
"Meta" : { "Meta" : {
"foo": "bar", "foo": "bar",
"gir": "zim" "gir": "zim"
@ -4245,7 +4240,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
config_entries { config_entries {
bootstrap { bootstrap {
Kind = "mesh" Kind = "mesh"
Name = "mesh"
Meta { Meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -4261,8 +4255,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.DataDir = dataDir rt.DataDir = dataDir
rt.ConfigEntryBootstrap = []structs.ConfigEntry{ rt.ConfigEntryBootstrap = []structs.ConfigEntry{
&structs.MeshConfigEntry{ &structs.MeshConfigEntry{
Kind: structs.MeshConfig,
Name: structs.MeshConfigMesh,
Meta: map[string]string{ Meta: map[string]string{
"foo": "bar", "foo": "bar",
"gir": "zim", "gir": "zim",

View File

@ -7,10 +7,11 @@ import (
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/testrpc"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/testrpc"
) )
func TestConfig_Get(t *testing.T) { func TestConfig_Get(t *testing.T) {
@ -48,6 +49,18 @@ func TestConfig_Get(t *testing.T) {
}, },
}, },
}, },
{
Datacenter: "dc1",
Entry: &structs.MeshConfigEntry{
TransparentProxy: structs.TransparentProxyMeshConfig{
CatalogDestinationsOnly: true,
},
Meta: map[string]string{
"key1": "value1",
"key2": "value2",
},
},
},
} }
for _, req := range reqs { for _, req := range reqs {
out := false out := false
@ -95,6 +108,37 @@ func TestConfig_Get(t *testing.T) {
_, err := a.srv.Config(resp, req) _, err := a.srv.Config(resp, req)
require.Error(t, errors.New("Must provide either a kind or both kind and name"), err) require.Error(t, errors.New("Must provide either a kind or both kind and name"), err)
}) })
t.Run("get the single mesh config", func(t *testing.T) {
req, _ := http.NewRequest("GET", "/v1/config/mesh/mesh", nil)
resp := httptest.NewRecorder()
obj, err := a.srv.Config(resp, req)
require.NoError(t, err)
ce, ok := obj.(*structs.MeshConfigEntry)
require.True(t, ok, "wrong type %T", obj)
// Set indexes to expected values for assertions
ce.CreateIndex = 12
ce.ModifyIndex = 13
out, err := a.srv.marshalJSON(req, obj)
require.NoError(t, err)
expected := `
{
"Kind": "mesh",
"TransparentProxy": {
"CatalogDestinationsOnly": true
},
"Meta":{
"key1": "value1",
"key2": "value2"
},
"CreateIndex": 12,
"ModifyIndex": 13
}
`
require.JSONEq(t, expected, string(out))
})
} }
func TestConfig_Delete(t *testing.T) { func TestConfig_Delete(t *testing.T) {

View File

@ -162,9 +162,11 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
return err return err
} }
if args.Kind != "" && !structs.ValidateConfigEntryKind(args.Kind) { if args.Kind != "" {
if _, err := structs.MakeConfigEntry(args.Kind, ""); err != nil {
return fmt.Errorf("invalid config entry kind: %s", args.Kind) return fmt.Errorf("invalid config entry kind: %s", args.Kind)
} }
}
return c.srv.blockingQuery( return c.srv.blockingQuery(
&args.QueryOptions, &args.QueryOptions,

View File

@ -428,8 +428,6 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
// mesh config entry // mesh config entry
meshConfig := &structs.MeshConfigEntry{ meshConfig := &structs.MeshConfigEntry{
Kind: structs.MeshConfig,
Name: structs.MeshConfigMesh,
TransparentProxy: structs.TransparentProxyMeshConfig{ TransparentProxy: structs.TransparentProxyMeshConfig{
CatalogDestinationsOnly: true, CatalogDestinationsOnly: true,
}, },

View File

@ -1674,8 +1674,6 @@ func TestState_WatchesAndUpdates(t *testing.T) {
CorrelationID: meshConfigEntryID, CorrelationID: meshConfigEntryID,
Result: &structs.ConfigEntryResponse{ Result: &structs.ConfigEntryResponse{
Entry: &structs.MeshConfigEntry{ Entry: &structs.MeshConfigEntry{
Kind: structs.MeshConfig,
Name: structs.MeshConfigMesh,
TransparentProxy: structs.TransparentProxyMeshConfig{}, TransparentProxy: structs.TransparentProxyMeshConfig{},
}, },
}, },

View File

@ -529,27 +529,12 @@ func MakeConfigEntry(kind, name string) (ConfigEntry, error) {
case ServiceIntentions: case ServiceIntentions:
return &ServiceIntentionsConfigEntry{Name: name}, nil return &ServiceIntentionsConfigEntry{Name: name}, nil
case MeshConfig: case MeshConfig:
return &MeshConfigEntry{Name: name}, nil return &MeshConfigEntry{}, nil
default: default:
return nil, fmt.Errorf("invalid config entry kind: %s", kind) return nil, fmt.Errorf("invalid config entry kind: %s", kind)
} }
} }
func ValidateConfigEntryKind(kind string) bool {
switch kind {
case ServiceDefaults, ProxyDefaults:
return true
case ServiceRouter, ServiceSplitter, ServiceResolver:
return true
case IngressGateway, TerminatingGateway:
return true
case ServiceIntentions:
return true
default:
return false
}
}
// ConfigEntryQuery is used when requesting info about a config entry. // ConfigEntryQuery is used when requesting info about a config entry.
type ConfigEntryQuery struct { type ConfigEntryQuery struct {
Kind string Kind string

View File

@ -1,15 +1,13 @@
package structs package structs
import ( import (
"encoding/json"
"fmt" "fmt"
"github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl"
) )
type MeshConfigEntry struct { type MeshConfigEntry struct {
Kind string
Name string
// TransparentProxy contains cluster-wide options pertaining to TPROXY mode // TransparentProxy contains cluster-wide options pertaining to TPROXY mode
// when enabled. // when enabled.
TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"` TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"`
@ -36,7 +34,7 @@ func (e *MeshConfigEntry) GetName() string {
return "" return ""
} }
return e.Name return MeshConfigMesh
} }
func (e *MeshConfigEntry) GetMeta() map[string]string { func (e *MeshConfigEntry) GetMeta() map[string]string {
@ -51,11 +49,7 @@ func (e *MeshConfigEntry) Normalize() error {
return fmt.Errorf("config entry is nil") return fmt.Errorf("config entry is nil")
} }
e.Kind = MeshConfig
e.Name = MeshConfigMesh
e.EnterpriseMeta.Normalize() e.EnterpriseMeta.Normalize()
return nil return nil
} }
@ -63,11 +57,6 @@ func (e *MeshConfigEntry) Validate() error {
if e == nil { if e == nil {
return fmt.Errorf("config entry is nil") return fmt.Errorf("config entry is nil")
} }
if e.Name != MeshConfigMesh {
return fmt.Errorf("invalid name (%q), only %q is supported", e.Name, MeshConfigMesh)
}
if err := validateConfigEntryMeta(e.Meta); err != nil { if err := validateConfigEntryMeta(e.Meta); err != nil {
return err return err
} }
@ -100,3 +89,19 @@ func (e *MeshConfigEntry) GetEnterpriseMeta() *EnterpriseMeta {
return &e.EnterpriseMeta return &e.EnterpriseMeta
} }
// MarshalJSON adds the Kind field so that the JSON can be decoded back into the
// correct type.
// This method is implemented on the structs type (as apposed to the api type)
// because that is what the API currently uses to return a response.
func (e *MeshConfigEntry) MarshalJSON() ([]byte, error) {
type Alias MeshConfigEntry
source := &struct {
Kind string
*Alias
}{
Kind: MeshConfig,
Alias: (*Alias)(e),
}
return json.Marshal(source)
}

View File

@ -19,6 +19,9 @@ func validateUnusedKeys(unused []string) error {
for _, k := range unused { for _, k := range unused {
switch { switch {
case k == "CreateIndex" || k == "ModifyIndex": case k == "CreateIndex" || k == "ModifyIndex":
case k == "kind" || k == "Kind":
// The kind field is used to determine the target, but doesn't need
// to exist on the target.
case strings.HasSuffix(strings.ToLower(k), "namespace"): case strings.HasSuffix(strings.ToLower(k), "namespace"):
err = multierror.Append(err, fmt.Errorf("invalid config key %q, namespaces are a consul enterprise feature", k)) err = multierror.Append(err, fmt.Errorf("invalid config key %q, namespaces are a consul enterprise feature", k))
default: default:

View File

@ -1310,7 +1310,6 @@ func TestDecodeConfigEntry(t *testing.T) {
name: "mesh", name: "mesh",
snake: ` snake: `
kind = "mesh" kind = "mesh"
name = "mesh"
meta { meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -1321,7 +1320,6 @@ func TestDecodeConfigEntry(t *testing.T) {
`, `,
camel: ` camel: `
Kind = "mesh" Kind = "mesh"
Name = "mesh"
Meta { Meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -1331,8 +1329,6 @@ func TestDecodeConfigEntry(t *testing.T) {
} }
`, `,
expect: &MeshConfigEntry{ expect: &MeshConfigEntry{
Kind: MeshConfig,
Name: MeshConfigMesh,
Meta: map[string]string{ Meta: map[string]string{
"foo": "bar", "foo": "bar",
"gir": "zim", "gir": "zim",

View File

@ -550,8 +550,6 @@ func TestListenersFromSnapshot(t *testing.T) {
snap.ConnectProxy.MeshConfigSet = true snap.ConnectProxy.MeshConfigSet = true
snap.ConnectProxy.MeshConfig = &structs.MeshConfigEntry{ snap.ConnectProxy.MeshConfig = &structs.MeshConfigEntry{
Kind: structs.MeshConfig,
Name: structs.MeshConfigMesh,
TransparentProxy: structs.TransparentProxyMeshConfig{ TransparentProxy: structs.TransparentProxyMeshConfig{
CatalogDestinationsOnly: true, CatalogDestinationsOnly: true,
}, },

View File

@ -295,7 +295,7 @@ func makeConfigEntry(kind, name string) (ConfigEntry, error) {
case ServiceIntentions: case ServiceIntentions:
return &ServiceIntentionsConfigEntry{Kind: kind, Name: name}, nil return &ServiceIntentionsConfigEntry{Kind: kind, Name: name}, nil
case MeshConfig: case MeshConfig:
return &MeshConfigEntry{Kind: kind, Name: name}, nil return &MeshConfigEntry{}, nil
default: default:
return nil, fmt.Errorf("invalid config entry kind: %s", kind) return nil, fmt.Errorf("invalid config entry kind: %s", kind)
} }

View File

@ -1,8 +1,8 @@
package api package api
import "encoding/json"
type MeshConfigEntry struct { type MeshConfigEntry struct {
Kind string
Name string
Namespace string `json:",omitempty"` Namespace string `json:",omitempty"`
TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"` TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"`
Meta map[string]string `json:",omitempty"` Meta map[string]string `json:",omitempty"`
@ -15,11 +15,11 @@ type TransparentProxyMeshConfig struct {
} }
func (e *MeshConfigEntry) GetKind() string { func (e *MeshConfigEntry) GetKind() string {
return e.Kind return MeshConfig
} }
func (e *MeshConfigEntry) GetName() string { func (e *MeshConfigEntry) GetName() string {
return e.Name return MeshConfigMesh
} }
func (e *MeshConfigEntry) GetNamespace() string { func (e *MeshConfigEntry) GetNamespace() string {
@ -37,3 +37,17 @@ func (e *MeshConfigEntry) GetCreateIndex() uint64 {
func (e *MeshConfigEntry) GetModifyIndex() uint64 { func (e *MeshConfigEntry) GetModifyIndex() uint64 {
return e.ModifyIndex return e.ModifyIndex
} }
// MarshalJSON adds the Kind field so that the JSON can be decoded back into the
// correct type.
func (e *MeshConfigEntry) MarshalJSON() ([]byte, error) {
type Alias MeshConfigEntry
source := &struct {
Kind string
*Alias
}{
Kind: MeshConfig,
Alias: (*Alias)(e),
}
return json.Marshal(source)
}

View File

@ -196,6 +196,64 @@ func TestAPI_ConfigEntries(t *testing.T) {
_, _, err = config_entries.Get(ServiceDefaults, "foo", nil) _, _, err = config_entries.Get(ServiceDefaults, "foo", nil)
require.Error(t, err) require.Error(t, err)
}) })
t.Run("Mesh", func(t *testing.T) {
mesh := &MeshConfigEntry{
TransparentProxy: TransparentProxyMeshConfig{CatalogDestinationsOnly: true},
Meta: map[string]string{
"foo": "bar",
"gir": "zim",
},
}
ce := c.ConfigEntries()
runStep(t, "set and get", func(t *testing.T) {
_, wm, err := ce.Set(mesh, nil)
require.NoError(t, err)
require.NotNil(t, wm)
require.NotEqual(t, 0, wm.RequestTime)
entry, qm, err := ce.Get(MeshConfig, MeshConfigMesh, nil)
require.NoError(t, err)
require.NotNil(t, qm)
require.NotEqual(t, 0, qm.RequestTime)
result, ok := entry.(*MeshConfigEntry)
require.True(t, ok)
// ignore indexes
result.CreateIndex = 0
result.ModifyIndex = 0
require.Equal(t, mesh, result)
})
runStep(t, "list", func(t *testing.T) {
entries, qm, err := ce.List(MeshConfig, nil)
require.NoError(t, err)
require.NotNil(t, qm)
require.NotEqual(t, 0, qm.RequestTime)
require.Len(t, entries, 1)
})
runStep(t, "delete", func(t *testing.T) {
wm, err := ce.Delete(MeshConfig, MeshConfigMesh, nil)
require.NoError(t, err)
require.NotNil(t, wm)
require.NotEqual(t, 0, wm.RequestTime)
// verify deletion
_, _, err = ce.Get(MeshConfig, MeshConfigMesh, nil)
require.Error(t, err)
})
})
}
func runStep(t *testing.T, name string, fn func(t *testing.T)) {
t.Helper()
if !t.Run(name, fn) {
t.FailNow()
}
} }
func TestDecodeConfigEntry(t *testing.T) { func TestDecodeConfigEntry(t *testing.T) {
@ -1141,7 +1199,6 @@ func TestDecodeConfigEntry(t *testing.T) {
body: ` body: `
{ {
"Kind": "mesh", "Kind": "mesh",
"Name": "mesh",
"Meta" : { "Meta" : {
"foo": "bar", "foo": "bar",
"gir": "zim" "gir": "zim"
@ -1152,8 +1209,6 @@ func TestDecodeConfigEntry(t *testing.T) {
} }
`, `,
expect: &MeshConfigEntry{ expect: &MeshConfigEntry{
Kind: "mesh",
Name: "mesh",
Meta: map[string]string{ Meta: map[string]string{
"foo": "bar", "foo": "bar",
"gir": "zim", "gir": "zim",

View File

@ -6,13 +6,14 @@ import (
"io" "io"
"time" "time"
"github.com/hashicorp/go-multierror"
"github.com/mitchellh/cli"
"github.com/mitchellh/mapstructure"
"github.com/hashicorp/consul/api" "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags" "github.com/hashicorp/consul/command/flags"
"github.com/hashicorp/consul/command/helpers" "github.com/hashicorp/consul/command/helpers"
"github.com/hashicorp/consul/lib/decode" "github.com/hashicorp/consul/lib/decode"
"github.com/hashicorp/go-multierror"
"github.com/mitchellh/cli"
"github.com/mitchellh/mapstructure"
) )
func New(ui cli.Ui) *cmd { func New(ui cli.Ui) *cmd {
@ -155,6 +156,12 @@ func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) {
} }
for _, k := range md.Unused { for _, k := range md.Unused {
switch k {
case "kind", "Kind":
// The kind field is used to determine the target, but doesn't need
// to exist on the target.
continue
}
err = multierror.Append(err, fmt.Errorf("invalid config key %q", k)) err = multierror.Append(err, fmt.Errorf("invalid config key %q", k))
} }
if err != nil { if err != nil {

View File

@ -1,6 +1,7 @@
package write package write
import ( import (
"bytes"
"io" "io"
"strings" "strings"
"testing" "testing"
@ -113,6 +114,36 @@ func TestConfigWrite(t *testing.T) {
require.NotEqual(t, 0, code) require.NotEqual(t, 0, code)
require.NotEmpty(t, ui.ErrorWriter.String()) require.NotEmpty(t, ui.ErrorWriter.String())
}) })
t.Run("mesh config entry", func(t *testing.T) {
stdin := new(bytes.Buffer)
stdin.WriteString(`
kind = "mesh"
meta {
"foo" = "bar"
"gir" = "zim"
}
transparent_proxy {
catalog_destinations_only = true
}
`)
ui := cli.NewMockUi()
c := New(ui)
c.testStdin = stdin
code := c.Run([]string{"-http-addr=" + a.HTTPAddr(), "-"})
require.Empty(t, ui.ErrorWriter.String())
require.Contains(t, ui.OutputWriter.String(),
`Config entry written: mesh/mesh`)
require.Equal(t, 0, code)
entry, _, err := client.ConfigEntries().Get(api.MeshConfig, api.MeshConfigMesh, nil)
require.NoError(t, err)
proxy, ok := entry.(*api.MeshConfigEntry)
require.True(t, ok)
require.Equal(t, map[string]string{"foo": "bar", "gir": "zim"}, proxy.Meta)
})
} }
// TestParseConfigEntry is the 'api' mirror image of // TestParseConfigEntry is the 'api' mirror image of
@ -2627,7 +2658,6 @@ func TestParseConfigEntry(t *testing.T) {
name: "mesh", name: "mesh",
snake: ` snake: `
kind = "mesh" kind = "mesh"
name = "mesh"
meta { meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -2638,7 +2668,6 @@ func TestParseConfigEntry(t *testing.T) {
`, `,
camel: ` camel: `
Kind = "mesh" Kind = "mesh"
Name = "mesh"
Meta { Meta {
"foo" = "bar" "foo" = "bar"
"gir" = "zim" "gir" = "zim"
@ -2650,7 +2679,6 @@ func TestParseConfigEntry(t *testing.T) {
snakeJSON: ` snakeJSON: `
{ {
"kind": "mesh", "kind": "mesh",
"name": "mesh",
"meta" : { "meta" : {
"foo": "bar", "foo": "bar",
"gir": "zim" "gir": "zim"
@ -2663,7 +2691,6 @@ func TestParseConfigEntry(t *testing.T) {
camelJSON: ` camelJSON: `
{ {
"Kind": "mesh", "Kind": "mesh",
"Name": "mesh",
"Meta" : { "Meta" : {
"foo": "bar", "foo": "bar",
"gir": "zim" "gir": "zim"
@ -2674,8 +2701,6 @@ func TestParseConfigEntry(t *testing.T) {
} }
`, `,
expect: &api.MeshConfigEntry{ expect: &api.MeshConfigEntry{
Kind: api.MeshConfig,
Name: api.MeshConfigMesh,
Meta: map[string]string{ Meta: map[string]string{
"foo": "bar", "foo": "bar",
"gir": "zim", "gir": "zim",