Skip to content
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

Shutdown agent when it is banned #3308

Merged
merged 3 commits into from
Aug 8, 2022

Conversation

MarcosDY
Copy link
Collaborator

@MarcosDY MarcosDY commented Aug 3, 2022

Make Agent shutdown when it is banned

Which issue this PR fixes
fixes #3278

Copy link
Member

@amartinezfayo amartinezfayo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @MarcosDY, looks great!

return isExpectedPermissionDenied(err, shouldReattest)
}

// ShouldAgentShutdown returns true if the Server returned an error worth shutdowning the Agent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ShouldAgentShutdown returns true if the Server returned an error worth shutdowning the Agent
// ShouldAgentShutdown returns true if the Server returned an error worth shutting down the Agent

func TestRotationFails(t *testing.T) {
caCert, caKey := testca.CreateCACertificate(t, nil, nil)

expiredStatus := status.New(codes.PermissionDenied, "agent is expired")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
Even if it's a test, it may be confusing

Suggested change
expiredStatus := status.New(codes.PermissionDenied, "agent is expired")
expiredStatus := status.New(codes.PermissionDenied, "agent is not active")

/opt/spire/bin/spire-server agent evict \
-spiffeID "spiffe://domain.test/spire/agent/x509pop/$(fingerprint conf/agent/agent.crt.pem)"

# Check at most 30 times (with one second in between) than agent is going shutdown
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Check at most 30 times (with one second in between) than agent is going shutdown
# Check at most 30 times (with one second in between) that the agent is going to shut down

sleep "${CHECKINTERVAL}"
done

fail-now "timed out waiting for agent to sync down entry"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fail-now "timed out waiting for agent to sync down entry"
fail-now "timed out waiting for agent to shut down"

MAXCHECKS=30
CHECKINTERVAL=1
for ((i=1;i<=MAXCHECKS;i++)); do
log-info "checking agent is not able to start ($i of $MAXCHECKS max)..."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log-info "checking agent is not able to start ($i of $MAXCHECKS max)..."
log-info "checking that the agent is not able to start ($i of $MAXCHECKS max)..."

Signed-off-by: Marcos Yacob <[email protected]>
Copy link
Member

@amartinezfayo amartinezfayo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@azdagron azdagron added this to the 1.4.1 milestone Aug 8, 2022
@MarcosDY MarcosDY merged commit 15c5cbe into spiffe:main Aug 8, 2022
@MarcosDY MarcosDY deleted the shutdown-agent-on-ban branch August 8, 2022 20:58
@evan2645 evan2645 modified the milestones: 1.4.1, 1.4.2 Sep 6, 2022
stevend-uber pushed a commit to stevend-uber/spire that referenced this pull request Oct 16, 2023
Shut down agent when it is banned

Signed-off-by: Marcos Yacob <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Banned agent keep alive
4 participants