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

Add new field with_lease_start_time to data.vault_generic_secret #1414

Merged
merged 4 commits into from
Apr 14, 2022
Merged

Add new field with_lease_start_time to data.vault_generic_secret #1414

merged 4 commits into from
Apr 14, 2022

Conversation

vinay-gopalan
Copy link
Contributor

This PR adds a new field with_lease_start_time to the vault_generic_secret data source. If set to false, the field lease_start_time is not set to the TF state.

Fixes: #873

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestDataSourceGenericSecret_v2'
=== RUN   TestDataSourceGenericSecret_v2
--- PASS: TestDataSourceGenericSecret_v2 (4.50s)
PASS

$ make testacc TESTARGS='-run=TestDataSourceGenericSecret'
=== RUN   TestDataSourceGenericSecret
--- PASS: TestDataSourceGenericSecret (3.80s)
PASS

@vinay-gopalan vinay-gopalan requested a review from benashz April 14, 2022 20:57
benashz
benashz reviewed Apr 14, 2022
View reviewed changes
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

Looks great! A few things to address and 👍

vault/data_source_generic_secret.go

if v, ok := d.GetOkExists("with_lease_start_time"); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

I recently discovered that you can use GetOk() on a boolean field, if it has a default value. It seems to have worked for me. May want to verify this however.

benashz
benashz approved these changes Apr 14, 2022
View reviewed changes
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

LGTM!

@vinay-gopalan vinay-gopalan merged commit 37cca58 into main Apr 14, 2022
@vinay-gopalan vinay-gopalan deleted the VAULT-5350/with-lease-start-time branch April 14, 2022 22:35
@benashz benashz added this to the 3.5.0 milestone Apr 19, 2022
marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022
@vinay-gopalan
Add new field with_lease_start_time to data.vault_generic_secret (h…
faa1ab0 
…ashicorp#1414)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benashz benashz benashz approved these changes

Assignees
No one assigned
Labels
documentation size/S
Projects
None yet
Milestone
3.5.0
Development

Successfully merging this pull request may close these issues.

data.vault_generic_secret requires new plan and apply on every run
2 participants
@vinay-gopalan @benashz