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

Addition of opinionated Terrafile support #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TrickMcCarthy
Copy link

Tested against python 2.7.6 and python 3.7.4

@raymondbutcher
Copy link
Contributor

Hi. I'm not sure about the way it matches module names. Here is how I would probably do it:

├── dev
│   ├── apps
│   │   └── main.tf
│   ├── ec2
│       └── main.tf
├── modules
    └── Terrafile
# Terrafile

example:
  source: https://github.com/joeblogs/example.git
  version: master
# dev/apps/main.tf

module "example1" {
  source = "../../modules/example"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example"
  name   = "example 2"
}

When pterrafile runs, it searches for module sources, rather than names, in files in the current directory. If a module source directory matches an entry in Terrafile, it gets downloaded.

This way, module names in your Terraform code don't have to match the name of the module. More importantly, it allows for using the same downloaded module multiple times - in the above example it gets used twice in the same file, but also multiple projects (apps, ec2, etc) could all refer to the same shared module directory and it only has to be downloaded once.

@raymondbutcher
Copy link
Contributor

The version feature is interesting, but I don't see any reason for it to be part of this pull request. 1 pull request = 1 feature only, please :)

If it is going to be implemented, I think it should be done similarly to https://www.terraform.io/docs/configuration/providers.html#version-provider-versions where you can specify an exact version or use prefixes like >=, <=, ~> etc. This could be done in the Terrafile yaml file version field, and not require any command line arguments.

# Terrafile

example:
  source: https://github.com/joeblogs/example.git
  version: ~> 1.2

I've never considered automatically upgrading Terraform modules in this way. It's very interesting to see how other people are using Terraform in different ways.

@TrickMcCarthy
Copy link
Author

TrickMcCarthy commented Oct 17, 2019

@raymondbutcher Thanks for getting back to me.
I understand and respect the requirement for single focussed change per PR. This additional feature occurred in error due to a force push to realign my local with remote forked repo. The open PR got updated automatically and I hadn't considered that would occur. I have now reverted that change back out. I intend to make a separate PR for that dynamic version change after I have successfully gotten this one merged, so I will leave the discussion about its implementation until then.

Let me clarify the new functionality with a couple of the use cases so you have better understanding of why its done in this manner and your suggested alternative will not meet the use case.(Even though there is obviously many ways to implement the same functionality).
First for clarity lets redefine the source in the Terrafile as remote_source and only use the word source to mean source in the terrafiles within a module block. Otherwise its gets confusing(I would suggest when the next major update to introduce this word change as a breaking change, cos it going to continue to make discussions more complicated than necessary)
Overview
Currently it recursively scans the subfolder /dev/apps for any terraform files (*.tf) and retrieves the module names and matching source information.
It then reads the central Terrafile and creates a data structure of just the matching module names and their attributes remote_source(note usage alias) and version information.
It then removes duplicates from the data structure, based on the remote_source and version information. In this manner it only retrieves down the remote_source repos once to the location specified by the module source keyword.

Use Case 1: Based on the above you can see that if the two attributes match then multiple modules will utilize the same retrieved repos. So your suggestion feature support is already encompassed, but it cannot be done in the manner you describe because you are not taking into consideration that the module source string only requires the repo name within it somewhere, but all the pre or post parts can be different.

Use Case 2: module sources suffix can have different subfolder suffixes, Here the functionality will retrieve the repo once, and the two modules will both reference the same source

example1:
  source: https://github.com/joeblogs/example.git
  version: "master"

example2:
  source: https://github.com/joeblogs/example.git
  version: "master"

# dev/apps/main.tf
module "example1" {
  source = "../../modules/example/subfolder1"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example/subfolder2"
  name   = "example 2"
}```

**Use Case 3:**
branch support - So in this case you want some remote repo on different branches. Example 1 on remote branch "branch1" and example2 using "master" then you just change the terraform module source retrieval so that the download location will be different and becos the central terrafile attributes of the two different entries will not match, due to version difference, it will attempt to retrieve both of them down.(i.e. one will not be removed by the deduplicate functionality)

```# Terrafile
example1:
  source: https://github.com/joeblogs/example.git
  version: "branch1"

