-
Notifications
You must be signed in to change notification settings - Fork 478
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
Integration tests for windows service support #3733
Integration tests for windows service support #3733
Conversation
Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
…tainers Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
@@ -20,7 +25,10 @@ type systemCall struct { | |||
} | |||
|
|||
func (s *systemCall) IsWindowsService() (bool, error) { | |||
return svc.IsWindowsService() | |||
// We are using a custom function because the svc.IsWindowsService() one still has an open issue in which it states | |||
// that it is not working properly in Windows containers: https://github.com/golang/go/issues/56335. Soon as we have |
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.
Is it worth submitting a PR? That issue has a help wanted tag and it seems like you have a fix below.
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.
A PR is already opened for this, in fact, the custom function with the workaround is based on the opened solution proposal: golang/sys#141
@@ -0,0 +1,6 @@ | |||
#!/bin/bash | |||
|
|||
pwd |
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.
why pwd is required?
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.
Not sure if it is actually required, I took it from another test sample (windows workload attestor). I'm going to check if it is really necessary.
create-service spire-server C:\\spire\\bin\\spire-server.exe | ||
start-service spire-server run -config C:/spire/conf/server/server.conf |
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.
is it possible to be consistent and use \\
or /
in all places?
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.
Sure! nice suggestion
docker-compose exec -T -u ContainerUser spire-base \ | ||
c:/spire/bin/spire-agent api fetch jwt -audience mydb fail-now "failed to fetch x509" |
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.
docker-compose exec -T -u ContainerUser spire-base \ | |
c:/spire/bin/spire-agent api fetch jwt -audience mydb fail-now "failed to fetch x509" | |
docker-compose exec -T -u ContainerUser spire-base \ | |
c:/spire/bin/spire-agent api fetch jwt -audience mydb || fail-now "failed to fetch JWT" |
|
||
FROM spire-server-windows:latest-local as spire-base | ||
|
||
COPY --from=spire-agent-windows C:/spire/bin/spire-agent.exe C:/spire/bin/spire-agent.exe |
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.
why you choose to use the same container to start both services?
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.
So, in this initial version, I tried to reproduce a test that I ran manually on my local machine (spire server and agent on the same host). Still, I see that a more realistic scenario would be running them in different hosts, right? I can update it :)
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.
yeah that is 'generally' the case
Signed-off-by: Guilherme Carvalho <[email protected]>
Signed-off-by: Guilherme Carvalho <[email protected]>
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.
Thank you @guilhermocc, LGTM!
* Create integration tests for windows service support Signed-off-by: Guilherme Carvalho <[email protected]>
Pull Request check list
Affected functionality
Running spire as a windows service inside windows containers.
Description of change
Which issue this PR fixes
Complement for #3490