Skip to content

Conversation

@arivankar-px
Copy link

@arivankar-px arivankar-px commented Jun 21, 2023

What this PR does / why we need it:
In this PR we have updated the packages used from aws sdk version 1 to aws sdk version 2. This has been made to maintain consistency in using the same aws sdk version across libopenstorage/secrets and portworx/pds-api repositories.

Which issue(s) this PR fixes (optional)
Closes #DS-5114

Special notes for your reviewer:

@arivankar-px arivankar-px marked this pull request as draft June 23, 2023 07:01
@arivankar-px arivankar-px marked this pull request as ready for review June 23, 2023 14:35
@arivankar-px arivankar-px requested a review from lprouza June 23, 2023 14:35
@arivankar-px arivankar-px requested a review from adityadani July 3, 2023 10:00
Copy link
Collaborator

@adityadani adityadani left a comment

Choose a reason for hiding this comment

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

looks good once the integration tests are fixed.

}
providers = append(providers, ec2RoleProvider)

} else if runningOnEc2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious, does AWS not allow chaining of providers anymore? Previously we were able to add EnvProvider and RoleProvider together. With this change we won't since each provider is in its own if condition.

// 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants