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

Refactor Terraform credential loading #44037

Merged
merged 11 commits into from
Jul 25, 2024

Conversation

hugoShaka
Copy link
Contributor

@hugoShaka hugoShaka commented Jul 10, 2024

Fixes #42437

This PR refactors Terraform's credential loading to stop YOLO-ing all the credentials into the client and praying that one of them works.

Now each credential is individually tested. Terraform also lists the supported credentials and can explain why a credential is active or not.

This change will open the way for 2 other changes:

  • have Terraform check if credentials are expired and warn the user
  • have Terraform support MachineID joining

Other notable changes are:

  • introduce a new KeyPair() function that builds a credential from DER material. This is neeed as creds built from tls.Config don't allow to report the expiry.
  • use terraform-plugin-log to provide structured logging. We'll be able to tell the users to diagnose with export TF_LOG=INFO

Changelog: clearer terraform-provider error and warning messages about its credentials.

Example output

No credential source

╷
│ Error: No active credentials source found
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│  - cannot read credentials from Key, Cert, and CA path because neither cert_path, key_path, TF_TELEPORT_CERT, nor TF_TELEPORT_KEY are set
│  - cannot read credentials from Key, Cert, and CA base64 because neither cert_base64, key_base64, TF_TELEPORT_CERT_BASE64, nor TF_TELEPORT_KEY_BASE64 are set
│  - cannot read credentials from the identity file path because neither identity_file_path, nor TF_TELEPORT_IDENTITY_FILE_PATH are set
│  - cannot read credentials from the identity file (passed as a string) because neither identity_file, nor TF_TELEPORT_IDENTITY_FILE are set
│  - cannot read credentials from the identity file (passed as a base64-encoded string) because neither identity_file_base64, nor TF_TELEPORT_IDENTITY_FILE_BASE64 are set
│  - cannot read credentials from the local profile because neither profile_name, profile_dir, TF_TELEPORT_PROFILE_NAME, nor TF_TELEPORT_PROFILE_PATH are set
│ 

Expired credentials

│ Warning: Credentials from the identity file (passed as a base64-encoded string) are expired
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│ The credentials from the identity file (passed as a base64-encoded string) are expired. Expiration is "2024-07-16 12:57:13 -0400 EDT" while current time is "2024-07-16
│ 12:57:37.066592 -0400 EDT"). You might need to refresh them. The provider will not attempt to use those credentials.
╵
╷
│ Error: Impossible to build Teleport client
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│ Every credential source provided has failed. The Terraform cannot connect to the Teleport cluster '0.0.0.0:3025'.
│ 
│ We tried building a client:
│  - from the identity file (passed as a base64-encoded string)
│ 
│ You can find more information about each credential source specific errors in the Terraform warnings above this error.
╵

Failed to build credentials

╷
│ Error: Failed to build credentials from the identity file (passed as a base64-encoded string)
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│ The Terraform provider tried to build credentials from the identity file (passed as a base64-encoded string) but received the following error:
│ 
│ decoding base64 identity file
│       illegal base64 data at input byte 4
│ 
│ The provider tried to use the credential source because either identity_file_base64, or TF_TELEPORT_IDENTITY_FILE_BASE64 are set. You must either address the error or
│ disable the credential source by removing its values.
╵

Failed to connect (teleport not running)

