civo / terraform-provider-civo

Terraform Civo provider
https://www.civo.com
Mozilla Public License 2.0
71 stars 56 forks source link

[FEATURE] Provider should take configuration from the CLI config (~/.civo.json) #246

Closed fernando-villalba closed 1 month ago

fernando-villalba commented 3 months ago

Description

Issue

When the CLI is configured with a token, terraform should automatically take the token config from the cli config file at from ~/.civo.json

Alternatively (or preferentially) it can be taken from the environment variable as it is possible to do now.

We should dissuade users from providing credentials via an input variable. These should be strictly reserved for short lived tokens which we don’t have now.

Acceptance Criteria

Screenshots

No response

Additional information

No response

Praveen005 commented 2 months ago

Proposed Approach for #246:

Hi Team,

I have been working on the feature request outlined in issue #246. I kindly request your review and feedback.

Here is what I have done:

  1. Removed the civo_token field from the provider schema and replaced it with a credential_file option.

  2. Implemented a priority-based token retrieval:

    1. Environment variable (CIVO_TOKEN).
    2. Token provided via a Credential file.
    3. Lastly, fall back to CLI config file (~/.civo.json)
  3. Added a getToken function to handle the token retrieval logic.

  4. Implemented readTokenFromFile to securely read tokens from JSON files.

  5. One additional thing that I have added is, the user can set the Credential file path in CIVO_CREDENTIAL_FILE environment variable as well. For this I took inspiration from here

  6. Updated and expanded test cases to cover:

    1. Reading token from environment variable
    2. Reading token from a specified credentials file
    3. Falling back to the CLI config file
  7. Updated the example in provider.tf to use the new credential_file option instead of civo_token.

The concerned code can be found in the gist here.

Screenshots:

It passes all the tests:

Screenshot 2024-07-08 121905

Test Coverage:

Screenshot 2024-07-08 121955

Thank you for your time and feedback.

Best regards,

Praveen

uzaxirr commented 2 months ago

Hey @Praveen005 Thank you for your amazing diagnosis, this approach looks fine to me can you go ahead and raise a PR. Please do test it locally and attach Screenshots in the PR

uzaxirr commented 1 month ago

Closed via: https://github.com/civo/terraform-provider-civo/pull/248