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

Nomad service name validation stricter than Consul validation #16691

Closed
wants to merge 6 commits into from

Conversation

Juanadelacuesta
Copy link
Member

@Juanadelacuesta Juanadelacuesta commented Mar 28, 2023

This PR addresses the bug reported on #11097

When a new service is created, the name should comply with RFC-1123, this validation is enforced on Nomad, but not on consul. This PR relaxes the Nomad validation and instead of an error, it shows a warning for the non compliant service name.

@Juanadelacuesta Juanadelacuesta changed the title func: add warnings function to Service definition Nomad service name validation stricter than Consul validation Mar 28, 2023
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

The code LGTM but I have two comments in terms of our intent:

  • The wording of the warning suggests that it's an error.
  • Do we really want to make this change? I realize that having consistency with the Consul services would be nice, but having a stricter validation for our own services means we could more easily add DNS support for Nomad SD without worrying about backwards compatibility later on.

nomad/structs/structs.go Outdated Show resolved Hide resolved
nomad/structs/structs.go Outdated Show resolved Hide resolved
Co-authored-by: Tim Gross <tgross@hashicorp.com>
Co-authored-by: Tim Gross <tgross@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants