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

Marking assets_dist as a sensitive output in typhoon as well #884

Closed
vroad opened this issue Nov 22, 2020 · 3 comments · Fixed by #885
Closed

Marking assets_dist as a sensitive output in typhoon as well #884

vroad opened this issue Nov 22, 2020 · 3 comments · Fixed by #885

Comments

@vroad
Copy link

vroad commented Nov 22, 2020

Description

Isn't assets_dist included by mistake in typhoon module output? The comment above the output says "outputs for debug". It's very long to read every time, not relevant if you are not debugging typhoon, and contains sensitive information that you should avoid printing to console.

https://github.com/poseidon/typhoon/blob/v1.19.4/aws/flatcar-linux/kubernetes/outputs.tf#L57-L59
https://github.com/poseidon/terraform-render-bootstrap/blob/49216ab82c236520204c4c85c8e52edbd722e1f4/outputs.tf#L20-L34

Steps to Reproduce

Do terraform apply for aws/flatcar-linux/kubernetes

Expected behavior

Not visible unless you explicitly terrafom output assets_dist, or unavailable from typhoon users at all

Environment

  • Platform: aws
  • OS: flatcar-linux
  • Release: v1.19.4
  • Terraform: v0.13.5

Possible Solution

Mark the output as sensitive, or just remove the output

@dghubble
Copy link
Member

Is assets_dist included by mistake in outputs?

Cluster generated assets are properties of the cluster, made accessible as outputs, by design. For example, you may need to provide the etcd client credentials to another component, need to know the cluster CA, or some workflows still write assets to a separate storage system. Also explained in #595 Its not a goal to make these generated assets inaccessible to users.

Improving console output to hide sensitive output is a good goal, esp. for CI systems. Though a typical plan and apply does not display the content you seem to be seeing. How are you defining your module and running terraform? Which apply (in the lifecycle)? Are you sure you're not running with TF_DEBUG or other options?

@vroad
Copy link
Author

vroad commented Nov 23, 2020

So it's not for debugging and designed to be used by typhoon users?
Then why the comment says "outputs for debug"? Shouldn't we remove that?

https://github.com/poseidon/typhoon/blob/v1.19.4/aws/flatcar-linux/kubernetes/outputs.tf#L55

Instead of using typhoon as a child module, I applied typhoon with Terragrunt. Sorry I should have mentioned that. Terragrunt clones repo to a temporary directory, checks out module directory, and run "terrafopm apply" treating typhoon as a root module, so output of the module is displayed.

Another way to reproduce the issue without terragrunt is fork typhoon repository (or one of its mirrors), go to module directory, add "terrform.tfvars" to git and apply.

Or create something like "prod.tfvars" and pass it with "-var-file" argument of terraform.

Even though terraform-render-bootstrap is often used as a child module, it already marks some of outputs as sensitive, so it makes sense to do the same for typhoon.

@vroad vroad changed the title Isn't assets_dist included by mistake in typhoon outputs? Marking assets_dist as a sensitive output in typhoon as well Nov 23, 2020
dghubble added a commit that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes #884
@dghubble
Copy link
Member

Thanks for clarifying your usage. #885 should resolve this and its worthwhile for Terraform v0.14 readiness.

Officially, I only recommend usage patterns shown in the docs. Mileage may vary with wrapper tools (out of scope). Terraform's tfvars files have limitations and using a root module with -var-file is not a practice I'd recommend. Better to write native Terraform declarations in version control, plan for a world with multiple clusters and other resources, and apply without arguments that modify what is done.

Concerning the comment, inspecting generated assets is another valid usage of asset_dist, often for debugging or learning. The comment doesn't need to enumerate every possible reason the feature is kept imo.

dghubble-robot pushed a commit to poseidon/terraform-digitalocean-kubernetes that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon/typhoon#884
dghubble-robot pushed a commit to poseidon/terraform-aws-kubernetes that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon/typhoon#884
dghubble-robot pushed a commit to poseidon/terraform-google-kubernetes that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon/typhoon#884
dghubble-robot pushed a commit to poseidon/terraform-azure-kubernetes that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon/typhoon#884
dghubble-robot pushed a commit to poseidon/terraform-onprem-kubernetes that referenced this issue Nov 23, 2020
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon/typhoon#884
Snaipe pushed a commit to aristanetworks/monsoon that referenced this issue Apr 13, 2023
* Mark `kubeconfig` and `asset_dist` as `sensitive` to
prevent the Terraform CLI displaying these values, esp.
for CI systems
* In particular, external tools or tfvars style uses (not
recommended) reportedly display all outputs and are improved
by setting sensitive
* For Terraform v0.14, outputs referencing sensitive fields
must also be annotated as sensitive

Closes poseidon#884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants