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

View File

@ -7,10 +7,11 @@ import (
"net/http/httptest"
"testing"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/testrpc"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/testrpc"
)
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 {
out := false
@ -95,6 +108,37 @@ func TestConfig_Get(t *testing.T) {
_, err := a.srv.Config(resp, req)
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) {

View File

@ -162,9 +162,11 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
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 c.srv.blockingQuery(
&args.QueryOptions,

View File

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

View File

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

View File

@ -529,27 +529,12 @@ func MakeConfigEntry(kind, name string) (ConfigEntry, error) {
case ServiceIntentions:
return &ServiceIntentionsConfigEntry{Name: name}, nil
case MeshConfig:
return &MeshConfigEntry{Name: name}, nil
return &MeshConfigEntry{}, nil
default:
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.
type ConfigEntryQuery struct {
Kind string

View File

@ -1,15 +1,13 @@
package structs
import (
"encoding/json"
"fmt"
"github.com/hashicorp/consul/acl"
)
type MeshConfigEntry struct {
Kind string
Name string
// TransparentProxy contains cluster-wide options pertaining to TPROXY mode
// when enabled.
TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"`
@ -36,7 +34,7 @@ func (e *MeshConfigEntry) GetName() string {
return ""
}
return e.Name
return MeshConfigMesh
}
func (e *MeshConfigEntry) GetMeta() map[string]string {
@ -51,11 +49,7 @@ func (e *MeshConfigEntry) Normalize() error {
return fmt.Errorf("config entry is nil")
}
e.Kind = MeshConfig
e.Name = MeshConfigMesh
e.EnterpriseMeta.Normalize()
return nil
}
@ -63,11 +57,6 @@ func (e *MeshConfigEntry) Validate() error {
if e == 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 {
return err
}
@ -100,3 +89,19 @@ func (e *MeshConfigEntry) GetEnterpriseMeta() *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 {
switch {
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"):
err = multierror.Append(err, fmt.Errorf("invalid config key %q, namespaces are a consul enterprise feature", k))
default:

View File

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

View File

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

View File

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

View File

@ -1,8 +1,8 @@
package api
import "encoding/json"
type MeshConfigEntry struct {
Kind string
Name string
Namespace string `json:",omitempty"`
TransparentProxy TransparentProxyMeshConfig `alias:"transparent_proxy"`
Meta map[string]string `json:",omitempty"`
@ -15,11 +15,11 @@ type TransparentProxyMeshConfig struct {
}
func (e *MeshConfigEntry) GetKind() string {
return e.Kind
return MeshConfig
}
func (e *MeshConfigEntry) GetName() string {
return e.Name
return MeshConfigMesh
}
func (e *MeshConfigEntry) GetNamespace() string {
@ -37,3 +37,17 @@ func (e *MeshConfigEntry) GetCreateIndex() uint64 {
func (e *MeshConfigEntry) GetModifyIndex() uint64 {
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)
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) {
@ -1141,7 +1199,6 @@ func TestDecodeConfigEntry(t *testing.T) {
body: `
{
"Kind": "mesh",
"Name": "mesh",
"Meta" : {
"foo": "bar",
"gir": "zim"
@ -1152,8 +1209,6 @@ func TestDecodeConfigEntry(t *testing.T) {
}
`,
expect: &MeshConfigEntry{
Kind: "mesh",
Name: "mesh",
Meta: map[string]string{
"foo": "bar",
"gir": "zim",

View File

@ -6,13 +6,14 @@ import (
"io"
"time"
"github.com/hashicorp/go-multierror"
"github.com/mitchellh/cli"
"github.com/mitchellh/mapstructure"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
"github.com/hashicorp/consul/command/helpers"
"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 {
@ -155,6 +156,12 @@ func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) {
}
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))
}
if err != nil {

View File

@ -1,6 +1,7 @@
package write
import (
"bytes"
"io"
"strings"
"testing"
@ -113,6 +114,36 @@ func TestConfigWrite(t *testing.T) {
require.NotEqual(t, 0, code)
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
@ -2627,7 +2658,6 @@ func TestParseConfigEntry(t *testing.T) {
name: "mesh",
snake: `
kind = "mesh"
name = "mesh"
meta {
"foo" = "bar"
"gir" = "zim"
@ -2638,7 +2668,6 @@ func TestParseConfigEntry(t *testing.T) {
`,
camel: `
Kind = "mesh"
Name = "mesh"
Meta {
"foo" = "bar"
"gir" = "zim"
@ -2650,7 +2679,6 @@ func TestParseConfigEntry(t *testing.T) {
snakeJSON: `
{
"kind": "mesh",
"name": "mesh",
"meta" : {
"foo": "bar",
"gir": "zim"
@ -2663,7 +2691,6 @@ func TestParseConfigEntry(t *testing.T) {
camelJSON: `
{
"Kind": "mesh",
"Name": "mesh",
"Meta" : {
"foo": "bar",
"gir": "zim"
@ -2674,8 +2701,6 @@ func TestParseConfigEntry(t *testing.T) {
}
`,
expect: &api.MeshConfigEntry{
Kind: api.MeshConfig,
Name: api.MeshConfigMesh,
Meta: map[string]string{
"foo": "bar",
"gir": "zim",