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

DRAFT: exploring sharing logic between sigv4 and sessions #103

Closed
wants to merge 1 commit into from

Conversation

sarahzinger
Copy link
Member

The logic for getting credentials from aws is nearly the same if you are fetching those credentials to sign requests vs if you are using those credentials to create a session object that is passed to the aws-sdk, and yet our code between the two is in separate places.

As we work to add more code to this, I wanted to explore if it would perhaps make sense to abstract this out in some way. I did a bit of an incomplete spike on this and I think it does make sense. I think I spotted a few bugs in this work (our sigv4 doesn't seem to support endpoint for example, and maybe doesn't use externalids correctly?), and I think having this fairly complicated logic in 2 different places is just very difficult to maintain.

That said it feels a bit scary to just merge something like this (even if I added many tests). This code is fairly high impact, and I'm not even sure I know all the datasources that consume our sigv4 code, (I think Opensearch and Prometheus?) so manual testing would likely be incomplete.

In an ideal world, I think we'd write this code in such a way that we were behind a feature toggle and slowly release over time. But my understanding is that our current feature toggles can not be used in the backend in external plugins so we'd either have to update our frontend to hit different endpoints based on feature toggle or we'd have to go about this without feature toggles. Maybe just abstracting a little bit over time over a series of prs?

I also don't think our team actually owns the sigv4 code here, which is arguably intentional? I think for now I'm going to put down this work for a bit and save it in a draft. Maybe we can revisit at another time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant