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

Revert changes to STS leases but keep the ttl field #20034

Merged
merged 6 commits into from
Apr 13, 2023

Conversation

kschoche
Copy link
Contributor

@kschoche kschoche commented Apr 6, 2023

@kschoche kschoche changed the title [wip] revert changes, create a lease for STS credentials but keep the ttl [wip] reverting changes, create a lease for STS credentials but keep the ttl Apr 6, 2023
@kschoche kschoche added this to the 1.13.2 milestone Apr 7, 2023
@kschoche kschoche added the backport/1.13.x Backport changes to `release/1.13.x` label Apr 12, 2023
@kschoche kschoche changed the title [wip] reverting changes, create a lease for STS credentials but keep the ttl Revert changes to STS leases but keep the ttl field Apr 12, 2023
@kschoche kschoche self-assigned this Apr 12, 2023
@kschoche kschoche marked this pull request as ready for review April 12, 2023 18:50
@kschoche kschoche requested review from a team, robmonte and austingebauer April 12, 2023 18:50
changelog/20034.txt Outdated Show resolved Hide resolved
Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com>
Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

LGTM

@alex-ikse
Copy link

Will fix: hashicorp/terraform-provider-vault#1805

@maxb
Copy link
Contributor

maxb commented Apr 13, 2023

But this is going to revert the efficiency optimization which was the original purpose of removing the leases too...

Wouldn't it be better to aim for a solution that gave clients the necessary compatible feedback in responses, but also retained the optimization?

Copy link
Member

@robmonte robmonte left a comment

Choose a reason for hiding this comment

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

👍

@alex-ikse
Copy link

Wouldn't it be better to aim for a solution that gave clients the necessary compatible feedback in responses, but also retained the optimization?

Terraform Vault provider must be updated before doing this kind of change. See: hashicorp/terraform-provider-vault#1805

@kschoche kschoche merged commit 8fa5605 into main Apr 13, 2023
85 checks passed
@kschoche
Copy link
Contributor Author

But this is going to revert the efficiency optimization which was the original purpose of removing the leases too...

Wouldn't it be better to aim for a solution that gave clients the necessary compatible feedback in responses, but also retained the optimization?

Hi @maxb - Thanks for pointing this out. Right now, it’s our first priority to fix the broken clients. We can circle back and introduce a no_lease option in the future for those who are concerned about the cost of the lease.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13.x Backport changes to `release/1.13.x` bug Used to indicate a potential bug secret/aws
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS secrets engine no longer returns lease_duration for STS credentials
6 participants