From 408ed18246bb845406f4767ff7a6f99c423715fb Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Tue, 28 May 2024 15:39:54 -0700 Subject: [PATCH] Backport of dns v2 - both empty string and default should be allowed for namespace and partition in CE into release/1.19.x (#21233) * backport of commit 8513eda6299c96f37b61e33d72bdad1bd794cf2e * backport of commit 329bdc1345949b7d1b19bc53e6756a12065f4b08 * backport of commit 0f5d0adebd9a66fb0180b5cf6892f213e1ea731e * backport of commit 8a1d017999419a226dca9620072c3aadd5ba1d45 --------- Co-authored-by: John Murret Co-authored-by: Michael Zalimeni --- .changelog/21230.txt | 3 ++ acl/acl_ce.go | 23 +++++++-- agent/discovery/query_fetcher_v1_ce.go | 6 ++- agent/discovery/query_fetcher_v1_ce_test.go | 53 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 .changelog/21230.txt diff --git a/.changelog/21230.txt b/.changelog/21230.txt new file mode 100644 index 0000000000..5a57333afa --- /dev/null +++ b/.changelog/21230.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +dns: new version was not supporting partition or namespace being set to 'default' in CE version. +``` \ No newline at end of file diff --git a/acl/acl_ce.go b/acl/acl_ce.go index 7d2b8513b8..0d207ad421 100644 --- a/acl/acl_ce.go +++ b/acl/acl_ce.go @@ -8,12 +8,25 @@ package acl const ( WildcardPartitionName = "" DefaultPartitionName = "" -) + // NonEmptyDefaultPartitionName is the name of the default partition that is + // not empty. An example of this being supplied is when a partition is specified + // in the request for DNS by consul-dataplane. This has been added to support + // DNS v1.5, which needs to be compatible with the original DNS subsystem which + // supports partition being "default" or empty. Otherwise, use DefaultPartitionName. + NonEmptyDefaultPartitionName = "default" -// Reviewer Note: This is a little bit strange; one might want it to be "" like partition name -// However in consul/structs/intention.go we define IntentionDefaultNamespace as 'default' and so -// we use the same here -const DefaultNamespaceName = "default" + // DefaultNamespaceName is used to mimic the behavior in consul/structs/intention.go, + // where we define IntentionDefaultNamespace as 'default' and so we use the same here. + // This is a little bit strange; one might want it to be "" like DefaultPartitionName. + DefaultNamespaceName = "default" + + // EmptyNamespaceName is the name of the default partition that is an empty string. + // An example of this being supplied is when a namespace is specifiedDNS v1. + // EmptyNamespaceName has been added to support DNS v1.5, which needs to be + // compatible with the original DNS subsystem which supports partition being "default" or empty. + // Otherwise, use DefaultNamespaceName. + EmptyNamespaceName = "" +) type EnterpriseConfig struct { // no fields in CE diff --git a/agent/discovery/query_fetcher_v1_ce.go b/agent/discovery/query_fetcher_v1_ce.go index 090db0e5f7..06299704bd 100644 --- a/agent/discovery/query_fetcher_v1_ce.go +++ b/agent/discovery/query_fetcher_v1_ce.go @@ -14,8 +14,12 @@ func (f *V1DataFetcher) NormalizeRequest(req *QueryPayload) { return } +// validateEnterpriseTenancy validates the tenancy fields for an enterprise request to +// make sure that they are either set to an empty string or "default" to align with the behavior +// in CE. func validateEnterpriseTenancy(req QueryTenancy) error { - if req.Namespace != "" || req.Partition != acl.DefaultPartitionName { + if !(req.Namespace == acl.EmptyNamespaceName || req.Namespace == acl.DefaultNamespaceName) || + !(req.Partition == acl.DefaultPartitionName || req.Partition == acl.NonEmptyDefaultPartitionName) { return ErrNotSupported } return nil diff --git a/agent/discovery/query_fetcher_v1_ce_test.go b/agent/discovery/query_fetcher_v1_ce_test.go index 717475c9dc..69cd2dea98 100644 --- a/agent/discovery/query_fetcher_v1_ce_test.go +++ b/agent/discovery/query_fetcher_v1_ce_test.go @@ -5,7 +5,60 @@ package discovery +import ( + "github.com/stretchr/testify/require" + "testing" +) + const ( defaultTestNamespace = "" defaultTestPartition = "" ) + +func Test_validateEnterpriseTenancy(t *testing.T) { + testCases := []struct { + name string + req QueryTenancy + expected error + }{ + { + name: "empty namespace and partition returns no error", + req: QueryTenancy{ + Namespace: defaultTestNamespace, + Partition: defaultTestPartition, + }, + expected: nil, + }, + { + name: "namespace and partition set to 'default' returns no error", + req: QueryTenancy{ + Namespace: "default", + Partition: "default", + }, + expected: nil, + }, + { + name: "namespace set to something other than empty string or `default` returns not supported error", + req: QueryTenancy{ + Namespace: "namespace-1", + Partition: "default", + }, + expected: ErrNotSupported, + }, + { + name: "partition set to something other than empty string or `default` returns not supported error", + req: QueryTenancy{ + Namespace: "default", + Partition: "partition-1", + }, + expected: ErrNotSupported, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := validateEnterpriseTenancy(tc.req) + require.Equal(t, tc.expected, err) + }) + } +}