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

New CredentialProvider for AWS’ credential_process facility #1

Merged
9 commits merged into from
Apr 20, 2022

Conversation

ghost
Copy link

@ghost ghost commented Apr 19, 2022

For CETUS-61.

This is working for me with the included manual test, when used with an AWS CLI profile created by aws-sso-util.

e.g. from the root of the repo:

backsaws $ AWS_PROFILE=sso.nas.read-prod test/manual/test_credential_process_manual.clj
Using AWS profile sso.nas.read-prod
2022-04-18 20:57:07.434:INFO::main: Logging initialized @2958ms to org.eclipse.jetty.util.log.StdErrLog
(latacora-cetus-nas-reports latacora-cetus-prod-nas-logs … <redacted>)

CETUS-61

Avi Flax added 7 commits April 17, 2022 19:25
Mainly for meander but also to skip `comment` forms.
I’m including this here as a discrete commit mainly to facilitate
better/easier peer review; this way a reviewer can diff the next commit
against this one to see exactly what changes I’ve made to the copy-pasta
code to adapt it to our needs.

tbh this isn’t the code *verbatim* but it’s close. I made a few tweaks
just so it would compile, e.g. I removed a reference to
`with-system-properties` and simplified function names a bit, minor
stuff like that.

CETUS-61
I have no idea why the original code in the pod is retrieving the config
key `credential_process` from the AWS CLI credentials file rather than
the config file.

In our case, wherein our profiles are created by aws-sso-util, that key
is in the profile definitions in our config files. So for this to work
for us, we need to retrieve this key from our config files.

I suppose perhaps there are some tools/people/cases that store that key
in the credentials file because I suppose the command could potentially
contain sensitive strings. I guess? But why would that matter? I mean,
it’s not like the credentials file is encrypted or something.

Regardless, it may be the case that the key *is* sometimes stored in the
credentials file, and we might want to support that case as well. TBD.

CETUS-61
Re: the removed content: I suspect this is unnecessary in the root
README. If someone really needs to specify a profile name
programmatically, they can read the source. Or maybe they’ll just
override AWS_PROFILE; that’d work just fine and shouldn’t be too tricky.

CETUS-61
Automated tests coming soon!

CETUS-61
@ghost ghost added the enhancement New feature or request label Apr 19, 2022
@ghost ghost self-assigned this Apr 19, 2022
I started with copy-pasta of the aws-vault provider tests. Which was
helpful!

CETUS-61
@ghost ghost marked this pull request as ready for review April 19, 2022 18:51
@ghost ghost requested a review from lvh April 19, 2022 18:51
CHANGELOG.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@ghost ghost changed the title WIP: New CredentialProvider for AWS’ credential_process facility New CredentialProvider for AWS’ credential_process facility Apr 19, 2022
Copy link
Contributor

@lvh lvh left a comment

Choose a reason for hiding this comment

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

LGTM modulo comments :)

As per feedback from @lvh in #1 — Thanks lvh!
@ghost ghost merged commit 1d7e468 into main Apr 20, 2022
@ghost ghost deleted the credential-process branch April 20, 2022 16:37
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant