feat: Add support for AWS provider 5.0

[MaxymVlasov]

Summary

We'd like to use AWS provider 5.0

Remove higher limit, as specified in TF best practices for this case.

How did you test this change?

terraform plan for our infrastructure

module "aws_config" {
  # source  = "lacework/config/aws"
  # version = "0.9.0"
  source = "./terraform-aws-config"

  count = module.this.enabled ? 1 : 0

  lacework_integration_name = "${module.this.tags.Name}-cfg"
  iam_role_name             = module.this.id
}

module "cloudtrail" {
  # https://github.com/lacework/terraform-aws-cloudtrail
  # source  = "lacework/cloudtrail/aws"
  # version = "2.6.0"
  source = "./terraform-aws-cloudtrail"

  count = module.this.enabled && var.cloudtrail_enabled ? 1 : 0

  lacework_integration_name = "${module.this.tags.Name}-ct"
  use_existing_iam_role     = true
  iam_role_arn              = module.aws_config[0].iam_role_arn
  iam_role_external_id      = module.aws_config[0].external_id
  iam_role_name             = module.aws_config[0].iam_role_name

  bucket_name               = module.this.id
  cloudtrail_name           = module.this.id
  cross_account_policy_name = module.this.id
  log_bucket_name           = "${module.this.id}-log"
  sns_topic_name            = module.this.id
  sqs_queue_name            = module.this.id
}

Issue

Relates to lacework/terraform-aws-config#65
Close #124
Close #125

[dmurray-lacework]

Hi @MaxymVlasov thank you for submitted this PR. @jrobison-sb has the same change with a different approach to version pinning.
lacework/terraform-aws-config#64

The previous approach we took was to change version pinning to ~> 4.0 and release major version increase of the module. Eg. the change to support aws provider "
~> 5.0 would be in terraform-aws-cloudtrail v3.0.0

[MaxymVlasov]

The previous approach we took was to change version pinning to ~> 4.0 and release major version increase of the module.

@dmurray-lacework I suggest taking a look at https://semver.org/ and https://developer.hashicorp.com/terraform/language/expressions/version-constraints#terraform-core-and-provider-versions

Because mentioned approach violate both of them

[afiune]

Hey @MaxymVlasov - We really appreciate your help with these PR's, I see we are talking about two different things here;

1. Terraform versioning best practices - Where, for Root modules it is encouraged to use a ~> constraint to set both a lower and upper bound on versions for each provider, and for Reusable modules it should constrain only their minimum allowed versions. We have been considering this to be a Root module but it sounds that we should change our approach and consider this a Reusable module. Is that a correct statement?

2. Semantic versioning in general - We agree that, not changing the lower-version constraint should be consider a minor change, but as mentioned in the previous point, we have been using the ~> constraint and if we apply a change say from ~> 4.0 to ~> 5.0, that is considered a major change since we are dropping support for 4.X.Y.

We like the approach you are suggesting, but before moving forward with it, we have one more scenario where we would like your opinion on.

Say we accept this change and;

• We release v2.7.0 of this module with aws = ">= 4.0"
• AWS releases a new major version (6.0.0) that breaks this module
• We create a change to set an upper-version constraint to aws = ">= 4.0, < 6.0.0"
• Would that ☝🏽 be a patch? minor? or major? version bump 🤔

[MaxymVlasov]

@afiune Usually AWS provider first deprecates some fields/resources and provides replacement during one of the minor releases, and only the next major version removes deprecated stuff.

So in that case, if you still have some deprecated stuff, you do next:

1. replace deprecated stuff (and use a moved block if needed)

2. Check if new fields/resources exist in the version specified in the lower contains version, just by looking in the docs

3. If exist - it's a patch.
   If not exist - you need to bump up lower constraints and that's breaking change because you will break the module for folks that still use outdated provider versions.
   3.1. if you'd like, before releasing breaking change, you can set

   We create a change to set an upper-version constraint to aws = ">= 4.0, < 6.0.0"

   as patch version and the last release for the current major release and only then go forward to >= 6.0

And let's imagine that we are unlucky and the AWS provider for some resource/field made breaking changes, without any "graceful migration period" - that's really bad but time to time happens. For that, we use the same algorithm above and release breaking change.

Now, when we described all these conditions, here simpler to answer why in the situation specified at the end of your comment, we just need to release a patch version - because it (p3.1) just Fix module to current users, which:

• for some reason violates TF best practices and does not pin their aws provider version yet
• would like to go forward to use a new provider

That should decrease maintenance work from the maintainers' side and decrease struggle from the users' side in most cases.

Here is the only one vote for specifying a higher constraint for child/reusable modules that I could imagine - when your users are both noobs in TF and programming best practices and had terrible search skills when they can find module repo and open issues where they just copy-paste error, but not able to open and read the documentation. If that too bothers you - then yes, better specify higher constraint preventively with the hope that it will decrease the number of issues

[MaxymVlasov]

1. Yes. All stuff published to terraform registry by definition is reusable. So it's a child module.
   Also, after TF1.4 going to live - Root modules are require terraform lock files for work (if it is not explicitly disabled via env var), and better to commit them to the repo.
2. Don't see reasons why you should drop prev provider support only because the new provider version was released.
   In projects where I'm a maintainer, I try to not make breaking changes until we can't avoid them - that provides a better user experience.

[afiune]

@MaxymVlasov You are spot on!

We completely agree with the process you described above, thank you for such a detailed response. We
will move forward with your changes and release a new version of the module!

Again, thank you for your help! 💯

[afiune]

Make it so (run the pipeline)

[afiune]

[MaxymVlasov]

@afiune Any time
Also, found that you using not strict pining for modules, so leave a few observations about it in #128