From 08ee7a7f15756afee7b7aec5c1e68f13f0cbad30 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 1 Feb 2022 16:28:00 -0800 Subject: [PATCH 1/6] chore(storage): internal client interface design --- storage/client.go | 145 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 storage/client.go diff --git a/storage/client.go b/storage/client.go new file mode 100644 index 00000000000..9faf82f04cd --- /dev/null +++ b/storage/client.go @@ -0,0 +1,145 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package storage + +import ( + "context" + + gax "github.com/googleapis/gax-go/v2" +) + +// storageClient is an internal-only interface designed to separate the +// transport-specific logic of making Storage API calls from the logic of the +// client library. +// +// Implementation requirements beyond implementing the interface include: +// * factory method(s) must accept a `userProject string` param +// * `settings` must be retained per instance +// * `storageOption`s must be resolved in the order they are received +// * all API errors must be wrapped in the gax-go APIError type +// * any unimplemented interface methods must return a StorageUnimplementedErr +type storageClient interface { + + // Bucket methods. + + CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) + DeleteBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error + GetBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) + ListBuckets(ctx context.Context, project string, opts ...storageOption) (BucketIterator, error) + UpdateBucket(ctx context.Context, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) + LockBucketRetentionPolicy(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error + + // Object metadata methods. + + DeleteObject(ctx context.Context, bucket, object string, conds *Conditions, opts ...storageOption) error + GetObject(ctx context.Context, bucket, object string, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) + ListObjects(ctx context.Context, bucket string, q *Query, opts ...storageOption) (*ObjectIterator, error) + UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) + + // Default Object ACL methods. + + DeleteDefaultObjectACL(ctx context.Context, bucket string, entity ACLEntity, opts ...storageOption) error + ListDefaultObjectACLs(ctx context.Context, bucket string, opts ...storageOption) ([]ACLRule, error) + UpdateDefaultObjectACL(ctx context.Context, opts ...storageOption) (ACLRule, error) + + // Bucket ACL methods. + + DeleteBucketACL(ctx context.Context, bucket string, entity ACLEntity, opts ...storageOption) error + ListBucketACLs(ctx context.Context, bucket string, opts ...storageOption) ([]ACLRule, error) + UpdateBucketACL(ctx context.Context, bucket string, entity ACLEntity, role ACLRole, opts ...storageOption) (ACLRule, error) + + // Object ACL Methods. + + DeleteObjectACL(ctx context.Context, bucket, object string, entity ACLEntity, opts ...storageOption) error + ListObjectACLs(ctx context.Context, bucket, object string, opts ...storageOption) ([]ACLRule, error) + UpdateObjectACL(ctx context.Context, bucket, object string, entity ACLEntity, role ACLRole, opts ...storageOption) (ACLRule, error) + + // Media operations. + + ComposeObject(ctx context.Context, req composeObjectRequest, opts ...storageOption) (*ObjectAttrs, error) + RewriteObject(ctx context.Context, req rewriteObjectRequest, opts ...storageOption) (rewriteObjectResponse, error) + + OpenReader(ctx context.Context, r *Reader, opts ...storageOption) error + OpenWriter(ctx context.Context, w *Writer, opts ...storageOption) error +} + +type settings struct { + retry *retryConfig + gax []gax.CallOption + idempotent bool +} + +type storageOption interface { + Apply(s *settings) +} + +func withGAXOptions(opts ...gax.CallOption) storageOption { + return &gaxOption{opts} +} + +type gaxOption struct { + opts []gax.CallOption +} + +func (o *gaxOption) Apply(s *settings) { s.gax = o.opts } + +func withRetryConfig(rc *retryConfig) storageOption { + return &retryOption{rc} +} + +type retryOption struct { + rc *retryConfig +} + +func (o *retryOption) Apply(s *settings) { s.retry = o.rc } + +func idempotent(i bool) storageOption { + return &idempotentOption{i} +} + +type idempotentOption struct { + idempotency bool +} + +func (o *idempotentOption) Apply(s *settings) { s.idempotent = o.idempotency } + +type composeObjectRequest struct { + dstBucket string + dstObject string + srcs []string + gen int64 + conds *Conditions + predefinedACL string +} + +type rewriteObjectRequest struct { + srcBucket string + srcObject string + dstBucket string + dstObject string + dstKeyName string + attrs *ObjectAttrs + gen int64 + conds *Conditions + predefinedACL string + token string +} + +type rewriteObjectResponse struct { + resource *ObjectAttrs + done bool + written int64 + token string +} From 9b8d52185beaea733ae2b1319b3edfea4118b8a5 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 15 Feb 2022 10:03:17 -0800 Subject: [PATCH 2/6] address feedback, add more comments --- storage/client.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/storage/client.go b/storage/client.go index 9faf82f04cd..ab980cae147 100644 --- a/storage/client.go +++ b/storage/client.go @@ -30,22 +30,27 @@ import ( // * `storageOption`s must be resolved in the order they are received // * all API errors must be wrapped in the gax-go APIError type // * any unimplemented interface methods must return a StorageUnimplementedErr +// +// TODO(noahdietz): This interface is currently not used in the production code +// paths. type storageClient interface { + // Top-level methods. + CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) + ListBuckets(ctx context.Context, project string, opts ...storageOption) (BucketIterator, error) + // Bucket methods. - CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) DeleteBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error GetBucket(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) - ListBuckets(ctx context.Context, project string, opts ...storageOption) (BucketIterator, error) UpdateBucket(ctx context.Context, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) LockBucketRetentionPolicy(ctx context.Context, bucket string, conds *BucketConditions, opts ...storageOption) error + ListObjects(ctx context.Context, bucket string, q *Query, opts ...storageOption) (*ObjectIterator, error) // Object metadata methods. DeleteObject(ctx context.Context, bucket, object string, conds *Conditions, opts ...storageOption) error GetObject(ctx context.Context, bucket, object string, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) - ListObjects(ctx context.Context, bucket string, q *Query, opts ...storageOption) (*ObjectIterator, error) UpdateObject(ctx context.Context, bucket, object string, uattrs *ObjectAttrsToUpdate, conds *Conditions, opts ...storageOption) (*ObjectAttrs, error) // Default Object ACL methods. @@ -75,12 +80,25 @@ type storageClient interface { OpenWriter(ctx context.Context, w *Writer, opts ...storageOption) error } +// settings contains transport-agnostic configuration for API calls made via +// the storageClient inteface. All implementations must utilize settings +// and respect those that are applicable. type settings struct { - retry *retryConfig - gax []gax.CallOption + // retry is the complete retry configuration to use when evaluating if an + // API call should be retried. + retry *retryConfig + + // gax is a set of gax.CallOption to be conveyed to gax.Invoke. + // Note: Not all storageClient interfaces will must use gax.Invoke. + gax []gax.CallOption + + // idempotent indicates if the call is idempotent or not when considering + // if the call should be retired or not. idempotent bool } +// storageOption is the transport-agnostic call option for the storageClient +// interface. type storageOption interface { Apply(s *settings) } From a69d9cf22ade03b38467f31bfc44438ebe65d920 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 15 Feb 2022 10:04:20 -0800 Subject: [PATCH 3/6] add todo --- storage/client.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/storage/client.go b/storage/client.go index ab980cae147..b56c132e0d5 100644 --- a/storage/client.go +++ b/storage/client.go @@ -20,6 +20,8 @@ import ( gax "github.com/googleapis/gax-go/v2" ) +// TODO(noahdietz): Move existing factory methods to this file. + // storageClient is an internal-only interface designed to separate the // transport-specific logic of making Storage API calls from the logic of the // client library. @@ -32,7 +34,7 @@ import ( // * any unimplemented interface methods must return a StorageUnimplementedErr // // TODO(noahdietz): This interface is currently not used in the production code -// paths. +// paths type storageClient interface { // Top-level methods. From 75aaf48624c23bec990c29ce54da035625152409 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 15 Feb 2022 10:59:36 -0800 Subject: [PATCH 4/6] add IAM, HMAC, and Service Account methods --- storage/client.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/storage/client.go b/storage/client.go index b56c132e0d5..7cec135fd3e 100644 --- a/storage/client.go +++ b/storage/client.go @@ -18,6 +18,7 @@ import ( "context" gax "github.com/googleapis/gax-go/v2" + iampb "google.golang.org/genproto/googleapis/iam/v1" ) // TODO(noahdietz): Move existing factory methods to this file. @@ -38,6 +39,8 @@ import ( type storageClient interface { // Top-level methods. + + GetServiceAccount(ctx context.Context, project string, opts ...storageOption) (string, error) CreateBucket(ctx context.Context, project string, attrs *BucketAttrs, opts ...storageOption) (*BucketAttrs, error) ListBuckets(ctx context.Context, project string, opts ...storageOption) (BucketIterator, error) @@ -67,7 +70,7 @@ type storageClient interface { ListBucketACLs(ctx context.Context, bucket string, opts ...storageOption) ([]ACLRule, error) UpdateBucketACL(ctx context.Context, bucket string, entity ACLEntity, role ACLRole, opts ...storageOption) (ACLRule, error) - // Object ACL Methods. + // Object ACL methods. DeleteObjectACL(ctx context.Context, bucket, object string, entity ACLEntity, opts ...storageOption) error ListObjectACLs(ctx context.Context, bucket, object string, opts ...storageOption) ([]ACLRule, error) @@ -80,6 +83,21 @@ type storageClient interface { OpenReader(ctx context.Context, r *Reader, opts ...storageOption) error OpenWriter(ctx context.Context, w *Writer, opts ...storageOption) error + + // IAM methods. + + GetIamPolicy(ctx context.Context, resource string, version int32, opts ...storageOption) (*iampb.Policy, error) + SetIamPolicy(ctx context.Context, resource string, policy *iampb.Policy, opts ...storageOption) error + TestIamPermissions(ctx context.Context, resource string, permissions []string, opts ...storageOption) ([]string, error) + + // HMAC Key methods. + // TODO(noahdietz): Determine how to work with HMACKeyOptions. + + GetHMACKey(ctx context.Context, project, accessID string, opts ...storageOption) (*HMACKey, error) + ListHMACKey(ctx context.Context, project string, opts ...storageOption) *HMACKeysIterator + UpdateHMACKey(ctx context.Context, project, accessID string, attrs *HMACKeyAttrsToUpdate, opts ...storageOption) (*HMACKey, error) + CreateHMACKey(ctx context.Context, project, serviceAccountEmail string, opts ...storageOption) (*HMACKey, error) + DeleteHMACKey(ctx context.Context, project, accessID string, opts ...storageOption) error } // settings contains transport-agnostic configuration for API calls made via From 920864ee982cfd3d932e9f0418dafa42eb0d2832 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 22 Feb 2022 08:44:51 -0800 Subject: [PATCH 5/6] switch to pointers for custom req/res objects --- storage/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/client.go b/storage/client.go index 7cec135fd3e..da7279134db 100644 --- a/storage/client.go +++ b/storage/client.go @@ -78,8 +78,8 @@ type storageClient interface { // Media operations. - ComposeObject(ctx context.Context, req composeObjectRequest, opts ...storageOption) (*ObjectAttrs, error) - RewriteObject(ctx context.Context, req rewriteObjectRequest, opts ...storageOption) (rewriteObjectResponse, error) + ComposeObject(ctx context.Context, req *composeObjectRequest, opts ...storageOption) (*ObjectAttrs, error) + RewriteObject(ctx context.Context, req *rewriteObjectRequest, opts ...storageOption) (*rewriteObjectResponse, error) OpenReader(ctx context.Context, r *Reader, opts ...storageOption) error OpenWriter(ctx context.Context, w *Writer, opts ...storageOption) error From 49cd35e0db9df730401e54e03f62304cd500b853 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 22 Feb 2022 08:50:18 -0800 Subject: [PATCH 6/6] use hmacKeyDesc --- storage/client.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/storage/client.go b/storage/client.go index da7279134db..c9fda866c46 100644 --- a/storage/client.go +++ b/storage/client.go @@ -91,13 +91,12 @@ type storageClient interface { TestIamPermissions(ctx context.Context, resource string, permissions []string, opts ...storageOption) ([]string, error) // HMAC Key methods. - // TODO(noahdietz): Determine how to work with HMACKeyOptions. - GetHMACKey(ctx context.Context, project, accessID string, opts ...storageOption) (*HMACKey, error) - ListHMACKey(ctx context.Context, project string, opts ...storageOption) *HMACKeysIterator - UpdateHMACKey(ctx context.Context, project, accessID string, attrs *HMACKeyAttrsToUpdate, opts ...storageOption) (*HMACKey, error) - CreateHMACKey(ctx context.Context, project, serviceAccountEmail string, opts ...storageOption) (*HMACKey, error) - DeleteHMACKey(ctx context.Context, project, accessID string, opts ...storageOption) error + GetHMACKey(ctx context.Context, desc *hmacKeyDesc, opts ...storageOption) (*HMACKey, error) + ListHMACKey(ctx context.Context, desc *hmacKeyDesc, opts ...storageOption) *HMACKeysIterator + UpdateHMACKey(ctx context.Context, desc *hmacKeyDesc, attrs *HMACKeyAttrsToUpdate, opts ...storageOption) (*HMACKey, error) + CreateHMACKey(ctx context.Context, desc *hmacKeyDesc, opts ...storageOption) (*HMACKey, error) + DeleteHMACKey(ctx context.Context, desc *hmacKeyDesc, opts ...storageOption) error } // settings contains transport-agnostic configuration for API calls made via