-
Notifications
You must be signed in to change notification settings - Fork 138
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
Load AWS credentials from Kubernetes secret #186
Load AWS credentials from Kubernetes secret #186
Conversation
opts := []func(*config.LoadOptions) error{ | ||
config.WithRegion(region), | ||
config.WithSharedConfigProfile(profile), | ||
config.WithHTTPClient(client), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not just set the AWS_SHARED_CREDENTIALS_FILE, this should also load credentials from a file, and you can just mount a secret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can set the location of the license file, but there does not seem to be a way to guarantee that the file is used. I can set the file, but the SDK will still use the entire chain, find an IRSA role and not even check the file
velero-plugin-for-aws/config.go
Outdated
@@ -80,3 +74,14 @@ func newS3Client(cfg aws.Config, url string, forcePathStyle bool) (*s3.Client, e | |||
|
|||
return s3.NewFromConfig(cfg, opts...), nil | |||
} | |||
|
|||
func loadCredentialsFromSecret(name string) (aws.Credentials, error) { | |||
// We don't do error handling for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if err != nil { return err }
is not too hard to add ;-)
@@ -144,7 +144,7 @@ func (o *ObjectStore) Init(config map[string]string) error { | |||
} | |||
} | |||
|
|||
cfg, err := newAWSConfig(region, credentialProfile, credentialsFile, insecureSkipTLSVerify, caCert) | |||
cfg, err := newAWSConfig(region, credentialProfile, credentialsFile, insecureSkipTLSVerify, caCert, bucket) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is the bucket coming from? can't seem to find what sets the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bucket is passed from the BackupStorageLocation
spec. Ideally I would like to pass the credential.name
for this, but it is not an accessible field in object_store
ea9c32e
to
dd67d44
Compare
@@ -147,8 +152,11 @@ func (o *ObjectStore) Init(config map[string]string) error { | |||
} | |||
} | |||
|
|||
useKubernetesCredentials := kubernetesSecret != "" | |||
cfg, err := newAWSConfig(region, credentialProfile, credentialsFile, insecureSkipTLSVerify, caCert, bucket, useKubernetesCredentials) | |||
credentialsConfig := CredentialsConfig{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe nicer to not duplicate the data, but add
func (c *CredentialConfig) UseSecret() bool {
return strings.TrimSpace(c.secretName) != ""
}
@reasonerjt Can you provide feedback on this implementation. The idea is to pass |
9935049
to
1dc5a8f
Compare
When Velero is using AWS IRSA for authentication, it is hard to use another provider in the chain, because the IRSA Role is always picked before a credentials file or static credentials in the environment. To mitigate this, add an extra configuration option `KubernetesSecret`, when this option is set, static credentials will be loaded from a k8s secret with that name. When the option is not set, the default CredentialsProviderChain is used. Signed-off-by: Rob Kenis <robke@softwareag.com>
1dc5a8f
to
491d53a
Compare
Will this handle the case when the credential file configures role-assumption looks like this?
Essentially, you want to ensure the credential file has higher priority even when the env var I have verified on my env but not submitted a PR yet, b/c we have to solve the problem also on velero side when the kopia connects to S3 and we can't do it via setting the env vars. |
Resolved by #191 |
When Velero is using AWS IRSA for authentication, it is hard to use another provider in the chain. To work around this, each BackupStorageLocation provisions a Secret in the velero namespace with the same name as the Bucket used in the BSL. Now Velero can retrieve the static credentials from the Secret, skip the entire credentials chain and just use the static credentials.
Fixes vmware-tanzu/velero#7302