Update shared config logic to resolve sso section #4868
Conversation
aws/session/shared_config.go
Outdated
| @@ -186,6 +197,20 @@ type sharedConfigFile struct { | |||
| IniData ini.Sections | |||
| } | |||
|
|
|||
| // SSOSession provides the shared configuration parameters of the sso-session | |||
| // section. | |||
| type SSOSession struct { | |||
There was a problem hiding this comment.
Question: Should this be public?
There was a problem hiding this comment.
I checked go v2's usage of SSOSession, it is used within config package only so I think we can change it to private
aws/session/shared_config.go
Outdated
| if err := cfg.validateCredentialsConfig(profile); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| // if not top level profile and has credentials, return with credentials. | ||
| if len(profiles) != 0 && cfg.Creds.HasKeys() { |
There was a problem hiding this comment.
Question: I'm not sure I understand why this was added, can you clarify for me what this is doing?
There was a problem hiding this comment.
This block is copied from go v2, it will be triggered when a source profile can get accessKey/secretAccessKey from shared config files, then the source shared config will be returned with credentials. But there might be some diff between v1 and v2's sharedConfig setting.
There was a problem hiding this comment.
I thought this part was added to v2 for resolving credentials as well.
There was a problem hiding this comment.
i still dont understand why this is needed in order to parse sso sessions? i dont think we should be copying anything from V2 just because its there that were not sure we also need.
There was a problem hiding this comment.
I agree, the remaining logic can still load the sso session for shared config. I will delete it.
| var ssoSession SSOSession | ||
| ssoSession.setFromIniSection(section) | ||
| ssoSession.Name = cfg.SSOSessionName | ||
| cfg.SSOSession = &ssoSession |
There was a problem hiding this comment.
Question: So this looks like we set a single SSoSession for the entire shared config. How does this work if we have an assume role chain? (this is mostly me trying to understand the existing Go v1 shared config loading).
e.g. with AWS_PROFILE = foo
foo doesn't directly refer to session1, what is the resulting sharedConfig struct in Go?
[profile foo]
role_arn = role1
source_profile = bar
[profile bar]
role_arn = role2
source_profile = baz
[profile baz]
role_arn = role3
sso_account_id = 1234
sso_region = us-east-1
sso_session = session1
[sso-session session1]
...
There was a problem hiding this comment.
According to v1's logic of resolving source profile , the current foo profile's credential will be cleaned and the referenced source profile bar will be loaded recursively to srcCfg and added as SourceProfile of foo. Finally the sharedConfig of foo will be sth like
sharedConfig {
Profile = foo
SourceProfile = sharedConfig {
Profile = bar
SourceProfile = sharedConfig {
Profile = baz
SSOSessionName = session1
SSOSession = ssoSession {
Name = session1
}
}
}
}
There was a problem hiding this comment.
Then during step of resolving user credentials from shared config, it will recursively detect source profile and resolve sso credential provider if present in sharedCfg
There was a problem hiding this comment.
Great thanks for clarifying this, that is helpful.
| @@ -507,6 +523,15 @@ func TestLoadSharedConfigFromFile(t *testing.T) { | |||
| S3UseARNRegion: true, | |||
| }, | |||
| }, | |||
| { | |||
| Profile: "sso-session-success", | |||
There was a problem hiding this comment.
Suggestion: Would be good to add some of the validation cases here as well, not just the happy path.
sync feat-sso-session branch to current branch
aws/session/shared_config.go
Outdated
| if err := cfg.validateCredentialsConfig(profile); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| // if not top level profile and has credentials, return with credentials. | ||
| if len(profiles) != 0 && cfg.Creds.HasKeys() { |
There was a problem hiding this comment.
i still dont understand why this is needed in order to parse sso sessions? i dont think we should be copying anything from V2 just because its there that were not sure we also need.
| // provided. If the session section is not found or incomplete an error | ||
| // will be returned. | ||
| if cfg.hasSSOTokenProviderConfiguration() { | ||
| skippedFiles = 0 |
There was a problem hiding this comment.
does V1 only have a concept of multiple shared config files?
There was a problem hiding this comment.
Yep, in v2's sharedConfig loading, sections will be extracted from config files first, but in v1 files will be used directly to load shared config
| @@ -507,6 +523,15 @@ func TestLoadSharedConfigFromFile(t *testing.T) { | |||
| S3UseARNRegion: true, | |||
| }, | |||
| }, | |||
| { | |||
| Profile: "sso-session-success", | |||
Update aws shared config logic to support resolving sso section in shared config file. Will resolve #4649 with following PRs on the same branch