Fix repository authentication when updating a helm_release with temporary credentials - #1687
Fix repository authentication when updating a helm_release with temporary credentials#1687kevinfrommelt wants to merge 5 commits into
Conversation
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
| var state HelmReleaseModel | ||
| diags := req.Plan.Get(ctx, &state) | ||
| var plan HelmReleaseModel | ||
| diags := req.Plan.Get(ctx, &plan) |
There was a problem hiding this comment.
I renamed this variable to plan for clarity, since it is using the plan model.
There was a problem hiding this comment.
Just wondering this is going to change the credentials to be used from the plan and removes it from the state? I assume this is the issue many people are having now where credentials are being saved in state and ephemeral tokens are timing out causing failures?
There was a problem hiding this comment.
The issue is actually at the bottom of the diff in the Update() function. It's getting the login credentials from the previously applied state, rather than using the currently fetched credentials.
There was a problem hiding this comment.
Ah I see it now, thank you for the clarification. Appreciate you submitting this!
kevinfrommelt
changed the title
|
I've built and tested this locally successfully. It resolved the problem I've been having with @jrhouston could you please review this and have it part of a patch release soon ? Thank you. |
|
Can this pls be prioritized 🙏. This been an issue for few months now and affecting meny people #1660. Everyone does manual delete/import as an workaround for helm modules on expired secrets. |
|
Hey, any updates here? |
|
any update from the maintainers? this bug has been going for months and it affects a core functionality of the provider |
|
@jrhouston Hi, could you look into this PR? It's blocking a migration to the latest version of this provider. |
| return | ||
| } | ||
| ociDiags := OCIRegistryLogin(ctx, meta, actionConfig, meta.RegistryClient, state.Repository.ValueString(), state.Chart.ValueString(), state.RepositoryUsername.ValueString(), state.RepositoryPassword.ValueString()) | ||
| ociDiags := OCIRegistryLogin(ctx, meta, actionConfig, meta.RegistryClient, plan.Repository.ValueString(), plan.Chart.ValueString(), plan.RepositoryUsername.ValueString(), plan.RepositoryPassword.ValueString()) |
There was a problem hiding this comment.
Use the credentials provided in the plan instead of the state. Users may have provided new credentials, especially when using temporary credentials that have a short expiration.
|
Any updates? It causes many issues |
|
@jrhouston Hi, could you look into this PR? It's blocking us for a long time. |
|
Hi, any updates here? |
|
Can this pls be prioritized? |
|
This is breaking my helm deployments from my ECR repository. Can we get a maintainer to review this or a workaround? |
10 participants
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No
Description
Repository login currently fails during updates when using temporary credentials because the credentials are being pulled from the state rather than from the current plan.
This PR changes
Update()to use the credentials from the plan model instead of the state model.Acceptance tests
Release Note
Release note for CHANGELOG:
References
Fixes #1660
Community Note