example2:
  source: https://github.com/joeblogs/example.git
  version: "master"

# dev/apps/main.if
module "example1" {
  source = "../../modules/branch1/example/subfolder1"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example/subfolder2"
  name   = "example 2"
}```

**Use Case 4** relative paths. These should be entered in the central Terrafile in the remote_source  attribute and should be relative to the central Terrafile current working directory.(Thats a little gotcha to watch out for). Useful for local testing when you are working on a module and other reasons.

@raymondbutcher
Copy link
Contributor

No worries about the version commit, and thanks for providing all of this detail.

I'll try to be clear with terminology too:

Terrafile files contain multiple Terrafile modules.

Each Terrafile module is comprised of a Terrafile module name, Terrafile module source and sometimes a Terrafile module version.

pterrafile uses the Terrafile module source to populate a directory based on the Terrafile module name which we'll call the Terrafile module path.

*.tf and *.tf.json files have Terraform modules.

Each Terraform module is comprised of a Terraform module name, Terraform module source.

Here is what I'm proposing:

pterrafile looks through all Terrafile modules and come up with a map of Terrafile module path -> Terrafile module source before doing anything with that information.

pterrafile then searches through *.tf and *.tf.json files and finds any Terraform module sources that match up with Terrafile module paths. If the Terraform module source is a subdirectory of a Terrafile module path then it is a match too.

pterrafile then populates the matched Terrafile module paths with their Terrafile module source using git clone or whatever is necessary depending on the source type.

That's it. It's just a new option that makes it only download modules if they are going to be used.

I don't think the name of a module in the Terraform code should have anything to do with the module directory name. We can achieve everything by using the source field. That's what the source field is for. This feature would not be "opinionated" and it wouldn't require much of an explanation. Nobody has to change how they use modules or pterrafile to use this feature.

I have translated your use cases (I think I got the intentions right) into how it would work with source matching. I think it works for all of them.

Use Case 1: use one module twice

# Terrafile
example:
  source: https://github.com/joeblogs/example.git
  version: "master"

# dev/apps/main.tf
module "example1" {
  source = "../../modules/example"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example"
  name   = "example 2"
}

Use Case 2: use two subfolders of one module

# Terrafile
example:
  source: https://github.com/joeblogs/example.git
  version: "master"

# dev/apps/main.tf
module "example1" {
  source = "../../modules/example/subfolder1"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example/subfolder2"
  name   = "example 2"
}

Use Case 3: use two branches of one module

# Terrafile
example1:
  source: https://github.com/joeblogs/example.git
  version: "branch1"

example2:
  source: https://github.com/joeblogs/example.git
  version: "master"

# dev/apps/main.if
module "example1" {
  source = "../../modules/example1"
  name   = "example 1"
}

module "example2" {
  source = "../../modules/example2"
  name   = "example 2"
}

Use Case 4 module with relative path

# Terrafile
example:
  source: ../../some/path

# dev/apps/main.if
module "example" {
  source = "../../modules/example"
  name   = "example 1"
}

@TrickMcCarthy
Copy link
Author

@raymondbutcher Thanks for taking the time to provide a more detailed explanation. I now get your suggested change of flipping the search and dropping the terraform module as index and utilising value from terraform module source instead and it makes sense as we are dealing with insignificant number of records and it is more compatible upgrade.

I am trying to identify any problems with it. There is a potential issue in that you have to search for all the module names from the Terrafile across all the terraform files module source strings. I believe this may easily result in some false positives as people inadvertently have Terrafile module names in terraform module source strings or utilising modules outside their control that utilise the aggregate multiple modules with single repo distinguishing by subfolder suffix (USE CASE 2).

I will look into prototyping the change and I will refer back.

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

Successfully merging this pull request may close these issues.

2 participants