Skip to content
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

[Resource][Data Source] ACR Scope Map and Tokens #9402

Closed
wants to merge 10 commits into from

Conversation

Lucretius
Copy link
Contributor

@Lucretius Lucretius commented Nov 19, 2020

  • 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

@shashken
Copy link

@Lucretius @tombuildsstuff Any updates on this PR?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Lucretius - sorry for the delay in reviewing this, overall it looks good but we'll need to fix up the merge conflicts and move to the new testing framework. Once thats done this should be good to merge!

ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := azure.ParseAzureResourceID(d.Id())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to use the new generated ID parsers here

MinItems: 1,
Elem: &schema.Schema{
Type: schema.TypeString,
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we validate this?

"description": {
Type: schema.TypeString,
Optional: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we validate this?

Type: schema.TypeString,
Required: true,
ForceNew: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we validate this?

Type: schema.TypeString,
Required: true,
ForceNew: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we validaate this?

@@ -0,0 +1,52 @@
package tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to change this to the new testing frame work to support binary testing


# azurerm_container_registry_scope_map

Manages an Azure Container Registry scope map. Scope Maps are a preview feature only available in Premium SKU Container registries.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is preview is there a feature to enable?

Manages an Azure Container Registry token. Tokens are a preview feature only available in Premium SKU Container registries.

```hcl
resource "azurerm_resource_group" "rg" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use example for all names

Suggested change
resource "azurerm_resource_group" "rg" {
resource "azurerm_resource_group" "example" {


```hcl
resource "azurerm_resource_group" "rg" {
name = "resourceGroup1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name = "resourceGroup1"
name = "example-resource-group"

}

resource "azurerm_container_registry" "acr" {
name = "containerRegistry1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name = "containerRegistry1"
name = "ExampleRegistry"

and so forth

@katbyte
Copy link
Collaborator

katbyte commented Apr 16, 2021

closing as this PR has been superseded by #11350

@katbyte katbyte closed this Apr 16, 2021
katbyte pushed a commit that referenced this pull request Apr 28, 2021
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
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
@ghost
Copy link

ghost commented Apr 30, 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 ...

@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Azure Container Registry tokens and scope
3 participants