-
Notifications
You must be signed in to change notification settings - Fork 2k
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
cli: forward request for job validation to nomad leader #14065
Conversation
b7faa80
to
1dc771f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I think the issue number in the PR message is wrong, it seems like it was supposed to be #13940.
This paragraph in docs could use some updating as well, I don't think that is even true anymore? 😅
func (j *Job) Validate(args *structs.JobValidateRequest, reply *structs.JobValidateResponse) error { | ||
if done, err := j.srv.forward("Job.Validate", args, args, reply); done { | ||
return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think request allows for stale right? But if we would were to allow validation without forwarding (like a local check) we could downgrade Vault validation errors to warnings. But since (I think?) stale requests are not allowed here we're probably good as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! The JobValidateRequest
rpc embeds a WriteRequest, which has no notion of stale
This PR changes the behavior of 'nomad job validate' to forward the request to the nomad leader, rather than responding from any server. This is because we need the leader when validating Vault tokens, since the leader is the only server with an active vault client.
1dc771f
to
7b46524
Compare
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. |
This PR changes the behavior of 'nomad job validate' to forward the
request to the nomad leader, rather than responding from any server.
This is because we need the leader when validating Vault tokens, since
the leader is the only server with an active vault client.
Closes #13940