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

feat: Add support for version constraints in tfupdate #436

Closed
wants to merge 1 commit into from

Conversation

MaxymVlasov
Copy link
Collaborator

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

Fixes #388

How can we test changes

  • pre-commit-config.yaml:
repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: 8e7a029eb1f796f1ff60ad8e07b911466381cdd1
  hooks:
    - id: tfupdate
      name: Autoupdate AWS provider versions
      args:
        - --args=--version '~> 4.2.0'
        - --args=provider aws

    - id: tfupdate
      name: Autoupdate Helm provider versions
      args:
        - --args=provider helm
    - id: tfupdate
      name: Update Google provider to specified version
      args:
        - --args=provider google
        - --args=--version ">= 4.18.0, < 5"
    - id: tfupdate
      name: Update Terraform provider to specified version
      args:
        - --args=terraform
        - --args=--version "> 1.1.8"

versions.tf

terraform {
  required_version = ">= 0.14"
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = ">= 3.0.0"
    }
    helm = {
      source  = "hashicorp/helm"
      version = "2.6.0"
    }
    google = {
      source  = "google/google"
      version = "2.7.0"
    }

  }
}

@MaxymVlasov MaxymVlasov added feature New feature or request estimate/2h Need 2 hours to be done hacktoberfest-accepted HacktoberFest'21 and '22 hook/tfupdate Bash hook labels Oct 3, 2022
@MaxymVlasov MaxymVlasov requested a review from yermulnik as a code owner October 3, 2022 19:16
@MaxymVlasov
Copy link
Collaborator Author

@chris3ware please test is that works correctly for you.

@yermulnik
Copy link
Collaborator

yermulnik commented Oct 3, 2022

Have you tested this with third --args element like --ignore-path "some/path/" (double quotes — or single quotes per your wish — are important), including placing it into a random position in list of args or even combining with values of other --args elements. As it seems like your test cases are locked down to what #388 is scoped to only.

Also it seems like this PR is somewhat a workaround for another workaround 😢 A fix for the #388 would be to properly quote values (as in elements of $args array), though for another reason quoting was droppped with a # shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]") comment. I beleive there was a good reson for that, though that workaround now needs another workaround from this PR.
I'm not quite sure and this needs testing, but replacing tfupdate ${args[@]} . (and alikes) with tfupdate ${args[*]} . or tfupdate "${args[*]}" . most probably would have had fixed that initial "issue" which needed shell quoting mechanism to be turned off and shellcheck forced to ignore this "mistake".

@MaxymVlasov
Copy link
Collaborator Author

Have you tested this with third --args element like --ignore-path "some/path/" (double quotes — or single quotes per your wish — are important), including placing it into a random position in list of args or even combining with values of other --args elements. As it seems like your test cases are locked down to what #388 is scoped to only.

Good catch, I'll do it.

Also it seems like this PR is somewhat a workaround for another workaround 😢 A fix for the #388 would be to properly quote values (as in elements of $args array), though for another reason quoting was droppped with a # shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]") comment. I beleive there was a good reson for that, though that workaround now needs another workaround from this PR.
I'm not quite sure and this needs testing, but replacing tfupdate ${args[@]} . (and alikes) with tfupdate ${args[]} . or tfupdate "${args[]}" . most probably would have had fixed that initial "issue" which needed shell quoting mechanism to be turned off and shellcheck forced to ignore this "mistake".

  • "${args[@]}" - didn't work at all

    rev: cdee1a365f34246cffbcc1652a85cbcb4fd436fa
  • "${args[*]}" - didn't work at all

    rev: d230cd87830976d991c926673dc0814073e53fe9
  • ${args[*]} - work only in 1/4 cases

    rev: 6c53823f8427081b5517645b876d6aa9d5caf6f4
  • ${args[@]} - work only in 1/4 cases

    rev: 0dece4b5fc696c3e1487cd3343fa2379f2999d89
  • "$args" - didn't work at all

    rev: bc2392499484819b9d738635db7e4e3ca32cbd26
  • $args - work only in 1/4 cases

    rev: febc0a46b65d762d20cffaa96340c93ae7cd6387

@MaxymVlasov
Copy link
Collaborator Author

Right now (v1.75.0), constructions with space inside don't works for all kind of elements, including --ignore-path.
That means, that next will fail:
- --args=--ignore-path "sdfs/sf dfs"

At the same time, for constructions without spaces, quotes are not needed, and that will work:
- --args=--ignore-path sdfs/sfdfs

At the same time, pre-commit have their own mechanisms to ignore paths, so, for that case is a workaround
https://pre-commit.com/#config-exclude

For now, in tfupdate no more config options, that need to include quotes.

However, in current rev: 8e7a029eb1f796f1ff60ad8e07b911466381cdd1, constructions like:

      args:
        - --args=provider google
        - --args=--ignore-path sdfs/sfdfs
        - --args=--version ">= 4.18.0, < 5"

will works, when next will fail:

      args:
        - --args=provider google
        - --args=--version ">= 4.18.0, < 5"
        - --args=--ignore-path sdfs/sfdfs

Which is not expected, so I try to find another solution

@antonbabenko
Copy link
Owner

This issue has been resolved in version 1.76.0 🎉

@MaxymVlasov MaxymVlasov deleted the feat/GH-388/tfupdate_float_version branch October 6, 2022 16:27
@chris3ware
Copy link

@chris3ware please test is that works correctly for you.

Hi, sorry for the delay. Thanks for adding this. Can confirm it works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimate/2h Need 2 hours to be done feature New feature or request hacktoberfest-accepted HacktoberFest'21 and '22 hook/tfupdate Bash hook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tfupdate version constraints
4 participants