Adding uri_sans to pki_secret_backend_cert #759
Conversation
ghost
added
theorlandog
changed the title
theorlandog
changed the title
|
@theorlandog This PR has changed all the file permissions to 755. The previous value of 644 was correct. If you revert that ( |
|
@bendrucker Sorry about that. Windows being weird. Just updated for you. |
|
No worries, I was just here checking in on another PR and figured I'd mention it so you don't waste a review cycle with an actual maintainer. Which I'm not FWIW, wish I could help you get this merged. Looks like the line endings got converted to Tweaking your git settings should help with that, specifically https://help.github.com/en/github/using-git/configuring-git-to-handle-line-endings |
|
@bendrucker ahh good looks. Definitely gotta spend the time to learn my git tricks some day. Thanks for helping me out. |
| Type: schema.TypeList, | ||
| Optional: true, | ||
| Description: "List of alternative URIs.", | ||
| ForceNew: true, |
There was a problem hiding this comment.
Should this really ForceNew if this field changes?
There was a problem hiding this comment.
Hi @tyrannosaurus-becks,
To be quite honest, I lifted most of this code for a similar PR raised against CAs. https://github.com/terraform-providers/terraform-provider-vault/pull/373/files#diff-318e4d5fc8ea223f57928c837b6ea671R60. In that PR, it was set to ForceNew as well. I would think force new matches this matches the intent, as you probably need the uri set in the cert if your are taking the time to set it in the tf.
There was a problem hiding this comment.
I don't see any option to update certificates in the API docs so I think ForceNew: true is correct here.
| @@ -167,6 +176,12 @@ func pkiSecretBackendCertCreate(d *schema.ResourceData, meta interface{}) error | |||
| ipSans = append(ipSans, iIpSan.(string)) | |||
| } | |||
|
|
|||
| iURISans := d.Get("uri_sans").([]interface{}) | |||
There was a problem hiding this comment.
Could we simplify this code if we did something more like:
if raw, ok := d.GetOk("uri_sans"); ok {
data["uri_sans"] = raw
}
Or is there a downside to that?
There was a problem hiding this comment.
Hi @tyrannosaurus-becks,
Same idea as the last comment. I jacked most of this code from another PR. https://github.com/terraform-providers/terraform-provider-vault/pull/373/files#diff-318e4d5fc8ea223f57928c837b6ea671R193. It smells like your block would be equivalent, but maybe it makes sense to keep the two related blocks to use the same form? Not sure your opinion there.
| Type: schema.TypeList, | ||
| Optional: true, | ||
| Description: "List of alternative URIs.", | ||
| ForceNew: true, |
There was a problem hiding this comment.
I don't see any option to update certificates in the API docs so I think ForceNew: true is correct here.
|
Hello all - this was released in |
Fixes #758
Community Note
Relates OR Closes #758
Release note for CHANGELOG: