From 8ec029ae6a58fe31383f43cc7bc9ae7886db0f2c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 9 Jun 2020 17:43:05 -0400 Subject: [PATCH] Update comments that reference PatchSliceOfMaps To reference decode.HookWeakDecodeFromSlice instead. Also removes a step from the adding config fields checklist which is no longer necessary. --- agent/config/runtime_test.go | 8 ++++---- agent/xds/clusters.go | 9 ++------- agent/xds/listeners.go | 9 ++------- contributing/checklist-adding-config-fields.md | 6 ------ 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 73bb16b161..20d9e70be7 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2499,8 +2499,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, { - // This tests that we correct added the nested paths to arrays of objects - // to the exceptions in PatchSliceOfMaps in config.go (for single service) + // Test that slices in structured config are preserved by + // decode.HookWeakDecodeFromSlice. desc: "service.connectsidecar_service with checks and upstreams", args: []string{ `-data-dir=` + dataDir, @@ -2628,8 +2628,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, }, { - // This tests that we correct added the nested paths to arrays of objects - // to the exceptions in PatchSliceOfMaps in config.go (for service*s*) + // Test that slices in structured config are preserved by + // decode.HookWeakDecodeFromSlice. desc: "services.connect.sidecar_service with checks and upstreams", args: []string{ `-data-dir=` + dataDir, diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 99c15a34a6..5ff5283c02 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -523,13 +523,8 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain( // arbitrary proto3 json format string or an error if it's invalid. // // For now we only support embedding in JSON strings because of the hcl parsing -// pain (see config.go comment above call to PatchSliceOfMaps). Until we -// refactor config parser a _lot_ user's opaque config that contains arrays will -// be mangled. We could actually fix that up in mapstructure which knows the -// type of the target so could resolve the slices to singletons unambiguously -// and it would work for us here... but we still have the problem that the -// config would render incorrectly in general in our HTTP API responses so we -// really need to fix it "properly". +// pain (see Background section in the comment for decode.HookWeakDecodeFromSlice). +// This may be fixed in decode.HookWeakDecodeFromSlice in the future. // // When we do that we can support just nesting the config directly into the // JSON/hcl naturally but this is a stop-gap that gets us an escape hatch diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 3eb150c8df..d79d9093bb 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -355,13 +355,8 @@ func makeListener(name, addr string, port int) *envoy.Listener { // arbitrary proto3 json format string or an error if it's invalid. // // For now we only support embedding in JSON strings because of the hcl parsing -// pain (see config.go comment above call to PatchSliceOfMaps). Until we -// refactor config parser a _lot_ user's opaque config that contains arrays will -// be mangled. We could actually fix that up in mapstructure which knows the -// type of the target so could resolve the slices to singletons unambiguously -// and it would work for us here... but we still have the problem that the -// config would render incorrectly in general in our HTTP API responses so we -// really need to fix it "properly". +// pain (see Background section in the comment for decode.HookWeakDecodeFromSlice). +// This may be fixed in decode.HookWeakDecodeFromSlice in the future. // // When we do that we can support just nesting the config directly into the // JSON/hcl naturally but this is a stop-gap that gets us an escape hatch diff --git a/contributing/checklist-adding-config-fields.md b/contributing/checklist-adding-config-fields.md index 42b2c45a25..fd33b5a47e 100644 --- a/contributing/checklist-adding-config-fields.md +++ b/contributing/checklist-adding-config-fields.md @@ -55,12 +55,6 @@ There are four specific cases covered with increasing complexity: state for client agent's RPC client. - [ ] Add a test to `agent/agent_test.go` similar to others with prefix `TestAgent_reloadConfig*`. - - [ ] **If** the new config field(s) include an array of structs or maps. - - [ ] Add the path to the call to `lib.PatchSliceOfMaps` in Parse in - `agent/config/config.go`. - - [ ] If none of the tests in `agent/config/runtime_test.go` failed before you did that, - then you didn't actually test the slice part yet, go back and add tests - that populate that slice. - [ ] Add documentation to `website/source/docs/agent/options.html.md`. Done! You can now use your new field in a client agent by accessing