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

Add sensitive flag for two outputs #294

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

hjoh95
Copy link
Contributor

@hjoh95 hjoh95 commented Apr 19, 2022

Running a terraform plan using the postgresql module resulted in two errors for these outputs. I believe this is the expected fix

@google-cla
Copy link

google-cla bot commented Apr 19, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@comment-bot-dev
Copy link

@hjoh95
Thanks for the PR! 🚀
✅ Lint checks have passed.

@morgante
Copy link
Contributor

This is technically not required, because this module is not meant as a root module, but we can add it anyways.

@morgante morgante merged commit 17da833 into terraform-google-modules:master Apr 19, 2022
@PhilBrammer
Copy link

Isn't this a breaking change, @morgante? I'm assuming that there are implementations that automate/screen against TF APPLY output.

@morgante
Copy link
Contributor

morgante commented May 4, 2022

@PhilBrammer I wouldn't consider a sensitive flag a breaking change. It shouldn't impact any automated configuration to switch an output to sensitive, since Terraform can still reference sensitive outputs.

Did this break you in some way?

@PhilBrammer
Copy link

Not directly. Working with @hjoh95 on this, but seems to me if I were using this module, I would've been getting a list of instances created as output. Now those would be omitted, correct? And to me, that seems like a breaking change for any consumer expecting this output. Sure they can adapt, but am wondering if this approach is safe "just pushing through" when these modules were being used in the wrong way anyhow? @hjoh95 created a wrapper to use this module correctly.

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 this pull request may close these issues.

None yet

4 participants