Rewrite package terratag/cli

[arybolovlev]

Hi there,

With this PR I would love to propose a small improvement for the terratag/cli package.
Instead of writing your own functions to manage CLI arguments, you can use the package 'flag' from the standard Golang library.

One of the benefits of using the package 'flag' is that it is coming with the build-in "help", it is done transparently by the package. Here is how it looks:

# terratag -help
Usage of terratag:
  -dir string
    	Directory to recursively search for .tf files and terratag them (default ".")
  -filter string
    	Only apply tags to the selected resource types (regex) (default ".*")
  -rename
    	Keep the original filename or replace it with <basename>.terratag.tf (default true)
  -skipTerratagFiles
    	Skips any previously tagged files (default true)
  -tags string
    	Tags as a valid JSON document
  -verbose
    	Enable verbose logging

Another benefit is the build-in check for the parameters of the argument:

# terratag -verbose=hello                                                                                                                               6s
invalid boolean value "hello" for -verbose: parse error

I am not sure if the descriptions for the options are right, please feel free to update them.

Thank you.

[yaronya]

@arybolovlev great suggestion 💯
It looks like you got some tests failing there

[arybolovlev]

Hi @yaronya,

I've fixed this. Tests were failing due to the nature of the flag package. When you use a default FlagSet, as I did in my first PR, all flags are set on the global level and cannot be re-set. Since tests were running in parallel each test starting from the second one was trying to set flags again and because of that, it was failing.

I also removed the unused field SkipTerratagFiles from the Args structure. I couldn't find anywhere in the code if it is used. I guess it was replaced with the IsSkipTerratagFiles.

Also, as you can see I've updated the terratag_test.go file to cut off the extra arguments that are added by the package testing when executing tests. It doesn't influence tests. For instance, I ran tests with the flag -time.timeout=30s to intentionally fail them due to timeout and it was working well. So cutting off extra arguments seems to look fine.

Locally all tests are green now.

Thank you!

[yaronya]

Very nicely done! Thank you for your contribution!
Left one last small comment 🙏

[yaronya]

Could you please extract os.Args[0] and os.Args[1:] into meaningful variables and use those instead?
i.e

programName := os.Args[0]
programArgs := os.Args[1:]

[arybolovlev]

Good catch, thank you! Updated also the test file just to have consistency.