-
Notifications
You must be signed in to change notification settings - Fork 149
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
Support for Terraform Cloud / Terraform Enterprise #458
Conversation
Codecov Report
@@ Coverage Diff @@
## v0.8 #458 +/- ##
==========================================
- Coverage 70.73% 70.73% -0.01%
==========================================
Files 286 287 +1
Lines 6445 6472 +27
==========================================
+ Hits 4559 4578 +19
- Misses 1516 1521 +5
- Partials 370 373 +3
|
Thanks @ayshiff for your contribution. You did great on a first step to support TF Cloud. But we should converge to what they do in Terraform for our user to use the support quite the same way. You can find all the information for the credentials part here. They rely on their credentials "app.terraform.io" {
token = "XXX"
} We should, thus, support 2 ways:
I didn't go further into terraform package but we could re-use one of their functions/methods to fetch the credentials from the file. Maybe they even have a function/method the fetch the state with the token and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! ππ½
Left few comments about error handling and tests. We also might discuss a bit about authentication, as there are multiple ways to authenticate to TFC. We usually don't perform integration tests for backends, so we have to discuss about that too.
I fixed the various issues that came up and added a new I still have to work on the way to retrieve the token from |
@wbeuil @sundowndev Maybe this could be splitted in another PR ? I'm fine with that |
Well I don't really know much about how credentials management work in TFC and what it implies to support the .terraformrc file, but yeah we could introduce this feature with the flag only for now. |
We could indeed split this PR into two. Just for the record, there is this function which seems to read and load everything there is inside a config file ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems great, I just left some comments about the code
a501e01
to
d964347
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments to be fully consistent with the new wording. Can you also update your branch to have the latest changes from @sundowndev ? Thanks !
} | ||
|
||
type Backend io.ReadCloser | ||
|
||
type Options struct { | ||
Headers map[string]string | ||
Headers map[string]string | ||
TerraformCloudToken string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we reworded everything with TFCloud
, I would suggest to do it everywhere to be consistent. Thus, let's call it TFCloudToken
.
) | ||
|
||
const BackendKeyTFCloud = "tfcloud" | ||
const TerraformCloudAPI = "https://app.terraform.io/api/v2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFCloudAPI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
Hey @all-contributors please add @ayshiff for code |
I've put up a pull request to add @ayshiff! π |
Description
This PR adds support for Terraform Cloud / Terraform Enterprise.
As described in the issue the logic is as follow:
hosted-state-download-url
from the API with the provided$WORKSPACE_ID
(tfstate+tfcloud://$WORKSPACE_ID
) and the API token through the headers (--headers 'Authorization=Bearer $API_TOKEN'
)HTTPReader
with the retrievedhosted-state-download-url
Note that the logic is the same for Terraform Cloud and Terraform Enterprise as the API is the same for both.
Here is a complete command example:
I also added some test the different cases we can have:
404
)401
)I have as well updated the documentation in snyk/driftctl-docs#47 to fit this new state reader.