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: allow terramate and stack config from multiple files #216

Merged
merged 24 commits into from
Feb 14, 2022

Conversation

katcipis
Copy link
Contributor

@katcipis katcipis commented Feb 11, 2022

Since the terramate block can include a different range of configurations, like:

  • version
  • git
  • code generation configs

It seemed useful to allow it to be defined on different files, multiple times. So this PR introduces that. For the stack block it seemed less useful, so it remains as it was before (only one block allowed).

Design wise the idea was to replace the function that would parse specific files/data for a function that given a dir parses the whole dir (all files matching *.tm or *tm.hcl). I kept the original logic as much as I could to avoid regressions + also did some piggybacking on the current tests to test with different filenames.

Globals and code generation still only loads the default file, so there is still more work to do.

As an example, this should work and list both stacks:

#!/bin/bash

set -o nounset
set -o errexit

basedir=$(mktemp -d)

cd "${basedir}"

# We need the project root config since it is not git
cat > terramate.tm.hcl <<- EOM
terramate {
  config {
  }
}
EOM

mkdir -p stacks/stack-{1,2}
terramate stacks init stacks/stack-1

cat > stacks/stack-2/stack.tm.hcl <<- EOM
stack {}
EOM

cat > stacks/stack-2/version.tm.hcl <<- EOM
terramate {
  required_version = "0.0.10"
}
EOM

cat > stacks/stack-2/config.tm.hcl <<- EOM
terramate {
  config {
    // stack specific config here
  }
}
EOM

terramate stacks list

@katcipis katcipis self-assigned this Feb 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2022

Codecov Report

Merging #216 (0c6a8ba) into main (7905b9e) will increase coverage by 0.27%.
The diff coverage is 86.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #216      +/-   ##
==========================================
+ Coverage   63.36%   63.64%   +0.27%     
==========================================
  Files          30       30              
  Lines        5408     5391      -17     
==========================================
+ Hits         3427     3431       +4     
+ Misses       1797     1777      -20     
+ Partials      184      183       -1     
Flag Coverage Δ
tests 63.64% <86.50%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
init.go 44.00% <20.00%> (+4.86%) ⬆️
generate/generate.go 88.98% <80.00%> (+0.09%) ⬆️
hcl/hcl.go 81.72% <90.62%> (+1.36%) ⬆️
config/config.go 96.15% <100.00%> (+3.99%) ⬆️
list.go 92.50% <100.00%> (+0.60%) ⬆️
stack/stack.go 75.90% <100.00%> (-1.88%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7905b9e...0c6a8ba. Read the comment docs.

@katcipis katcipis marked this pull request as ready for review February 11, 2022 20:20
i4ki
i4ki previously approved these changes Feb 14, 2022
Copy link
Contributor

@i4ki i4ki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm but left some improvement comments, nothing serious.

hcl/hcl.go Outdated Show resolved Hide resolved
generate/generate.go Outdated Show resolved Hide resolved
hcl/hcl.go Show resolved Hide resolved
hcl/hcl.go Show resolved Hide resolved
hcl/hcl.go Outdated Show resolved Hide resolved
return nil, errutil.Chain(ErrHCLSyntax, diags)
}

logger.Trace().Msg("Terramate config file parsed successfully")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

hcl/hcl.go Outdated Show resolved Hide resolved
hcl/hcl_test.go Outdated Show resolved Hide resolved
hcl/hcl_test.go Outdated Show resolved Hide resolved
list.go Show resolved Hide resolved
@i4ki
Copy link
Contributor

i4ki commented Feb 14, 2022

@katcipis

Globals are still loaded from terramate.tm.hcl: https://github.com/mineiros-io/terramate/blob/ff66ae70f2175ac9b5557a3d3c6f9ba6bee566fb/globals.go#L233

so having a separate globals.tm.hcl does not work.

$ ls apps/a
globals.tm.hcl  terramate.tm.hcl
$ cat apps/a/globals.tm.hcl
globals {
        test = 1
        something = "else"
}

$ terramate stacks globals
$

@katcipis
Copy link
Contributor Author

katcipis commented Feb 14, 2022

@katcipis

Globals are still loaded from terramate.tm.hcl:

https://github.com/mineiros-io/terramate/blob/ff66ae70f2175ac9b5557a3d3c6f9ba6bee566fb/globals.go#L233

so having a separate globals.tm.hcl does not work.

$ ls apps/a
globals.tm.hcl  terramate.tm.hcl
$ cat apps/a/globals.tm.hcl
globals {
        test = 1
        something = "else"
}
$ terramate stacks globals
$

Ah yes, Im aware, I also added a comment on the PR description:

Globals and code generation still only loads the default file, so there is still more work to do.

The idea is to incrementally bring all the code generation features to using multiple config files, but right now export_as_locals/globals and generate_hcl wont work yet. Since they would still work on terramate.tm.hcl and nothing would break I thought it would be OK to work on them incrementaly in a different PR. WDYT ?

katcipis and others added 4 commits February 14, 2022 18:04
Co-authored-by: Tiago de Bem Natel de Moura <t.nateldemoura@gmail.com>
Co-authored-by: Tiago de Bem Natel de Moura <t.nateldemoura@gmail.com>
Co-authored-by: Tiago de Bem Natel de Moura <t.nateldemoura@gmail.com>
Co-authored-by: Tiago de Bem Natel de Moura <t.nateldemoura@gmail.com>
@i4ki
Copy link
Contributor

i4ki commented Feb 14, 2022

I thought it would be OK to work on them incrementaly in a different PR. WDYT ?

I guess it's ok.

@katcipis katcipis requested a review from i4ki February 14, 2022 17:17
…ineiros-io/terramate into katcipis-load-tm-config-multiple-files
Copy link
Contributor

@i4ki i4ki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@katcipis katcipis merged commit 00e6f7b into main Feb 14, 2022
@katcipis katcipis deleted the katcipis-load-tm-config-multiple-files branch February 14, 2022 17:47
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.

3 participants