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

azurerm_key_vault_secret - support for not_before_date and expiration_date #4873

Merged
merged 3 commits into from
Nov 19, 2019
Merged

azurerm_key_vault_secret - support for not_before_date and expiration_date #4873

merged 3 commits into from
Nov 19, 2019

Conversation

aqche
Copy link
Contributor

@aqche aqche commented Nov 14, 2019

Fixes #4858

Adds the not_before_date and expiration_date options to the azurerm_key_vault_secret resource.

--- PASS: TestAccAzureRMKeyVaultSecret_complete (219.48s)
--- PASS: TestAccAzureRMKeyVaultSecret_disappearsWhenParentKeyVaultDeleted (224.89s)
--- PASS: TestAccAzureRMKeyVaultSecret_basic (235.53s)
--- PASS: TestAccAzureRMKeyVaultSecret_update (238.80s)
--- PASS: TestAccAzureRMKeyVaultSecret_disappears (244.47s)
--- PASS: TestAccAzureRMKeyVaultSecret_basicClassic (247.29s)

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

Hey @aqche, this PR is nearly there. We just need to do a nil check and remove a duplicated line and we should be good

@@ -280,6 +334,14 @@ func resourceArmKeyVaultSecretRead(d *schema.ResourceData, meta interface{}) err
d.Set("version", respID.Version)
d.Set("content_type", resp.ContentType)

if v := resp.Attributes.NotBefore; v != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Attributes has the potential to be nil so we should that it isn't nil before referencing NotBefore and Expires

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, added a nil check for Attributes

@@ -153,6 +153,9 @@ func TestAccAzureRMKeyVaultSecret_complete(t *testing.T) {
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMKeyVaultSecretExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "not_before_date", "2019-01-01T01:02:03Z"),
resource.TestCheckResourceAttr(resourceName, "expiration_date", "2020-01-01T01:02:03Z"),
resource.TestCheckResourceAttr(resourceName, "tags.hello", "world"),
Copy link
Member

Choose a reason for hiding this comment

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

The same check is done just a couple lines below. Can we remove this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, removed!

@aqche
Copy link
Contributor Author

aqche commented Nov 18, 2019

@mbfrahry thanks for the review! addressed the comments. please take a look again when you have the chance.

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks for addressing that

Comment on lines 154 to 157
notBeforeDate, err := time.Parse(time.RFC3339, v.(string))
if err != nil {
return fmt.Errorf("error parsing `not_before_date` time: %s", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

technically we don't need to check for an error here because the validation function should catch all non valid time strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense, removed 👍

Comment on lines 163 to 166
expirationDate, err := time.Parse(time.RFC3339, v.(string))
if err != nil {
return fmt.Errorf("error parsing `expiration_date` time: %s", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

TestAccAzureRMPostgreSQLDatabase_collationWithHyphen

Comment on lines 226 to 229
notBeforeDate, err := time.Parse(time.RFC3339, v.(string))
if err != nil {
return fmt.Errorf("error parsing `not_before_date` time: %s", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

TestAccAzureRMPostgreSQLDatabase_collationWithHyphen

Comment on lines 235 to 238
expirationDate, err := time.Parse(time.RFC3339, v.(string))
if err != nil {
return fmt.Errorf("error parsing `expiration_date` time: %s", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

TestAccAzureRMPostgreSQLDatabase_collationWithHyphen

@katbyte katbyte added this to the v1.37.0 milestone Nov 19, 2019
@aqche
Copy link
Contributor Author

aqche commented Nov 19, 2019

@katbyte updated per your suggestion, let me know what you think!

@mbfrahry mbfrahry changed the title Add not_before_date and expiration_date args to azurerm_key_vault_secret azurerm_key_vault_secret - support for not_before_date and expiration_date Nov 19, 2019
@mbfrahry mbfrahry merged commit 784e232 into hashicorp:master Nov 19, 2019
@mbfrahry
Copy link
Member

Perfect! Thanks for getting this written. We'll get that out in the next release

mbfrahry added a commit that referenced this pull request Nov 19, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

This has been released in version 1.37.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 = "~> 1.37.0"
}
# ... other configuration ...

@aqche aqche deleted the azurerm_key_vault_secret_date_options branch December 5, 2019 05:42
@ghost
Copy link

ghost commented Dec 19, 2019

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Dec 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for set and get activation and expiration date on azurerm_key_vault_secret
3 participants