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

Fix issue 5875 #6283

Merged
merged 2 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/6283-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #5875. Since Kopia has supported IAM, Velero should not require static credentials all the time
1 change: 0 additions & 1 deletion pkg/backup/backup_pv_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,4 @@ func (a *PVCAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runti
}

return &unstructured.Unstructured{Object: pvcMap}, []velero.ResourceIdentifier{pv}, nil

}
12 changes: 8 additions & 4 deletions pkg/repository/config/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,27 @@ func GetS3ResticEnvVars(config map[string]string) (map[string]string, error) {

// GetS3Credentials gets the S3 credential values according to the information
// of the provided config or the system's environment variables
func GetS3Credentials(config map[string]string) (credentials.Value, error) {
func GetS3Credentials(config map[string]string) (*credentials.Value, error) {
if len(os.Getenv("AWS_ROLE_ARN")) > 0 {
return nil, nil
}

credentialsFile := config[CredentialsFileKey]
if credentialsFile == "" {
credentialsFile = os.Getenv("AWS_SHARED_CREDENTIALS_FILE")
}

if credentialsFile == "" {
return credentials.Value{}, errors.New("missing credential file")
return nil, errors.New("missing credential file")
}

creds := credentials.NewSharedCredentials(credentialsFile, "")
credValue, err := creds.Get()
if err != nil {
return credValue, err
return nil, err
}

return credValue, nil
return &credValue, nil
}

// GetAWSBucketRegion returns the AWS region that a bucket is in, or an error
Expand Down
10 changes: 6 additions & 4 deletions pkg/repository/provider/unified_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,11 +423,13 @@ func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, cr
if err != nil {
return map[string]string{}, errors.Wrap(err, "error get s3 credentials")
}
result[udmrepo.StoreOptionS3KeyID] = credValue.AccessKeyID
result[udmrepo.StoreOptionS3Provider] = credValue.ProviderName
result[udmrepo.StoreOptionS3SecretKey] = credValue.SecretAccessKey
result[udmrepo.StoreOptionS3Token] = credValue.SessionToken

if credValue != nil {
result[udmrepo.StoreOptionS3KeyID] = credValue.AccessKeyID
result[udmrepo.StoreOptionS3Provider] = credValue.ProviderName
result[udmrepo.StoreOptionS3SecretKey] = credValue.SecretAccessKey
result[udmrepo.StoreOptionS3Token] = credValue.SessionToken
}
case repoconfig.AzureBackend:
storageAccount, accountKey, err := getAzureCredentials(config)
if err != nil {
Expand Down
28 changes: 21 additions & 7 deletions pkg/repository/provider/unified_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestGetStorageCredentials(t *testing.T) {
credStoreError error
credStorePath string
getAzureCredentials func(map[string]string) (string, string, error)
getS3Credentials func(map[string]string) (awscredentials.Value, error)
getS3Credentials func(map[string]string) (*awscredentials.Value, error)
getGCPCredentials func(map[string]string) string
expected map[string]string
expectedErr string
Expand Down Expand Up @@ -89,8 +89,8 @@ func TestGetStorageCredentials(t *testing.T) {
},
},
},
getS3Credentials: func(config map[string]string) (awscredentials.Value, error) {
return awscredentials.Value{
getS3Credentials: func(config map[string]string) (*awscredentials.Value, error) {
return &awscredentials.Value{
AccessKeyID: "from: " + config["credentialsFile"],
}, nil
},
Expand All @@ -115,8 +115,8 @@ func TestGetStorageCredentials(t *testing.T) {
},
credFileStore: new(credmock.FileStore),
credStorePath: "credentials-from-credential-key",
getS3Credentials: func(config map[string]string) (awscredentials.Value, error) {
return awscredentials.Value{
getS3Credentials: func(config map[string]string) (*awscredentials.Value, error) {
return &awscredentials.Value{
AccessKeyID: "from: " + config["credentialsFile"],
}, nil
},
Expand All @@ -138,13 +138,27 @@ func TestGetStorageCredentials(t *testing.T) {
},
},
},
getS3Credentials: func(config map[string]string) (awscredentials.Value, error) {
return awscredentials.Value{}, errors.New("fake error")
getS3Credentials: func(config map[string]string) (*awscredentials.Value, error) {
return nil, errors.New("fake error")
},
credFileStore: new(credmock.FileStore),
expected: map[string]string{},
expectedErr: "error get s3 credentials: fake error",
},
{
name: "aws, credential file not exist",
backupLocation: velerov1api.BackupStorageLocation{
Spec: velerov1api.BackupStorageLocationSpec{
Provider: "velero.io/aws",
Config: map[string]string{},
},
},
getS3Credentials: func(config map[string]string) (*awscredentials.Value, error) {
return nil, nil
},
credFileStore: new(credmock.FileStore),
expected: map[string]string{},
},
{
name: "azure, Credential section exists in BSL",
backupLocation: velerov1api.BackupStorageLocation{
Expand Down
12 changes: 2 additions & 10 deletions pkg/repository/udmrepo/kopialib/backend/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,8 @@ func (c *S3Backend) Setup(ctx context.Context, flags map[string]string) error {
return err
}

c.options.AccessKeyID, err = mustHaveString(udmrepo.StoreOptionS3KeyID, flags)
if err != nil {
return err
}

c.options.SecretAccessKey, err = mustHaveString(udmrepo.StoreOptionS3SecretKey, flags)
if err != nil {
return err
}

c.options.AccessKeyID = optionalHaveString(udmrepo.StoreOptionS3KeyID, flags)
c.options.SecretAccessKey = optionalHaveString(udmrepo.StoreOptionS3SecretKey, flags)
c.options.Endpoint = optionalHaveString(udmrepo.StoreOptionS3Endpoint, flags)
c.options.Region = optionalHaveString(udmrepo.StoreOptionOssRegion, flags)
c.options.Prefix = optionalHaveString(udmrepo.StoreOptionPrefix, flags)
Expand Down
15 changes: 0 additions & 15 deletions pkg/repository/udmrepo/kopialib/backend/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,6 @@ func TestS3Setup(t *testing.T) {
flags: map[string]string{},
expectedErr: "key " + udmrepo.StoreOptionOssBucket + " not found",
},
{
name: "must have access key Id",
flags: map[string]string{
udmrepo.StoreOptionOssBucket: "fake-bucket",
},
expectedErr: "key " + udmrepo.StoreOptionS3KeyID + " not found",
},
{
name: "must have access key",
flags: map[string]string{
udmrepo.StoreOptionOssBucket: "fake-bucket",
udmrepo.StoreOptionS3KeyID: "fake-key-id",
},
expectedErr: "key " + udmrepo.StoreOptionS3SecretKey + " not found",
},
}

for _, tc := range testCases {
Expand Down