-
Notifications
You must be signed in to change notification settings - Fork 2k
task env: add NOMAD_UNIX_ADDR var #25598
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
base: main
Are you sure you want to change the base?
Conversation
for easier task setup when using workload identity and nomad CLI
a61728b
to
cda7f39
Compare
.changelog/25598.txt
Outdated
@@ -0,0 +1,3 @@ | |||
```release-note:improvement | |||
task environment: add NOMAD_UNIX_ADDR env var when using workload identity |
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.
"when using workload identity" doesn't quite feel like the right way to describe this. It's for use with workload identity. Maybe point out that it points to the Task API socket?
client/taskenv/env.go
Outdated
@@ -620,10 +624,12 @@ func (b *Builder) buildEnv(allocDir, localDir, secretsDir string, | |||
// Build the Nomad Workload Token | |||
if b.workloadTokenDefault != "" { | |||
envMap[WorkloadToken] = b.workloadTokenDefault | |||
envMap[UnixAddr] = "unix:" + filepath.Join(secretsDir, "api.sock") |
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.
Aren't these usually of the form: unix://$PATH
? ex. unix:///secrets/api.sock
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.
Right?? That's part of why this is all needed - the world is not consistent. With a caddy
reverse proxy, for example, it's unix//path/to/sock
🤷
In our particular case, it can be either unix:{path}
or unix://{path}
(but not unix:/{path}
nor unix/{path}
).
Would you prefer unix://
?
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.
the world is not consistent
Yeah, there's no real standard here. The Go parser (which we're using in the api
package) handles both https://go.dev/play/p/S2tCmK6la8H I think I'd prefer unix://
just because it matches the file://
URL scheme (from RFC1738 and friends). But that's a matter of taste.
Minor nit: this is a new feature, so it wouldn't get backported to 1.9.x/1.8.x |
I keep thinking of this, and finally I'm doin it. Allows for easier usage in task setup:
I named it "_UNIX_ADDR" instead of "_ADDR_UNIX" because I figure there may be services out there called "UNIX" that would cause a name collision, but I'm open to other names.
E2E spot check on my linux laptop: