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

aws/session: SDK should be able to load multiple custom shared config files. #1258

Closed
hongchaodeng opened this issue May 9, 2017 · 16 comments
Labels
feature-request A feature should be added or improved.

Comments

@hongchaodeng
Copy link
Contributor

Version of AWS SDK for Go?

v1.8.20

Version of Go (go version)?

go 1.8.1

Description

Feature request

According to this doc: https://docs.aws.amazon.com/sdk-for-go/api/aws/session/ , currently we could only load credentials/config (for client) from shared ENV. For a single program running only one aws account, this seems fine. But for a program that operates multiple aws accounts, there is no way to create different clients for each accounts.

For example, a program which is helping users to manage files on s3. User A specifies bucket "B1", credentials "S1", config "C1"; User B might specifies bucket "B2", credentials "S2", config "C2". While currently it allows loading credentials and config from shared config ENV like AWS_SHARED_CREDENTIALS_FILE, AWS_CONFIG_FILE, I can't find a way for creating individual clients.

@jasdel jasdel added the guidance Question that needs advice or information. label May 10, 2017
@jasdel
Copy link
Contributor

jasdel commented May 10, 2017

Hi @hongchaodeng thanks for contacting us. You can configure individual clients to use specific credentials two ways.

First would be to create a new session specifically for the client. The NewSessionWithOptions Session constructor takes an option where you can specify the shared credentials file profile to use. This allows multiple sets of credentials bucketed by profiles to be used by the same application.

// Load session with "profileA"'s credentials.
sessA := session.Must(session. NewSessionWithOptions(session.Option{
    Profile: "profileA",
}

// Load session with "profileB"'s credentials.
sessB := session.Must(session. NewSessionWithOptions(session.Option{
    Profile: "profileB",
}

Alternatively you can create a SharedCredentials credential provider and assign that to the Config when creating the service client. The first option above is preferable if the credentials will be shared by multiple clients.

credsFilename := `~/.aws/credentials`

sess := session.Must(session.NewSession())

s3SvcA := s3.New(sess, &aws.Config{
    Credentials: NewSharedCredentials(credsFilename, "profileA"),
})

s3SvcB := s3.New(sess, &aws.Config{
    Credentials: NewSharedCredentials(credsFilename, "profileb"),
})

In addition you can actually define your own credentials provider with the Provider interface that the SDK will use to retrieve credentials from custom sources.

@hongchaodeng
Copy link
Contributor Author

Hi @jasdel Thanks for replying. I have looked into all three methods you mentioned. That still does not work. Here's why:

Session constructor takes an option where you can specify the shared credentials file profile to use.

The multi-tenancy model above is per profile, but not per account.

Alternatively you can create a SharedCredentials credential provider and assign that to the Config when creating the service client.

Actually we are doing this as a workaround. The problem with this is that it couldn't get the "config" file, the file referred to by env AWS_CONFIG_FILE. Currently we need to pass in parameters like "region" to work around.

define your own credentials provider with the Provider interface

This has the same issue as using NewSharedCredentials().

Feel free to ping me if you need ask more info.

@jasdel
Copy link
Contributor

jasdel commented May 10, 2017

Thanks for the update @hongchaodeng.

The multi-tenancy model above is per profile, but not per account.

Could you clarify this? The profile is just a name that is used to group AWS credentials in the shared credentials file. Each shared credentials file can contain multiple profiles. In addition the credentials files can contain profiles for multiple accounts. Profiles do not need to have any association with one another.

The problem with this is that it couldn't get the "config" file, the file referred to by env AWS_CONFIG_FILE.

Ah this makes sense, the SDK does not read the shared configuration file by default, only the credentials file. The Sessions from Shared Config section of the session package docs should be helpful here. The AWS_SDK_LOAD_CONFIG=1 environment variable needs to be set, or NewSessionWithOption used when creating a Session with SharedConfigState set to session.SharedConfigEnable.

// Force enable Shared Config support
sess := session.Must(session.NewSessionWithOptions(session.Options{
	SharedConfigState: session.SharedConfigEnable,
}))

Currently we need to pass in parameters like "region" to work around.

Could you go into more details about this? Is this setting the Region parameter of the SDK's Config type? If the SDK's shared configuration support mentioned above, the SDK will read the region from the shared configuration file. If your application is operating in a single environment the AWS_REGION environment variable for setting the region via the environment. The Environment Variables section of the session package docs should be helpful here.

@hongchaodeng
Copy link
Contributor Author

hongchaodeng commented May 10, 2017

Could you clarify this? The profile is just a name that is used to group AWS credentials in the shared credentials file. Each shared credentials file can contain multiple profiles. In addition the credentials files can contain profiles for multiple accounts. Profiles do not need to have any association with one another.

Our model looks like:
model

Each user will pass in his AWS credentials when registering. This is dynamic and user accounts could not be known ahead.

Even though this shared credentials file could contain multiple accounts, does that mean we need to generate all accounts, profiles ahead?

What's more, it's scary for different users to share one file.

Ah this makes sense, the SDK does not read the shared configuration file by default, only the credentials file. The Sessions from Shared Config section of the session package docs should be helpful here. The AWS_SDK_LOAD_CONFIG=1 environment variable needs to be set, or NewSessionWithOption used when creating a Session with SharedConfigState set to session.SharedConfigEnable.

The AWS_SDK_LOAD_CONFIG=1 needs load shared file or use shared env, which could not be achieved if we create multiple clients in one program.

Could you go into more details about this? Is this setting the Region parameter of the SDK's Config type? If the SDK's shared configuration support mentioned above ...

You are right but this is not achievable for shared environment. Ideally, we want to read individual config file not from ENV, but from a param in config:

cfg := aws.Config{
  CredentialsFile: ...
  ConfigFile: ...
}
session.NewSession(cfg)

... or ...

session.NewSession(credsFile, cfgFile)

While we can load credentials file using NewSharedCredentials(), we can't load the "config" file like this. If we use env AWS_CONFIG_FILE, this could not allow creating multiple clients.

@jasdel
Copy link
Contributor

jasdel commented May 10, 2017

Thanks for the update and additional information @hongchaodeng. Out of curiosity how are the credentials shared with your service? Does User provide them through some interface and they are stored server side? Are these credentials used for multiple operations, or short lived for a single task?

I suggest instead of the User providing credentials directly to your application, and your application using those credentials on the User's behalf would be to provide instructions to the users how they can create a cross account IAM role that can be assumed from an account you control. This would allow your users to never provide your application with credentials directly, and it keeps their credentials secure because they are not sharing them.

This tutorial on Delegate Access Across AWS Accounts Using IAM Roles should help with this.

Generally this will be a much safer and secure means for the customer to give permission to your application to make modifications to their account's resources. The customer can provide find grained permissions as needed. This also reduces the risk your service would need to manage because of the sensitivity of the user's credentials potentially leaking.

I recommend not storing User credentials in a plain text config file. If your application does need to the user's the credentials, and cross account IAM roles is not an option, storing the credentials with strong encryption should be used. This guide on How to Manage Secrets for Amazon EC2 Container Service–Based Applications by Using Amazon S3 and Docker But again, I strongly recommend using cross account IAM roles instead.

@hongchaodeng
Copy link
Contributor Author

Isn't it valid to create client from credentials and config without sharing ENV? This SDK has provided NewSharedCredentials() to parse credentials file. Isn't it OK to also provide methods to parse config file?

@jasdel
Copy link
Contributor

jasdel commented May 10, 2017

Thanks for the clarification. The SDK does not currently export the functionality to load the shared configuration file directly. Only the shared credential's file functionality is exported. The shared configuration file is only available internally when creating a session. But, this makes it difficult to use multiple shared configuration files concurrently.

I think the best way to add this functionality would be to export the SDK's sharedConfig functionality. We'll need to explore the best way to integrate a passed in or constructed with Session or aws.Config.

@hongchaodeng
Copy link
Contributor Author

Sure. Thanks for the information.

For now, we are trying to modify this line of code:

cfgFiles := []string{envCfg.SharedConfigFile, envCfg.SharedCredentialsFile}

to change envCfg.SharedConfigFile to some param and export it to outside.

It would be great if this official SDK could provide it too :)

@jasdel jasdel changed the title How to create client from credentials and config file without sharing ENV aws/session: SDK should be able to load multiple custom shared config files. May 10, 2017
@jasdel jasdel added the feature-request A feature should be added or improved. label May 10, 2017
@jasdel
Copy link
Contributor

jasdel commented May 10, 2017

One additional way to work around this issue, is a preprocess step that would run before creating a Session which would merge the contents of the multiple shared config files into a single file. You'd need to ensure that the profile names of each file do not clash though. This could probably be accomplished by adding a prefix to the existing profile names. This is probably the least invasive way to accomplish this today until the SDK supports this feature.

This suggestion assumes that you that your application is only concerned about multiple shared config files once at startup. If your application deals with multiple shared configuration files throughout the lifetime of the application another solution would be needed.

@jasdel jasdel removed the guidance Question that needs advice or information. label May 10, 2017
@hongchaodeng
Copy link
Contributor Author

If your application deals with multiple shared configuration files throughout the lifetime of the application another solution would be needed.

Yeah. This is the context I have been talking in.

@hongchaodeng
Copy link
Contributor Author

hongchaodeng commented May 11, 2017

Hi @jasdel .

Currently we are trying to work around this by locking on shared env, e.g. AWS_CONFIG_FILE.
Basically, the code looks like:

// Assume that this method do not have side effect
func createS3Client() ... {
  mutex.Lock()
  defer mutex.Unlock()

  os.Setenv("AWS_SHARED_CREDENTIALS_FILE", "...")
  defer os.Unsetenv("AWS_SHARED_CREDENTIALS_FILE")

  os.Setenv("AWS_CONFIG_FILE", "...")
  defer os.Unsetenv("AWS_CONFIG_FILE")

  session.NewSession()
  ...
}

Is it true that the credentials and config file will only be used once and won't be reloaded?

Note that this is just a work-around. After this, we would like the official SDK to export functionality to pass in config file name. Are you going to work on this? Or are you fine to assign me to take care of this issue?

Thanks in advance!

@jasdel
Copy link
Contributor

jasdel commented May 11, 2017

@hongchaodeng, We'd be glad to review a PR for this feature. I've not started work on it.

I think the best way to support this is to update the session.Option struct to have an additional optional member which is a list of files the user wants to load the credentials from.

sess := session.Must(session.NewSessionWithOptions(session.Options{
     // Ordered list of files the SDK will load configuration from
     SharedConfigFiles: []string{file1, file2, file3}, 
}))

This would mean the newSession function would use this list instead of the default if it was set.

In doing so the we'd probably also want to export the sharedCredentialsFilename and sharedConfigFilename functions.

@hongchaodeng
Copy link
Contributor Author

I think the best way to support this is to update the session.Option struct to have an additional optional member which is a list of files the user wants to load the credentials from.

sess := session.Must(session.NewSessionWithOptions(session.Options{
    // Ordered list of files the SDK will load configuration from
    SharedConfigFiles: []string{file1, file2, file3}, 
}))

Will do that.

In doing so the we'd probably also want to export the sharedCredentialsFilename and sharedConfigFilename functions.

I'm not catching it. Can you clarify?

jasdel pushed a commit that referenced this issue May 12, 2017
Add SharedConfigFiles option for users to load custom config files.
This will override any related files loaded based on environment variables.

Fix #1258
@hongchaodeng
Copy link
Contributor Author

@jasdel
Any ETA for coming release?

@jasdel
Copy link
Contributor

jasdel commented May 12, 2017

This will be included in our next release. I don't have an exact date for that, but I expect it to be early/mid next week. I've also updated the pending changelog for this feature, and the release notes will include details about this change.

@jasdel
Copy link
Contributor

jasdel commented May 15, 2017

Hi @hongchaodeng This change is now included with the SDK's release that was cut today. 1.8.23. The release should be published a little bit later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants