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

Introduce BucketProvider interface #596

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Feb 28, 2022

This PR introduces a BucketProvider interface for fetch operations
against object storage provider buckets. Allowing for easier
introduction of new provider implementations.

The algorithm for conditionally downloading object files is the same,
whether you are using GCP storage or an S3/Minio-compatible
bucket. The only thing that differs is how the respective clients
handle enumerating through the objects in the bucket; by implementing
just that in each provider, I can have the select-and-fetch code in
once place.

The client implementations do now include safe-guards to ensure the
fetched object is the same as metadata has been collected for. In
addition, minor changes have been made to the object fetch operation
to take into account that:

  • Etags can change between composition of index and actual fetch, in
    which case the etag is now updated.
  • Objects can disappear between composition of index and actual fetch,
    in which case the item is removed from the index.

Lastly, the requirement for authentication has been removed (and not
referring to a Secret at all is thus allowed), to provide support
for e.g. public buckets.

Supersedes #455

@hiddeco hiddeco added enhancement New feature or request area/bucket Bucket related issues and pull requests labels Feb 28, 2022
@hiddeco hiddeco force-pushed the bucket-provider-interface-dev branch 2 times, most recently from cac61ff to 3a31d1c Compare February 28, 2022 14:29
@hiddeco hiddeco force-pushed the bucket-provider-interface-dev branch from 3a31d1c to 380dd32 Compare February 28, 2022 14:34
@hiddeco hiddeco marked this pull request as ready for review February 28, 2022 15:10
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Very neat.
LGTM!

controllers/bucket_controller.go Show resolved Hide resolved
pkg/minio/minio.go Outdated Show resolved Hide resolved
@@ -0,0 +1,284 @@
/*
Copyright 2021 The Flux authors
Copy link
Contributor

Choose a reason for hiding this comment

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

New file:

Suggested change
Copyright 2021 The Flux authors
Copyright 2022 The Flux authors

controllers/bucket_controller_fetch_test.go Outdated Show resolved Hide resolved
controllers/bucket_controller_fetch_test.go Outdated Show resolved Hide resolved
controllers/bucket_controller.go Outdated Show resolved Hide resolved
pkg/minio/minio_test.go Outdated Show resolved Hide resolved
@hiddeco hiddeco force-pushed the bucket-provider-interface-dev branch from 380dd32 to 9fda293 Compare March 1, 2022 06:31
This commit introduces a BucketProvider interface for fetch operations
against object storage provider buckets. Allowing for easier
introduction of new provider implementations.

The algorithm for conditionally downloading object files is the same,
whether you are using GCP storage or an S3/Minio-compatible
bucket. The only thing that differs is how the respective clients
handle enumerating through the objects in the bucket; by implementing
just that in each provider, I can have the select-and-fetch code in
once place.

The client implementations do now include safe-guards to ensure the
fetched object is the same as metadata has been collected for. In
addition, minor changes have been made to the object fetch operation
to take into account that:

- Etags can change between composition of index and actual fetch, in
  which case the etag is now updated.
- Objects can disappear between composition of index and actual fetch,
  in which case the item is removed from the index.

Lastly, the requirement for authentication has been removed (and not
referring to a Secret at all is thus allowed), to provide support
for e.g. public buckets.

Co-authored-by: Hidde Beydals <hello@hidde.co>
Co-authored by: Michael Bridgen <michael@weave.works>
Signed-off-by: pa250194 <pa250194@ncr.com>
@hiddeco hiddeco force-pushed the bucket-provider-interface-dev branch from 9ed66bb to ed6c6eb Compare March 1, 2022 09:15
@hiddeco hiddeco merged commit a4012f2 into main Mar 1, 2022
@hiddeco hiddeco deleted the bucket-provider-interface-dev branch March 1, 2022 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bucket Bucket related issues and pull requests enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants