minamijoyo / tfupdate

Update version constraints in your Terraform configurations
MIT License
539 stars 23 forks source link

tfupdate module command support regex matches #108

Closed fr12k closed 5 months ago

fr12k commented 6 months ago

The match parameter of the tfupdate module command supports regex matches.

tfupdate module -v 2.14.1 terraform-aws-modules/s3-bucket/* main.tf

What do you think of this feature ?

minamijoyo commented 6 months ago

Hi @fr12k, Thank you for your suggestion!

Just by looking at it for a bit, it should be an opt-in flag, at least because the change breaks backward compatibility. Anyway, could you describe the use case first for adding new features? It helps me understand what is the best way.

fr12k commented 6 months ago

Hi @minamijoyo,

the use case is we have a terraform module Github repository that host multiple modules.

For example.

module "network" {
   source = "git@github.com:fr12k/terraform-modules.git//network?ref=v109"
}

module "vpc" {
   source = "git@github.com:fr12k/terraform-modules.git//vpc?ref=v109"
}

We would like to update both module references from above with just one command because we have 50+ modules in that repository. Instead of writing 50+ tfupdate commands we would like to update all references with one command.

That for example is possible with the following regex.

tfupdate module -v 214 git@github.com:fr12k/terraform-modules.git/* -r .

We can implement that feature in a way that it is backward compatible. For example let's add a new option '-t' that specify the match type.

tfupdate module -v 214 -t full git@github.com:fr12k/terraform-modules.git/vpc -r .
tfupdate module -v 214 -t prefix git@github.com:fr12k/terraform-modules.git/ -r .
tfupdate module -v 214 -t regex git@github.com:fr12k/terraform-modules.git/* -r .

To ensure backward compatibility the default value for the match type option is full

I can change the Pull Request to implement that new option match type if you agree with it. We really like your tfupdate project and want to replace our internal develop tool with it.

Have a nice day

minamijoyo commented 5 months ago

Thank you for the explanation. It makes sense to me, and I'm open to accepting the new feature to support your use case.

I have some random thoughts about the user interface design, so please give me time to investigate and think. I prefer to keep consistency with the existing design and potential future requests as much as possible.

minamijoyo commented 5 months ago

First, the scope of this problem is specific to modules because providers are typically served as one provider per repository. Modules often serve as a mono-repo that stores multiple modules in one repository. Possible variants of module source syntax can be found here: https://developer.hashicorp.com/terraform/language/modules/sources

Regarding the syntax of the regular expression, you may assume that * means a wildcard, but there are several dialects of regular expressions; Go's standard regexp library uses the RE2 syntax: https://pkg.go.dev/regexp

In addition, the tfupdate already uses regular expressions in the --ignore-path flag, which uses Go's standard regexp. Therefore, I'd like to use the same RE2 syntax in new features for consistency unless there is no strong reason.

In RE2 syntax, . matches any single character, and /* is interpreted as zero or more repetitions of /. Looking at your use case of git@github.com:fr12k/terraform-modules.git/*, it can match both of the following by coincidence:

FYI: online tester results by https://regex101.com/

image

In general, escaping special characters is correct, but in practice, the variations of the module source attribute are limited, so the intended string may be matched even without escaping. A set of all special characters are \.+*?()|[]{}^$.

Therefore, I don't think it is necessary to implement prefix matching, at least in the initial implementation, since regex matching can be used like prefix matching in simple cases, and it also has more flexibility when strictness is needed.

Regarding the new flag, it could be a bool opt-in flag, but I agree with the idea of declaring it as a string flag to leave room for future extensions. However, I hesitate to consume a shortened flag that may only be used by a few users. I prefer to start from a long-style flag like --source-match-type=regex. If it proves helpful to many users, then it is worth revisiting.

With that in mind, my proposal is as follows:

For example, optimistically behaves like a prefix:

$ tfupdate module -v 214 --source-match-type=regex git@github.com:fr12k/terraform-modules.git/ -r .

Or strict escaped version:

$ tfupdate module -v 214 --source-match-type=regex ^git@github\.com:fr12k/terraform-modules\.git//.+ -r .

What do you think?

fr12k commented 5 months ago

First of all thanks for taking the time and the effort to think about this new regex feature with this level of detail.

Totally agree to this proposal

With that in mind, my proposal is as follows:

  • Introduce a new opt-in --source-match-type=regex flag, which allows us to match multiple module source addresses with a regular expression in RE2 syntax.
  • To keep backward compatibility, the --source-match-type flag defaults to full.
  • Implementing prefix matching is unnecessary because regex matching is expected to behave similarly to prefix matching in simple use cases.

Will provide the code changes based on this proposal in the next days.

fr12k commented 5 months ago

Hi how are you today ?

today i had the time to prepare the first draft for the implementation. I hope i din't miss anything. When you have time let me know what should be changed.

Have a nice day.

minamijoyo commented 5 months ago

@fr12k Thank you for working on this! I've left some review comments. Please check them. If you have any questions, feel free to ask anything.

fr12k commented 5 months ago

Hi i addressed all the review comments. Have a look again and maybe you have some idea to make it more beautiful.

Have a nice day.

minamijoyo commented 5 months ago

@fr12k We're almost there. I pointed out one more thing that needs to be fixed before merging.

minamijoyo commented 5 months ago

@fr12k Thank you for your contributions! I've cut a new release v0.8.1 🚀

fr12k commented 5 months ago

Thanks for your patience and the work you put in tfupdate. Keep going.