Feat add azure containers cache #24040

mhaligowski · 2023-11-28T00:37:00Z

No description provided.

aristosvo

Hi @mhaligowski! Thanks for all this! I gave you a few pointers to start with, as I don't expect a review from the HC maintainers these days, they will have probably a few more remarks.

There are two tests failing as well:

------- Stdout: -------
=== RUN   TestAccContainerRegistryCacheRule_basic
=== PAUSE TestAccContainerRegistryCacheRule_basic
=== CONT  TestAccContainerRegistryCacheRule_basic
    testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
        Error: Unsupported argument
          on terraform_plugin_test.tf line 35, in resource "azurerm_container_registry_cache_rule" "test":
          35:   resource_group_name = azurerm_resource_group.test.name
        An argument named "resource_group_name" is not expected here.
--- FAIL: TestAccContainerRegistryCacheRule_basic (2.78s)
FAIL

------- Stdout: -------
=== RUN   TestAccContainerRegistryCacheRule_requiresImport
=== PAUSE TestAccContainerRegistryCacheRule_requiresImport
=== CONT  TestAccContainerRegistryCacheRule_requiresImport
    testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
        Error: Unsupported argument
          on terraform_plugin_test.tf line 35, in resource "azurerm_container_registry_cache_rule" "test":
          35:   resource_group_name = azurerm_resource_group.test.name
        An argument named "resource_group_name" is not expected here.
--- FAIL: TestAccContainerRegistryCacheRule_requiresImport (3.10s)
FAIL

Hope you enjoy the last bits of 2023!

aristosvo · 2023-12-29T20:01:44Z

internal/services/containers/container_registry_cache_rule_resource.go

