[Resource][Data Source] ACR Scope Map and Tokens #11350
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
katbyte
requested changes
Apr 16, 2021
There was a problem hiding this comment.
Thanks for taking on this pr @MattiasAng - overall this looks great and most of my comments are minor. this should be good to merge once they have been addressed
azurerm/internal/services/containers/container_registry_scope_map_data_source.go
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_data_source.go
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_resource.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_resource.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_resource_test.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_token_resource_test.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_token_resource.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_token_resource.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_data_source.go
Outdated
Show resolved
Hide resolved
|
see my comment reply left inline @MattiasAng |
katbyte
requested changes
Apr 28, 2021
There was a problem hiding this comment.
Thanks, this should be good to merge once the build is fix:
Run GOARCH=386 GOOS=linux go build -o 32bitbuild .
# github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers
azurerm/internal/services/containers/container_registry_scope_map_data_source.go:30:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_scope_map_resource.go:42:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_scope_map_resource.go:57:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_token_data_source.go:27:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_token_data_source.go:33:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_token_resource.go:42:19: undefined: ValidateContainerRegistryName
azurerm/internal/services/containers/container_registry_token_resource.go:51:19: undefined: ValidateContainerRegistryName
Error: Process completed with exit code 2.
katbyte
approved these changes
Apr 28, 2021
There was a problem hiding this comment.
Thanks @MattiasAng - aside from 2 comments this looks good 👍
azurerm/internal/services/containers/container_registry_token_resource_test.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/containers/container_registry_scope_map_data_source_test.go
Outdated
Show resolved
Hide resolved
…map_data_source_test.go
katbyte
added a commit
that referenced
this pull request
Apr 28, 2021
alvintang
pushed a commit
to alvintang/terraform-provider-azurerm
that referenced
this pull request
Apr 29, 2021
Supersede hashicorp#9402 after email exchange with @Lucretius which didn't have time to complete it currently. I fixed the comments from last review. Data Source for azurerm_container_registry_token Data Source for azurerm_container_registry_scope_map Resource for azurerm_container_registry_token Resource for azurerm_container_registry_scope_map Documentation for each Tests for each, they all pass running against my own Azure environment. There are passwords for the token which I would like to expose but there is no way to delete them once generated so they didn't really fit into the lifecycle management of the token itself. Perhaps they might be better suited as a separate data source, similar to how we handle the SAS tokens? Also note that these are still both preview features. As such I've vendor-included the SDK preview code and only used it in these specific preview features (see client.go) This PR resolves hashicorp#6505
alvintang
pushed a commit
to alvintang/terraform-provider-azurerm
that referenced
this pull request
Apr 29, 2021
|
This has been released in version 2.57.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 = "~> 2.57.0"
}
# ... other configuration ... |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersede #9402 after email exchange with @Lucretius which didn't have time to complete it currently.
I fixed the comments from last review.
Data Source for azurerm_container_registry_token
Data Source for azurerm_container_registry_scope_map
Resource for azurerm_container_registry_token
Resource for azurerm_container_registry_scope_map
Documentation for each
Tests for each, they all pass running against my own Azure environment.
There are passwords for the token which I would like to expose but there is no way to delete them once generated so they didn't really fit into the lifecycle management of the token itself. Perhaps they might be better suited as a separate data source, similar to how we handle the SAS tokens?
Also note that these are still both preview features. As such I've vendor-included the SDK preview code and only used it in these specific preview features (see client.go)
This PR resolves #6505