Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dns v2 - both empty string and default should be allowed for namespace and partition in CE #21230

Merged
merged 4 commits into from
May 28, 2024

Conversation

jmurret
Copy link
Member

@jmurret jmurret commented May 28, 2024

Description

tenancy was being validated on DNS queries from consul-dataplane to only allow partition to be set to an empty string. It was not allow default in either case.

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@jmurret jmurret added the backport/1.19 This release series is longer active on CE, use backport/ent/1.19 label May 28, 2024
@jmurret jmurret force-pushed the jm/dns-v2-tenancy-validation branch from 5d12cb5 to 329bdc1 Compare May 28, 2024 20:22
@jmurret jmurret requested a review from zalimeni May 28, 2024 20:45
Copy link
Member

@zalimeni zalimeni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment, otherwise LGTM - thank you!

agent/discovery/query_fetcher_v1.go Outdated Show resolved Hide resolved
agent/discovery/query_fetcher_v1_ce.go Outdated Show resolved Hide resolved
@jmurret jmurret requested a review from a team as a code owner May 28, 2024 21:55
Comment on lines +11 to +28
// 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 = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.19 This release series is longer active on CE, use backport/ent/1.19
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants