agent: Add agent process supervisor tests #20741

averche · 2023-05-24T16:11:20Z

This introduces 6 integration tests for agent running in process supervisor mode (#20739):

simple test to ensure environment variables are injected correctly
test when the child process exits early
test when the child process exits early with an error
test sending sigterm to the child process
test sending sigusr1 to the child process
test child process ignoring signals

This PR is part of a larger effort to implement secrets as environment variable support in agent (VLT-253)

This PR was started in #20722 by @dhuckins

…nv-var

Co-authored-by: Anton Averchenkov <[email protected]>

Signed-off-by: Daniel Huckins <[email protected]>

…nv-var

…nv-var-config

Signed-off-by: Daniel Huckins <[email protected]>

Co-authored-by: Anton Averchenkov <[email protected]>

Signed-off-by: Daniel Huckins <[email protected]>

Co-authored-by: Anton Averchenkov <[email protected]>

Signed-off-by: Daniel Huckins <[email protected]>

…o agent-runner-env-var

Signed-off-by: Daniel Huckins <[email protected]>

command/agent/exec/test-app/main.go

command/agent/exec/exec_test.go

VioletHynes · 2023-05-25T13:33:06Z

command/agent/exec/exec_test.go

+                "request_id": "8af096e9-518c-7351-eff5-5ba20554b21f",
+                "lease_id": "",
+                "renewable": false,
+                "lease_duration": 0,


I'd like to see a test on a dynamic secret, as well as static (it would likely make more sense on a real Vault server, as mentioned above).

Dynamic secret might be a bit difficult to set up on a real server. I'm thinking we can use fake server that simply scrambles the response on each request.

You could request a token? That should be easy enough to set up, I think. For example, while TestAgent_Cache_DynamicSecret uses the API proxy subsystem of Agent, I don't see why it couldn't be doable using templating?

VioletHynes · 2023-05-25T13:38:29Z

command/agent/exec/exec_test.go

+			}},
+			testAppArgs:       []string{"--stop-after", "10s"},
+			testAppStopSignal: syscall.SIGTERM,
+			testAppPort:       34001,


Instead of static ports, could we use something similar to generateListenerAddress in agent_test to try and avoid collisions?

