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.

Sign up for GitHub

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

Jump to bottom

Adding uri_sans to pki_secret_backend_cert #759

Merged
merged 5 commits into from
Aug 24, 2020
Merged

Adding uri_sans to pki_secret_backend_cert #759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
40c0992
adding uri_sans to pki_secret_backend_cert
orlando-jamie May 15, 2020
cbbc56a
fixing test case
orlando-jamie May 15, 2020
756bebe
Fix file permissions
theorlandog May 18, 2020
6d31985
running dos2unix
theorlandog May 18, 2020
1c3da56
running dos2unix
theorlandog May 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions vault/resource_pki_secret_backend_cert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ func pkiSecretBackendCertResource() *schema.Resource {
Type: schema.TypeString,
},
},
"uri_sans": {
Type: schema.TypeList,
Optional: true,
Description: "List of alternative URIs.",
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this really ForceNew if this field changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any option to update certificates in the API docs so I think ForceNew: true is correct here.

Elem: &schema.Schema{
Type: schema.TypeString,
},
},
"other_sans": {
Type: schema.TypeList,
Optional: true,
Expand Down Expand Up @@ -167,6 +176,12 @@ func pkiSecretBackendCertCreate(d *schema.ResourceData, meta interface{}) error
ipSans = append(ipSans, iIpSan.(string))
}

iURISans := d.Get("uri_sans").([]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

uriSans := make([]string, 0, len(iURISans))
for _, iUriSan := range iURISans {
uriSans = append(uriSans, iUriSan.(string))
}

iOtherSans := d.Get("other_sans").([]interface{})
otherSans := make([]string, 0, len(iOtherSans))
for _, iOtherSan := range iOtherSans {
Expand All @@ -189,6 +204,10 @@ func pkiSecretBackendCertCreate(d *schema.ResourceData, meta interface{}) error
data["ip_sans"] = strings.Join(ipSans, ",")
}

if len(uriSans) > 0 {
data["uri_sans"] = strings.Join(uriSans, ",")
}

if len(otherSans) > 0 {
data["other_sans"] = strings.Join(otherSans, ",")
}
Expand Down
4 changes: 4 additions & 0 deletions vault/resource_pki_secret_backend_cert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ func TestPkiSecretBackendCert_basic(t *testing.T) {
resource.TestCheckResourceAttr("vault_pki_secret_backend_cert.test", "backend", intermediatePath),
resource.TestCheckResourceAttr("vault_pki_secret_backend_cert.test", "common_name", "cert.test.my.domain"),
resource.TestCheckResourceAttr("vault_pki_secret_backend_cert.test", "ttl", "720h"),
resource.TestCheckResourceAttr("vault_pki_secret_backend_cert.test", "uri_sans.#", "1"),
resource.TestCheckResourceAttr("vault_pki_secret_backend_cert.test", "uri_sans.0", "spiffe://test.my.domain"),
),
},
},
Expand Down Expand Up @@ -123,6 +125,7 @@ resource "vault_pki_secret_backend_role" "test" {
name = "test"
allowed_domains = ["test.my.domain"]
allow_subdomains = true
allowed_uri_sans = ["spiffe://test.my.domain"]
max_ttl = "3600"
key_usage = ["DigitalSignature", "KeyAgreement", "KeyEncipherment"]
}
Expand All @@ -132,6 +135,7 @@ resource "vault_pki_secret_backend_cert" "test" {
backend = "${vault_pki_secret_backend.test-intermediate.path}"
name = "${vault_pki_secret_backend_role.test.name}"
common_name = "cert.test.my.domain"
uri_sans = ["spiffe://test.my.domain"]
ttl = "720h"
min_seconds_remaining = 60
}`, rootPath, intermediatePath)
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/pki_secret_backend_cert.html.md
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ The following arguments are supported:

* `ip_sans` - (Optional) List of alternative IPs

* `uri_sans` - (Optional) List of alternative URIs

* `other_sans` - (Optional) List of other SANs

* `ttl` - (Optional) Time to live
Expand Down