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

azurerm_container_registry - deprecated classic sku and storage_account_id #11165

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ func dataSourceContainerRegistry() *schema.Resource {
Computed: true,
},

"storage_account_id": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

someone might be using this in a config so can we just deprecate it for now?

Type: schema.TypeString,
Computed: true,
},

"tags": tags.SchemaDataSource(),
},
}
Expand Down Expand Up @@ -96,10 +91,6 @@ func dataSourceContainerRegistryRead(d *schema.ResourceData, meta interface{}) e
d.Set("sku", string(sku.Tier))
}

if account := resp.StorageAccount; account != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

and it could be an old account so maybe we should leave this till 3.0 - should be migrated isn't a 100% yes they are all migrated

d.Set("storage_account_id", account.ID)
}

if *resp.AdminUserEnabled {
credsResp, err := client.ListCredentials(ctx, resourceGroup, name)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ func resourceContainerRegistry() *schema.Resource {
"sku": {
Type: schema.TypeString,
Optional: true,
Default: string(containerregistry.Classic),
Default: string(containerregistry.Basic),
DiffSuppressFunc: suppress.CaseDifference,
ValidateFunc: validation.StringInSlice([]string{
string(containerregistry.Classic),
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is probably fine if puts where failing, but what if someone had an old one that never udpated?

string(containerregistry.Basic),
string(containerregistry.Standard),
string(containerregistry.Premium),
Expand Down Expand Up @@ -89,12 +88,6 @@ func resourceContainerRegistry() *schema.Resource {
Default: true,
},

"storage_account_id": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, i don't see any reason to remove it now instead of deprecating it and removing in 3.0

Type: schema.TypeString,
Optional: true,
ForceNew: true,
},

"login_server": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -330,18 +323,6 @@ func resourceContainerRegistryCreate(d *schema.ResourceData, meta interface{}) e
Tags: tags.Expand(t),
}

if v, ok := d.GetOk("storage_account_id"); ok {
if !strings.EqualFold(sku, string(containerregistry.Classic)) {
return fmt.Errorf("`storage_account_id` can only be specified for a Classic (unmanaged) Sku.")
}

parameters.StorageAccount = &containerregistry.StorageAccountProperties{
ID: utils.String(v.(string)),
}
} else if strings.EqualFold(sku, string(containerregistry.Classic)) {
return fmt.Errorf("`storage_account_id` must be specified for a Classic (unmanaged) Sku.")
}

future, err := client.Create(ctx, resourceGroup, name, parameters)
if err != nil {
return fmt.Errorf("Error creating Container Registry %q (Resource Group %q): %+v", name, resourceGroup, err)
Expand Down Expand Up @@ -633,10 +614,6 @@ func resourceContainerRegistryRead(d *schema.ResourceData, meta interface{}) err
d.Set("sku", string(sku.Tier))
}

if account := resp.StorageAccount; account != nil {
d.Set("storage_account_id", account.ID)
}

if *resp.AdminUserEnabled {
credsResp, errList := client.ListCredentials(ctx, resourceGroup, name)
if errList != nil {
Expand Down
2 changes: 0 additions & 2 deletions website/docs/d/container_registry.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ The following attributes are exported:

* `sku` - The SKU of this Container Registry, such as `Basic`.

* `storage_account_id` - The ID of the Storage Account used for this Container Registry. This is only returned for `Classic` SKU's.

* `tags` - A map of tags assigned to the Container Registry.

## Timeouts
Expand Down
6 changes: 1 addition & 5 deletions website/docs/r/container_registry.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ The following arguments are supported:

* `admin_enabled` - (Optional) Specifies whether the admin user is enabled. Defaults to `false`.

* `storage_account_id` - (Required for `Classic` Sku - Forbidden otherwise) The ID of a Storage Account which must be located in the same Azure Region as the Container Registry. Changing this forces a new resource to be created.

* `sku` - (Optional) The SKU name of the container registry. Possible values are `Basic`, `Standard` and `Premium`. `Classic` (which was previously `Basic`) is supported only for existing resources.

~> **NOTE:** The `Classic` SKU is Deprecated and will no longer be available for new resources from the end of March 2019.
* `sku` - (Optional) The SKU name of the container registry. Possible values are `Basic`, `Standard` and `Premium`.

* `tags` - (Optional) A mapping of tags to assign to the resource.

Expand Down