-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 content_type
to azurerm_storage_blob
#1304
Changes from all commits
0cb75e6
57cfc2e
da7b3a5
7e6daf7
86f17b7
96d8258
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ func resourceArmStorageBlob() *schema.Resource { | |
return &schema.Resource{ | ||
Create: resourceArmStorageBlobCreate, | ||
Read: resourceArmStorageBlobRead, | ||
Update: resourceArmStorageBlobUpdate, | ||
Exists: resourceArmStorageBlobExists, | ||
Delete: resourceArmStorageBlobDelete, | ||
|
||
|
@@ -53,6 +54,12 @@ func resourceArmStorageBlob() *schema.Resource { | |
Default: 0, | ||
ValidateFunc: validateArmStorageBlobSize, | ||
}, | ||
"content_type": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Default: "application/octet-stream", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where does this default come from? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. The default value comes from |
||
ConflictsWith: []string{"source_uri"}, | ||
}, | ||
"source": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
|
@@ -149,6 +156,7 @@ func resourceArmStorageBlobCreate(d *schema.ResourceData, meta interface{}) erro | |
blobType := d.Get("type").(string) | ||
cont := d.Get("storage_container_name").(string) | ||
sourceUri := d.Get("source_uri").(string) | ||
contentType := d.Get("content_type").(string) | ||
|
||
log.Printf("[INFO] Creating blob %q in storage account %q", name, storageAccountName) | ||
if sourceUri != "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refer to this article to simplify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can pull this out into separate methods - but I think that's outside of the scope of this PR tbh; so let's do that in a separate PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @tombuildsstuff . The blob storage code is messy and the fields also make people confused. So let's do it in another PR if possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, at some point in the future (read: when the SDK's are more stable) we'll be switching over to the new Storage SDKs; which would probably be a sensible time to do this refactoring |
||
|
@@ -174,7 +182,7 @@ func resourceArmStorageBlobCreate(d *schema.ResourceData, meta interface{}) erro | |
if source != "" { | ||
parallelism := d.Get("parallelism").(int) | ||
attempts := d.Get("attempts").(int) | ||
if err := resourceArmStorageBlobBlockUploadFromSource(cont, name, source, blobClient, parallelism, attempts); err != nil { | ||
if err := resourceArmStorageBlobBlockUploadFromSource(cont, name, source, contentType, blobClient, parallelism, attempts); err != nil { | ||
return fmt.Errorf("Error creating storage blob on Azure: %s", err) | ||
} | ||
} | ||
|
@@ -183,7 +191,7 @@ func resourceArmStorageBlobCreate(d *schema.ResourceData, meta interface{}) erro | |
if source != "" { | ||
parallelism := d.Get("parallelism").(int) | ||
attempts := d.Get("attempts").(int) | ||
if err := resourceArmStorageBlobPageUploadFromSource(cont, name, source, blobClient, parallelism, attempts); err != nil { | ||
if err := resourceArmStorageBlobPageUploadFromSource(cont, name, source, contentType, blobClient, parallelism, attempts); err != nil { | ||
return fmt.Errorf("Error creating storage blob on Azure: %s", err) | ||
} | ||
} else { | ||
|
@@ -193,6 +201,7 @@ func resourceArmStorageBlobCreate(d *schema.ResourceData, meta interface{}) erro | |
container := blobClient.GetContainerReference(cont) | ||
blob := container.GetBlobReference(name) | ||
blob.Properties.ContentLength = size | ||
blob.Properties.ContentType = contentType | ||
err := blob.PutPageBlob(options) | ||
if err != nil { | ||
return fmt.Errorf("Error creating storage blob on Azure: %s", err) | ||
|
@@ -210,7 +219,7 @@ type resourceArmStorageBlobPage struct { | |
section *io.SectionReader | ||
} | ||
|
||
func resourceArmStorageBlobPageUploadFromSource(container, name, source string, client *storage.BlobStorageClient, parallelism, attempts int) error { | ||
func resourceArmStorageBlobPageUploadFromSource(container, name, source, contentType string, client *storage.BlobStorageClient, parallelism, attempts int) error { | ||
workerCount := parallelism * runtime.NumCPU() | ||
|
||
file, err := os.Open(source) | ||
|
@@ -228,6 +237,7 @@ func resourceArmStorageBlobPageUploadFromSource(container, name, source string, | |
containerRef := client.GetContainerReference(container) | ||
blob := containerRef.GetBlobReference(name) | ||
blob.Properties.ContentLength = blobSize | ||
blob.Properties.ContentType = contentType | ||
err = blob.PutPageBlob(options) | ||
if err != nil { | ||
return fmt.Errorf("Error creating storage blob on Azure: %s", err) | ||
|
@@ -387,7 +397,7 @@ type resourceArmStorageBlobBlock struct { | |
id string | ||
} | ||
|
||
func resourceArmStorageBlobBlockUploadFromSource(container, name, source string, client *storage.BlobStorageClient, parallelism, attempts int) error { | ||
func resourceArmStorageBlobBlockUploadFromSource(container, name, source, contentType string, client *storage.BlobStorageClient, parallelism, attempts int) error { | ||
workerCount := parallelism * runtime.NumCPU() | ||
|
||
file, err := os.Open(source) | ||
|
@@ -432,6 +442,7 @@ func resourceArmStorageBlobBlockUploadFromSource(container, name, source string, | |
|
||
containerReference := client.GetContainerReference(container) | ||
blobReference := containerReference.GetBlobReference(name) | ||
blobReference.Properties.ContentType = contentType | ||
options := &storage.PutBlockListOptions{} | ||
err = blobReference.PutBlockList(blockList, options) | ||
if err != nil { | ||
|
@@ -524,6 +535,40 @@ func resourceArmStorageBlobBlockUploadWorker(ctx resourceArmStorageBlobBlockUplo | |
} | ||
} | ||
|
||
func resourceArmStorageBlobUpdate(d *schema.ResourceData, meta interface{}) error { | ||
armClient := meta.(*ArmClient) | ||
ctx := armClient.StopContext | ||
|
||
resourceGroupName := d.Get("resource_group_name").(string) | ||
storageAccountName := d.Get("storage_account_name").(string) | ||
|
||
blobClient, accountExists, err := armClient.getBlobStorageClientForStorageAccount(ctx, resourceGroupName, storageAccountName) | ||
if err != nil { | ||
return fmt.Errorf("Error getting storage account %s: %+v", storageAccountName, err) | ||
} | ||
if !accountExists { | ||
return fmt.Errorf("Storage account %s not found in resource group %s", storageAccountName, resourceGroupName) | ||
} | ||
|
||
name := d.Get("name").(string) | ||
storageContainerName := d.Get("storage_container_name").(string) | ||
|
||
container := blobClient.GetContainerReference(storageContainerName) | ||
blob := container.GetBlobReference(name) | ||
|
||
if d.HasChange("content_type") { | ||
blob.Properties.ContentType = d.Get("content_type").(string) | ||
} | ||
|
||
options := &storage.SetBlobPropertiesOptions{} | ||
err = blob.SetProperties(options) | ||
if err != nil { | ||
return fmt.Errorf("Error setting properties of blob %s (container %s, storage account %s): %+v", name, storageContainerName, storageAccountName, err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func resourceArmStorageBlobRead(d *schema.ResourceData, meta interface{}) error { | ||
armClient := meta.(*ArmClient) | ||
ctx := armClient.StopContext | ||
|
@@ -556,6 +601,14 @@ func resourceArmStorageBlobRead(d *schema.ResourceData, meta interface{}) error | |
|
||
container := blobClient.GetContainerReference(storageContainerName) | ||
blob := container.GetBlobReference(name) | ||
|
||
options := &storage.GetBlobPropertiesOptions{} | ||
err = blob.GetProperties(options) | ||
if err != nil { | ||
return fmt.Errorf("Error getting properties of blob %s (container %s, storage account %s): %+v", name, storageContainerName, storageAccountName, err) | ||
} | ||
d.Set("content_type", blob.Properties.ContentType) | ||
|
||
url := blob.GetURL() | ||
if url == "" { | ||
log.Printf("[INFO] URL for %q is empty", name) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should do a state migration test here? Just to make sure it doesn't break anything downstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I run the new
terraform plan
after the oldterraform apply
, and it turns out nothing has been changed. I think there is not needs to do state migration here. @katbyte @tombuildsstuff @paultyng what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related question below: where does this default value come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also document this new field in the docs (
website/docs/r/storage_blob.html.markdown
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. The default value comes from
Content-Type
section in https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob#request-headers-all-blob-types. And blobs created by old plugin have theapplication/octet-stream
content type as well.