tfv: Fix kms key ring iam asset name#6762
Conversation
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. TF Validator: Diff ( 10 files changed, 339 insertions(+), 484 deletions(-)) |
roaks3
left a comment
There was a problem hiding this comment.
Overall looks good to me, just had one minor question
| if len(splits) == 3 { | ||
| assetNameTemplate = fmt.Sprintf("//cloudkms.googleapis.com/projects/%s/locations/%s/keyRings/%s", splits[0], splits[1], splits[2]) | ||
| } else if len(splits) == 2 { | ||
| assetNameTemplate = fmt.Sprintf("//cloudkms.googleapis.com/projects/{{project}}/locations/%s/keyRings/%s", splits[0], splits[1]) | ||
| } |
There was a problem hiding this comment.
Nit: I can't quite tell what would happen if the key_ring_id is malformed, and we don't reach either of these conditions. Perhaps a schema validation prevents this?
There was a problem hiding this comment.
key_ring_id is from user input, it can go wrong easily, if neither condition is met, then it uses the original string, and I think the code has tried its best to convert in that case.
b/256892857
Fix google_kms_key_ring_iam asset name
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintto ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)