╷
│ Warning: Failed to connect with credentials from the identity file (passed as a base64-encoded string)
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│ The client built from the credentials from the identity file (passed as a base64-encoded string) failed to connect to "0.0.0.0:3025" with the error: all connection methods
│ failed
│       
│       failed to connect to addr 0.0.0.0:3025 as a web proxy
│               context deadline exceeded: connection error: desc = "transport: Error while dialing: failed to dial: Get \"https://0.0.0.0:3025/webapi/find\": dial tcp 0.0.0.0:3025:
│ connect: connection refused", 
│       failed to connect to addr 0.0.0.0:3025 as a reverse tunnel proxy
│               context deadline exceeded: connection error: desc = "transport: Error while dialing: failed to dial: dial tcp 0.0.0.0:3025: connect: connection refused", 
│       failed to connect to addr 0.0.0.0:3025 as an auth server
│               context deadline exceeded: connection error: desc = "transport: Error while dialing: failed to dial: dial tcp 0.0.0.0:3025: connect: connection refused", 
│       failed to connect to addr 0.0.0.0:3025 with TLS Routing with ALPN connection upgrade dialer
│               context deadline exceeded: connection error: desc = "transport: Error while dialing: failed to dial: Get \"https://0.0.0.0:3025/webapi/find\": dial tcp 0.0.0.0:3025:
│ connect: connection refused", 
│       failed to connect to addr 0.0.0.0:3025 with TLS Routing dialer
│               context deadline exceeded: connection error: desc = "transport: Error while dialing: failed to dial: Get \"https://0.0.0.0:3025/webapi/find\": dial tcp 0.0.0.0:3025:
│ connect: connection refused".
╵
╷
│ Error: Impossible to build Teleport client
│ 
│   with provider["terraform.releases.teleport.dev/gravitational/teleport"],
│   on main.tf line 11, in provider "teleport":
│   11: provider "teleport" {
│ 
│ Every credential source provided has failed. The Terraform cannot connect to the Teleport cluster '0.0.0.0:3025'.
│ 
│ We tried building a client:
│  - from the identity file (passed as a base64-encoded string)
│ 
│ You can find more information about each credential source specific errors in the Terraform warnings above this error.
╵

Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@hugoShaka hugoShaka force-pushed the hugo/cleanup-terraform-credential-handling branch 2 times, most recently from abec015 to 397b7de Compare July 16, 2024 16:51
@hugoShaka hugoShaka force-pushed the hugo/cleanup-terraform-credential-handling branch from 3a97915 to 5d3f724 Compare July 23, 2024 21:41
@hugoShaka
Copy link
Contributor Author

hugoShaka commented Jul 23, 2024

@marcoandredinis I addressed all your feedback and also caught a breaking change: the provider used to fallback to the local profile when no credential is specified. This makes everything super slow but I can't just break this in v16. I implemented a backward compatible behaviour in this PR and will remove it in v17.

@hugoShaka hugoShaka force-pushed the hugo/cleanup-terraform-credential-handling branch from 645fc09 to cbdd0d8 Compare July 25, 2024 20:35
Copy link

🤖 Vercel preview here: https://docs-itxc1qiny-goteleport.vercel.app/docs/ver/preview

@hugoShaka hugoShaka added this pull request to the merge queue Jul 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 25, 2024
@hugoShaka hugoShaka added this pull request to the merge queue Jul 25, 2024
@hugoShaka hugoShaka removed this pull request from the merge queue due to a manual request Jul 25, 2024
@hugoShaka hugoShaka added this pull request to the merge queue Jul 25, 2024
Merged via the queue into master with commit aadc044 Jul 25, 2024
43 checks passed
@hugoShaka hugoShaka deleted the hugo/cleanup-terraform-credential-handling branch July 25, 2024 21:54
@public-teleport-github-review-bot

@hugoShaka See the table below for backport results.

Branch Result
branch/v16 Failed

hugoShaka added a commit that referenced this pull request Jul 26, 2024
* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs
hugoShaka added a commit that referenced this pull request Aug 1, 2024
* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs
hugoShaka added a commit that referenced this pull request Aug 2, 2024
* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs
hugoShaka added a commit that referenced this pull request Aug 6, 2024
* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs
marcoandredinis pushed a commit that referenced this pull request Aug 26, 2024
* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs
github-merge-queue bot pushed a commit that referenced this pull request Aug 26, 2024
)

* Introduce the `tctl terraform env` command (#43664)

* Introduce the `tctl terrafor env` command

* fix tests

* address marco's feedback + use correct b64 lib

* add license

* add created-by label as specified in the RFD

* Update tool/tctl/common/terraform_command.go

Co-authored-by: Roman Tkachenko <roman@goteleport.com>

* Apply suggestions from code review

Co-authored-by: Roman Tkachenko <roman@goteleport.com>

* Have telpeort create the Terraform default role

* rename use-existing-role -> role, and stop hijacking identity.SSHCACertBytes

* Make the terraform provider role a real preset, rename to 'terraform-provider'

* lint

* Fix tbot's invocation after rebase

---------

Co-authored-by: Roman Tkachenko <roman@goteleport.com>

* Refactor Terraform credential loading (#44037)

* Refactor Terraform credential loading

* Warn about expiry

* kip expired credentials

* fixup! kip expired credentials

* Use constants everywhere + add godocs

* fixup! Use constants everywhere + add godocs

* Address marco's feedback

* fixup! Address marco's feedback

* tidy go mod

* lint

* re-render TF docs

* Update v16 version in error message

* Add Terraform Provider native MachineID support (#44306)

* Add Terraform Provider native MachineID support

* Reject 'token' join method

* lint: fix imports

* re-render TF docs

* fix tests + add license

* lint

* tidy go mod

* use v16 client.Expiry() function

---------

Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: terraform provider should warn about expired credentials
3 participants