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

Use regex to filter filenames #1253

Merged

Conversation

surminus
Copy link
Contributor

@surminus surminus commented Nov 6, 2020

I ran into an issue where I have a directory of templates I use that I name foo.tf.template. I found that Atlantis was running against this directory and failing, which was unexpected behaviour.

This is because we're just checking if .tf is contained within a filename, rather than checking if it's the suffix of the filename.

Instead, we can use regex to ensure that we're only filtering on actual Terraform files, inclusive of .tf and .tfvars suffixes.

There are obviously some alternative ways I could get around this: rename my templates to foo.template without the .tf.

I also really liked the idea of a .atlantisignore file previously cited[1], but it was rejected in favour of explicitly defining exactly
which directories to run it in. We have a large number of directories so this wasn't appealing to me.

I felt this behaviour was sufficiently unexpected that it was worth making a more explicit filter.

[1] #26

I ran into an issue where I have a directory of templates I use that I
name `foo.tf.template`. I found that Atlantis was running against this
directory and failing, which was unexpected behaviour.

This is because we're just checking if `.tf` is contained within a
filename, rather than checking if it's the suffix of the filename.

Instead, we can use regex to ensure that we're only filtering on actual
Terraform files, inclusive of `.tf` and `.tfvars` suffixes.

There are obviously some alternative ways I could get around this:
rename my templates to `foo.template` without the `.tf`.

I also really liked the idea of a `.atlantisignore` file previously
cited[1], but it was rejected in favour of explicitly defining exactly
which directories to run it in. We have a large number of directories so
this wasn't appealing to me.

I felt this behaviour was sufficiently unexpected that it was worth
making a more explicit filter.

[1] runatlantis#26
@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #1253 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1253   +/-   ##
=======================================
  Coverage   69.98%   69.99%           
=======================================
  Files          71       71           
  Lines        5381     5382    +1     
=======================================
+ Hits         3766     3767    +1     
  Misses       1265     1265           
  Partials      350      350           
Impacted Files Coverage Δ
server/events/project_finder.go 90.90% <100.00%> (+0.11%) ⬆️

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 cb7dd0e...732c90c. Read the comment docs.

@chenrui333
Copy link
Member

Thanks @surminus for the improvement.

@chenrui333 chenrui333 merged commit b6e8403 into runatlantis:master Dec 13, 2020
@gek0
Copy link
Contributor

gek0 commented Jan 4, 2021

This seems to ignore *.auto.tfvars.json files for the initial auto-plan feature
I'm able to run plan with -d directory flag to run it.
I'm not using --disable-autoplan option!

[INFO] Terraform/aws#1024: Automatically determined that there were 0 projects modified in this pull request: []
We store most of the variables in those *.auto.tfvars.json files, so those are changed in 90% of cases
And since those are per terraform conventions, those extensions should be loaded properly as well

Can I somehow change those extensions via server-side config?

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.

4 participants