Repository: google/exposure-notifications-server
Pull request #1000 (2020-09-16 18:12:22)

set text/plain content type when uploading index.txt files
## Proposed Changes

* Allow for setting of content type on storage.CreateObject
* Set `text/plain` as the content type for index.txt files

**Release Note**

```release-note
Set content type for index files on blob storage.
```

google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1000#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1000#pullrequestreview-489897143" title="Approved">gurayAlsac</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1000#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1000#pullrequestreview-489897143" title="Approved">gurayAlsac</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1000#" title="Author self-approved">mikehelmick</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1000#pullrequestreview-489902172" title="LGTM">sethvargo</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick,sethvargo]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/internal/export/worker.go b/internal/export/worker.go
index 72cad4e..3b26291 100644
--- a/internal/export/worker.go
+++ b/internal/export/worker.go
@@ -28,6 +28,7 @@ import (
 	coredb "github.com/google/exposure-notifications-server/internal/database"
 	exportdatabase "github.com/google/exposure-notifications-server/internal/export/database"
 	publishdatabase "github.com/google/exposure-notifications-server/internal/publish/database"
+	"github.com/google/exposure-notifications-server/internal/storage"
 
 	"github.com/google/exposure-notifications-server/internal/export/model"
 	publishmodel "github.com/google/exposure-notifications-server/internal/publish/model"
@@ -274,7 +275,7 @@ func (s *Server) createFile(ctx context.Context, cfi createFileInfo) (string, er
 	logger.Infof("Created file %v, signed with %v keys", objectName, len(signers))
 	ctx, cancel := context.WithTimeout(ctx, blobOperationTimeout)
 	defer cancel()
-	if err := s.env.Blobstore().CreateObject(ctx, cfi.exportBatch.BucketName, objectName, data, true); err != nil {
+	if err := s.env.Blobstore().CreateObject(ctx, cfi.exportBatch.BucketName, objectName, data, true, storage.ContentTypeDefault); err != nil {
 		return "", fmt.Errorf("creating file %s in bucket %s: %w", objectName, cfi.exportBatch.BucketName, err)
 	}
 	return objectName, nil
@@ -350,8 +351,8 @@ func (s *Server) createIndex(ctx context.Context, eb *model.ExportBatch, newObje
 	indexObjectName := exportIndexFilename(eb)
 	ctx, cancel := context.WithTimeout(ctx, blobOperationTimeout)
 	defer cancel()
-	if err := s.env.Blobstore().CreateObject(ctx, eb.BucketName, indexObjectName, data, false); err != nil {
-		return "", 0, fmt.Errorf("creating file %s in bucket %s: %w", indexObjectName, eb.BucketName, err)
+	if err := s.env.Blobstore().CreateObject(ctx, eb.BucketName, indexObjectName, data, false, storage.ContentTypeTextPlain); err != nil {
+		return "", 0, fmt.Errorf("creating index file %s in bucket %s: %w", indexObjectName, eb.BucketName, err)
 	}
 	return indexObjectName, len(objects), nil
 }
diff --git a/internal/storage/aws_s3.go b/internal/storage/aws_s3.go
index ee5f4d2..8f1066a 100644
--- a/internal/storage/aws_s3.go
+++ b/internal/storage/aws_s3.go
@@ -51,18 +51,22 @@ func NewAWSS3(ctx context.Context) (Blobstore, error) {
 }
 
 // CreateObject creates a new S3 object or overwrites an existing one.
-func (s *AWSS3) CreateObject(ctx context.Context, bucket, key string, contents []byte, cacheable bool) error {
+func (s *AWSS3) CreateObject(ctx context.Context, bucket, key string, contents []byte, cacheable bool, contentType string) error {
 	cacheControl := "public, max-age=86400"
 	if !cacheable {
 		cacheControl = "no-cache, max-age=0"
 	}
 
-	if _, err := s.svc.PutObjectWithContext(ctx, &s3.PutObjectInput{
+	putInput := s3.PutObjectInput{
 		Bucket:       aws.String(bucket),
 		Key:          aws.String(key),
 		CacheControl: aws.String(cacheControl),
 		Body:         bytes.NewReader(contents),
-	}); err != nil {
+	}
+	if contentType != "" {
+		putInput.ContentType = aws.String(contentType)
+	}
+	if _, err := s.svc.PutObjectWithContext(ctx, &putInput); err != nil {
 		return fmt.Errorf("storage.CreateObject: %w", err)
 	}
 	return nil
diff --git a/internal/storage/azure_blobstore.go b/internal/storage/azure_blobstore.go
index d66cabd..6ff5a01 100644
--- a/internal/storage/azure_blobstore.go
+++ b/internal/storage/azure_blobstore.go
@@ -112,17 +112,21 @@ func NewAzureBlobstore(ctx context.Context) (Blobstore, error) {
 }
 
 // CreateObject creates a new blobstore object or overwrites an existing one.
-func (s *AzureBlobstore) CreateObject(ctx context.Context, container, name string, contents []byte, cacheable bool) error {
+func (s *AzureBlobstore) CreateObject(ctx context.Context, container, name string, contents []byte, cacheable bool, contentType string) error {
 	cacheControl := "public, max-age=86400"
 	if !cacheable {
 		cacheControl = "no-cache, max-age=0"
 	}
 
 	blobURL := s.serviceURL.NewContainerURL(container).NewBlockBlobURL(name)
+	headers := azblob.BlobHTTPHeaders{
+		CacheControl: cacheControl,
+	}
+	if contentType != "" {
+		headers.ContentType = contentType
+	}
 	if _, err := azblob.UploadBufferToBlockBlob(ctx, contents, blobURL, azblob.UploadToBlockBlobOptions{
-		BlobHTTPHeaders: azblob.BlobHTTPHeaders{
-			CacheControl: cacheControl,
-		},
+		BlobHTTPHeaders: headers,
 	}); err != nil {
 		return fmt.Errorf("storage.CreateObject: %w", err)
 	}
diff --git a/internal/storage/filesystem_storage.go b/internal/storage/filesystem_storage.go
index e4f9721..8217daa 100644
--- a/internal/storage/filesystem_storage.go
+++ b/internal/storage/filesystem_storage.go
@@ -38,7 +38,8 @@ func NewFilesystemStorage(ctx context.Context) (Blobstore, error) {
 
 // CreateObject creates a new object on the filesystem or overwrites an existing
 // one.
-func (s *FilesystemStorage) CreateObject(ctx context.Context, folder, filename string, contents []byte, cacheable bool) error {
+// contentType is ignored for this storage implementation.
+func (s *FilesystemStorage) CreateObject(ctx context.Context, folder, filename string, contents []byte, cacheable bool, contentType string) error {
 	pth := filepath.Join(folder, filename)
 	if err := ioutil.WriteFile(pth, contents, 0644); err != nil {
 		return fmt.Errorf("failed to create object: %w", err)
diff --git a/internal/storage/google_cloud_storage.go b/internal/storage/google_cloud_storage.go
index ab9ccc8..f87d2ea 100644
--- a/internal/storage/google_cloud_storage.go
+++ b/internal/storage/google_cloud_storage.go
@@ -44,7 +44,7 @@ func NewGoogleCloudStorage(ctx context.Context) (Blobstore, error) {
 }
 
 // CreateObject creates a new cloud storage object or overwrites an existing one.
-func (s *GoogleCloudStorage) CreateObject(ctx context.Context, bucket, objectName string, contents []byte, cacheable bool) error {
+func (s *GoogleCloudStorage) CreateObject(ctx context.Context, bucket, objectName string, contents []byte, cacheable bool, contentType string) error {
 	cacheControl := "public, max-age=86400"
 	if !cacheable {
 		cacheControl = "no-cache, max-age=0"
@@ -52,6 +52,9 @@ func (s *GoogleCloudStorage) CreateObject(ctx context.Context, bucket, objectNam
 
 	wc := s.client.Bucket(bucket).Object(objectName).NewWriter(ctx)
 	wc.CacheControl = cacheControl
+	if contentType != "" {
+		wc.ContentType = contentType
+	}
 
 	if _, err := wc.Write(contents); err != nil {
 		return fmt.Errorf("storage.Writer.Write: %w", err)
diff --git a/internal/storage/memory.go b/internal/storage/memory.go
index 83ea547..23f8000 100644
--- a/internal/storage/memory.go
+++ b/internal/storage/memory.go
@@ -38,7 +38,8 @@ func NewMemory(_ context.Context) (Blobstore, error) {
 }
 
 // CreateObject creates a new object.
-func (s *Memory) CreateObject(_ context.Context, folder, filename string, contents []byte, cacheable bool) error {
+// contentType is ignored in this implementation.
+func (s *Memory) CreateObject(_ context.Context, folder, filename string, contents []byte, cacheable bool, contentType string) error {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
diff --git a/internal/storage/noop.go b/internal/storage/noop.go
index 04c6e5e..ad3973f 100644
--- a/internal/storage/noop.go
+++ b/internal/storage/noop.go
@@ -26,7 +26,7 @@ func NewNoop(ctx context.Context) (Blobstore, error) {
 	return &Noop{}, nil
 }
 
-func (s *Noop) CreateObject(_ context.Context, _, _ string, _ []byte, _ bool) error {
+func (s *Noop) CreateObject(_ context.Context, _, _ string, _ []byte, _ bool, _ string) error {
 	return nil
 }
 
diff --git a/internal/storage/storage.go b/internal/storage/storage.go
index b12c7c6..370dc2f 100644
--- a/internal/storage/storage.go
+++ b/internal/storage/storage.go
@@ -22,10 +22,16 @@ import (
 
 var ErrNotFound = fmt.Errorf("storage object not found")
 
+const (
+	ContentTypeTextPlain = "text/plain"
+	ContentTypeDefault   = ""
+)
+
 // Blobstore defines the minimum interface for a blob storage system.
 type Blobstore interface {
 	// CreateObject creates or overwrites an object in the storage system.
-	CreateObject(ctx context.Context, parent, name string, contents []byte, cacheable bool) error
+	// If contentType is blank, the default for the chosen storage implementation is used.
+	CreateObject(ctx context.Context, parent, name string, contents []byte, cacheable bool, contentType string) error
 
 	// DeleteObject deletes an object or does nothing if the object doesn't exist.
 	DeleteObject(ctx context.Context, parent, bame string) error


Test patch:
diff --git a/internal/storage/filesystem_storage_test.go b/internal/storage/filesystem_storage_test.go
index a094c2b..ac4d1a4 100644
--- a/internal/storage/filesystem_storage_test.go
+++ b/internal/storage/filesystem_storage_test.go
@@ -67,7 +67,7 @@ func TestFilesystemStorage_CreateObject(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			err = storage.CreateObject(ctx, tc.folder, tc.filepath, tc.contents, false)
+			err = storage.CreateObject(ctx, tc.folder, tc.filepath, tc.contents, false, ContentTypeDefault)
 			if (err != nil) != tc.err {
 				t.Fatal(err)
 			}
diff --git a/internal/storage/google_cloud_storage_test.go b/internal/storage/google_cloud_storage_test.go
index 0f277fa..253ba83 100644
--- a/internal/storage/google_cloud_storage_test.go
+++ b/internal/storage/google_cloud_storage_test.go
@@ -155,7 +155,7 @@ func TestGoogleCloudStorage_CreateObject(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			err = gcsStorage.CreateObject(ctx, tc.bucket, tc.object, tc.contents, false)
+			err = gcsStorage.CreateObject(ctx, tc.bucket, tc.object, tc.contents, false, ContentTypeDefault)
 			if (err != nil) != tc.err {
 				t.Fatal(err)
 			}