chore: address code review comments

Signed-off-by: Kirat Singh <kirat.singh@beacon.io>

docs/configuration.md

@@ -100,8 +100,8 @@ storage:
     rootdirectory: /az/object/name/prefix
     credentials:
       type: client_secret
-      clientId: client_id_string
-      tenantId: tenant_id_string
+      clientid: client_id_string
+      tenantid: tenant_id_string
       secret: secret_string
   gcs:
     bucket: bucketname

registry/storage/blobwriter.go

@@ -94,7 +94,7 @@ func (bw *blobWriter) Commit(ctx context.Context, desc distribution.Descriptor)
 // the writer and canceling the operation.
 func (bw *blobWriter) Cancel(ctx context.Context) error {
 	dcontext.GetLogger(ctx).Debug("(*blobWriter).Cancel")
-	if err := bw.fileWriter.Cancel(); err != nil {
+	if err := bw.fileWriter.Cancel(ctx); err != nil {
 		return err
 	}
 

registry/storage/driver/azure/azure.go

@@ -59,7 +59,7 @@ func New(params *Parameters) (*Driver, error) {
 	if err != nil {
 		return nil, err
 	}
-	client := azClient.GetContainerClient()
+	client := azClient.ContainerClient()
 	d := &driver{
 		azClient:      azClient,
 		client:        client,
@@ -159,16 +159,13 @@ func (d *driver) Writer(ctx context.Context, path string, append bool) (storaged
 	blobName := d.blobName(path)
 	blobRef := d.client.NewBlobClient(blobName)
 
-	var blobExists bool
 	props, err := blobRef.GetProperties(ctx, nil)
+	blobExists := true
 	if err != nil {
-		if is404(err) {
-			blobExists = false
-		} else {
+		if !is404(err) {
 			return nil, err
 		}
-	} else {
-		blobExists = true
+		blobExists = false
 	}
 
 	var size int64
@@ -412,7 +409,6 @@ func is404(err error) bool {
 
 type writer struct {
 	driver    *driver
-	ctx       context.Context
 	path      string
 	size      int64
 	bw        *bufio.Writer
@@ -424,7 +420,6 @@ type writer struct {
 func (d *driver) newWriter(ctx context.Context, path string, size int64) storagedriver.FileWriter {
 	return &writer{
 		driver: d,
-		ctx:    ctx,
 		path:   path,
 		size:   size,
 		bw: bufio.NewWriterSize(&blockWriter{
@@ -461,15 +456,15 @@ func (w *writer) Close() error {
 	return w.bw.Flush()
 }
 
-func (w *writer) Cancel() error {
+func (w *writer) Cancel(ctx context.Context) error {
 	if w.closed {
 		return fmt.Errorf("already closed")
 	} else if w.committed {
 		return fmt.Errorf("already committed")
 	}
 	w.cancelled = true
 	blobRef := w.driver.client.NewBlobClient(w.path)
-	_, err := blobRef.Delete(w.ctx, nil)
+	_, err := blobRef.Delete(ctx, nil)
 	return err
 }
 
@@ -486,6 +481,8 @@ func (w *writer) Commit() error {
 }
 
 type blockWriter struct {
+	// We construct transient blockWriter objects to encapsulate a write
+	// and need to keep the context passed in to the original FileWriter.Write
 	ctx    context.Context
 	client *container.Client
 	path   string

registry/storage/driver/azure/azure_auth.go

@@ -15,32 +15,28 @@ import (
 )
 
 const (
-	UdcGracePeriodInMinutes = 30.0
-	UdcExpiryTimeHours      = 48.0
+	UdcGracePeriod = 30.0 * time.Minute
+	UdcExpiryTime  = 48.0 * time.Hour
 )
 
 type azureClient interface {
 	SignBlobURL(ctx context.Context, blobURL string, expires time.Time) (string, error)
-	GetContainerClient() *container.Client
+	ContainerClient() *container.Client
 }
 
 type azureClientSharedKey struct {
-	accountName string
-	container   string
-	serviceURL  string
-	cred        *azblob.SharedKeyCredential
-	client      *azblob.Client
+	container string
+	cred      *azblob.SharedKeyCredential
+	client    *azblob.Client
 }
 
 type azureClientTokenCredential struct {
-	accountName string
-	container   string
-	serviceURL  string
-	cred        azcore.TokenCredential
-	client      *azblob.Client
-	udcMutex    sync.Mutex
-	udc         *service.UserDelegationCredential
-	udcExpiry   time.Time
+	container string
+	cred      azcore.TokenCredential
+	client    *azblob.Client
+	udcMutex  sync.Mutex
+	udc       *service.UserDelegationCredential
+	udcExpiry time.Time
 }
 
 func NewAzureClient(params *Parameters) (azureClient, error) {
@@ -54,11 +50,9 @@ func NewAzureClient(params *Parameters) (azureClient, error) {
 			return nil, err
 		}
 		return &azureClientSharedKey{
-			accountName: params.AccountName,
-			container:   params.Container,
-			serviceURL:  params.ServiceURL,
-			cred:        cred,
-			client:      client,
+			container: params.Container,
+			cred:      cred,
+			client:    client,
 		}, nil
 	}
 
@@ -78,11 +72,9 @@ func NewAzureClient(params *Parameters) (azureClient, error) {
 		return nil, err
 	}
 	return &azureClientTokenCredential{
-		accountName: params.AccountName,
-		container:   params.Container,
-		serviceURL:  params.ServiceURL,
-		cred:        cred,
-		client:      client,
+		container: params.Container,
+		cred:      cred,
+		client:    client,
 	}, nil
 }
 
@@ -99,7 +91,7 @@ func makeBlobSignatureValues(urlParts *sas.URLParts, expires time.Time) sas.Blob
 	return res
 }
 
-func (a *azureClientSharedKey) GetContainerClient() *container.Client {
+func (a *azureClientSharedKey) ContainerClient() *container.Client {
 	return a.client.ServiceClient().NewContainerClient(a.container)
 }
 
@@ -115,7 +107,7 @@ func (a *azureClientSharedKey) SignBlobURL(ctx context.Context, blobURL string,
 	return urlParts.String(), nil
 }
 
-func (a *azureClientTokenCredential) GetContainerClient() *container.Client {
+func (a *azureClientTokenCredential) ContainerClient() *container.Client {
 	return a.client.ServiceClient().NewContainerClient(a.container)
 }
 
@@ -124,10 +116,10 @@ func (a *azureClientTokenCredential) refreshUDC(ctx context.Context) error {
 	defer a.udcMutex.Unlock()
 
 	now := time.Now().UTC()
-	if a.udc == nil || a.udcExpiry.Sub(now).Minutes() < UdcGracePeriodInMinutes {
+	if a.udc == nil || a.udcExpiry.Sub(now) < UdcGracePeriod {
 		// reissue user delegation credential
 		startTime := now.Add(-10 * time.Second)
-		expiryTime := startTime.Add(UdcExpiryTimeHours * time.Hour)
+		expiryTime := startTime.Add(UdcExpiryTime)
 		info := service.KeyInfo{
 			Start:  to.Ptr(startTime.UTC().Format(sas.TimeFormat)),
 			Expiry: to.Ptr(expiryTime.UTC().Format(sas.TimeFormat)),

registry/storage/driver/azure/parser.go

@@ -13,8 +13,8 @@ const (
 
 type Credentials struct {
 	Type     string `yaml:"type"`
-	ClientID string `yaml:"clientId"`
-	TenantID string `yaml:"tenantId"`
+	ClientID string `yaml:"clientid"`
+	TenantID string `yaml:"tenantid"`
 	Secret   string `yaml:"secret"`
 }
 

registry/storage/driver/filesystem/driver.go

@@ -139,7 +139,7 @@ func (d *driver) PutContent(ctx context.Context, subPath string, contents []byte
 	defer writer.Close()
 	_, err = io.Copy(writer, bytes.NewReader(contents))
 	if err != nil {
-		writer.Cancel()
+		writer.Cancel(ctx)
 		return err
 	}
 	return writer.Commit()
@@ -387,7 +387,7 @@ func (fw *fileWriter) Close() error {
 	return nil
 }
 
-func (fw *fileWriter) Cancel() error {
+func (fw *fileWriter) Cancel(ctx context.Context) error {
 	if fw.closed {
 		return fmt.Errorf("already closed")
 	}

registry/storage/driver/inmemory/driver.go

@@ -293,7 +293,7 @@ func (w *writer) Close() error {
 	return nil
 }
 
-func (w *writer) Cancel() error {
+func (w *writer) Cancel(ctx context.Context) error {
 	if w.closed {
 		return fmt.Errorf("already closed")
 	} else if w.committed {

registry/storage/driver/s3-aws/s3.go

@@ -1425,7 +1425,7 @@ func (w *writer) Close() error {
 	return w.flushPart()
 }
 
-func (w *writer) Cancel() error {
+func (w *writer) Cancel(ctx context.Context) error {
 	if w.closed {
 		return fmt.Errorf("already closed")
 	} else if w.committed {

registry/storage/driver/storagedriver.go

@@ -103,7 +103,7 @@ type FileWriter interface {
 	Size() int64
 
 	// Cancel removes any written content from this FileWriter.
-	Cancel() error
+	Cancel(context.Context) error
 
 	// Commit flushes all content written to this FileWriter and makes it
 	// available for future calls to StorageDriver.GetContent and

registry/storage/driver/swift/swift.go

@@ -850,14 +850,14 @@ func (w *writer) Close() error {
 	return nil
 }
 
-func (w *writer) Cancel() error {
+func (w *writer) Cancel(ctx context.Context) error {
 	if w.closed {
 		return fmt.Errorf("already closed")
 	} else if w.committed {
 		return fmt.Errorf("already committed")
 	}
 	w.cancelled = true
-	return w.driver.Delete(context.Background(), w.path)
+	return w.driver.Delete(ctx, w.path)
 }
 
 func (w *writer) Commit() error {

registry/storage/driver/testdriver/testdriver.go

@@ -61,9 +61,9 @@ func (tfw *testFileWriter) Close() error {
 	return tfw.FileWriter.Close()
 }
 
-func (tfw *testFileWriter) Cancel() error {
+func (tfw *testFileWriter) Cancel(ctx context.Context) error {
 	tfw.Write(nil)
-	return tfw.FileWriter.Cancel()
+	return tfw.FileWriter.Cancel(ctx)
 }
 
 func (tfw *testFileWriter) Commit() error {