I had a similar concern (port collisions when running the tests from multiple PR's) but if I understand correctly, GHA should run each PR in an isolated/containerized environment so it should not be an issue

VioletHynes · 2023-05-25T13:40:38Z

command/agent/exec/exec_test.go

+	}()
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {


Can we use t.Parallel() on the next line here (and at the start of the function)? I can't see anything that might conflict, but I could be missing something

I've tried t.Parallel but it's failing the tests -- possibly something related to the data race.

Unfortunately, the tests currently can't be run in parallel since there is a shared setup (a common test app that is built ahead of time). This would probably work better in a test suite/fixture, but these don't seem to be used in Vault.

VioletHynes · 2023-05-25T14:34:30Z

Oh, as an FYI, there's a data race test failure, in case you've not seen it:

WARNING: DATA RACE
Write at 0x00c000c46cc0 by goroutine 163:
  github.com/hashicorp/consul-template/child.(*Child).kill.func1()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:439 +0x86
  runtime.deferreturn()
      /opt/hostedtoolcache/go/1.20.4/x64/src/runtime/panic.go:476 +0x32
  github.com/hashicorp/consul-template/child.(*Child).internalStop()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:296 +0x1d5
  github.com/hashicorp/consul-template/child.(*Child).Stop()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:274 +0x1877
  github.com/hashicorp/vault/command/agent/exec.(*Server).Run()
      /home/runner/work/vault/vault/command/agent/exec/exec.go:131 +0x1857
  github.com/hashicorp/vault/command/agent/exec.TestServer_Run.func2.1()
      /home/runner/work/vault/vault/command/agent/exec/exec_test.go:203 +0x67

Previous read at 0x00c000c46cc0 by goroutine 184:
  github.com/hashicorp/consul-template/child.(*Child).kill.func2()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:457 +0x84

Goroutine 163 (running) created at:
  github.com/hashicorp/vault/command/agent/exec.TestServer_Run.func2()
      /home/runner/work/vault/vault/command/agent/exec/exec_test.go:202 +0xbbe
  testing.tRunner()
      /opt/hostedtoolcache/go/1.20.4/x64/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.20.4/x64/src/testing/testing.go:1629 +0x47

Goroutine 184 (running) created at:
  github.com/hashicorp/consul-template/child.(*Child).kill()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:455 +0x4ab
  github.com/hashicorp/consul-template/child.(*Child).internalStop()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:296 +0x1d5
  github.com/hashicorp/consul-template/child.(*Child).Stop()
      /home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/child/child.go:274 +0x1877
  github.com/hashicorp/vault/command/agent/exec.(*Server).Run()
      /home/runner/work/vault/vault/command/agent/exec/exec.go:131 +0x1857
  github.com/hashicorp/vault/command/agent/exec.TestServer_Run.func2.1()
      /home/runner/work/vault/vault/command/agent/exec/exec_test.go:203 +0x67
==================
2023-05-24T21:12:28.749Z [INFO]  exec server stopped
    testing.go:1446: race detected during execution of test

=== FAIL: command/agent/exec TestServer_Run (71.43s)

averche · 2023-05-25T14:53:54Z

We've been looking into this data race and trying to figure it out, gives us a concern about the existing code 😄

averche · 2023-05-25T20:47:48Z

The data race seems to be internal to consul-template. I've created an issue hashicorp/consul-template#1753 with a minimal example of failure.

command/agent/exec/test-app/main.go

…tests

dhuckins and others added 30 commits May 8, 2023 11:16

added exec and env_template config/parsing

92d738a

add tests

6d331c8

we can reuse ctconfig here

aacd796

do not create a non-nil map

ef2f4c0

check defaults

18ea7d4

Merge branch 'main' of github.com:hashicorp/vault into agent-runner-e…

2588973

…nv-var

Apply suggestions from code review

efbcfb9

Co-authored-by: Anton Averchenkov <[email protected]>

Apply suggestions from code review

baa4700

Co-authored-by: Anton Averchenkov <[email protected]>

first go of exec server

78bc24b

Signed-off-by: Daniel Huckins <[email protected]>

convert to list

1a09e78

Signed-off-by: Daniel Huckins <[email protected]>

convert to list

af3b535

Signed-off-by: Daniel Huckins <[email protected]>

refactor

bcf3f17

Signed-off-by: Daniel Huckins <[email protected]>

sig test

654b0a1

Signed-off-by: Daniel Huckins <[email protected]>

add failing example

ed56bf0

Signed-off-by: Daniel Huckins <[email protected]>

Merge branch 'agent-runner-env-var-config' into agent-runner-env-var

545ac2a

refactor for config changes

27a8a50

Signed-off-by: Daniel Huckins <[email protected]>

Merge branch 'main' of github.com:hashicorp/vault into agent-runner-e…

deb0270

…nv-var

Merge branch 'main' of github.com:hashicorp/vault into agent-runner-e…

4d14b06

…nv-var-config

add test for invalid signal

a83a3ab

Signed-off-by: Daniel Huckins <[email protected]>

Merge branch 'agent-runner-env-var-config' into agent-runner-env-var

cf2827d

account for auth token changes

7f4011e

Signed-off-by: Daniel Huckins <[email protected]>

only start the runner once we have a token

df1f0b2

test case

118404f

Signed-off-by: Daniel Huckins <[email protected]>

tests in diff branch

3bf8057

Signed-off-by: Daniel Huckins <[email protected]>

Apply suggestions from code review

e6035ee

Co-authored-by: Anton Averchenkov <[email protected]>

fix rename

c7041c6

Signed-off-by: Daniel Huckins <[email protected]>

Update command/agent/exec/exec.go

9bdb47e

Co-authored-by: Anton Averchenkov <[email protected]>

apply suggestions from code review

5dd1aa0

Signed-off-by: Daniel Huckins <[email protected]>

Merge branch 'agent-runner-env-var' of github.com:hashicorp/vault int…

6147b75

…o agent-runner-env-var

cleanup

71dac97

Signed-off-by: Daniel Huckins <[email protected]>

averche requested a review from a team as a code owner May 24, 2023 21:05

Merge branch 'main' into agent-process-supervisor-tests

5001082

vercel bot deployed to Preview May 24, 2023 21:16 View deployment

VioletHynes self-requested a review May 25, 2023 12:47

VioletHynes reviewed May 25, 2023

View reviewed changes

averche added 2 commits May 25, 2023 16:41

fix exitCh race condition

b2364b6

skip the zombie proc test

5935543

test doc and cleanup

487cca7

VioletHynes reviewed May 26, 2023

View reviewed changes

command/agent/exec/test-app/main.go Show resolved Hide resolved

VioletHynes approved these changes May 26, 2023

View reviewed changes

averche added 5 commits May 26, 2023 11:19

comment at the top of main.go

c05b2f1

more comments

10dee8d

revert to testAppStartedCh

5704422

fix exitCh race condition

38d833c

Merge branch 'agent-fix-exit-ch-issue' into agent-process-supervisor-…

9b7b782

…tests

averche changed the base branch from main to agent-fix-exit-ch-issue May 26, 2023 18:00

vercel bot deployed to Preview May 26, 2023 18:05 View deployment

averche added the backport/1.14.x label May 29, 2023

Base automatically changed from agent-fix-exit-ch-issue to main May 30, 2023 16:23

Merge branch 'main' into agent-process-supervisor-tests

e7dc3e3

vercel bot deployed to Preview May 30, 2023 16:32 View deployment

averche enabled auto-merge (squash) May 30, 2023 16:58

Shorter wait time to complete tests faster

bba2160

averche merged commit 21eccf8 into main May 30, 2023

averche deleted the agent-process-supervisor-tests branch May 30, 2023 18:06

hc-github-team-secure-vault-core mentioned this pull request May 30, 2023

Backport of agent: Add agent process supervisor tests into release/1.14.x #20866

Merged

dhuckins mentioned this pull request May 31, 2023

agent runner env var test #20722

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

agent: Add agent process supervisor tests #20741

agent: Add agent process supervisor tests #20741

averche commented May 24, 2023 •

edited

Loading

VioletHynes May 25, 2023

averche May 25, 2023

VioletHynes May 25, 2023

VioletHynes May 25, 2023

averche May 25, 2023

VioletHynes May 25, 2023

averche May 25, 2023

averche May 25, 2023

VioletHynes commented May 25, 2023

averche commented May 25, 2023

averche commented May 25, 2023

agent: Add agent process supervisor tests #20741

agent: Add agent process supervisor tests #20741

Conversation

averche commented May 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VioletHynes commented May 25, 2023

averche commented May 25, 2023

averche commented May 25, 2023

averche commented May 24, 2023 •

edited

Loading