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

Disable EC2 Instance Stop Protection #986

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

sstoops
Copy link
Contributor

@sstoops sstoops commented Apr 26, 2023

AWS recently launched the ability to enable "stop protection" on EC2 instance, similar to the existing "termination protection"

This PR implemented an error-state routine to disable stop protection and retry termination.

Open Question: A new feature flag was added called DisableEC2InstanceStopProtection at the top level since this doesn't exactly fit under the DisableDeletionProtection hierarchy. I'd considered adding this flag under that group and just calling it something like EC2InstanceStop to differentiate it, but I felt that might be confusing. Thoughts?

I went back and forth on how to handle the retry-routines from two possible error states, but what I came up with works and I tried to document it inline with some comments.

Testing

Create a set of EC2 instances with the following script:

# Get default VPC ID
echo "getting default VPC ID"
VPC_ID=$(aws ec2 describe-vpcs --filters Name=isDefault,Values=true --query 'Vpcs[0].VpcId' --output text)

# Get default AWS Security Group ID as a comma separated string
echo "getting default security group"
SEC_GROUP_ID=$(aws ec2 describe-security-groups --filters Name=vpc-id,Values=$VPC_ID --query 'SecurityGroups[0].GroupId' --output text | tr '\t' ',')

# Launch an EC2 instance
echo "launching EC2 instance"
INSTANCE_ID=$(aws ec2 run-instances --image-id ami-09d3b3274b6c5d4aa --count 1 --instance-type t2.micro --security-group-ids $SEC_GROUP_ID --query 'Instances[0].InstanceId' --output text)

# Enable termination protection
echo "enabling termination protection"
aws ec2 modify-instance-attribute --instance-id $INSTANCE_ID --disable-api-termination

# Launch an EC2 instance
echo "launching EC2 instance"
INSTANCE_ID=$(aws ec2 run-instances --image-id ami-09d3b3274b6c5d4aa --count 1 --instance-type t2.micro --security-group-ids $SEC_GROUP_ID --query 'Instances[0].InstanceId' --output text)

# Enable stop protection
echo "enabling stop protection"
aws ec2 modify-instance-attribute --instance-id $INSTANCE_ID --disable-api-stop

# Launch an EC2 instance
echo "launching EC2 instance"
INSTANCE_ID=$(aws ec2 run-instances --image-id ami-09d3b3274b6c5d4aa --count 1 --instance-type t2.micro --security-group-ids $SEC_GROUP_ID --query 'Instances[0].InstanceId' --output text)

# Enable termination and stop protection
echo "enabling termination and stop protection"
aws ec2 modify-instance-attribute --instance-id $INSTANCE_ID --disable-api-termination
aws ec2 modify-instance-attribute --instance-id $INSTANCE_ID --disable-api-stop

Running AWS Nuke with EC2Instance enabled successfully terminates all instances.

@sstoops sstoops requested a review from a team as a code owner April 26, 2023 23:25
Copy link
Member

@bjoernhaeuser bjoernhaeuser left a comment

Choose a reason for hiding this comment

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

I am aware that you just amended the existing logic to also cater for stop protection, which is totally fine me. Though I was thinking if it would be better to add the two different attributes to the EC2Instance struct and act accordingly.

With this I would see upsides:
1.) We do not need to check error messages
2.) If both things are enabled (is this even possible), both things can be deactivated and then the instance terminated. Maybe even before trying to delete them.

@sstoops
Copy link
Contributor Author

sstoops commented May 23, 2023

@bjoernhaeuser Thank you for your review!

I'd considered rewriting this logic in a manner you described but similar to the original developer who implemented the termination protection logic, the problem I faced is both the termination and stop protection attributes are not returned by the regular DescribeInstances API call. To retrieve the state of these attributes, you must make two additional API calls to DescribeInstanceAttribute, one for each type of protection.

Since I suspect these two attributes will not be a common issue for most users of AWS Nuke, I felt it was an unnecessary addition of API calls to every EC2 Instance. In an effort to keep AWS Nuke as light as possible for most users, my personal preference is to maintain the approach of calling these API endpoints only in the event of a termination error.

I am, however, open to continued discussion and brainstorming!

@bjoernhaeuser
Copy link
Member

Thanks for the reminder, now I remember why it was implemented like this.

@sstoops
Copy link
Contributor Author

sstoops commented Jun 27, 2023

Greetings @svenwltr @der-eismann , would you be okay getting this PR merged? Thank you!

@der-eismann der-eismann merged commit d7be6e0 into rebuy-de:main Jun 27, 2023
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.

4 participants