Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Feature/git modules #28

Merged
merged 4 commits into from
May 14, 2018

Conversation

melbit-jonathanb
Copy link
Contributor

Hi,

I needed support for Git based modules, so I put this together.

A few points:

  • relies on system git and in CI/CD environments, credentials stored in git credential helpers or a ssh key. In non CI/CD environments, it does work with interactive authentication passed through from the git client.
  • Only supports git style repositories (git or https git), not hosted via terraform's registry, nor other cvs.
  • didn't mess with setup.py, but did add GitPython to requirements.txt
  • It will download each repo every time init is called, I thought about stopping this, but I would prefer a fresh copy of each repo each time to stop any oddness I've seen with CI/CD recycling directory contents.
  • hard coded temp directory, but with Windows and *Nix support.

Let me know what you think.

cheers,

Jon.

@melbit-jonathanb
Copy link
Contributor Author

Found an issue and an edge case. One tic

Fixed variable names clashing in directory parsing.
@melbit-jonathanb
Copy link
Contributor Author

Right, found two issues which would have caused problems.

  1. some modules have ?ref=// - this is not really mentioned in terraform, but seems to be something which can happen. I have pulled apart the directory specification and ignored it, as the tool parses all directories.
  2. I had re-used some of the directory scanning and failed to change the variable names, which in some cases caused incorrect attempts to load files which weren't there. Fixed by changing variable names.

@JEHAL981
Copy link

JEHAL981 commented May 14, 2018

@elmundio87 Hey, just wondering if this going to be merged too master?

@elmundio87
Copy link
Owner

Thanks for the PR!

@melbit-jonathanb & @JEHAL981 I will review the code changes tonight, and if all's good I'll create a new release.

-Ed

@elmundio87 elmundio87 merged commit cfd2ad0 into elmundio87:master May 14, 2018
@elmundio87
Copy link
Owner

Released in 2.7.0

elmundio87 pushed a commit that referenced this pull request May 16, 2018
This reverts commit 15f910c.
atward added a commit to atward/terraform_validate that referenced this pull request May 18, 2018
@melbit-jonathanb
Copy link
Contributor Author

@elmundio87 & @atward
Hi,
I've performed a number of changes as suggested. I now have it following the load order closer as per Adam's comments and have fixed up the module loading issues observed.

Changes:

  • Fix elif issue.
  • Tool now correctly parses only .tf files in nominated directory - or .tf files in a nominated module.
  • Fixed error with evaluating source of module.
  • Added condition that tf files must be over 20 bytes before evaluation.
    • I had to add this as Hashicorp have said it's ok to have blank .tf files and this allows someone to have blank file with a comment in it. I'm not sure how to get around this in any other manner, as the pyhcl tool seems to raise an exception if there's nothing to load.
  • Added support for multiple "identical" repo directories via semi-random dirname.
    • I am not deleting the /tmp/terraform_validate contents after finishing. This is mainly to allow for debugging. I am open to cleaning up the directory after.
  • Added exception and message if no valid terraform is loaded.

Let me know what you think.
cheers,

Jon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants