You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Provide support for AWS credentials obtained through SSO #17
However, it seems never versions of the JavaScript SDK also support using these credentials directly. However, I think that would require a major upgrade of the AWS SDK used by Goldstack:
I hit this issue today while evaluating goldstack (which looks to be great btw)
An alternate approach might be to add support for credential_process. This is neatly summarised in a comment in the same thread referenced above: aws/aws-cli#4982 (comment)
This would not require an upgrade of the AWS SDK since it exists in JS v2 as AWS.ProcessCredentials. Note that credential_process should be found in ~/.aws/config according to mostotherdocs (not in ~/.aws/credentials)
Then for this type of users it would instantiate AWS.ProcessCredentials in infraAws.ts#L282 and that should then provide a good way to get SSO credentials, correct?
And just using the profile provider would not work, correct? infraAws.ts#L262
Yes. It would probably be worth adding an option for fileName which "defaults to ~/.aws/credentials or defined by AWS_SHARED_CREDENTIALS_FILE process env var" and seems to be wrong in most cases.
Then for this type of users it would instantiate AWS.ProcessCredentials in infraAws.ts#L282 and that should then provide a good way to get SSO credentials, correct?
If you're taking an explicit approach, this makes good sense.
And just using the profile provider would not work, correct? infraAws.ts#L262
Just using the current implementation of profile provider doesn't seem to work for me and the error message 'Please perform aws login for the profile using the AWS CLI.' (infraAws.ts#L275) threw me off a bit.
Reading the docs, AWS.SharedIniFileCredentials expects to find aws_access_key_id and aws_secret_access_key in the INI file and therefore would ignore credential_process
NB. I didn't see a contributor guide here and I'm not sure how to work on a submodule in the yarn workspace / pnp setup. If you can help with that, I'd be happy to either contribute a PR and/or test it with my setup here.
Thanks for the thoughts and pointers. I have put together a draft PR: #95
A couple of questions:
Does the way to override the default path for the credentials file look okay? (To me a bit confusing here is that they talk about the configuration path but it would always be the path to the credentials file rather than the config file?)
Instead of adding a new user type, I've added an additional proprty to the profile user type, "processCredentials": true,. Does that make sense?
Not sure how well this would work with assume-role? (The way AWS credentials are acquired really seems to be a science all to itself!)
I have also started putting together a Contribute section (#94) - but this is still WIP. Anything that stand out to you that would be worthwhile to add?
Does the way to override the default path for the credentials file look okay? (To me a bit confusing here is that they talk about the configuration path but it would always be the path to the credentials file rather than the config file?)
Yeah, it's definitely confusing. In fact I thought it was the other way around (i.e. AWS.ProcessCredentials talks about the credential path in their docs but it would always be the path to config.).
Every other doc I've seen puts the credential_process property in ~/.aws/config.
I ran some tests today and if you set AWS_SDK_LOAD_CONFIG=1, then it will search for profiles in both config & credentials. No need to provide a filename.
Instead of adding a new user type, I've added an additional proprty to the profile user type, "processCredentials": true,. Does that make sense?
Yes, this makes good sense. it's definitely feels like it should sit under profile instead of being a new user type. I also like that you can specify which provider/behaviour you want to use rather than relying on some opaque precedence rules.
Elsewhere I've noticed credential_source can be used to influence the logic in some places so naming might be better as credentialSource: "process". Leaving it blank could perhaps try a few methods?
Not sure how well this would work with assume-role?
I ran some tests today and if you set AWS_SDK_LOAD_CONFIG=1, then it will search for profiles in both config & credentials. No need to provide a filename.
btw, the SSO credentials expire after 60 minutes and some of the terraform scripts can take a while to run (especially if waiting on DNS propagation). The full batch can easily take over an hour on a fresh install.
getPromise() will refresh the credentials if they are due to expire (15 second window). Passing these values to a docker container as environment vars effectively removes the ability to refresh / extend the session for the lifetime of the container.
I'll see if I can find a mechanism to allow terraform to refresh credentials from within a docker container.
Seems like it was looking up the default file location rather than the one provided in the constructor. Will try to investigate further tomorrow. Agreed it seems to make more sense to put the credentials_process into the config since it's just config and not a credential!
Somehow setting AWS_SDK_LOAD_CONFIG=1 led to the test to fail
Seems like it was looking up the default file location rather than the one provided in the constructor. Will try to investigate further tomorrow.
Ah yes, I did read the code path in IniLoader (the underlying file loader) and it seems to ignore the file name property if AWS_SDK_LOAD_CONFIG is set.
I suspect that if you set AWS_CONFIG_FILE then it will work.
I have merged and released what is there so far, which should now support using process credentials with new templates (or by upgrading a package to the latest version of the template dependency).
Thanks for the quick response @mxro, this is working for me now (with a workaround**) 🚀
I found some typos and other minor inconsistencies so I'll put a review on the PR for you.
I still can't make changes locally using yarn link * or yarn patch but I made a bit of progress and I'll raise a few issues related to that. Happy to be added as a collaborator if that's a useful path.
** I had to either provide awsConfigFileName with the default ~/.aws/config location, OR execute yarn infra with a prefix (e.g. AWS_SDK_LOAD_CONFIG=1 yarn infra init dev) to get this working. I'll add this to the PR review also!
That's great to hear. Yeah a lots of bits and pieces of logic esp around the env variables, so definitely need to tweak this a bit to make it work reliabily. Thanks for your help. Started a new PR based on the comments #102 - but still WIP for now.
** I had to either provide awsConfigFileName with the default ~/.aws/config location, OR execute yarn infra with a prefix (e.g. AWS_SDK_LOAD_CONFIG=1 yarn infra init dev) to get this working.
Will want to get this working but not sure if the new PR above would resolve this.
Also added you as a collaborator. This should enable you to create and contribute to branches and the CI should run on them I hope.
Activity
chrismilleruk commentedon Feb 16, 2022
I hit this issue today while evaluating goldstack (which looks to be great btw)
An alternate approach might be to add support for
credential_process. This is neatly summarised in a comment in the same thread referenced above: aws/aws-cli#4982 (comment)This would not require an upgrade of the AWS SDK since it exists in JS v2 as AWS.ProcessCredentials. Note that
credential_processshould be found in~/.aws/configaccording to most other docs (not in~/.aws/credentials)It seems to be a valid workaround with plenty of CLI tools that fill the gap: aws-sso-creds-helper, aws-sso-util, aws-vault, aws2-wrap
Would this be a good path forward?
mxro commentedon Feb 16, 2022
Thank you that looks very promising. So with this, configuration would only need to contain the following:
{ "users": [ { "name": "dev-user", "type": "credentialProcess", "config": { "profile": "default" } } ] }(will need to implement that of course)
Then for this type of users it would instantiate AWS.ProcessCredentials in infraAws.ts#L282 and that should then provide a good way to get SSO credentials, correct?
And just using the
profileprovider would not work, correct? infraAws.ts#L262chrismilleruk commentedon Feb 17, 2022
Yes. It would probably be worth adding an option for
fileNamewhich "defaults to~/.aws/credentialsor defined byAWS_SHARED_CREDENTIALS_FILEprocess env var" and seems to be wrong in most cases.My
~/.aws/configfile looks something like this:So
config.jsonmight need to look something like this (assuming the config gets passed toAWS.ProcessCredentialsunaltered){ "users": [ { "name": "dev-user", "type": "credentialProcess", "config": { "profile": "profile goldstackApp.GoldstackAccess", "fileName": "~/.aws/config", } } ] }If you're taking an explicit approach, this makes good sense.
Just using the current implementation of
profileprovider doesn't seem to work for me and the error message 'Please performaws loginfor the profile using the AWS CLI.' (infraAws.ts#L275) threw me off a bit.Reading the docs, AWS.SharedIniFileCredentials expects to find
aws_access_key_idandaws_secret_access_keyin the INI file and therefore would ignorecredential_processIf you did want to keep this under the
profileprovider, I think something likeAWS.CredentialProviderChaincould be used. e.g. https://gist.github.com/pahud/836481ae759147d3f493d3ead1f5406aNB. I didn't see a contributor guide here and I'm not sure how to work on a submodule in the yarn workspace / pnp setup. If you can help with that, I'd be happy to either contribute a PR and/or test it with my setup here.
mxro commentedon Feb 17, 2022
Thanks for the thoughts and pointers. I have put together a draft PR: #95
A couple of questions:
configurationpath but it would always be the path to thecredentialsfile rather than theconfigfile?)profileuser type,"processCredentials": true,. Does that make sense?{ "name": "process", "type": "profile", "config": { "profile": "with-process", "awsDefaultRegion": "us-west-2", "processCredentials": true, "awsConfigFileName": "~/.aws/mycredentials" } }I have also started putting together a Contribute section (#94) - but this is still WIP. Anything that stand out to you that would be worthwhile to add?
chrismilleruk commentedon Feb 18, 2022
Yeah, it's definitely confusing. In fact I thought it was the other way around (i.e. AWS.ProcessCredentials talks about the credential path in their docs but it would always be the path to config.).
Every other doc I've seen puts the
credential_processproperty in~/.aws/config.I ran some tests today and if you set
AWS_SDK_LOAD_CONFIG=1, then it will search for profiles in bothconfig&credentials. No need to provide a filename.Yes, this makes good sense. it's definitely feels like it should sit under
profileinstead of being a new user type. I also like that you can specify which provider/behaviour you want to use rather than relying on some opaque precedence rules.Elsewhere I've noticed credential_source can be used to influence the logic in some places so naming might be better as
credentialSource: "process". Leaving it blank could perhaps try a few methods?Not sure either, I suspect it would be OK. The expected response from
credential_processis very similar to response fromsts assume-role. I wouldn't be surprised if many of those tools mentioned above usests assume-rolebehind the scenes.Yes, unfortunately I'm just a lab assistant :). @benkehoe is the chief scientist in my book theres-a-better-way
chrismilleruk commentedon Feb 18, 2022
chrismilleruk commentedon Feb 18, 2022
btw, the SSO credentials expire after 60 minutes and some of the terraform scripts can take a while to run (especially if waiting on DNS propagation). The full batch can easily take over an hour on a fresh install.
getPromise()will refresh the credentials if they are due to expire (15 second window). Passing these values to a docker container as environment vars effectively removes the ability to refresh / extend the session for the lifetime of the container.I'll see if I can find a mechanism to allow terraform to refresh credentials from within a docker container.
Changing configuration for credentials source process as per #17 (com…
mxro commentedon Feb 19, 2022
Thanks for testing things out and for your ideas!
I think
"credentialsSource": "process",should work very nicely and changed the config in the PR to reflect that https://github.com/goldstack/goldstack/pull/95/files#diff-d3e9e88cc2f0ee2aad83c3b15bdd7ddc45341b6c988a2bbbe1b06e63f2947667R85Somehow setting
AWS_SDK_LOAD_CONFIG=1led to the test to fail (disabled it for now https://github.com/goldstack/goldstack/pull/95/files#diff-e43f64484d679c0de71cca9f787c4dc4d20e104fbd0faada8b38efc275670918R84)https://github.com/goldstack/goldstack/runs/5256339838?check_suite_focus=true
Seems like it was looking up the default file location rather than the one provided in the constructor. Will try to investigate further tomorrow. Agreed it seems to make more sense to put the
credentials_processinto theconfigsince it's just config and not a credential!chrismilleruk commentedon Feb 19, 2022
Ah yes, I did read the code path in IniLoader (the underlying file loader) and it seems to ignore the file name property if
AWS_SDK_LOAD_CONFIGis set.I suspect that if you set
AWS_CONFIG_FILEthen it will work.Chris
mxro commentedon Feb 20, 2022
Got it working with both using
credentialsandconfigfiles, see https://github.com/goldstack/goldstack/pull/95/files#diff-d3e9e88cc2f0ee2aad83c3b15bdd7ddc45341b6c988a2bbbe1b06e63f2947667R110. Had to do some fiddinling around with the environment variables to make it work 😱 https://github.com/goldstack/goldstack/pull/95/files#diff-e43f64484d679c0de71cca9f787c4dc4d20e104fbd0faada8b38efc275670918R83 So happy for any better ideas.I have merged and released what is there so far, which should now support using process credentials with new templates (or by upgrading a package to the latest version of the template dependency).
Also updated the configuration with a section on Process Credentials.
I raised a seperate issue for tracking the issue you identified regarding long running infra operations: #99
I think this should (if it works!) provide at least some way to use an SSO-based login for local development? Thanks for all your work so far!!!
chrismilleruk commentedon Feb 21, 2022
Thanks for the quick response @mxro, this is working for me now (with a workaround**) 🚀
I found some typos and other minor inconsistencies so I'll put a review on the PR for you.
I still can't make changes locally using
yarn link *oryarn patchbut I made a bit of progress and I'll raise a few issues related to that. Happy to be added as a collaborator if that's a useful path.** I had to either provide
awsConfigFileNamewith the default~/.aws/configlocation, OR executeyarn infrawith a prefix (e.g.AWS_SDK_LOAD_CONFIG=1 yarn infra init dev) to get this working. I'll add this to the PR review also!mxro commentedon Feb 21, 2022
That's great to hear. Yeah a lots of bits and pieces of logic esp around the env variables, so definitely need to tweak this a bit to make it work reliabily. Thanks for your help. Started a new PR based on the comments #102 - but still WIP for now.
Will want to get this working but not sure if the new PR above would resolve this.
Also added you as a collaborator. This should enable you to create and contribute to branches and the CI should run on them I hope.
mxro commentedon Mar 2, 2022
Does this work now and can be closed?