New resource: `azurerm_container_registry_token_password` #15939

magodo · 2022-03-22T11:37:01Z

Supersedes #13782.

This PR creates a new resource azurerm_container_registry_token_password, which adds supports of password to ACR token. The password related API is a bit special:

The API allows one or two password, the name of the password can be either password1 or password2
The API can only be used to regenerate a password, but can't be used to cleanup a generated password. Instead, you have to use the PATCH of the ACR token.
The value of the generated password is only available in the response of this API. Especially, when you generate for the 2nd password, the response will contain the information for the 1st and the 2nd password, while only the 2nd password has the value

Due to above fact, I'm implementing the password by referencing the aws_iam_user_login_profle resource, where it sets the password in the create or update method, but not in the read method. This result into the import doesn't work for the password block.

Test

terraform-provider-azurerm on  container_registry_password via 🐹 v1.18
💤  TF_ACC=1 go test -v -timeout=20h ./internal/services/containers  -run='TestAccContainerRegistryTokenPassword_update'
=== RUN   TestAccContainerRegistryTokenPassword_update
=== PAUSE TestAccContainerRegistryTokenPassword_update
=== CONT  TestAccContainerRegistryTokenPassword_update
--- PASS: TestAccContainerRegistryTokenPassword_update (393.65s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/containers    393.665s

Issues

Fixes #12810

This commit create a separate resource for the token password.

ksaffarian · 2022-04-05T05:38:49Z

Hi
Is there any plan to release this new resource?
In the original GitHub issue (#13782), it is said that the resource in available in azurerm v3.0, but I could not see it even in 3.0.2.
Can you please advise?
Thanks

katbyte

Test failure @magodo

------- Stdout: -------
=== RUN   TestAccContainerRegistryTokenPassword_requiresImport
=== PAUSE TestAccContainerRegistryTokenPassword_requiresImport
=== CONT  TestAccContainerRegistryTokenPassword_requiresImport
    testcase.go:110: Step 2/2, expected an error with pattern, no match on: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_container_registry_token_password" "import":
          43: resource "azurerm_container_registry_token_password" "import" {
        
        The argument "container_registry_token_id" is required, but no definition was
        found.
        
        Error: Insufficient password blocks
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_container_registry_token_password" "import":
          43: resource "azurerm_container_registry_token_password" "import" {
        
        At least 1 "password" blocks are required.
    testing_new.go:70: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_container_registry_token_password" "import":
          43: resource "azurerm_container_registry_token_password" "import" {
        
        The argument "container_registry_token_id" is required, but no definition was
        found.
        
        Error: Insufficient password blocks
        
          on terraform_plugin_test.tf line 43, in resource "azurerm_container_registry_token_password" "import":
          43: resource "azurerm_container_registry_token_password" "import" {
        
        At least 1 "password" blocks are required.
--- FAIL: TestAccContainerRegistryTokenPassword_requiresImport (108.18s)
FAIL

magodo · 2022-04-06T02:19:49Z

@katbyte Sorry, I missed the requiresImport acctest config 😣. Updated now, please take another look..

ksaffarian · 2022-04-13T23:21:38Z

Thanks for the great work.
Is this now ready @katbyte , @magodo?

davidkarlsen · 2022-05-19T10:59:09Z

Anything missing here? Can this be merged? ACR is largely useless as it is now because:

a) service-accounts is to coarse-grained access
b) you can't easily set a pw using IaC, you need to shell out and do lots of hoop-jumping just to do a trivial thing as setting a pw.

As we now have a lot of AZ infra, this is the missing piece to get off ECR, but is not feasible for now...

pedrolsazevedo · 2022-06-20T14:37:48Z

This is extremely useful, are there any news on releasing this on the azurerm provider?

…password_separate

avinashpancham · 2022-07-11T18:11:26Z

@katbyte could this get another review? It seems ready and I think multiple people would welcome the addition of this new resource!

admincasper · 2022-08-15T13:58:19Z

This would be a great addition. We're automating our onboarding flow and without setting passwords automatically it will lengthen our onboarding process considerably.

katbyte · 2022-09-04T01:10:08Z

internal/services/containers/container_registry_token_password_resource.go

+		"password": {
+			Type:     pluginsdk.TypeList,
+			Required: true,
+			MaxItems: 2,
+			Elem: &pluginsdk.Resource{
+				Schema: map[string]*pluginsdk.Schema{
+					"name": {
+						Type:     pluginsdk.TypeString,
+						Required: true,
+						ValidateFunc: validation.StringInSlice([]string{
+							// TODO: Use below SDK enum once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/18339
+							// string(containerregistry.PasswordNamePassword),
+							"password1",
+							string(containerregistry.PasswordNamePassword2),
+						}, false),
+					},


Instead of a password list, that each take a name, can we instead make this two blocks: password1, password2 where the block name IS the name we pass in the the API? that way resource.password1.value is the value for the password with the name "password1"

does that makes sense? i think it would make for a better UX, WDYT?

@katbyte
In this case the, the HCL would be:

password1 {}

or

password1 { expiry = "..." }

The first form looks weired, tbh. On the other hand, to reference the value of the value of password1, users would be using password1.0.value.

I think the current config is better in either configuring or referencing facts.

Naming the blocks password1 and password2 is what we came to internally to avoid magic constants. If you want to control if to send or not send the password required you could toss an enabled in there or enforce expiry as required? but the point is to highlight there are 2 passwords only, and these are their names.

@katbyte That makes sense, I've made the change accordingly, please take another look:

💢 TF_ACC=1 go test -v -timeout=20h ./internal/services/containers -run='TestAccContainerRegistryTokenPassword_' === RUN TestAccContainerRegistryTokenPassword_basic === PAUSE TestAccContainerRegistryTokenPassword_basic === RUN TestAccContainerRegistryTokenPassword_complete === PAUSE TestAccContainerRegistryTokenPassword_complete === RUN TestAccContainerRegistryTokenPassword_update === PAUSE TestAccContainerRegistryTokenPassword_update === RUN TestAccContainerRegistryTokenPassword_requiresImport === PAUSE TestAccContainerRegistryTokenPassword_requiresImport === CONT TestAccContainerRegistryTokenPassword_basic === CONT TestAccContainerRegistryTokenPassword_update === CONT TestAccContainerRegistryTokenPassword_requiresImport === CONT TestAccContainerRegistryTokenPassword_complete --- PASS: TestAccContainerRegistryTokenPassword_requiresImport (152.96s) --- PASS: TestAccContainerRegistryTokenPassword_basic (194.71s) --- PASS: TestAccContainerRegistryTokenPassword_complete (204.42s) --- PASS: TestAccContainerRegistryTokenPassword_update (213.02s) PASS

…password_separate

katbyte

Thanks @magodo - LGTM 🔏

internal/services/containers/container_registry_token_password_resource.go

…_resource.go

github-actions · 2022-09-09T03:17:34Z

This functionality has been released in v3.22.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-10-10T02:41:59Z

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.

New resource: azurerm_container_registry_token_password

dfe0c39

This commit create a separate resource for the token password.

github-actions bot added documentation size/XL labels Mar 22, 2022

magodo mentioned this pull request Mar 22, 2022

azurerm_container_registry_token - add supports for password #13782

Closed

fix linting issue

ea160e8

katbyte requested changes Apr 6, 2022

View reviewed changes

requiresImport acctest

cfd2651

Merge remote-tracking branch 'upstream/main' into container_registry_…

5d47ead

…password_separate

katbyte requested changes Sep 4, 2022

View reviewed changes

katbyte added the waiting-response label Sep 4, 2022

Merge remote-tracking branch 'upstream/main' into container_registry_…

77e3a36

…password_separate

github-actions bot added waiting-response and removed waiting-response labels Sep 5, 2022

refactor the schema

2d4f328

github-actions bot removed the waiting-response label Sep 7, 2022

katbyte approved these changes Sep 9, 2022

View reviewed changes

internal/services/containers/container_registry_token_password_resource.go Outdated Show resolved Hide resolved

Update internal/services/containers/container_registry_token_password…

e25f5cc

…_resource.go

katbyte merged commit dedeb25 into hashicorp:main Sep 9, 2022

katbyte added a commit that referenced this pull request Sep 9, 2022

CHANGELOG #15939

5e199cd

github-actions bot added this to the v3.22.0 milestone Sep 9, 2022

github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource: `azurerm_container_registry_token_password` #15939

New resource: `azurerm_container_registry_token_password` #15939

magodo commented Mar 22, 2022

ksaffarian commented Apr 5, 2022

katbyte left a comment

magodo commented Apr 6, 2022

ksaffarian commented Apr 13, 2022

davidkarlsen commented May 19, 2022 •

edited

Loading

pedrolsazevedo commented Jun 20, 2022

avinashpancham commented Jul 11, 2022

admincasper commented Aug 15, 2022

katbyte Sep 4, 2022

magodo Sep 5, 2022 •

edited

Loading

katbyte Sep 6, 2022

magodo Sep 7, 2022

katbyte left a comment

github-actions bot commented Sep 9, 2022

github-actions bot commented Oct 10, 2022

New resource: azurerm_container_registry_token_password #15939

New resource: azurerm_container_registry_token_password #15939

Conversation

magodo commented Mar 22, 2022

Test

Issues

ksaffarian commented Apr 5, 2022

katbyte left a comment

Choose a reason for hiding this comment

magodo commented Apr 6, 2022

ksaffarian commented Apr 13, 2022

davidkarlsen commented May 19, 2022 • edited Loading

pedrolsazevedo commented Jun 20, 2022

avinashpancham commented Jul 11, 2022

admincasper commented Aug 15, 2022

katbyte Sep 4, 2022

Choose a reason for hiding this comment

magodo Sep 5, 2022 • edited Loading

Choose a reason for hiding this comment

katbyte Sep 6, 2022

Choose a reason for hiding this comment

magodo Sep 7, 2022

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 9, 2022

github-actions bot commented Oct 10, 2022

New resource: `azurerm_container_registry_token_password` #15939

New resource: `azurerm_container_registry_token_password` #15939

davidkarlsen commented May 19, 2022 •

edited

Loading

magodo Sep 5, 2022 •

edited

Loading