-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add helm release addon non-sensitive outputs #700
Conversation
Signed-off-by: Fernando Miguel <github@FernandoMiguel.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FernandoMiguel interesting PR. Would you be able to add some more details in the motivation? What is the use-case for being able to consume these outputs? Maybe an code example would help.
Also the CI check is failing. Running pre-commit run terraform_docs
should fix that.
@askulkarni2 I was looking to get the version of the charts installed, and also looking for newer versions that I could upgrade to. I also have a single liner that does what this does, but without descriptions, and also includes the sets and value files, if the team prefers that. if you can provide me with further instructions on the pre-commit command, I can try that tomorrow. |
instead of adding all outputs, would it be possible to only export the outputs that are needed? we could also do a single full export like: output "helm_release" {
description = "Attributes of Helm release created"
value = helm_release.addon
} |
The password attribute is sensitive and would block the all output downstream. |
Signed-off-by: Fernando Miguel <github@FernandoMiguel.net>
Signed-off-by: Fernando Miguel <github@FernandoMiguel.net>
…l/terraform-aws-eks-blueprints into add-addons-outputs
Signed-off-by: Fernando Miguel <github@FernandoMiguel.net>
Pls review. |
description = "Describes the desired status of a chart in a kubernetes cluster." | ||
value = try( | ||
{ | ||
for k, v in helm_release.addon : k => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the value here would be:
value = try({ for k, v in helm_release.addon : k => v if k != "repository_password" }, {})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either works in my testing.
I tend to go with not setting explicitly anything, hence null.
But not all providers handle that well.
I'm fine following your existing style, if you prefer.
see comment above and it looks like there are CI failures to address (pre-commit) |
thank you for picking up this @bryantbiggs |
Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
What does this PR do?
add helm_addons outpus
Motivation
So practitioners can add outputs to modules
More
pre-commit run -a
with this PRNote: Not all the PRs required examples and docs except a new pattern or add-on added.
For Moderators