+			Description: "The name of the cache rule.",
+		},
+
+		"registry": {


I would opt for container_registry_id here, which contains both the resource group name and registry name, as input.

How to work with that ID is shown here:

terraform-provider-azurerm/internal/services/containers/container_registry_task_resource.go

Line 126 in 46a95c1

ContainerRegistryId string `tfschema:"container_registry_id"`

aristosvo · 2023-12-29T20:07:17Z

internal/services/containers/container_registry_cache_rule_resource.go

+}
+
+func (ContainerRegistryCacheRule) IDValidationFunc() pluginsdk.SchemaValidateFunc {
+	return validate.ContainerRegistryCacheRuleID


I believe you can replace the validate package with the content of cacherules in the new SDK, like this:

Suggested change

return validate.ContainerRegistryCacheRuleID

return cacherules.ValidateCacheRuleID

aristosvo · 2023-12-29T20:08:40Z

internal/services/containers/resourceids.go

@@ -5,5 +5,6 @@ package containers

 //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=Cluster -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.ContainerService/managedClusters/cluster1
 //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=NodePool -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.ContainerService/managedClusters/cluster1/agentPools/pool1
+//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=ContainerRegistryCacheRule -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/group1/providers/Microsoft.ContainerRegistry/registries/registry1/cacheRules/rule1


If you don't need the validate package for ID validation, you can remove this resource ID entry as well!

aristosvo · 2023-12-29T20:10:32Z

internal/services/containers/client/client.go

@@ -37,6 +40,12 @@ type Client struct {
 }

 func NewContainersClient(o *common.ClientOptions) (*Client, error) {
+	cacheRulesClient, err := cacherules.NewCacheRulesClientWithBaseURI(o.Environment.ResourceManager)


I don't think this declaration of the cacheRulesClient is necessary for the new SDK clients anymore

aristosvo · 2023-12-29T20:12:05Z

internal/services/containers/container_registry_cache_rule_resource_test.go

+
+type ContainerRegistryCacheRuleResource struct{}
+
+func TestAccContainerRegistryCacheRuleName_validation(t *testing.T) {


This validate function is already tested in its own package (and you won't need it after all)

mhaligowski · 2024-01-05T07:37:02Z

Hi @mhaligowski! Thanks for all this! I gave you a few pointers to start with, as I don't expect a review from the HC maintainers these days, they will have probably a few more remarks.

There are two tests failing as well:

------- Stdout: -------
=== RUN   TestAccContainerRegistryCacheRule_basic
=== PAUSE TestAccContainerRegistryCacheRule_basic
=== CONT  TestAccContainerRegistryCacheRule_basic
    testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
        Error: Unsupported argument
          on terraform_plugin_test.tf line 35, in resource "azurerm_container_registry_cache_rule" "test":
          35:   resource_group_name = azurerm_resource_group.test.name
        An argument named "resource_group_name" is not expected here.
--- FAIL: TestAccContainerRegistryCacheRule_basic (2.78s)
FAIL

------- Stdout: -------
=== RUN   TestAccContainerRegistryCacheRule_requiresImport
=== PAUSE TestAccContainerRegistryCacheRule_requiresImport
=== CONT  TestAccContainerRegistryCacheRule_requiresImport
    testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
        Error: Unsupported argument
          on terraform_plugin_test.tf line 35, in resource "azurerm_container_registry_cache_rule" "test":
          35:   resource_group_name = azurerm_resource_group.test.name
        An argument named "resource_group_name" is not expected here.
--- FAIL: TestAccContainerRegistryCacheRule_requiresImport (3.10s)
FAIL

Hope you enjoy the last bits of 2023!

I did, thanks :) I'll try pushing it through in the first week of 2024 :)

katbyte · 2024-03-21T23:17:16Z

@mhaligowski - is this PR now ready to be reviewed? it has been in draft for a while

stephybun · 2024-05-15T14:18:59Z

Going to close this for the moment since we haven't heard back. Let us know when you're ready to come back to this @mhaligowski and we can re-open this!

github-actions · 2024-06-16T02:10:25Z

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 bot added dependencies size/XL labels Nov 28, 2023

github-actions bot added the waiting-response label Dec 7, 2023

mhaligowski force-pushed the feat-add-azure-containers-cache branch from 63af448 to 2351da9 Compare December 11, 2023 03:11

github-actions bot removed the waiting-response label Dec 11, 2023

aristosvo suggested changes Dec 29, 2023

View reviewed changes

github-actions bot added the waiting-response label Dec 29, 2023

github-actions bot added waiting-response and removed waiting-response labels Jan 5, 2024

mhaligowski added 9 commits January 15, 2024 21:28

Add an SDK client for Cache Rules

fe88662

Synch vendor directory

326dc00

Generate parser and validator

7110c21

Verify the name of the cache rule

5001174

Fix the vendor versions

563cf3f

Fix the vendor versions

74193ab

Create and Read

05bd024

add registration and dummy test

eb8af7c

Adjust for the correct layout

09a00ff

mhaligowski force-pushed the feat-add-azure-containers-cache branch from 6d164a0 to 09a00ff Compare January 16, 2024 05:29

github-actions bot removed the waiting-response label Jan 16, 2024

stephybun closed this May 15, 2024

This was referenced May 20, 2024

Feat add azure containers cache #26033

Closed

New resource: azurerm_container_registry_cache_rule #26034

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat add azure containers cache #24040

Feat add azure containers cache #24040

mhaligowski commented Nov 28, 2023

aristosvo left a comment

aristosvo Dec 29, 2023

aristosvo Dec 29, 2023

aristosvo Dec 29, 2023

aristosvo Dec 29, 2023

aristosvo Dec 29, 2023

mhaligowski commented Jan 5, 2024

katbyte commented Mar 21, 2024

stephybun commented May 15, 2024

github-actions bot commented Jun 16, 2024

	return validate.ContainerRegistryCacheRuleID
	return cacherules.ValidateCacheRuleID


		type ContainerRegistryCacheRuleResource struct{}

		func TestAccContainerRegistryCacheRuleName_validation(t *testing.T) {

Feat add azure containers cache #24040

Feat add azure containers cache #24040

Conversation

mhaligowski commented Nov 28, 2023

aristosvo left a comment

Choose a reason for hiding this comment

aristosvo Dec 29, 2023

Choose a reason for hiding this comment

aristosvo Dec 29, 2023

Choose a reason for hiding this comment

aristosvo Dec 29, 2023

Choose a reason for hiding this comment

aristosvo Dec 29, 2023

Choose a reason for hiding this comment

aristosvo Dec 29, 2023

Choose a reason for hiding this comment

mhaligowski commented Jan 5, 2024

katbyte commented Mar 21, 2024

stephybun commented May 15, 2024

github-actions bot commented Jun 16, 2024