Add support for new Okta config in home directory with support for multiple profiles #162
Conversation
…perate configuration sections
…iple okta profiles
…rent directory to avoid mixing configurations
cmd/root/root.go
Outdated
| @@ -233,6 +241,28 @@ to collect a proper IAM role for the AWS CLI operator.`, | |||
| if vipAwsRegion != "" && os.Getenv(awsRegionEnvVar) == "" { | |||
| _ = os.Setenv(awsRegionEnvVar, vipAwsRegion) | |||
| } | |||
| } else { | |||
There was a problem hiding this comment.
This and the previous logic should probably be moved to a function.
internal/config/config.go
Outdated
| Profile: viper.GetString(ProfileFlag), | ||
| QRCode: viper.GetBool(QRCodeFlag), | ||
| WriteAWSCredentials: viper.GetBool(WriteAWSCredentialsFlag), | ||
| AWSCredentials: viper.GetString(getFlagNameFromProfile(oktaProfile, AWSCredentialsFlag)), |
There was a problem hiding this comment.
If the .env file is present and config is not, which config values would actually need to exist to work to use profiles? profilename.OKTA_AWS_CLI_ORG_DOMAIN ?
|
Hi @MatthewJohn we just GA'd v2.0.0 okta-aws-cli and it was quite a large code refactor. Can you see if your PR still works against the latest master branch? Thanks |
|
Hey @monde - sure I'll take a look! Thanks Also, we were discussing internally and have refactored the change a bit, so that the profiles in the config are keyed by the AWS profile, meaning that the okta-profile is no longer necessary. |
…AWS config. Search for available configurations based on AWS profile dict in config before falling back to the root version of the config. This also allows common configurations (such as Okta domain) to be specified in the config outside of a profile
|
That was easy - merge conflicts appear to be resolve :) I've also pushed my changes to replace the new "okta profile" with just configuration overrides of AWS profiles :) |
|
@MatthewJohn I'll bring this in, in a different PR. There will be on change, the config file pattern for Okta tools is the yaml file |
|
Thanks @monde , just let me know if you'd like me to make any modifications to this PR and I'll happily take a look :) |
Bringing in MatthewJohn's PR #162 - multiple config profiles in okta.yaml
This (draft) PR adds support for multiple independent profiles.
If the user has multiple (possibly tens or hundreds :cough:) AWS applications, this allows a single config file to be present (or even distributed between teams).
The YAML config file contains root keys of the AWS profile, which would remain passed as a command line argument.
It checks for configs in the AWS profile section of the new config, but falls back to root-level configs.
The config would look something like:
This means that, with all of the configurations present, the user can run okta-aws-cli with just the AWS profile and the rest of the configurations are obtained for that profile (without risk of passing an incorrect app ID).
This increases security incredibly compared to users manually running the full commands (as mixing a, seemingly unique, app ID with the AWS profile, could lead to authenticating an AWS profile to the wrong application and leading to accidental changes to incorrect AWS accounts). The only approach at the moment is to have .env files spread account directories per-account, which is an awful user experience, especially as the user must change directories.
The mixing of the configurations (in the code base) is obviously not ideal, but it retains backwards compatibility. If a .env file exists in the current directory, it ignores the new YML config (to avoid mixing of loading config types (I haven't used viper before and don't know how well it would play with this).
I'm not sure that, if a config is loaded from the yaml file, the env variables no longer override it - but since the config definitions are performed and then the config is loaded, I can't think of a great way to load the config file, obtain the profile and then ignore env variables for the remaining configs (without multiple config load passes, which is horrible!)
Anyway, it needs some tidying up, but a POC to see if it's something that could be merged and, meanwhile, we will test it thoroughly.