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.

Sign up for GitHub

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

Jump to bottom

New data source: azurerm_storage_account_blob_container_sas #4195

Merged

tombuildsstuff merged 13 commits into hashicorp:master from r0bnet:data-storage-account-blob-container-sas

Sep 4, 2019

Contributor

r0bnet commented Aug 30, 2019 •

edited by tombuildsstuff

Implements what's requested here: #3068

Fixes #3068

r0bnet added 10 commits

August 15, 2019 09:26


          add validation function for shared access signature ip

44cc35f


          add first untested implementation of data source for blob container s…

c771f5b

…as token


          Merge branch 'master' into data-storage-account-blob-container-sas

b2f50f9

# Conflicts:
#	azurerm/provider.go
#	go.mod


          fix formatting

c6038e8


          Merge branch 'master' into data-storage-account-blob-container-sas

8d0879b

# Conflicts:
#	go.mod
#	vendor/github.com/Azure/azure-sdk-for-go/services/recoveryservices/mgmt/2018-01-10/siterecovery/client.go
#	vendor/github.com/Azure/azure-sdk-for-go/services/recoveryservices/mgmt/2018-01-10/siterecovery/version.go


          Merge branch 'master' into data-storage-account-blob-container-sas

738024a

# Conflicts:
#	go.sum
#	vendor/github.com/Azure/go-autorest/autorest/adal/token.go
#	vendor/github.com/Azure/go-autorest/autorest/go.mod
#	vendor/github.com/Azure/go-autorest/autorest/go.sum
#	vendor/github.com/Azure/go-autorest/autorest/sender.go
#	vendor/github.com/Azure/go-autorest/autorest/version.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method_azure_cli_token.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method_client_cert.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method_client_secret.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method_client_secret_multi_tenant.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/auth_method_msi.go
#	vendor/github.com/hashicorp/go-azure-helpers/authentication/config.go
#	vendor/modules.txt


          validate start and expiry properties

53e71cc


          add tests for blob container sas token

43518d0


          add documentation


          Merge branch 'master' into data-storage-account-blob-container-sas

37447b3

ghost added size/XL documentation labels

r0bnet mentioned this pull request

Feature Request: Add support for Azure Container SAS tokens #3068

Closed

tombuildsstuff added new-data-source service/storage labels

tombuildsstuff approved these changes

View reviewed changes

Member

tombuildsstuff left a comment

Thanks for this PR - taking a look through besides removing the ForceNew's (since this is a Data Source, they're not needed) and adding some validation this otherwise LGTM 👍

Thanks!

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"container_name": {
+              				Type:     schema.TypeString,
+              				Required: true,
+              				ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

can we remove ForceNew/add:

Suggested change

      
            				ForceNew: true,
          
            				ValidateFunc: validate.NoEmptyStrings,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              				Type:      schema.TypeString,
+              				Required:  true,
+              				ForceNew:  true,
+              				Sensitive: true,

Member

tombuildsstuff Sep 3, 2019

can we add:

Suggested change

      
            				Sensitive: true,
          
            				Sensitive: true,
          
            				ValidateFunc: validate.NoEmptyStrings,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"connection_string": {
+              				Type:      schema.TypeString,
+              				Required:  true,
+              				ForceNew:  true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              				Type:     schema.TypeBool,
+              				Optional: true,
+              				Default:  true,
+              				ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"ip": {
+              				Type:         schema.TypeString,
+              				Optional:     true,
+              				ForceNew:     true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              						"read": {
+              							Type:     schema.TypeBool,
+              							Required: true,
+              							ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"content_disposition": {
+              				Type:     schema.TypeString,
+              				Optional: true,
+              				ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"content_encoding": {
+              				Type:     schema.TypeString,
+              				Optional: true,
+              				ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              			"content_language": {
+              				Type:     schema.TypeString,
+              				Optional: true,
+              				ForceNew: true,

Member

tombuildsstuff Sep 3, 2019

since this is a Data Source we don't need the ForceNew here (since this'll be recomputed every time)

Suggested change

ForceNew: true,

azurerm/data_source_storage_account_blob_container_sas.go Outdated Show resolved Hide resolved

r0bnet added 2 commits

September 3, 2019 14:05


          fixed PR comments


          Merge branch 'master' into data-storage-account-blob-container-sas

dd2c3ca

tombuildsstuff added this to the v1.34.0 milestone


          rename ip field to ip_address

24eec92

tombuildsstuff approved these changes

View reviewed changes

Member

tombuildsstuff left a comment

Thanks for pushing those changes - besides the one rename this otherwise LGTM 👍

azurerm/data_source_storage_account_blob_container_sas.go Outdated

+              				Default:  true,
+              			},
+              			"ip": {

Member

tombuildsstuff Sep 3, 2019

🤔 on reflection could we make this ip_address to match the other resources?

Member

tombuildsstuff commented Sep 4, 2019

Tests pass:

tombuildsstuff merged commit c0e99ec into hashicorp:master

tombuildsstuff added a commit that referenced this pull request


          updating to include #4195

941264f

ibayer commented Sep 5, 2019 •

edited

Thanks for this great MR, fine grand SAS is really important for me.

@tombuildsstuff
I couldn't find any fixed release cycle for this provider. When can we expect this MR to be included?
Even an tentative estimate would help me to decide if I need to use a workaround or if I can wait for the next release.

r0bnet deleted the data-storage-account-blob-container-sas branch

September 5, 2019 08:31

Contributor Author

r0bnet commented Sep 5, 2019

Hey @ibayer
happy to hear that this helps you. I heard rumors that they're aiming for releasing 1.34.0 somewhere next week. But no guarantee on this.

Here you can see the status / progress for next releases: https://github.com/terraform-providers/terraform-provider-azurerm/milestones

ghost commented Sep 18, 2019

This has been released in version 1.34.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.34.0"
}
# ... other configuration ...

ghost commented Oct 4, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

ghost locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.