Conversation
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
kfox1111
requested review from
MarcosDY,
amartinezfayo,
evan2645,
rturner3 and
sorindumitru
as code owners
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
|
|
||
| func (c *ConfigOS) AddOSFlags(flags *flag.FlagSet) { | ||
| flags.StringVar(&c.socketPath, "socketPath", DefaultSocketPath, "Path to the SPIRE Agent API Unix domain socket") | ||
| flags.StringVar(&c.instance, "i", "", "Instance name to substitute into socket templates (env SPIRE_AGENT_PUBLIC_SOCKET_TEMPLATE). If omitted and the env var is set, defaults to 'main'.") |
There was a problem hiding this comment.
I wonder if it's not a better UX to error out instead of defaulting to main, since main is not particularly meaningful in any way and it might not be an instance name.
I'm thinking that people might forget the -i in the case where those env vars are available and they accidentally target the wrong instance.
There was a problem hiding this comment.
without instance flag support, it defaults to /tmp/xxxxxxx, so there is precedent where people expect not having a flag to "just work"
I'm kind of trying to reproduce that feeling too when configured with system packages and not just use /tmp on a systemd system which is pretty insecure. The system packages would set the path by default and then if you only have one instance, things "just work" like they always have.
But, if its a show stopper, requiring -i if the env vars set is still probably better then not having support at all
There was a problem hiding this comment.
I kind of envisioned the env vars here along with a default -i, fulfilling the role of:
#5770
There was a problem hiding this comment.
Discussed this today and we would prefer erroring out if the instance is not present. Any default here is likely to be very opinionated and likely not useful for a lot of people.
For #5770 I think we can still use SPIRE_SERVER_ADMIN_SOCKET, SPIRE_AGENT_ADMIN_SOCKET and SPIFFE_ENDPOINT_SOCKET. That would cover the non-instanced use case and would not require any instance specified.
There was a problem hiding this comment.
ok. so, if no instance is specified, SPIRE_SERVER_ADMIN_SOCKET, SPIRE_AGENT_ADMIN_SOCKET and SPIFFE_ENDPOINT_SOCKET will be used if set and the regular defaults if not set.
If -instance is set and the template env vars are set, then those are used. otherwise, it will complain
There was a problem hiding this comment.
actually, I ran into the issue with SPIFFE_ENDPOINT_SOCKET being incompatible with socket path before since one expects unix:// in front and the other does not. For consistency, I coded up everything to have the _TEMPLATE equivalent variable name without the _TEMPLATE
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
|
Ok, this should be ready |
2 participants
Pull Request check list
Description of change
Adds easy multiinstance support
Which issue this PR fixes
fixes #6247