From c1d1be2a4babe8176a6b48809a12f11991baa299 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 4 May 2021 17:27:56 -0400 Subject: [PATCH] Merge pull request #10155 from hashicorp/dnephin/config-entry-remove-fields config-entry: remove Kind and Name field from Mesh config entry --- agent/config/runtime_test.go | 8 --- agent/config_endpoint_test.go | 48 +++++++++++++++++- agent/consul/config_endpoint.go | 6 ++- agent/consul/fsm/snapshot_oss_test.go | 2 - agent/proxycfg/state_test.go | 2 - agent/structs/config_entry.go | 17 +------ agent/structs/config_entry_mesh.go | 31 +++++++----- agent/structs/config_entry_oss.go | 3 ++ agent/structs/config_entry_test.go | 4 -- agent/xds/listeners_test.go | 2 - api/config_entry.go | 2 +- api/config_entry_cluster.go | 22 ++++++-- api/config_entry_test.go | 61 +++++++++++++++++++++-- command/config/write/config_write.go | 13 +++-- command/config/write/config_write_test.go | 37 +++++++++++--- 15 files changed, 190 insertions(+), 68 deletions(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index dffcc4be96..85db6d3be9 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -4157,7 +4157,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { "bootstrap": [ { "kind": "mesh", - "name": "mesh", "meta" : { "foo": "bar", "gir": "zim" @@ -4174,7 +4173,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { config_entries { bootstrap { kind = "mesh" - name = "mesh" meta { "foo" = "bar" "gir" = "zim" @@ -4190,8 +4188,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", @@ -4212,7 +4208,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { "bootstrap": [ { "Kind": "mesh", - "Name": "mesh", "Meta" : { "foo": "bar", "gir": "zim" @@ -4229,7 +4224,6 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { config_entries { bootstrap { Kind = "mesh" - Name = "mesh" Meta { "foo" = "bar" "gir" = "zim" @@ -4245,8 +4239,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", diff --git a/agent/config_endpoint_test.go b/agent/config_endpoint_test.go index ec18747e0d..13ff251d85 100644 --- a/agent/config_endpoint_test.go +++ b/agent/config_endpoint_test.go @@ -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) { diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 2061e76afa..4825a78b3a 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -162,8 +162,10 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe return err } - if args.Kind != "" && !structs.ValidateConfigEntryKind(args.Kind) { - return fmt.Errorf("invalid config entry kind: %s", 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( diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go index 99de8ebc9a..cb05399870 100644 --- a/agent/consul/fsm/snapshot_oss_test.go +++ b/agent/consul/fsm/snapshot_oss_test.go @@ -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, }, diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index f22558330b..021b4465d2 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -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{}, }, }, diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index afa5f34fc1..3f7145ee2b 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -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 diff --git a/agent/structs/config_entry_mesh.go b/agent/structs/config_entry_mesh.go index 8e7688a06c..33792b381b 100644 --- a/agent/structs/config_entry_mesh.go +++ b/agent/structs/config_entry_mesh.go @@ -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) +} diff --git a/agent/structs/config_entry_oss.go b/agent/structs/config_entry_oss.go index a3ca07c1b8..9773643dde 100644 --- a/agent/structs/config_entry_oss.go +++ b/agent/structs/config_entry_oss.go @@ -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: diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 38c10deb0b..cace510bb4 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -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", diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index 2cf9a67838..bf1d0b6cb7 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -540,8 +540,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, }, diff --git a/api/config_entry.go b/api/config_entry.go index 6ae23be898..5047d07a5b 100644 --- a/api/config_entry.go +++ b/api/config_entry.go @@ -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) } diff --git a/api/config_entry_cluster.go b/api/config_entry_cluster.go index 057059f8aa..48fcc44fcf 100644 --- a/api/config_entry_cluster.go +++ b/api/config_entry_cluster.go @@ -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) +} diff --git a/api/config_entry_test.go b/api/config_entry_test.go index 4bf5e23681..85ee5c4364 100644 --- a/api/config_entry_test.go +++ b/api/config_entry_test.go @@ -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", diff --git a/command/config/write/config_write.go b/command/config/write/config_write.go index e150b40af9..c7b3a72fd4 100644 --- a/command/config/write/config_write.go +++ b/command/config/write/config_write.go @@ -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 { diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index ee122f2b38..20f20af43e 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -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",