Skip to content

Support Graviton portscan instances#897

Merged
mcdonnnj merged 1 commit intodevelopfrom
improvement/allow_arm64_portscanners
Sep 18, 2025
Merged

Support Graviton portscan instances#897
mcdonnnj merged 1 commit intodevelopfrom
improvement/allow_arm64_portscanners

Conversation

@mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Sep 18, 2025

🗣 Description

Add the ability spin up Graviton-based portscan instances in addition to x86_64 ones.

💭 Motivation and context

As part of our move to use Graviton-based instances (mainly for cost-benefits), and with the addition of ARM64-based AMIs added in #867, we can use portscan instances running on Graviton instances due to their straightforward configuration. Any Graviton-based instances will be deployed after any x86_64-based instances in sequence. Thus, with the configuration

nmap_instance_count = {
  arm64  = 1
  x86_64 = 1
}

portscan1 will be x86_64 and portscan2 will be ARM64. This method was chosen because it is of minimal impact to our existing configuration and helper scripts.

Warning

This is a breaking change because the format of the nmap_instance_count variable has changed from a number to an object where each key is an architecture and the value is the number of instances using that architecture.

🧪 Testing

Automated tests pass. I deployed Graviton-based portscan instances in my testing environment both in addition to x86_64 instances and exclusively. I verified that they received work and were able to process it successfully.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
  • All new and existing tests pass.

@mcdonnnj mcdonnnj self-assigned this Sep 18, 2025
@mcdonnnj mcdonnnj added the breaking change This issue or pull request involves changes to existing functionality label Sep 18, 2025
@mcdonnnj mcdonnnj added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use hacktoberfest-accepted Pull request that should count toward Hacktoberfest participation labels Sep 18, 2025
@github-actions github-actions bot added documentation This issue or pull request improves or adds to documentation terraform Pull requests that update Terraform code labels Sep 18, 2025
@mcdonnnj mcdonnnj requested review from a team and Copilot September 18, 2025 18:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for Graviton (ARM64) instances for the portscan service alongside existing x86_64 instances. This is a breaking change that modifies the nmap_instance_count variable from a simple number to an object specifying counts per architecture.

  • Changed nmap_instance_count variable from number to object with arm64 and x86_64 keys
  • Added separate AMI data sources for ARM64 and x86_64 architectures
  • Updated instance provisioning to use appropriate AMI and instance type based on architecture
  • Modified all count references to use the total calculated from both architectures

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
terraform/variables.tf Updates nmap_instance_count variable type and description
terraform/locals.tf Adds calculation for total nmap instance count across architectures
terraform/cyhy_nmap_ec2.tf Implements dual-architecture AMI selection and instance type logic
terraform/cyhy_nmap_cloud_init.tf Updates cloud init count to use total instance count
terraform/cyhy_mongo_ec2.tf Updates commander configuration to use total instance count
terraform/README.md Updates documentation to reflect variable type changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍

Add the ability to specify the number of ARM64 and AMD64 `portscan`
instances to deploy. The current configuration will deploy AMD64-based
instances first and then ARM64-based instances. This was done to make
the implementation as easy as possible given all of the other elements
of the configuration that interact with these instances.
@mcdonnnj mcdonnnj force-pushed the improvement/allow_arm64_portscanners branch from f305789 to 1d210fe Compare September 18, 2025 20:22
@mcdonnnj mcdonnnj merged commit 24386be into develop Sep 18, 2025
18 checks passed
@mcdonnnj mcdonnnj deleted the improvement/allow_arm64_portscanners branch September 18, 2025 20:41
@mcdonnnj
Copy link
Member Author

@cisagov/vm-dev I have updated the production tfvars file to align with the changes in this pull request (specifically the change to the nmap_instance_count variable).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change This issue or pull request involves changes to existing functionality documentation This issue or pull request improves or adds to documentation hacktoberfest-accepted Pull request that should count toward Hacktoberfest participation improvement This issue or pull request will add or improve functionality, maintainability, or ease of use terraform Pull requests that update Terraform code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants