cmd/tailscale/cli,feature: add support for identity federation #17529
Conversation
mcoulombe
force-pushed
the
max/identity-federation-support
branch
from
3a9a986 to
2e648e5
Compare
mcoulombe
force-pushed
the
max/identity-federation-support
branch
from
2e648e5 to
0e8c3df
Compare
| } | ||
| // Try to resolve the auth key via workload identity federation if that functionality | ||
| // is available and no auth key is yet determined. | ||
| if f, ok := tailscale.HookResolveAuthKeyViaWIF.GetOk(); ok && authKey == "" { |
There was a problem hiding this comment.
Note for future us to double check if we have credential precedence documented somewhere and will need to add this as a result
There was a problem hiding this comment.
Yeah it'll be worth mentioning in the command doc
There was a problem hiding this comment.
Consider calling this from ResolveAuthKey instead? That way it'll work in tsnet too (the original purpose of my ResolveAuthKey PR was to move common logic out into a place where both the CLI and tsnet could use it)
There was a problem hiding this comment.
ResolveAuthKey & ResolveAuthKeyViaWIF diverge enough that it feels best to keep them separate. They'll grow apart even more as we expand the ResolveAuthKeyViaWIF implementation while we work on the GA release.
For our beta release we do not plan to support tsnet just yet. We want a bit more time in the incubator before we cover more than tailscale up.
| return authkey, nil | ||
| } | ||
|
|
||
| func parseOptionalAttributes(clientID string) (ephemeral bool, preauthorized bool, err error) { |
There was a problem hiding this comment.
Nit: might be good to put this somewhere common that can be used by the resolveAuthKey path as a followup
There was a problem hiding this comment.
Do you know where it'd be appropriate to share code between different hooks? I tried to find a common place but nothing seemed obvious.
mcoulombe
force-pushed
the
max/identity-federation-support
branch
from
0e8c3df to
98effb6
Compare
mcoulombe
force-pushed
the
max/identity-federation-support
branch
from
98effb6 to
1e1dbd6
Compare
|
@tendstofortytwo sup, adding you to this PR since you worked on |
cmd/tailscale/cli/up.go
Outdated
| upf.BoolVar(&upArgs.advertiseDefaultRoute, "advertise-exit-node", false, "offer to be an exit node for internet traffic for the tailnet") | ||
| upf.BoolVar(&upArgs.postureChecking, "report-posture", false, hidden+"allow management plane to gather device posture information") | ||
| upf.StringVar(&upArgs.wifClientID, "wif-client-id", "", "Client ID used to generate authkeys via workload identity federation") | ||
| upf.StringVar(&upArgs.wifIDToken, "wif-id-token", "", "ID token from the identity provider to exchange with the control server for workload identity federation") |
There was a problem hiding this comment.
maybe worth gating on the feature being present?
There was a problem hiding this comment.
Sorry can you explain what you mean or give me an example? I don't see other arguments being conditionally gated.
| } | ||
| // Try to resolve the auth key via workload identity federation if that functionality | ||
| // is available and no auth key is yet determined. | ||
| if f, ok := tailscale.HookResolveAuthKeyViaWIF.GetOk(); ok && authKey == "" { |
There was a problem hiding this comment.
Consider calling this from ResolveAuthKey instead? That way it'll work in tsnet too (the original purpose of my ResolveAuthKey PR was to move common logic out into a place where both the CLI and tsnet could use it)
Sorry didn't know you were on PTO, I'll ask in #client thanks! |
mcoulombe
force-pushed
the
max/identity-federation-support
branch
3 times, most recently
from
6ffb19e to
3f3eeed
Compare
Add new arguments to `tailscale up` so authkeys can be generated dynamically via identity federation. Updates #9192 Signed-off-by: mcoulombe <max@tailscale.com>
mcoulombe
force-pushed
the
max/identity-federation-support
branch
from
3f3eeed to
ed90cc4
Compare
4 participants
Add new arguments to
tailscale upso authkeys can be generated dynamically via identity federation.Example command:
Updates #9192