Skip to content

Commit

Permalink
dns v2 - both empty string and default should be allowed for namespac…
Browse files Browse the repository at this point in the history
…e and partition in CE (hashicorp#21230)

* dns v2 - both empty string and default should be allowed for namespace and partition in Ce

* add changelog

* use default partition constant

* use constants in validation.

---------

Co-authored-by: Michael Zalimeni <michael.zalimeni@hashicorp.com>
  • Loading branch information
jmurret and zalimeni committed May 28, 2024
1 parent ad9ada8 commit 11bcf52
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 6 deletions.
3 changes: 3 additions & 0 deletions .changelog/21230.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
dns: new version was not supporting partition or namespace being set to 'default' in CE version.
```
23 changes: 18 additions & 5 deletions acl/acl_ce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion agent/discovery/query_fetcher_v1_ce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 53 additions & 0 deletions agent/discovery/query_fetcher_v1_ce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 11bcf52

Please sign in to comment.