feat: replace bash cleanup script with native Go implementation #433

ArangoGutierrez · 2025-07-30T14:35:48Z

Migrate AWS VPC cleanup functionality from bash script to a native Go package and CLI command, improving maintainability, error handling, and integration with the holodeck toolchain.

BREAKING CHANGE: The cleanup functionality is now available through holodeck cleanup command instead of scripts/awscleanup.sh

Changes:

Add pkg/cleanup package with comprehensive AWS resource deletion
- Handles EC2 instances, security groups, subnets, route tables, IGWs
- Includes GitHub job status checking via API
- Implements retry logic for VPC deletion (3 attempts, 30s delay)
- Better error handling with partial failure support
Add holodeck cleanup CLI command
- Accepts multiple VPC IDs in a single invocation
- --region flag for AWS region specification
- --force flag to skip GitHub job status checks
- Comprehensive help documentation
Update periodic GitHub workflow
- Now uses holodeck cleanup instead of bash script
- Builds Go binary as part of the workflow
- Better error handling and logging
- Handles multiple VPCs more efficiently
Add documentation
- docs/commands/cleanup.md with detailed usage examples
- Updated README.md with cleanup examples
- Updated command reference documentation
Add deprecation warning to scripts/awscleanup.sh
- Script still functional but warns users to migrate
- Can be removed in a future release

Benefits:

Type safety and better error handling
Consistent with other holodeck commands
Easier to test and maintain
More efficient handling of multiple VPCs
Better integration with CI/CD pipelines

Fixes: Improves reliability of periodic cleanup jobs
Refs: Migration from bash to Go for better maintainability

Copilot

Pull Request Overview

This PR migrates the AWS VPC cleanup functionality from a bash script to a native Go implementation, improving maintainability, error handling, and integration with the holodeck toolchain. The cleanup functionality is now available through the holodeck cleanup command instead of scripts/awscleanup.sh.

Key Changes

Added comprehensive AWS resource cleanup package (pkg/cleanup) with native Go implementation
Introduced new holodeck cleanup CLI command with support for multiple VPCs and enhanced flags
Updated periodic GitHub workflow to use the new Go-based cleanup command instead of bash script

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`scripts/awscleanup.sh`	Added deprecation warning to existing bash script
`pkg/cleanup/cleanup.go`	New comprehensive AWS resource cleanup package implementation
`pkg/cleanup/cleanup_test.go`	Basic test coverage for cleanup package structures
`cmd/cli/cleanup/cleanup.go`	New CLI command implementation for cleanup functionality
`cmd/cli/main.go`	Integration of new cleanup command into main CLI
`docs/commands/cleanup.md`	Comprehensive documentation for the new cleanup command
`docs/commands/README.md`	Added cleanup command to command reference
`README.md`	Updated with cleanup command usage example
`.github/workflows/periodic.yaml`	Updated workflow to use new Go-based cleanup command

Comments suppressed due to low confidence (1)

pkg/cleanup/cleanup_test.go:48

Test creates AWS resources without proper cleanup or mocking. This test will fail in environments without AWS credentials and may create actual AWS resources. Consider using mocks or dependency injection to test the cleaner creation logic without requiring real AWS credentials.

	cleaner, err := New(log, "us-east-1")

Copilot · 2025-07-30T14:36:54Z

pkg/cleanup/cleanup.go

+	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/aws/aws-sdk-go/aws"


Mixed AWS SDK imports detected. You're importing both v2 (github.com/aws/aws-sdk-go-v2/...) and v1 (github.com/aws/aws-sdk-go/aws) packages. Consider using only v2 SDK imports for consistency and to avoid potential conflicts.

Suggested change

"github.com/aws/aws-sdk-go/aws"

// Removed unused v1 SDK import

Copilot · 2025-07-30T14:36:55Z

pkg/cleanup/cleanup.go

+				Name:   aws.String("resource-id"),
+				Values: []string{vpcID},
+			},
+			{
+				Name:   aws.String("key"),


Using v1 SDK aws.String() with v2 SDK types. Since you're using v2 SDK throughout, this should be a string pointer using & operator or v2 equivalent: Name: &"resource-id"

Suggested change

Name: aws.String("resource-id"),

Values: []string{vpcID},

},

{

Name: aws.String("key"),

Name: &"resource-id",

Values: []string{vpcID},

},

{

Name: &"key",

Copilot · 2025-07-30T14:36:55Z

pkg/cleanup/cleanup.go

+				Name:   aws.String("resource-id"),
+				Values: []string{vpcID},
+			},
+			{
+				Name:   aws.String("key"),


Using v1 SDK aws.String() with v2 SDK types. Since you're using v2 SDK throughout, this should be a string pointer using & operator or v2 equivalent: Name: &"key"

Suggested change

Name: aws.String("resource-id"),

Values: []string{vpcID},

},

{

Name: aws.String("key"),

Name: &"resource-id",

Values: []string{vpcID},

},

{

Name: &"key",

Copilot · 2025-07-30T14:36:55Z

pkg/cleanup/cleanup.go

+	input := &ec2.DescribeInstancesInput{
+		Filters: []types.Filter{
+			{
+				Name:   aws.String("vpc-id"),


Using v1 SDK aws.String() with v2 SDK types. This pattern appears multiple times throughout the file and should use v2 SDK string pointer syntax consistently.

coveralls · 2025-07-30T14:37:38Z

Pull Request Test Coverage Report for Build 16627726132

Details

7 of 355 (1.97%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-2.0%) to 12.277%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/cleanup/cleanup.go	7	355	1.97%

Totals
Change from base Build 16625221596:	-2.0%
Covered Lines:	268
Relevant Lines:	2183

💛 - Coveralls

Migrate AWS VPC cleanup functionality from bash script to a native Go package and CLI command, improving maintainability, error handling, and integration with the holodeck toolchain. BREAKING CHANGE: The cleanup functionality is now available through `holodeck cleanup` command instead of scripts/awscleanup.sh Changes: - Add pkg/cleanup package with comprehensive AWS resource deletion - Handles EC2 instances, security groups, subnets, route tables, IGWs - Includes GitHub job status checking via API - Implements retry logic for VPC deletion (3 attempts, 30s delay) - Better error handling with partial failure support - Add `holodeck cleanup` CLI command - Accepts multiple VPC IDs in a single invocation - --region flag for AWS region specification - --force flag to skip GitHub job status checks - Comprehensive help documentation - Update periodic GitHub workflow - Now uses `holodeck cleanup` instead of bash script - Builds Go binary as part of the workflow - Better error handling and logging - Handles multiple VPCs more efficiently - Add documentation - docs/commands/cleanup.md with detailed usage examples - Updated README.md with cleanup examples - Updated command reference documentation - Add deprecation warning to scripts/awscleanup.sh - Script still functional but warns users to migrate - Can be removed in a future release Benefits: - Type safety and better error handling - Consistent with other holodeck commands - Easier to test and maintain - More efficient handling of multiple VPCs - Better integration with CI/CD pipelines Fixes: Improves reliability of periodic cleanup jobs Refs: Migration from bash to Go for better maintainability Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>

shivakunv · 2025-08-14T17:13:34Z

.github/workflows/periodic.yaml

+          cache: true
+
+      - name: Build holodeck CLI
+        run: make build-cli


Can we download a stable release instead of building it .

ArangoGutierrez requested review from shivakunv and Copilot July 30, 2025 14:35

ArangoGutierrez self-assigned this Jul 30, 2025

ArangoGutierrez requested review from elezar, tariq1890 and cdesiniotis as code owners July 30, 2025 14:35

Copilot AI reviewed Jul 30, 2025

View reviewed changes

ArangoGutierrez force-pushed the cleanup_cmd branch 2 times, most recently from 47029bc to 2f2dc95 Compare July 30, 2025 15:46

ArangoGutierrez force-pushed the cleanup_cmd branch from 2f2dc95 to c27dbd6 Compare July 30, 2025 16:04

shivakunv reviewed Aug 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: replace bash cleanup script with native Go implementation #433

feat: replace bash cleanup script with native Go implementation #433

Uh oh!

ArangoGutierrez commented Jul 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 30, 2025

Uh oh!

Copilot AI Jul 30, 2025

Uh oh!

Copilot AI Jul 30, 2025

Uh oh!

Copilot AI Jul 30, 2025

Uh oh!

coveralls commented Jul 30, 2025 •

edited

Loading

Uh oh!

shivakunv Aug 14, 2025

Uh oh!

Uh oh!

	"github.com/aws/aws-sdk-go/aws"
	// Removed unused v1 SDK import

feat: replace bash cleanup script with native Go implementation #433

Are you sure you want to change the base?

feat: replace bash cleanup script with native Go implementation #433

Uh oh!

Conversation

ArangoGutierrez commented Jul 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 16627726132

Details

💛 - Coveralls

Uh oh!

shivakunv Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Jul 30, 2025 •

edited

Loading