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

Terraform Provider: Kubeconfig as Base64 string #114

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

m-ildefons
Copy link
Contributor

Accept kubeconfig as base64 string.
When configuring the Harvester Terraform provider, the kubeconfig can either be given as path to a file, from which to read, or the contents can be supplied directly as base64 string.
This allows for greater flexibility in supplying this sensitive data to the Terraform provider.

related-to: harvester/harvester#6234

Accept kubeconfig as base64 string.
When configuring the Harvester Terraform provider, the kubeconfig can
either be given as path to a file, from which to read, or the contents
can be supplied directly as base64 string.
This allows for greater flexibility in supplying this sensitive data to
the Terraform provider.

related-to: harvester/harvester#6234

Signed-off-by: Moritz Röhrich <[email protected]>
@m-ildefons m-ildefons added the enhancement New feature or request label Jan 6, 2025
@m-ildefons m-ildefons self-assigned this Jan 6, 2025
@ihcsim
Copy link

ihcsim commented Jan 6, 2025

Are we ok with the cluster user private key (users.user[*].client-key-data) being stored as plain text in the Terraform's state file? I am tempted to say we shouldn't support anything more than what the Terraform Kubernetes provider supports. Anything more feels like user CI-specific problem.

@m-ildefons
Copy link
Contributor Author

This is indeed a concern, but Terraform Enterprise or HCP Terraform are able to encrypt sensitive data before storing it:
https://developer.hashicorp.com/terraform/tutorials/configuration-language/sensitive-variables#sensitive-values-in-state

@ihcsim
Copy link

ihcsim commented Jan 7, 2025

If it's up to the user to secure their statefile while using our provider, then I think we should document it somewhere. Normally, a provider would generate sensitive data as an output or data. Then the user can decide if they want to write them to disk or use in memory.

@JasonIamAUnixAdmin
Copy link

This would help us with an issue we have getting this to work with Terraform cloud

@sdt-ssexton
Copy link

Looking forward to this as I think this would fix an issue we're encountering

Document the risk of leaking sensitive information via the Terraform
state files when supplying the kubeconfig as base64 encoded string.

Signed-off-by: Moritz Röhrich <[email protected]>
@m-ildefons
Copy link
Contributor Author

I've added a warning in the documentation/example that informs the reader of the risk of leaking sensitive information via the Terraform state files.

Copy link
Member

@FrankYang0529 FrankYang0529 left a comment

Choose a reason for hiding this comment

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

LGTM. I can use kubeconfig with base64 encode. Thanks.

FieldProviderKubeContext = "kubecontext"
FieldProviderKubeConfig = "kubeconfig"
FieldProviderKubeContext = "kubecontext"
FieldProviderKubeConfigBase64 = "kubeconfig_base64"
Copy link

Choose a reason for hiding this comment

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

It doesn't look like this is being used.

@@ -59,3 +68,16 @@ func NewClient(kubeConfig, kubeContext string) (*Client, error) {
HarvesterNetworkClient: harvNetworkClient,
}, nil
}

func restConfigFromFile(kubeConfig, kubeContext string) (*rest.Config, error) {
Copy link

Choose a reason for hiding this comment

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

Will it make sense to move the homedir.Expand(d.Get(constants.FieldProviderKubeConfig).(string)) at

kubeConfig, err := homedir.Expand(d.Get(constants.FieldProviderKubeConfig).(string))
if err != nil {
return nil, diag.FromErr(err)
}
into this new function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants