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.

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

Specifying Email Contacts for azurerm_key_vault_certificate #1301

Closed
ksista-ahc opened this issue May 25, 2018 · 10 comments · Fixed by #8937
Closed

Specifying Email Contacts for azurerm_key_vault_certificate #1301

ksista-ahc opened this issue May 25, 2018 · 10 comments · Fixed by #8937

Comments

@ksista-ahc
Copy link

ksista-ahc commented May 25, 2018

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Specifying email contacts to be updated in Azure Key vault. In the resource 'azurerm_key_vault_certificate', we can only specify action_type as 'EmailContacts' but we cannot specify the email addresses to update email contacts for a specific key vault

New or Affected Resource(s)

  • azurerm_key_vault_certificate
@tombuildsstuff
Copy link
Member

hey @kalyanrajsista

Thanks for opening this issue :)

Taking a quick look into this - it appears that support for this is available in the SDK using the SetCertificateContacts method on the Key Vault Base Client - as such it looks like there's no SDK blockers for this.

Thanks!

@Lucretius
Copy link
Contributor

Lucretius commented Mar 19, 2019

@tombuildsstuff this seems to be something we would want to have specified on the Key Vault as a whole. Perhaps we can expose this property on the azurerm_key_vault resource instead? If so, I can pick this up.

EDIT: It also may be reasonable to just make a separate azurerm_key_vault_certificate_contacts resource. If we add this to either the azurerm_key_vault or the azurerm_key_vault_certificate resources, we end up having to make two calls when creating or updating the resource, which causes the operation to be non-atomic.

@tombuildsstuff
Copy link
Member

@Lucretius thinking about this I think this'd make more sense within the azurerm_key_vault resource rather than being split out into it's own resource, since this is likely to be managed by the owners of the Key Vault, rather than anyone else - WDYT?

@Lucretius
Copy link
Contributor

Lucretius commented Mar 20, 2019

The only thing I was concerned about was it being a separate call after the resource already exists in order to add the contacts via the SDK. Still learning how the provider works - would putting this logic after the SetId function call ensure that - if for some reason the SetCertificateContacts call fails - that the following plan/apply just applies the contacts piece and doesn’t trigger a forceNew on the entire keyvault?

@tombuildsstuff
Copy link
Member

tombuildsstuff commented Mar 28, 2019

@Lucretius sorry for the delayed reply - we tend to have Create call the Update method (which calls Read, instead of Create -> Read) when this happens and then make these changes inside of the Update function - WDYT?

@Lucretius
Copy link
Contributor

The Key Vault resource currently uses one function for both Create and Update - I do have a working prototype of this where I check if the contacts length is > 0 and if so, set the contacts, otherwise delete them, and I make that call just after the key vault itself is created. Seems to be working just need to fix a few bugs and write some tests. But I also have some open PRs I want to close out before I go any further on this one. Thanks @tombuildsstuff for the explainer on the existing CRUD pattern for resources - it definitely is helpful when trying to figure out how best to approach these problems!

@Lucretius
Copy link
Contributor

Lucretius commented Apr 4, 2019

@tombuildsstuff I am running into the following case:

The access policy of the Azure entity (service principal/MSI/etc) executing terraform must have the managecontacts certificate permission enabled in order to actually GetCertificateContacts/SetCertificateContacts on the contact list. I am not sure how I can determine whether or not this permission is enabled. The problem comes from trying to read the certificate contacts list. I cannot try to perform the GetCertificateContacts call everytime or else we will run into API failures if that permission is not applied (maybe the user does not want to manage contacts at all).

If there is not a way to check this, I don't see a way around handling this in a separate resource. What do you think?

@njuCZ
Copy link
Contributor

njuCZ commented May 13, 2020

any updates? is it OK for me to take over this issue and continue ? After investigating, I think an individual resource azurerm_key_vault_certificate_contact might be better? Because reading/listing contacts needs to have manage contacts access policy, and it serves azurerm_key_vault_certificate. If put it in the azurerm_key_vault resource, the read function will invoke listing contacts function and will throw error if users don't have manage contact permission

@katbyte katbyte added this to the v2.34.0 milestone Oct 27, 2020
katbyte pushed a commit that referenced this issue Oct 27, 2020
fix #1301

making certificate contact as a standalone resource is pushed back. submit this PR to make it as a embeded field in "key_vault" resource
@ghost
Copy link

ghost commented Oct 29, 2020

This has been released in version 2.34.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.34.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Nov 27, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.