google_certificate_manager_certificate - self_managed block is not sensitive

[ruanda]

According to the documentation (https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/certificate_manager_certificate#self_managed) self_managed block should be sensitive and not visible in plan. google-provider v4.48.0 does not mark this property as sensitive and I can see pem_private_key in clear text in plan.

[edwardmedia]

@ruanda I see below attributes are set sensitive. Don't you want the key to be visible? We want to hide its value, for sure

terraform-provider-google/google/resource_certificate_manager_certificate.go

Line 194 in 576870f

Sensitive: true,

terraform-provider-google/google/resource_certificate_manager_certificate.go

Line 216 in 576870f

Sensitive: true,

[ruanda]

Looks like only private_key_pem and certificate_pem are set to sensitive. Both are also marked as deprecated in favor of pem_certificate and pem_private_key which are not sensitive. I followed the docs and used new attributes and got surprised with private key visible in plan. Anyway, I don't think certificate_pem and pem_certificate should be sensitive, it does contain public cert after all.

[rileykarson]

@ruanda: If I've got this right, you'd prefer we unmark self_managed as sensitive in that case, is that right? For the deprecated subfields we should preserve it imo (and just remove those fields when we get the chance in a major version- they aren't supposed to be there).

That will mean the bottom-level fields that are currently sensitive will remain sensitive, while the top-level field's documentation will no longer suggest all its subfields are sensitive.

[ruanda]

Let me clarify

1. private_key_pem and pem_private_key should both be sensitive. They contain private key, so definitely sensitive data. For now only depracated private_key_pem is sensitive.
2. certificate_pem and pem_certificate contains public cert, I don't think they should be sensitive. For now certificate_pem is sensitive.
3. self_managed block is sensitive but I am not sure what it means exactly as my plan showed me diff in plain text for pem_private_key in self_managed block. Probably should not be sensitive as it doesn't work the way it is described in the documentation and marking it as sensitive is misleading.

[rileykarson]

Ah, I'd missed that one field should likely be sensitive while the other shouldn't. Thanks!

I'd prefer we keep certificate_pem sensitive as it's not clear whether removing it requires a major version or not, and the value of removing it is fairly low. It probably doesn't require one.

That means our delta would be:

• remove sensitive from self_managed since adding it to a parent field doesn't appear to do anything
• add sensitive to pem_private_key

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.