Update hashicorp/terraform-provider-aws version

[ialidzhikov]

/platform aws
/kind bug

We have the following issues reported in the upstream terraform-provider-aws:

• Error finding route after creating it: Unable to find matching route for Route Table (<rtb_id>) and destination CIDR block (<destionation_cidr>) hashicorp/terraform-provider-aws#12073 - fixed in 3.34.0
• resource/aws_subnet: an unexpected new value for was present, but now absent. hashicorp/terraform-provider-aws#12829 - fixed in 3.34.0
• [UPDATE] aws_default_security_group cannot be destroyed when vpc is manually deleted hashicorp/terraform-provider-aws#19965 - fixed in 3.50.0

They continuously reproduce on our environments and require manual ops effort to be fixed.
It seems that now both of the issues are fixed (hopefully) in 3.34.0 (see CHANGELOG). So we should update to terraform-provider-aws version >= 3.34.0 to adopt these fixes.

However terraform-provider-aws >= 3.29.1 requires additional permission that is currently not part of our recommended AWS IAM policy - see https://github.com/gardener/gardener-extension-provider-aws/blob/v1.22.2/docs/usage-as-end-user.md#permissions. The corresponding upstream change is hashicorp/terraform-provider-aws#5904 - it requires additional permission iam:ListRolePolicies to be able to successfully apply an aws_iam_role resource.

The required change to the AWS IAM policy:

diff --git a/docs/usage-as-end-user.md b/docs/usage-as-end-user.md
index 8e2bec76..eb94fe26 100644
--- a/docs/usage-as-end-user.md
+++ b/docs/usage-as-end-user.md
@@ -61,6 +61,7 @@ Please make sure that the provided credentials have the correct privileges. You
           "iam:GetRole",
           "iam:GetRolePolicy",
           "iam:ListPolicyVersions",
+          "iam:ListRolePolicies",
           "iam:ListAttachedRolePolicies",
           "iam:ListInstanceProfilesForRole",
           "iam:CreateInstanceProfile",

So before update to terraform-provider-aws >= 3.29.1 we need to update our recommended AWS IAM policy that is currently maintained in few separate places:

• gardener/dashboard -> Apply the upcoming changes to the AWS IAM policy dashboard#1007
• gardener/gardener-extension-provider-aws -> Adapt the IAM policy for terraformer-provider-aws >= 3.29.1 #322

We should also treat this AWS IAM policy change as breaking one and we need to document in properly in the release notes.

As hashicorp/terraform-provider-aws is under active development, terraform-provider-aws@latest can potentially have other breaking changes. So that's why I propose to first update to 3.32.0 to tackle the breaking change in the AWS IAM policy. Then we can work on updating to terraform-provider-aws@latest which will include the above bug fixes.

Tasks related to g/terraformer (update to terraform-provider-aws@3.32.0):

• Update to terraform-provider-aws@3.32.0 -> Update hashicorp/terraform-provider-aws terraformer#98
• Cut a g/terraformer release with the above change (if possible, cut a standalone release that contains only the terraform-provider-aws version bump - to make any potential roll-backs easier) -> https://github.com/gardener/terraformer/releases/tag/v2.8.0
• Adopt the g/terraformer release in g/provider-aws -> [ci:component:github.com/gardener/terraformer:v2.7.0->v2.8.0] #380
• Cut a g/provider-aws release with terraform-provider-aws@3.32.0 -> https://github.com/gardener/gardener-extension-provider-aws/releases/tag/v1.27.0

Tasks related to g/terraformer (update to terraform-provider-aws version >= 3.34.0):

• Update to terraform 0.12.31 -> Update hashicorp/terraform terraformer#94, [ci:component:github.com/gardener/terraformer:v2.6.0->v2.7.0] #350
• Update to terraform-provider-aws >= 3.34.0 (if possible terraform-provider-aws@latest) -> Update hashicorp/terraform-provider-aws terraformer#101
• Cut a g/terraformer release with the above change (if possible, cut a standalone release that contains only the terraform-provider-aws version bump - to make any potential roll-backs easier) -> https://github.com/gardener/terraformer/releases/tag/v2.10.0
• Adopt the g/terraformer release in g/provider-aws -> [ci:component:github.com/gardener/terraformer:v2.9.0->v2.10.0] #392
• Cut a g/provider-aws release with terraform-provider-aws >= 3.34.0 -> https://github.com/gardener/gardener-extension-provider-aws/releases/tag/v1.28.0

[rfranzke]

Thanks for opening the issue @ialidzhikov. As this is a breaking change for end-users, I'm thinking about having some fallback mechanism to the current provider plugin version in case the user's account does not include the required new permission. Do you think something like would be reasonable to prevent too many clusters from becoming stuck in a failed state, or would you rather go the "standard" way (operators announcing the change so that end-users can adapt until a certain time, and only afterwards the version is updated)?

[ialidzhikov]

Such fallback mechanism can be implemented using the operation in question - try to ListRolePolicies, if the operation fails assume that the current user does not have the new permission -> fallback to the old terraformer version.
My initial idea was to give end-user enough period of time after announcing the upcoming breaking change (at least 2-3 months). And proactively check that most of them already adapted their policies before we roll-out the new terraformer.

To be honest I am not sure. What would we gain from such fallback mechanism? I assume that there will be always folks that will miss the action required and will potentially update after the Infrastructure reconciliation is failing. I rather see complexity that will be added on top:

• We will have to "maintain" 2 terraform-provider-aws versions
• We have to make sure that the update from terraformer@legacy to terraformer@latest is always possible and works without any issues. You known that there are many restrictions coming into the game - don't skip minor version when you update the terraform version; update to the latest patch release of the current minor version, before you update to the next minor version, etc.

So I would rather vote for "announce the upcoming change; give enough period for adaptation; actively monitor the adaptation and roll-out when we are sure".

[rfranzke]

Yeah, I agree, let's do it as usual. I was just brain-storming about potential other ways how to deal with this situation and wanted to get some feedback. The points you raised are very valid and there is no good reason to not follow the usual approach.

[vpnachev]

Should we add an empty commit with a breaking release note that we are planning to upgrade the terraform-provider-aws in June/July/August and it requires additional permissions in the AWS IAM? Or/and we should send a notification to https://groups.google.com/g/gardener and https://kubernetes.slack.com/archives/CB57N0BFG?

[ialidzhikov]

Or/and we should send a notification to https://groups.google.com/g/gardener and https://kubernetes.slack.com/archives/CB57N0BFG?

Ref https://groups.google.com/g/gardener/c/sIwiQp6ak_4/m/y2mUd3_lAAAJ and https://kubernetes.slack.com/archives/CB57N0BFG/p1619106318319700

[ialidzhikov]

I am closing this issue as all subtasks are completed (11/11).

/close