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

fix(parameters): refresh AppConfig session token after 24 hrs #1916

Conversation

dreamorosi
Copy link
Contributor

Description of your changes

As discussed in the linked issue, the AppConfig provider in the Parameters utility stores a session token returned by the service that should be used for the subsequent call. By default the token has a duration of up to 24hrs and if the provider attempts to retrieve a configuration using an expired token it'll result in a bad request exception.

For most Lambda-based workloads this is not an issue since the execution environment is likely to have been recycled before the token has the chance to expire. For those customers who use provisioned concurrency or who decide to use the Parameters utility outside of Lambda (i.e. containers or others) this is an issue.

This PR improves the session token handling by storing the expiration timestamp together with the token. The timestamp is then checked by the AppConfigProvider before making a call to the service, and if expired a new session token is requested.

With this change customers using the Parameters utility in long lived execution environments should see decreased bad requests exception.

Related issues, RFCs

Issue number: #1798

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Jan 16, 2024
@dreamorosi dreamorosi requested a review from a team as a code owner January 16, 2024 09:29
@boring-cyborg boring-cyborg bot added parameters This item relates to the Parameters Utility tests PRs that add or change tests labels Jan 16, 2024
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Jan 16, 2024
@github-actions github-actions bot added the bug Something isn't working label Jan 16, 2024
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@am29d am29d left a comment

Choose a reason for hiding this comment

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

Nice 🛳️

@am29d am29d merged commit 559ef2d into main Jan 16, 2024
11 checks passed
@am29d am29d deleted the 1798-bug-appconfig-value-retrieval-fails-when-there-is-more-than-24-hours-between-calls branch January 16, 2024 15:16
dreamorosi added a commit that referenced this pull request Jan 27, 2024
* fix(parameters): refresh appconfig session after 24 hrs

* chore: revert change in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parameters This item relates to the Parameters Utility size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: AppConfig value retrieval fails when there is more than 24 hours between calls
2 participants