Skip to content

Add --tf-download-url configuration option#787

Closed
cullenmcdermott wants to merge 3 commits intorunatlantis:masterfrom
cullenmcdermott:add_download_flag
Closed

Add --tf-download-url configuration option#787
cullenmcdermott wants to merge 3 commits intorunatlantis:masterfrom
cullenmcdermott:add_download_flag

Conversation

@cullenmcdermott
Copy link
Copy Markdown
Contributor

@cullenmcdermott cullenmcdermott commented Sep 24, 2019

This fixes #614

I'm new to Go so I'm sure I'll need plenty of changes before this can be merged. The main thing I'm unsure about is how I implemented the downloadUrl. I feel as though the modifications I made would fit better as part of Downloader however I'm not entirely sure the best way to go about modifying that.

I don't currently have access to an internal mirror of releases.hashicorp.com so I haven't had a chance to fully test this functionality myself. I'm hoping to have the time to do that later this week.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 24, 2019 •
edited
Loading

Codecov Report

Merging #787 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #787      +/-   ##
==========================================
+ Coverage   71.83%   71.86%   +0.02%     
==========================================
  Files          65       65              
  Lines        5213     5217       +4     
==========================================
+ Hits         3745     3749       +4     
  Misses       1183     1183              
  Partials      285      285
Impacted Files Coverage Δ
server/user_config.go 100% <ø> (ø) ⬆️
server/server.go 63.69% <100%> (+0.11%) ⬆️
cmd/server.go 78.97% <100%> (+0.19%) ⬆️
server/events/terraform/terraform_client.go 78.04% <100%> (+0.1%) ⬆️

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 777956c...3b21f31. Read the comment docs.

lkysow
lkysow requested changes Oct 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Looks great! Almost there. I don't think it makes sense to be in Downloader actually, I like where you put it. Downloader is just used as a shim during testing.

server/events/terraform/terraform_client_test.go

Ok(t, err)
Equals(t, "0.11.10", c.DefaultVersion().String())
baseURL := "https://releases.hashicorp.com/terraform/0.11.10"
Copy link
Copy Markdown
Member

@lkysow lkysow Oct 4, 2019

Choose a reason for hiding this comment

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

maybe set this to https://my-mirror.releases.mycompany.com in this test to make it clear it's being tested

Copy link
Copy Markdown
Contributor Author

@cullenmcdermott cullenmcdermott Nov 4, 2019

Choose a reason for hiding this comment

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

I think I adjusted this to match what you are asking, can you double check?

Copy link
Copy Markdown
Member

@lkysow lkysow Nov 4, 2019

Choose a reason for hiding this comment

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

yes this looks good

@lkysow lkysow added the waiting-on-response Waiting for a response from the user label Oct 29, 2019
@cullenmcdermott
Copy link
Copy Markdown
Contributor Author

cullenmcdermott commented Nov 4, 2019

I just tried to test this with atlantis testdrive and it looks like there's an issue with the version verification now.

Error: creating atlantis server: timeout, logs:
2019/11/03 17:37:01-0700 [WARN] cmd: No GitHub webhook secret set. This could allow attackers to spoof requests from GitHub
2019/11/03 17:37:01-0700 [WARN] cmd: No Azure DevOps webhook user and password set. This could allow attackers to spoof requests from Azure DevOps.
Error: initializing server: initializing terraform: Malformed version: https://releases.hashicorp.com

@lkysow
Copy link
Copy Markdown
Member

lkysow commented Nov 4, 2019

initializing terraform

Atlantis runs terraform version. What's the output when you run that.

@lkysow lkysow mentioned this pull request Jan 20, 2020
Merged
@lkysow lkysow closed this in #903 Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lkysow lkysow lkysow requested changes

Assignees

No one assigned

Labels

waiting-on-response Waiting for a response from the user

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow configuring custom releasesURL

2 participants

@cullenmcdermott @lkysow