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: Reflect the changes in the ebs_csi driver #326

Merged

Conversation

prysmakou-sa
Copy link
Contributor

@prysmakou-sa prysmakou-sa commented Jan 16, 2023

Description

modules/iam-role-for-service-accounts-eks -
EBS-CSI driver policy - fix condition string for DeleteVolume action to reflect the changes in the upstream

Motivation and Context

Issue - #325
The change in the upstream - kubernetes-sigs/aws-ebs-csi-driver#1450

Breaking Changes

There are no breaking changes

How Has This Been Tested?

  • I have executed pre-commit run -a on my pull request
  • tested on the EKS 1.23, Chart: aws-ebs-csi-driver-2.11.1, the driver version: 1.11.3

@prysmakou-sa prysmakou-sa changed the title Fix ebs_csi to reflect the changes in the upstream Fix: ebs_csi to reflect the changes in the upstream Jan 16, 2023
@prysmakou-sa prysmakou-sa changed the title Fix: ebs_csi to reflect the changes in the upstream fix: ebs_csi to reflect the changes in the upstream Jan 16, 2023
@prysmakou-sa prysmakou-sa changed the title fix: ebs_csi to reflect the changes in the upstream fix: Reflect the changes in the ebs_csi driver Jan 16, 2023
@bryantbiggs
Copy link
Member

so the change looks great - makes sense. However, for users who are using versions < 2.14.1, this will break their permissions - *I think.

So we'll need to somehow factor that into the change - I don't know if the CSI drivers still support the old route with the cluster owned, open to ideas though

@prysmakou-sa
Copy link
Contributor Author

prysmakou-sa commented Jan 17, 2023

@bryantbiggs just to be on the safe side we could let loose the permissions for a while: to save the current/former statement and add the new one. I mean not to replace the condition but to add an additional one. What do you think?

Instead of replacing the condition we add new to support (for a while)
users of older versions of the EBS driver.
@prysmakou-sa
Copy link
Contributor Author

I have updated the PR so now we do not replace the condition but add the new statement.

@antonbabenko antonbabenko merged commit cadfe47 into terraform-aws-modules:master Jan 19, 2023
antonbabenko pushed a commit that referenced this pull request Jan 19, 2023
### [5.11.1](v5.11.0...v5.11.1) (2023-01-19)

### Bug Fixes

* Reflect the changes in the ebs_csi driver ([#326](#326)) ([cadfe47](cadfe47))
@antonbabenko
Copy link
Member

This PR is included in version 5.11.1 🎉

@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 Feb 19, 2023
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.

3 participants