Skip to content

Conversation

@mikeland73
Copy link
Contributor

@mikeland73 mikeland73 commented Sep 20, 2023

Summary

Use consistent cache locations everywhere.

Fix envsec aws credentials cache bug where cache key did not take into account changing organizations.

How was it tested?

@mikeland73 mikeland73 changed the title [runx] Use same cache location as auth pkg [runx+envsec] Use same cache location as auth pkg Sep 20, 2023
Copy link
Contributor

@loreto loreto left a comment

Choose a reason for hiding this comment

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

LGTM

  1. What's your thought on whether we should make the directory go.jetpack.io vs simply jetpack.io for everything?

  2. Could we use SHA256 instead of MD5 for the hash. Most modern stores are deprecating MD5 (docker uses SHA256, git uses MD5 but introduced support for SHA256 with the hope of migrating to it over time)

  3. Thought for later: would it make sense for awsfed to become part of the auth library, and that library can then handle caching for all the tokens (without having to store them in an envsec specific place)

@mikeland73 mikeland73 force-pushed the landau/cache branch 6 times, most recently from 084914c to f5db8a1 Compare September 20, 2023 23:02
@mikeland73
Copy link
Contributor Author

mikeland73 commented Sep 20, 2023

What's your thought on whether we should make the directory go.jetpack.io vs simply jetpack.io for everything

I'm good either way as long as we're consistent. I guess jetpack.io is a bit better because it is less golang-ish

Could we use SHA256 instead of MD5 f

will change in this PR

Thought for later: would it make sense for awsfed to become part of the auth library

agree, but id federation stuff is somewhat unique/different from regular auth. Still, it can be factored out so that cache can be built in.

@mikeland73 mikeland73 merged commit 417d4b7 into main Sep 20, 2023
@mikeland73 mikeland73 deleted the landau/cache branch September 20, 2023 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants