From 8513eda6299c96f37b61e33d72bdad1bd794cf2e Mon Sep 17 00:00:00 2001 From: John Murret Date: Tue, 28 May 2024 14:18:30 -0600 Subject: [PATCH 1/4] dns v2 - both empty string and default should be allowed for namespace and partition in Ce --- agent/discovery/query_fetcher_v1.go | 1 + agent/discovery/query_fetcher_v1_ce.go | 2 +- agent/discovery/query_fetcher_v1_ce_test.go | 53 +++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/agent/discovery/query_fetcher_v1.go b/agent/discovery/query_fetcher_v1.go index da76d744dd61..1b39f020b598 100644 --- a/agent/discovery/query_fetcher_v1.go +++ b/agent/discovery/query_fetcher_v1.go @@ -424,6 +424,7 @@ RPC: } func (f *V1DataFetcher) ValidateRequest(_ Context, req *QueryPayload) error { + f.logger.Debug(fmt.Sprintf("req %+v", req)) if req.EnableFailover { return ErrNotSupported } diff --git a/agent/discovery/query_fetcher_v1_ce.go b/agent/discovery/query_fetcher_v1_ce.go index 090db0e5f789..5d500644a3f0 100644 --- a/agent/discovery/query_fetcher_v1_ce.go +++ b/agent/discovery/query_fetcher_v1_ce.go @@ -15,7 +15,7 @@ func (f *V1DataFetcher) NormalizeRequest(req *QueryPayload) { } func validateEnterpriseTenancy(req QueryTenancy) error { - if req.Namespace != "" || req.Partition != acl.DefaultPartitionName { + if !(req.Namespace == "" || req.Namespace == acl.DefaultNamespaceName) || !(req.Partition == acl.DefaultPartitionName || req.Partition == "default") { 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 717475c9dccd..69cd2dea98d9 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) + }) + } +} From 329bdc1345949b7d1b19bc53e6756a12065f4b08 Mon Sep 17 00:00:00 2001 From: John Murret Date: Tue, 28 May 2024 14:21:44 -0600 Subject: [PATCH 2/4] add changelog --- .changelog/21230.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/21230.txt diff --git a/.changelog/21230.txt b/.changelog/21230.txt new file mode 100644 index 000000000000..5a57333afa9d --- /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 From 0f5d0adebd9a66fb0180b5cf6892f213e1ea731e Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Tue, 28 May 2024 17:27:22 -0400 Subject: [PATCH 3/4] use default partition constant --- agent/discovery/query_fetcher_v1_ce.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/discovery/query_fetcher_v1_ce.go b/agent/discovery/query_fetcher_v1_ce.go index 5d500644a3f0..9ba772216d25 100644 --- a/agent/discovery/query_fetcher_v1_ce.go +++ b/agent/discovery/query_fetcher_v1_ce.go @@ -7,6 +7,7 @@ package discovery import ( "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/api" ) func (f *V1DataFetcher) NormalizeRequest(req *QueryPayload) { @@ -15,7 +16,7 @@ func (f *V1DataFetcher) NormalizeRequest(req *QueryPayload) { } func validateEnterpriseTenancy(req QueryTenancy) error { - if !(req.Namespace == "" || req.Namespace == acl.DefaultNamespaceName) || !(req.Partition == acl.DefaultPartitionName || req.Partition == "default") { + if !(req.Namespace == "" || req.Namespace == acl.DefaultNamespaceName) || !(req.Partition == "" || req.Partition == api.PartitionDefaultName) { return ErrNotSupported } return nil From 8a1d017999419a226dca9620072c3aadd5ba1d45 Mon Sep 17 00:00:00 2001 From: John Murret Date: Tue, 28 May 2024 15:46:02 -0600 Subject: [PATCH 4/4] use constants in validation. --- acl/acl_ce.go | 23 ++++++++++++++++++----- agent/discovery/query_fetcher_v1.go | 1 - agent/discovery/query_fetcher_v1_ce.go | 7 +++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/acl/acl_ce.go b/acl/acl_ce.go index 7d2b8513b832..0d207ad4211a 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" + + // 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" -// 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" + // 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.go b/agent/discovery/query_fetcher_v1.go index 1b39f020b598..da76d744dd61 100644 --- a/agent/discovery/query_fetcher_v1.go +++ b/agent/discovery/query_fetcher_v1.go @@ -424,7 +424,6 @@ RPC: } func (f *V1DataFetcher) ValidateRequest(_ Context, req *QueryPayload) error { - f.logger.Debug(fmt.Sprintf("req %+v", req)) if req.EnableFailover { return ErrNotSupported } diff --git a/agent/discovery/query_fetcher_v1_ce.go b/agent/discovery/query_fetcher_v1_ce.go index 9ba772216d25..06299704bdc9 100644 --- a/agent/discovery/query_fetcher_v1_ce.go +++ b/agent/discovery/query_fetcher_v1_ce.go @@ -7,7 +7,6 @@ package discovery import ( "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/api" ) func (f *V1DataFetcher) NormalizeRequest(req *QueryPayload) { @@ -15,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.Namespace == acl.DefaultNamespaceName) || !(req.Partition == "" || req.Partition == api.PartitionDefaultName) { + if !(req.Namespace == acl.EmptyNamespaceName || req.Namespace == acl.DefaultNamespaceName) || + !(req.Partition == acl.DefaultPartitionName || req.Partition == acl.NonEmptyDefaultPartitionName) { return ErrNotSupported } return nil