-
Notifications
You must be signed in to change notification settings - Fork 455
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
Support running SPIRE as a Windows service #3625
Changes from 1 commit
91676fa
43b918a
3d5a483
0186320
7634851
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,12 +168,13 @@ type rateLimitConfig struct { | |
UnusedKeys []string `hcl:",unusedKeys"` | ||
} | ||
|
||
func NewRunCommand(logOptions []log.Option, allowUnknownConfig bool) cli.Command { | ||
return newRunCommand(common_cli.DefaultEnv, logOptions, allowUnknownConfig) | ||
func NewRunCommand(ctx context.Context, logOptions []log.Option, allowUnknownConfig bool) cli.Command { | ||
return newRunCommand(ctx, common_cli.DefaultEnv, logOptions, allowUnknownConfig) | ||
} | ||
|
||
func newRunCommand(env *common_cli.Env, logOptions []log.Option, allowUnknownConfig bool) *Command { | ||
func newRunCommand(ctx context.Context, env *common_cli.Env, logOptions []log.Option, allowUnknownConfig bool) *Command { | ||
return &Command{ | ||
ctx: ctx, | ||
env: env, | ||
logOptions: logOptions, | ||
allowUnknownConfig: allowUnknownConfig, | ||
|
@@ -182,6 +183,7 @@ func newRunCommand(env *common_cli.Env, logOptions []log.Option, allowUnknownCon | |
|
||
// Run Command struct | ||
type Command struct { | ||
ctx context.Context | ||
logOptions []log.Option | ||
env *common_cli.Env | ||
allowUnknownConfig bool | ||
|
@@ -241,7 +243,11 @@ func (cmd *Command) Run(args []string) int { | |
|
||
s := server.New(*c) | ||
|
||
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) | ||
ctx := cmd.ctx | ||
if ctx == nil { | ||
ctx = context.Background() | ||
} | ||
Comment on lines
+246
to
+249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may this initialization be moved to newRunCommand or NewRunCommand? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if there is a real benefit of doing that. Currently, the context is being initialized here also, there is no change with that (https://github.com/spiffe/spire/blob/main/cmd/spire-server/cli/run/run.go#L244). |
||
ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) | ||
defer stop() | ||
|
||
err = s.Run(ctx) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -166,6 +166,13 @@ the following flags are available: | |
| `-trustBundleUrl` | URL to download the SPIRE server CA bundle | | | ||
| `-trustDomain` | The trust domain that this agent belongs to (should be no more than 255 characters) | | | ||
|
||
#### Running SPIRE Agent as a Windows service | ||
|
||
On Windows platform, SPIRE Agent can optionally be run as a Windows service. When running as a Windows service, the only command supported is the `run` command. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how is this validation done? may we prevent it programmatically? what happens if a user try to create a service that run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add a validation to fail with ERROR_BAD_ARGUMENTS if an argument indicating a command different to "run" is used when running as a service. |
||
|
||
_Note: SPIRE does not automatically create the service in the system, it must be created by the user. | ||
When starting the service, all the arguments to execute SPIRE Agent with the `run` command must be passed as service arguments._ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is something useful to provide an example here of a configuration that allows this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I'll add an example of how the service can be created and how the service can be run. |
||
|
||
### `spire-agent api fetch` | ||
|
||
Calls the workload API to fetch an X509-SVID. This command is aliased to `spire-agent api fetch x509`. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
//go:build !windows | ||
// +build !windows | ||
|
||
package entrypoint | ||
|
||
import ( | ||
"context" | ||
"os" | ||
) | ||
|
||
type EntryPoint struct { | ||
runCmdFn func(ctx context.Context, args []string) int | ||
} | ||
|
||
func NewEntryPoint(runFn func(ctx context.Context, args []string) int) *EntryPoint { | ||
return &EntryPoint{ | ||
runCmdFn: runFn, | ||
} | ||
} | ||
|
||
func (e *EntryPoint) Main() int { | ||
return e.runCmdFn(context.Background(), os.Args[1:]) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
//go:build !windows | ||
// +build !windows | ||
|
||
package entrypoint | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestEntryPoint(t *testing.T) { | ||
assert.Equal(t, | ||
NewEntryPoint(func(ctx context.Context, args []string) int { return 0 }).Main(), | ||
0) | ||
|
||
assert.Equal(t, | ||
NewEntryPoint(func(ctx context.Context, args []string) int { return 1 }).Main(), | ||
1) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
//go:build windows | ||
// +build windows | ||
|
||
package entrypoint | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
|
||
"golang.org/x/sys/windows/svc" | ||
) | ||
|
||
type systemCaller interface { | ||
IsWindowsService() (bool, error) | ||
Run(name string, handler svc.Handler) error | ||
} | ||
|
||
type systemCall struct { | ||
} | ||
|
||
func (s *systemCall) IsWindowsService() (bool, error) { | ||
return svc.IsWindowsService() | ||
} | ||
|
||
func (s *systemCall) Run(name string, handler svc.Handler) error { | ||
return svc.Run(name, handler) | ||
} | ||
|
||
type EntryPoint struct { | ||
handler svc.Handler | ||
runCmdFn func(ctx context.Context, args []string) int | ||
sc systemCaller | ||
} | ||
|
||
func NewEntryPoint(runCmdFn func(ctx context.Context, args []string) int) *EntryPoint { | ||
return &EntryPoint{ | ||
runCmdFn: runCmdFn, | ||
handler: &service{ | ||
RunFn: func(ctx context.Context, stop context.CancelFunc, args []string) int { | ||
retCode := runCmdFn(ctx, args[1:]) | ||
stop() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may we move stop to a defer? |
||
return retCode | ||
}, | ||
}, | ||
sc: &systemCall{}, | ||
} | ||
} | ||
|
||
func (e *EntryPoint) Main() int { | ||
isWindowsService, err := e.sc.IsWindowsService() | ||
if err != nil { | ||
fmt.Fprintf(os.Stderr, "Could not determine if running as a Windows service: %v", err) | ||
return 1 | ||
} | ||
if isWindowsService { | ||
errChan := make(chan error) | ||
go func() { | ||
// Since the service runs in its own process, the service name is ignored. | ||
// https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatcherw | ||
errChan <- e.sc.Run("", e.handler) | ||
}() | ||
err = <-errChan | ||
if err != nil { | ||
Comment on lines
+65
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we be sure that returned error was logged? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Errors will be logged as part of the normal execution of the "run" command, in the same way that is done when running as a regular process. |
||
return 1 | ||
} | ||
return 0 | ||
} | ||
|
||
return e.runCmdFn(context.Background(), os.Args[1:]) | ||
} |
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'm worried that with this approach, any command will try to verify if our binary is running as a service,
but that is only true for "run" commands
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.
We discussed this and we agreed that in order to avoid blocking SPIRE to run only because the
svc.IsWindowsService()
fails, we will fallback to run SPIRE as a regular process ifIsWindowsService()
returns an error.