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.

Sign up for GitHub

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

Ds 5114 update the aws secrets provider to aws sdk v2 #76

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Ds 5114 update the aws secrets provider to aws sdk v2 #76

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
087660a
DS-5114-Draft changes
arivankar-px Jun 14, 2023
e1853a0
DS-5114-Draft changes
arivankar-px Jun 14, 2023
795621a
DS-5114-Draft changes
arivankar-px Jun 14, 2023
6d3bf6a
Revert "Update minikube setup versions (#74)"
arivankar-px Jun 14, 2023
b29a619
Revert "Revert "Update minikube setup versions (#74)""
arivankar-px Jun 14, 2023
0304ae7
Updated code to support AWS SDK V2
arivankar-px Jun 21, 2023
78bf645
Changed retention period to 7 days for integration test
arivankar-px Jun 23, 2023
e0fa474
Add a delay to allow time for deletion to propagate
arivankar-px Jun 23, 2023
cd3f98a
Add a delay to allow time for deletion to propagate
arivankar-px Jun 23, 2023
c073ad0
Add a delay to allow time for deletion to propagate
arivankar-px Jun 23, 2023
bd0945e
Add a delay to allow time for deletion to propagate
arivankar-px Jun 23, 2023
61157c5
reduce sleep time
arivankar-px Jun 23, 2023
1762ab5
reduce sleep time
arivankar-px Jun 23, 2023
5f953e7
Changed error handling by using secretmanager error handler instead o…
arivankar-px Sep 1, 2023
4c53935
Made changes to the struct
arivankar-px Sep 3, 2023
6a1cf46
Made changes based on review comments
arivankar-px Sep 3, 2023
d367c77
Removed keyContext value
arivankar-px Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions aws/aws_kms/aws_kms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package aws_kms

import (
"context"
"encoding/base64"
"fmt"
"github.com/libopenstorage/secrets/aws/utils"
"os"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/kms"
"github.com/libopenstorage/secrets/aws/utils"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/credentials"
"github.com/aws/aws-sdk-go-v2/service/kms"
"github.com/aws/aws-sdk-go-v2/service/kms/types"
"github.com/libopenstorage/secrets"
sc "github.com/libopenstorage/secrets/aws/credentials"
"github.com/libopenstorage/secrets/pkg/store"
Expand All @@ -28,9 +30,8 @@ const (
)

type awsKmsSecrets struct {
client *kms.KMS
creds *credentials.Credentials
sess *session.Session
client *kms.Client
creds *aws.Credentials
cmk string
asc sc.AWSCredentials
ps store.PersistenceStore
Expand Down Expand Up @@ -84,22 +85,34 @@ func New(
if err != nil {
return nil, fmt.Errorf("Failed to get credentials: %v", err)
}
config := &aws.Config{
Credentials: creds,
Region: &region,
credProv := credentialsToProvider(creds)
config := aws.Config{
Credentials: credProv,
Region: region,
}
sess := session.New(config)
kmsClient := kms.New(sess)

kmsClient := kms.NewFromConfig(config)

return &awsKmsSecrets{
client: kmsClient,
sess: sess,
creds: creds,
cmk: cmk,
asc: asc,
ps: ps,
}, nil
}

func credentialsToProvider(creds *aws.Credentials) aws.CredentialsProvider {
return credentials.StaticCredentialsProvider{
arivankar-px marked this conversation as resolved.
Show resolved Hide resolved
Value: aws.Credentials{
AccessKeyID: creds.AccessKeyID,
SecretAccessKey: creds.SecretAccessKey,
SessionToken: creds.SessionToken,
Source: creds.Source,
},
}
}

func (a *awsKmsSecrets) String() string {
return Name
}
Expand Down Expand Up @@ -139,10 +152,10 @@ func (a *awsKmsSecrets) GetSecret(
decodedCipherBlob = cipherBlob
}
input := &kms.DecryptInput{
EncryptionContext: getAWSKeyContext(keyContext),
EncryptionContext: keyContext,
CiphertextBlob: decodedCipherBlob,
}
output, err := a.client.Decrypt(input)
output, err := a.client.Decrypt(context.TODO(), input)
if err != nil {
return nil, secrets.NoVersion, err
}
Expand Down Expand Up @@ -203,11 +216,11 @@ func (a *awsKmsSecrets) PutSecret(
keySpec := "AES_256"
input := &kms.GenerateDataKeyInput{
KeyId: &a.cmk,
EncryptionContext: getAWSKeyContext(keyContext),
KeySpec: &keySpec,
EncryptionContext: keyContext,
KeySpec: types.DataKeySpec(keySpec),
}

output, err := a.client.GenerateDataKey(input)
output, err := a.client.GenerateDataKey(context.TODO(), input)
if err != nil {
return secrets.NoVersion, err
}
Expand Down
4 changes: 4 additions & 0 deletions aws/aws_kms/aws_kms_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package aws_kms
import (
"os"
"testing"
"time"

"github.com/libopenstorage/secrets"
"github.com/libopenstorage/secrets/aws/utils"
Expand Down Expand Up @@ -164,6 +165,9 @@ func (a *awsSecretTest) TestDeleteSecret(t *testing.T) error {
err := a.s.DeleteSecret(secretIdWithData, nil)
assert.NoError(t, err, "Expected DeleteSecret to succeed")

// Add a delay to allow time for deletion to propagate
time.Sleep(time.Second * 90)

// Get of a deleted key should fail
_, _, err = a.s.GetSecret(secretIdWithData, nil)
assert.EqualError(t, secrets.ErrInvalidSecretId, err.Error(), "Unexpected error on GetSecret after delete")
Expand Down
66 changes: 40 additions & 26 deletions aws/aws_secrets_manager/aws_scm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ package aws_secrets_manager
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"strconv"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/secretsmanager"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/credentials"
"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
"github.com/aws/smithy-go"
"github.com/libopenstorage/secrets"
sc "github.com/libopenstorage/secrets/aws/credentials"
"github.com/libopenstorage/secrets/aws/utils"
Expand All @@ -26,7 +27,7 @@ const (

// AWSSecretsMgr is backend for secrets.SecretStore.
type AWSSecretsMgr struct {
scm *secretsmanager.SecretsManager
scm *secretsmanager.Client
}

// New creates new instance of AWSSecretsMgr with provided configuration.
Expand All @@ -39,7 +40,7 @@ func New(

awsConfig, ok := secretConfig[utils.AwsConfigKey]
if ok {
awsConfig, ok := awsConfig.(*aws.Config)
awsConfig, ok := awsConfig.(aws.Config)
if !ok {
return nil, utils.ErrAWSConfigWrongType
}
Expand Down Expand Up @@ -68,21 +69,30 @@ func New(
if err != nil {
return nil, fmt.Errorf("failed to get credentials: %v", err)
}
config := &aws.Config{
Credentials: creds,
Region: &region,
credProv := CredentialsToProvider(creds)
arivankar-px marked this conversation as resolved.
Show resolved Hide resolved
config := aws.Config{
Credentials: credProv,
Region: region,
}

return NewFromAWSConfig(config)
}

// NewFromAWSConfig creates new instance of AWSSecretsMgr with provided AWS configuration (aws.Config).
func NewFromAWSConfig(config *aws.Config) (*AWSSecretsMgr, error) {
sess, err := session.NewSession(config)
if err != nil {
return nil, fmt.Errorf("failed to create a session: %v", err)
// credentialsToProvider converts a aws.Credential object to a aws.CredentialProvider object
func CredentialsToProvider(creds *aws.Credentials) aws.CredentialsProvider {
return credentials.StaticCredentialsProvider{
Value: aws.Credentials{
AccessKeyID: creds.AccessKeyID,
SecretAccessKey: creds.SecretAccessKey,
SessionToken: creds.SessionToken,
Source: creds.Source,
},
}
scm := secretsmanager.New(sess)
}

// NewFromAWSConfig creates new instance of AWSSecretsMgr with provided AWS configuration (aws.Config).
func NewFromAWSConfig(config aws.Config) (*AWSSecretsMgr, error) {
scm := secretsmanager.NewFromConfig(config)
return &AWSSecretsMgr{
scm: scm,
}, nil
Expand Down Expand Up @@ -175,15 +185,17 @@ func (a *AWSSecretsMgr) Rencrypt(
}

func (a *AWSSecretsMgr) get(secretID string) (map[string]interface{}, secrets.Version, error) {
secretValueOutput, err := a.scm.GetSecretValue(&secretsmanager.GetSecretValueInput{
secretValueOutput, err := a.scm.GetSecretValue(context.TODO(), &secretsmanager.GetSecretValueInput{
SecretId: aws.String(secretID),
})
if err != nil {
if aerr, ok := err.(awserr.Error); ok {
if aerr.Code() == secretsmanager.ErrCodeResourceNotFoundException {
var apiErr smithy.APIError
arivankar-px marked this conversation as resolved.
Show resolved Hide resolved
if errors.As(err, &apiErr) {
// aerr, ok := err.(awserr.Error); ok {
if apiErr.ErrorCode() == "ResourceNotFoundException" {
return nil, secrets.NoVersion, secrets.ErrInvalidSecretId
} else if aerr.Code() == secretsmanager.ErrCodeInvalidRequestException &&
strings.Contains(aerr.Error(), "marked for deletion") {
} else if apiErr.ErrorCode() == "InvalidRequestException" &&
arivankar-px marked this conversation as resolved.
Show resolved Hide resolved
strings.Contains(apiErr.ErrorCode(), "Marked for deletion") {
return nil, secrets.NoVersion, secrets.ErrInvalidSecretId
}
}
Expand Down Expand Up @@ -214,12 +226,12 @@ func (a *AWSSecretsMgr) put(
return secrets.NoVersion, fmt.Errorf("failed to marshal secret data: %v", err)
}
// Check if there already exists a key.
_, err = a.scm.GetSecretValue(&secretsmanager.GetSecretValueInput{
_, err = a.scm.GetSecretValue(context.TODO(), &secretsmanager.GetSecretValueInput{
SecretId: aws.String(secretID),
})
if err == nil {
// Update the existing secret
secretValueOutput, putErr := a.scm.PutSecretValue(&secretsmanager.PutSecretValueInput{
secretValueOutput, putErr := a.scm.PutSecretValue(context.TODO(), &secretsmanager.PutSecretValueInput{
SecretId: aws.String(secretID),
SecretString: aws.String(string(secretBytes)),
})
Expand All @@ -231,10 +243,12 @@ func (a *AWSSecretsMgr) put(
}
return secrets.Version(*secretValueOutput.VersionId), nil
} else {
if aerr, ok := err.(awserr.Error); ok {
if aerr.Code() == secretsmanager.ErrCodeResourceNotFoundException {
// if aerr, ok := err.(awserr.Error); ok {
var apiErr smithy.APIError
if errors.As(err, &apiErr) {
if apiErr.ErrorCode() == "ResourceNotFoundException" {
// Create a new secret
secretValueOutput, createErr := a.scm.CreateSecret(&secretsmanager.CreateSecretInput{
secretValueOutput, createErr := a.scm.CreateSecret(context.TODO(), &secretsmanager.CreateSecretInput{
SecretString: aws.String(string(secretBytes)),
Name: aws.String(secretID),
})
Expand Down Expand Up @@ -278,7 +292,7 @@ func (a *AWSSecretsMgr) delete(
}
}

_, err := a.scm.DeleteSecret(deleteSecretInput)
_, err := a.scm.DeleteSecret(context.TODO(), deleteSecretInput)
if err != nil {
return &secrets.ErrProviderInternal{Reason: err.Error(), Provider: Name}
}
Expand Down
14 changes: 12 additions & 2 deletions aws/aws_secrets_manager/aws_scm_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package aws_secrets_manager
import (
"os"
"testing"
"time"

"github.com/libopenstorage/secrets"
"github.com/libopenstorage/secrets/aws/utils"
Expand Down Expand Up @@ -107,18 +108,27 @@ func (a *awsSecretTest) TestListSecrets(t *testing.T) error {

func (a *awsSecretTest) TestDeleteSecret(t *testing.T) error {
// Delete of a key that exists should succeed
err := a.s.DeleteSecret(a.secretIdWithData, nil)
keyContext := make(map[string]string)
keyContext[SecretRetentionPeriodInDaysKey] = "7"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you ad a retention period then the subsequent GetSecret check is going to fail ? The integration tests are failing for SecretsManager. Can you check?


err := a.s.DeleteSecret(a.secretIdWithData, keyContext)
assert.NoError(t, err, "Expected DeleteSecret to succeed")

// Add a delay to allow time for deletion to propagate
time.Sleep(time.Second * 200)

// Get of a deleted key should fail
_, version, err := a.s.GetSecret(a.secretIdWithData, nil)
assert.EqualError(t, secrets.ErrInvalidSecretId, err.Error(), "Unexpected error on GetSecret after delete")
assert.Equal(t, version, secrets.NoVersion)

// Delete of a key that exists should succeed
err = a.s.DeleteSecret(a.secretIdWithoutData, nil)
err = a.s.DeleteSecret(a.secretIdWithoutData, keyContext)
assert.NoError(t, err, "Expected DeleteSecret to succeed")

// Add a delay to allow time for deletion to propagate
time.Sleep(time.Second * 200)

// GetSecret using a secretId without data
_, version, err = a.s.GetSecret(a.secretIdWithoutData, nil)
assert.EqualError(t, secrets.ErrInvalidSecretId, err.Error(), "Unexpected error on GetSecret after delete")
Expand Down
Loading