Skip to content

Conversation

erezzarum
Copy link
Contributor

@erezzarum erezzarum commented Sep 11, 2025

Description

Karpenter supports for Capacity Reservation introduce a new IAM permissions, a previous PR resolved this but was missing the ec2:DescribeCapacityReservations permission.

Motivation and Context

To allow users use Karpenter with Capacity Reservation

Changes from the following upstream PRs:

Breaking Changes

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@bryantbiggs bryantbiggs changed the title fix: Update Karpenter controller policy to support CR feat: Update Karpenter controller policy and permissions to match upstream project Sep 11, 2025
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

Looks like there's been a few other changes so added those in to re-align with upstream. Thank you!

@bryantbiggs bryantbiggs merged commit 131db39 into terraform-aws-modules:master Sep 11, 2025
22 checks passed
antonbabenko pushed a commit that referenced this pull request Sep 11, 2025
## [21.2.0](v21.1.5...v21.2.0) (2025-09-11)

### Features

* Update Karpenter controller policy and permissions to match upstream project ([#3510](#3510)) ([131db39](131db39))
@antonbabenko
Copy link
Member

This PR is included in version 21.2.0 🎉

@davivcgarcia
Copy link

davivcgarcia commented Sep 16, 2025

Hi @bryantbiggs! Not sure why, but I see that ec2:DescribeCapacityReservations was removed in your second commit on this PR. So, the version 21.2.0 and 21.3.0 still lacks required permissions to handle On-Demand Capacity Reservation. Could you please take a look again?

sid = "AllowRegionalReadActions"
resources = ["*"]
actions = [
"ec2:DescribeCapacityReservations",

Choose a reason for hiding this comment

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

@bryantbiggs Why was this permission removed?

Copy link
Member

Choose a reason for hiding this comment

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

@davivcgarcia in the time its taken you to post 3+ different messages you could have simply opened a PR to fix this error - try to put a little effort in would ya

Choose a reason for hiding this comment

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

Sorry for that, but I didn't know if that was in purpose or not. :-(

Copy link
Member

Choose a reason for hiding this comment

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

clearly not - we want to align with the upstream policy but there was an error made in aligning the permissions

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