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

fix: Insufficient permissions for karpenter policy when not using karpenter discovery tags on security group #294

Conversation

apamildner
Copy link
Contributor

@apamildner apamildner commented Oct 26, 2022

Fixes an issue where the policy was not sufficient if you use security group ids in the Karpenter AWSNodeTemplate instead of relying on discovery tags. Then the security groups weren't tagged, which led to insufficient permissions for Karpenter to launch the node.

Description

Removes the need for the security group to be tagged with the Karpenter discovery tag
for Karpenter to be able to use it when launching an instance.

Motivation and Context

If we create an AWSNodeTemplate like this:

apiVersion: karpenter.k8s.aws/v1alpha1
kind: AWSNodeTemplate
metadata:
  name: default
spec:
  subnetSelector:
    aws-ids: subnet-abc,subnet-xyz
  securityGroupSelector:
    aws-ids: sg-abc,sg-xyz
  tags:
    karpenter.sh/discovery/foo-cluster: foo-cluster

which is perfectly valid according to Karpenter docs, we will not
be able to use the module policy as is, since the security group we are referring to doesn't have the correct discovery tag (since we don't want to use discovery tags).

The change here changes the policy such that this constraint is relaxed. This makes the security group part of the policy behave like the subnet part, which wasn't dependent on the tag being present.

Breaking Changes

No breaking changes, just a little bit more permissive policy.

How Has This Been Tested?

  • 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
  • I have updated the policy manually and verified in karpenter

…y group ids in the karpenter provisioners instead of relying on discovery tags. Then the security groups wasn't tagged, which led to insufficient permissions
@bryantbiggs bryantbiggs changed the title Fix: Insufficient permissions for karpenter policy when not using karpenter discovery tags fix: Insufficient permissions for karpenter policy when not using karpenter discovery tags on security group Oct 26, 2022
@bryantbiggs
Copy link
Member

change looks good - we'll get the tflint warnings fixed first #296 and then merge this to keep the CI green

@bryantbiggs bryantbiggs merged commit 5ad496b into terraform-aws-modules:master Oct 26, 2022
antonbabenko pushed a commit that referenced this pull request Oct 26, 2022
### [5.5.4](v5.5.3...v5.5.4) (2022-10-26)

### Bug Fixes

* Insufficient permissions for karpenter policy when not using karpenter discovery tags on security group ([#294](#294)) ([5ad496b](5ad496b))
@antonbabenko
Copy link
Member

This PR is included in version 5.5.4 🎉

@apamildner
Copy link
Contributor Author

Thanks for the quick response on mr PR's! Much appreciated 🥳

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insufficient permissions when using karpenter irsa without discovery tags
3 participants