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

chore: replace TF Cloud implementation with TF CLI #1955

Merged
merged 41 commits into from Dec 15, 2022

Conversation

DanielMSchmidt
Copy link
Contributor

@DanielMSchmidt DanielMSchmidt commented Jul 25, 2022

This will allow us to integrate better with TFC and TFE without additional work being spend on polishing this part.

TODO

@DanielMSchmidt DanielMSchmidt changed the title WIP chore: replace TF Cloud implementation with TF CLI Jul 25, 2022
@DanielMSchmidt DanielMSchmidt marked this pull request as ready for review July 26, 2022 07:58
@xiehan xiehan added this to the 0.15 (tentative) milestone Sep 23, 2022
@DanielMSchmidt DanielMSchmidt marked this pull request as draft November 4, 2022 08:21
@ansgarm
Copy link
Member

ansgarm commented Nov 22, 2022

Okay, so #490 does not quite work yet with this PR and requires a follow-up:
Screenshot 2022-11-22 at 11 56 11

@michaellzc
Copy link
Contributor

michaellzc commented Nov 26, 2022

Hey, this seems like a better approach otherwise cdktf will always have to play catch-up with TFC/TFE, e.g. #2243.

but... when I tried out this branch, and got this error when running cdktf destroy

[2022-11-25T19:30:29.198] [ERROR] default - ╷
│ Error: Saving a generated plan is currently not supported
│
│ Terraform Cloud does not support saving the generated execution plan
output  ╷
        │ Error: Saving a generated plan is currently not supported
        │
        │ Terraform Cloud does not support saving the generated execution plan
        │ locally at this time.

Apparently, TFC/TFE does not support saving the plan locally through terraform cli when remote execution is enabled, and the raw plan is only exposed through their API. (TIL we just migrated to TFC not that long ago)

Therefore, it seems almost impossible to get to the state where cdktf has zero knowledge of how TFC/TFE works, and always requires some special handling.

Is such a huge discrepancy between normal terraform behaviour and TFC/TFE something on the terraform and cdktf team radar? What is the plan for cdktf to address it?

@ansgarm
Copy link
Member

ansgarm commented Nov 28, 2022

Hi @michaellzc 👋

Wow, you are quite the adventurer by daring to test this draft PR 😁👏

Yeah, I noticed that as well when I took over Daniel's work on this. In the long term, we're working with the team working on the Terraform CLI on a solution that allows us to close this gap (I don't know yet whether they'll support saving a TFC/E remote plan in a file that can be applied or whether a different solution will be preferred).

However, as we need to support current Terraform CLI versions as well, our current approach is to use the Terraform CLI in an interactive way (via node-pty). So I'm currently working on switching the implementation from cdktf deploy = terraform plan -out <plan> && terraform apply <plan> to cdktf deploy = terraform apply (and relaying the "Do you want to apply? yes / no"-question). This will work for all cases but still has uncertainties that we need to test.

…or authenticated requests when determining available pre-built providers for Go

We can't use CDKTF_ in a prefix as that collides with yargs arguments from env vars and its strict mode.
Refer to: yargs/yargs#873
@team-tf-cdk
Copy link
Collaborator

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@team-tf-cdk
Copy link
Collaborator

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@team-tf-cdk
Copy link
Collaborator

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@team-tf-cdk
Copy link
Collaborator

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants