support os_profile_secrets #150
Conversation
|
|
|
Hi @AliAllomani Thanks for your contribution. Would you mind add a test in test/fixture and add the corresponding fields in README? |
|
Hi @AliAllomani After some changes, the test has failed too. |
|
@yupwei68 Thanks for your comments, i've fixed the required. |
|
Hi @AliAllomani Thanks for contribution. This PR looks almost good to me. The test has passed. Would you add this feature into the example in README to show how to use this feature? |
|
@yupwei68 I added the usage instructions to README, i skipped adding a full example as it would be so long to include the whole key vault resources definitions, let me know if that looks good to you or if you have any other suggestions. |
README.md
Outdated
| os_profile_secrets = [{ | ||
| source_vault_id = azurerm_key_vault.test.id | ||
| certificate_url = azurerm_key_vault_certificate.test.secret_id | ||
| certificate_store = "My" | ||
| }] |
There was a problem hiding this comment.
Here, could we add the features in the Advanced Usage example? Could we add two data source azurerm_key_vault and azurerm_key_vault_certificate and refer them here, like (here we use name example):
data "azurerm_key_vault" "example" {
name = "examplekeyvault"
resource_group_name = azurerm_resource_group.example.name
}
data "azurerm_key_vault_certificate" "example" {
name = "example-kv-cert"
key_vault_id = data.azurerm_key_vault.example.id
}
...
os_profile_secrets = [{
source_vault_id = data.azurerm_key_vault.example.id
certificate_url = data.azurerm_key_vault_certificate.example.secret_id
certificate_store = "My"
}]
....
README.md
Outdated
| You can install custom certificates / secrets on the virtual machine from Key Vault by using the variable `os_profile_secrets`. | ||
|
|
||
| The variable accepts a list of maps with the following keys: | ||
|
|
||
| * source_vault_id : The ID of the Key Vault Secret which contains the encrypted Certificate. | ||
| * certificate_url : The certificate URL in Key Vault | ||
| * certificate_store : The certificate store on the Virtual Machine where the certificate should be added to (Windows Only). |
There was a problem hiding this comment.
Could we add this in the 4th note of Advanced Usage?
test/fixture/main.tf
Outdated
|
|
There was a problem hiding this comment.
Could we remove the unnecessary changes?
There was a problem hiding this comment.
Here we need to add this feature into linux vm to test it as well. Please add it here:
os_profile_secrets = [{
source_vault_id = azurerm_key_vault.test.id
certificate_url = azurerm_key_vault_certificate.test.secret_id
certificate_store = "My"
}]
variables.tf
Outdated
| @@ -220,3 +220,9 @@ variable "identity_ids" { | |||
| type = list(string) | |||
| default = [] | |||
| } | |||
|
|
|||
| variable "os_profile_secrets" { | |||
| description = "Specifies a list of certificates to be installed on the VM, each list item is a map with the keys source_vault_id, certificate_url, certificate_store (Windows Only)" | |||
There was a problem hiding this comment.
Here is not only for Windows, it's for linux too.
description = "Specifies a list of certificates to be installed on the VM, each list item is a map with the keys source_vault_id, certificate_url, certificate_store "
There was a problem hiding this comment.
"Windows Only" here is referring to the key "certificate_store" only, but i can see that this may create some confusion that way that the whole "os_profile_secrets" is windows only, i will remove it from here and keep it in readme only.
adding support for os_profile_secrets
example variable usage