velero icon indicating copy to clipboard operation
velero copied to clipboard

Add credentials to volume snapshot locations.

Open sseago opened this issue 2 years ago • 4 comments

This is analogous to the BSL creds work that was done previously, but for VSLs instead.

Originally the per-BSL credentials support was intended to be added to VolumeSnapshotLocations as well. Some initial work was already done for this but was reverted when the VSL work was de-scoped from a prior release (the AWS plugin work to support this remained in place, though). This issue is for going though with the enhancement.

Signed-off-by: Scott Seago [email protected]

Please add a summary of your change

Does your change fix a particular issue?

Fixes #4862

Please indicate you've done the following:

  • [x ] Accepted the DCO. Commits without the DCO will delay acceptance.
  • [x ] Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • [ ] Updated the corresponding documentation in site/content/docs/main.

sseago avatar Apr 26 '22 18:04 sseago

We'll target this to post-v1.9

reasonerjt avatar May 30 '22 03:05 reasonerjt

@sseago I found the comment by Bridget: https://github.com/vmware-tanzu/velero/issues/4493#issuecomment-1010285577

For most cloud providers, it's not possible to take snapshots in different accounts so there isn't really a need to support multiple locations with unique credentials. This is different from the BSL case where it's possible to store the serialized contents of a backup in many storage locations.

So will it cause problems for some cloud providers if we allow them to set different credentials for BSL and VSL?

reasonerjt avatar Jul 12 '22 16:07 reasonerjt

@sseago I found the comment by Bridget: #4493 (comment)

For most cloud providers, it's not possible to take snapshots in different accounts so there isn't really a need to support multiple locations with unique credentials. This is different from the BSL case where it's possible to store the serialized contents of a backup in many storage locations.

So will it cause problems for some cloud providers if we allow them to set different credentials for BSL and VSL?

I don't believe so. She's only saying that there's no need for multiple VSLs to coexist in the same cluster for the same provider. However, BSL and VSL can definitely have different credentials from each other. It could be that the default credentials are the wrong credentials for the VSL, so we want VSL credentials. Also, some users may want to manage all of their credentials in the same way, never creating the default secret but always setting credentials when they create a VSL or a BSL.

sseago avatar Jul 12 '22 19:07 sseago

I've gone through the BSL credentials commits after the initial implementation (some refactoring and validation changes) -- making similar changes here.

sseago avatar Aug 05 '22 19:08 sseago

@sseago Thanks, the code looks good to me. Could you fix the minor issues with the comment and update the doc accordingly to reflect the changes in API and CLI?

reasonerjt avatar Aug 29 '22 08:08 reasonerjt

@reasonerjt PR updated with comment fixes and doc updates.

sseago avatar Sep 07 '22 23:09 sseago