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

Make validate command ignore environment variables in service name #3558

Merged
merged 8 commits into from
Nov 17, 2017

Conversation

preetapan
Copy link
Contributor

Fixes #3532

be resolved at run time. Invalid characters outside of environment variable names will not pass
validation.
For example, if the service name is set to `my_service${NOMAD_NODE_NAME}` it will fail
validation. However, if a service name is set to `myservice{NOMAD_NODE_NAME}` and the node name
Copy link
Contributor

Choose a reason for hiding this comment

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

myservice${NOMAD_NODE_NAME} ?

@@ -28,6 +28,14 @@ and supports `go-getter` syntax.
On successful validation, exit code 0 will be returned, otherwise an exit code
of 1 indicates an error.

Service names are validated according to [RFC-1123](https://tools.ietf.org/html/rfc1123).
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove from here and instead add to: https://www.nomadproject.io/docs/job-specification/service.html#name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of any specific validation details on this page since then we will end up documenting validation details here and on the specific stanzas.

@@ -89,6 +89,12 @@ does not automatically enable service discovery.
- `${GROUP}` - the name of the group
- `${TASK}` - the name of the task
- `${BASE}` - shorthand for `${JOB}-${GROUP}-${TASK}`

Any environment variables in the name are ignored during validation, because some environment variables can only
be resolved at run time. Invalid characters outside of environment variable names will not pass
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

Validation of the name occurs in two parts. When the job is registered, an initial validation pass checks that the service name adheres to RFC-1123 §2.1 and the length limit, excluding any variables requiring interpolation. Once the client receives the service and all interpretable values are available, the service name will be interpolated and revalidated. This can cause certain service names to pass validation at submit time but fail at runtime.

Preetha added 2 commits November 17, 2017 08:38
@@ -28,6 +28,14 @@ and supports `go-getter` syntax.
On successful validation, exit code 0 will be returned, otherwise an exit code
of 1 indicates an error.

Service names are validated according to [RFC-1123](https://tools.ietf.org/html/rfc1123).
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of any specific validation details on this page since then we will end up documenting validation details here and on the specific stanzas.

@dadgar
Copy link
Contributor

dadgar commented Nov 17, 2017

LGTM assuming we remove the new verbiage from the validate commands page

@preetapan preetapan merged commit 7bf4d34 into master Nov 17, 2017
@preetapan preetapan deleted the b-nomad-validate-with-defaults branch January 25, 2018 19:02
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants