Skip to content

feat: add ami-updater lambda function with tests and config #4488

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

dgokcin
Copy link

@dgokcin dgokcin commented Mar 20, 2025

Summary

This PR introduces a new AMI Updater Lambda function with comprehensive configuration, testing, and infrastructure setup.

Changes

  • Added AMIManager class with methods to handle AMI updates
  • Added unit tests for AMIManager methods and Lambda handler
  • Added configuration management for environment variables
  • Added ESLint and TypeScript configuration
  • Added Terraform configuration for Lambda infrastructure
  • Added IAM policies and CloudWatch permissions
  • Added README documentation for the AMI Updater module
  • Added package.json with build and test scripts
  • Added local run function for testing
  • Added Vitest configuration for testing

Additional Notes

  • This is the first time I dive this deep into this codebase so I probably messed up, thats why the PR is still a draft.

dgokcin added 4 commits March 20, 2025 14:02
- create new terraform module for ami updater lambda function
- add comprehensive documentation in readme
- implement all required resources including lambda, iam, eventbridge, and ssm
- add variables for flexible configuration
- include outputs for module integration
- implement core ami updater logic with ec2 and ssm integration
- add comprehensive unit tests for all components
- configure eslint and typescript for code quality
- include vitest for testing framework
- add aws powertools for logging and metrics
- implement dry run mode for safe testing
- add error handling and logging throughout
@dgokcin dgokcin requested a review from a team as a code owner March 20, 2025 13:17
@dgokcin dgokcin marked this pull request as draft March 20, 2025 13:19
dgokcin added 6 commits March 20, 2025 19:52
- add s3 bucket, key, and version variables for lambda deployment
- make lambda zip path configurable via variable
- add support for additional lambda tags
- update lambda resource to handle both local and s3 deployments
- modify dist script to create zip file in dist/ami-updater.zip instead of root directory
- ensure zip file location matches terraform module's expected path
…ling

- update zip file location in package.json to match other modules
- add ami-updater module configuration in root main.tf with consistent variable mapping
- add ami_updater_lambda variables in root variables.tf for zip, memory and timeout
- enable optional ami-updater functionality through enable_ami_updater variable
@npalm
Copy link
Member

npalm commented Mar 31, 2025

Created an PR to get the AMI ID via SSM supported via the launch template. You feedback would be very welcome. I think once we have that better done in the core. An auto update like this PR would be a welcome additon. See #4517

@dgokcin
Copy link
Author

dgokcin commented Apr 1, 2025

Hey @npalm!

I’ve added a single question about your preference for using the arn over the parameter name in issue #4517. Considering backward compatibility is your priority, I believe this PR (with some potential changes) aligns well with your changes and could enable automated AMI updates. (This would address my use case, but I’m not sure if it’s a common one.)

npalm added a commit that referenced this pull request Apr 15, 2025
# Description

This PR migrates the launch template to use SSM to foor looking up AMI
ID. This PR is inteded to migrate to use SSM for AMI lookups. In later
PR we will remove logic from scale-up and remove old parateters.

By default, the module still uses the same AMI filters, but difference
is the resulting AMI ID is stored in SSM instead of the launch template.
As alternative the module user can provide `ami_id_ssm_parameter_arn`
pointing to an SSM parameter containing the AMI ID to be used (data type
aws:ec2:image).

When using a custom managed AMI this will avoid drift in terraform when
`ami.id_ssm_parameter_arn` is used instead of the deprecated
`ami_id_ssm_parameter_name`. Another way to avoid the drift is to ensure
the filter is poiting to the same image. See also #4460

In PR #4488 and updater via Lambda is introduced. I think we should firt
think how we can better support the AMI id in the core. What this PR is
supposed todo. NExt the update can be an nice way to auto update AMI's
without running terraform.

The PR also refactor the variables for `ami_*` and move the to a
dedicated object `ami`. The old ones remains till the next major
release. Deprecattion warnings are added to the output.

## Notes

- Adjuest multi runner examples
- Test with multi runners
- Test with default runners + pool

## Fix

- Deprecation warning inline_poilicy for pool

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: github-aws-runners-pr|bot <github-aws-runners-pr[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: runners-releaser[bot] <194412594+runners-releaser[bot]@users.noreply.github.com>
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.

2 participants