New DataSource: `azurerm_key_vault_certificates` #19498

harshavmb · 2022-11-30T10:17:39Z

Support new data source azurerm_key_vault_certificates

Fixes 19451

katbyte

Thanks @harshavmb, looks good overall but we have a test failure:

------- Stdout: -------
=== RUN   TestAccDataSourceKeyVaultCertificates_basic
=== PAUSE TestAccDataSourceKeyVaultCertificates_basic
=== CONT  TestAccDataSourceKeyVaultCertificates_basic
    testcase.go:110: Step 1/1 error: Error running apply: exit status 1
        
        Error: making Read request on Azure KeyVault "Vault: (Name \"acctestkeyvaultoo3qd\" / Resource Group \"acctestRG-221205172152005995\")": keyvault.BaseClient#GetCertificates: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=*******;oid=3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21;numgroups=9;iss=https://sts.windows.net/*******/' does not have certificates list permission on key vault 'acctestkeyvaultoo3qd;location=westus2'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
        
          with data.azurerm_key_vault_certificates.test,
          on terraform_plugin_test.tf line 157, in data "azurerm_key_vault_certificates" "test":
         157: data "azurerm_key_vault_certificates" "test" {
        
--- FAIL: TestAccDataSourceKeyVaultCertificates_basic (474.80s)
FAIL

internal/services/keyvault/key_vault_certificates_data_source.go

harshavmb · 2022-12-06T20:55:05Z

Thanks @harshavmb, looks good overall but we have a test failure:

------- Stdout: -------
=== RUN   TestAccDataSourceKeyVaultCertificates_basic
=== PAUSE TestAccDataSourceKeyVaultCertificates_basic
=== CONT  TestAccDataSourceKeyVaultCertificates_basic
    testcase.go:110: Step 1/1 error: Error running apply: exit status 1
        
        Error: making Read request on Azure KeyVault "Vault: (Name \"acctestkeyvaultoo3qd\" / Resource Group \"acctestRG-221205172152005995\")": keyvault.BaseClient#GetCertificates: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=*******;oid=3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21;numgroups=9;iss=https://sts.windows.net/*******/' does not have certificates list permission on key vault 'acctestkeyvaultoo3qd;location=westus2'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
        
          with data.azurerm_key_vault_certificates.test,
          on terraform_plugin_test.tf line 157, in data "azurerm_key_vault_certificates" "test":
         157: data "azurerm_key_vault_certificates" "test" {
        
--- FAIL: TestAccDataSourceKeyVaultCertificates_basic (474.80s)
FAIL

Thanks @harshavmb, looks good overall but we have a test failure:

------- Stdout: -------
=== RUN   TestAccDataSourceKeyVaultCertificates_basic
=== PAUSE TestAccDataSourceKeyVaultCertificates_basic
=== CONT  TestAccDataSourceKeyVaultCertificates_basic
    testcase.go:110: Step 1/1 error: Error running apply: exit status 1
        
        Error: making Read request on Azure KeyVault "Vault: (Name \"acctestkeyvaultoo3qd\" / Resource Group \"acctestRG-221205172152005995\")": keyvault.BaseClient#GetCertificates: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=*******;oid=3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21;numgroups=9;iss=https://sts.windows.net/*******/' does not have certificates list permission on key vault 'acctestkeyvaultoo3qd;location=westus2'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
        
          with data.azurerm_key_vault_certificates.test,
          on terraform_plugin_test.tf line 157, in data "azurerm_key_vault_certificates" "test":
         157: data "azurerm_key_vault_certificates" "test" {
        
--- FAIL: TestAccDataSourceKeyVaultCertificates_basic (474.80s)
FAIL

Hello @katbyte ,

I've added missing permissions for tests to succeed.

tombuildsstuff

hey @harshavmb

Thanks for this PR - I've taken a look through and left some comments inline, but if we can fix those up then we should be able to take another look/get this merged 👍

Thanks!

internal/services/keyvault/key_vault_certificates_data_source.go

tombuildsstuff · 2022-12-07T11:39:01Z

internal/services/keyvault/key_vault_certificates_data_source_test.go

+				check.That(data.ResourceName).Key("names.#").HasValue("9"),
+			),
+		},
+	})
+}
+
+func (KeyVaultCertificatesDataSource) basic(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%s
+
+resource "azurerm_key_vault_certificate" "test2" {
+  count = 10


we're creating 10 of these, so this would indicate that we're not paging or something?

Paging happens after 25 resources as per doc here.
So, left untouched.

so could we create 26 for test purposes to check we cross the boundary?

internal/services/keyvault/key_vault_certificates_data_source_test.go

website/docs/d/key_vault_certificates.html.markdown

harshavmb · 2022-12-07T15:34:15Z

hey @harshavmb

Thanks for this PR - I've taken a look through and left some comments inline, but if we can fix those up then we should be able to take another look/get this merged 👍

Thanks!

Hi @tombuildsstuff ,

I've made changes as per your review. Let me know if it looks good.

tombuildsstuff

One comment but this otherwise LGTM 👍

tombuildsstuff · 2022-12-12T15:28:04Z

internal/services/keyvault/key_vault_certificates_data_source_test.go

+				check.That(data.ResourceName).Key("names.#").HasValue("9"),
+			),
+		},
+	})
+}
+
+func (KeyVaultCertificatesDataSource) basic(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%s
+
+resource "azurerm_key_vault_certificate" "test2" {
+  count = 10


so could we create 26 for test purposes to check we cross the boundary?

harshavmb · 2022-12-16T10:23:18Z

One comment but this otherwise LGTM 👍

Sure, I've increased tests to 30 now just to be in sync with secrets data source.

From my state file ::

"names": [ "certificate-0", "certificate-1", "certificate-10", "certificate-11", "certificate-12", "certificate-13", "certificate-14", "certificate-15", "certificate-16", "certificate-17", "certificate-18", "certificate-19", "certificate-2", "certificate-20", "certificate-21", "certificate-22", "certificate-23", "certificate-24", "certificate-25", "certificate-26", "certificate-27", "certificate-28", "certificate-29", "certificate-3", "certificate-4", "certificate-5", "certificate-6", "certificate-7", "certificate-8", "certificate-9" ],

katbyte

Thanks @harshavmb - looks good aside from a test failure:

------- Stdout: -------
=== RUN   TestAccDataSourceKeyVaultCertificates_basic
=== PAUSE TestAccDataSourceKeyVaultCertificates_basic
=== CONT  TestAccDataSourceKeyVaultCertificates_basic
    testcase.go:110: Step 1/1 error: Error running apply: exit status 1
        
        Error: A resource with the ID "/subscriptions/*******/resourceGroups/acctestRG-221219175714268125/providers/Microsoft.KeyVault/vaults/acctestkeyvault8ck08/objectId/3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_key_vault_access_policy" for more information.
        
          with azurerm_key_vault_access_policy.certificates,
          on terraform_plugin_test.tf line 157, in resource "azurerm_key_vault_access_policy" "certificates":
         157: resource "azurerm_key_vault_access_policy" "certificates" {
        
        
        Error: retrieving Vault: (Name "acctestkeyvault8ck08" / Resource Group "acctestRG-221219175714268125"): keyvault.BaseClient#GetCertificates: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="The user, group or application 'appid=*******;oid=3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21;numgroups=9;iss=https://sts.windows.net/*******/' does not have certificates list permission on key vault 'acctestkeyvault8ck08;location=westeurope'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"ForbiddenByPolicy"}
        
          with data.azurerm_key_vault_certificates.test,
          on terraform_plugin_test.tf line 172, in data "azurerm_key_vault_certificates" "test":
         172: data "azurerm_key_vault_certificates" "test" {
        
--- FAIL: TestAccDataSourceKeyVaultCertificates_basic (769.17s)
FAIL

once thats fixed up this should be good to merge!

StefanSchoof · 2023-01-20T08:09:29Z

@harshavmb I really like, to have this data source. Do you know, if you have time to look at this in the near future?

harshavmb · 2023-01-23T18:46:00Z

@harshavmb I really like, to have this data source. Do you know, if you have time to look at this in the near future?

I tried my best to push for this PR. If tests are failing due to 403 error, I'm not going to do anything. Also, I can't test on my Azure environment as resource group creation is restricted.

mbfrahry

Hey @harshavmb, the test isn't passing because of a 403 but because we're managing certificate access policies in the template but you've declared a resource to manage certificate access policies so Terraform is stuck.

Error: A resource with the ID "/subscriptions/*******/resourceGroups/acctestRG-221219175714268125/providers/Microsoft.KeyVault/vaults/acctestkeyvault8ck08/objectId/3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_key_vault_access_policy" for more information.

I've detailed how to fix this in a comment below

mbfrahry · 2023-01-24T19:08:17Z

internal/services/keyvault/key_vault_certificates_data_source_test.go

+  key_vault_id = azurerm_key_vault.test.id
+}
+
+resource "azurerm_key_vault_access_policy" "certificates" {


There is an issue with this resource because we're managing certificate access policies inside of the template. If you remove resource "azurerm_key_vault_access_policy" "certificates" { and add "List" to the linked block, this test should pass

…ough resource template as per review

harshavmb · 2023-01-25T09:03:38Z

Hi @mbfrahry ,

I've pushed changes as you asked. Please let me know if tests run fine now.

katbyte

LGTM aside from 1 mincor change

katbyte · 2023-01-25T17:55:18Z

internal/services/keyvault/key_vault_certificates_data_source_test.go

+		{
+			Config: r.basic(data),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).Key("names.#").HasValue("9"),


Suggested change

check.That(data.ResourceName).Key("names.#").HasValue("9"),

check.That(data.ResourceName).Key("names.#").HasValue("31"),

i can't edit this PR, @harshavmb once this change is made this is good to go

Hi @katbyte ,

I've pushed the change from my end.

mbfrahry

LGTM! Thanks for getting that test fixed

github-actions · 2023-01-27T04:28:22Z

This functionality has been released in v3.41.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 · 2023-02-27T02:15:31Z

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.

Support new data source azurerm_key_vault_certificates

2534005

github-actions bot added documentation service/key-vault Key Vault size/L labels Nov 30, 2022

katbyte requested changes Dec 5, 2022

View reviewed changes

internal/services/keyvault/key_vault_certificates_data_source.go Show resolved Hide resolved

github-actions bot added the waiting-response label Dec 5, 2022

Adding certificate_permissions access_policy for certificates

bb18efe

github-actions bot removed the waiting-response label Dec 6, 2022

Fixing formatting

5c893a7

harshavmb requested a review from katbyte December 6, 2022 20:56

Merge branch 'hashicorp:main' into azurerm_key_vault_certificates

e890efa

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

tombuildsstuff suggested changes Dec 7, 2022

View reviewed changes

harshavmb added 2 commits December 7, 2022 16:23

Changse as per review

6c113f2

accepting changes from remote

0fc2d71

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

harshavmb requested review from tombuildsstuff and katbyte and removed request for katbyte and tombuildsstuff December 7, 2022 15:50

tombuildsstuff approved these changes Dec 12, 2022

View reviewed changes

increasing certificate count to 30 to test paging as per the review

0497d2f

harshavmb requested review from katbyte and tombuildsstuff and removed request for katbyte December 16, 2022 10:23

katbyte reviewed Dec 19, 2022

View reviewed changes

katbyte added the waiting-response label Dec 19, 2022

github-actions bot removed the waiting-response label Jan 23, 2023

mbfrahry requested changes Jan 24, 2023

View reviewed changes

github-actions bot added the waiting-response label Jan 24, 2023

Removing azurerm_key_vault_access_policy resource and managing it thr…

68ecdb2

…ough resource template as per review

github-actions bot removed the waiting-response label Jan 25, 2023

harshavmb requested review from mbfrahry and removed request for tombuildsstuff January 25, 2023 09:03

harshavmb requested review from katbyte and mbfrahry and removed request for mbfrahry and katbyte January 25, 2023 15:02

katbyte approved these changes Jan 25, 2023

View reviewed changes

github-actions bot added the waiting-response label Jan 25, 2023

Amending test results as per review

e428fd5

github-actions bot removed the waiting-response label Jan 25, 2023

mbfrahry approved these changes Jan 26, 2023

View reviewed changes

mbfrahry changed the title ~~Support new data source azurerm_key_vault_certificates~~ New DataSource: azurerm_key_vault_certificates Jan 26, 2023

mbfrahry added this to the v3.41.0 milestone Jan 26, 2023

mbfrahry merged commit 34b2614 into hashicorp:main Jan 26, 2023

mbfrahry added a commit that referenced this pull request Jan 26, 2023

Update changelog for #19498

7808d49

github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New DataSource: `azurerm_key_vault_certificates` #19498

New DataSource: `azurerm_key_vault_certificates` #19498

harshavmb commented Nov 30, 2022

katbyte left a comment

harshavmb commented Dec 6, 2022

tombuildsstuff left a comment

tombuildsstuff Dec 7, 2022

harshavmb Dec 7, 2022

tombuildsstuff Dec 12, 2022

harshavmb commented Dec 7, 2022

tombuildsstuff left a comment

tombuildsstuff Dec 12, 2022

harshavmb commented Dec 16, 2022

katbyte left a comment

StefanSchoof commented Jan 20, 2023

harshavmb commented Jan 23, 2023

mbfrahry left a comment

mbfrahry Jan 24, 2023

harshavmb commented Jan 25, 2023

katbyte left a comment

katbyte Jan 25, 2023

katbyte Jan 25, 2023

harshavmb Jan 25, 2023

mbfrahry left a comment

github-actions bot commented Jan 27, 2023

github-actions bot commented Feb 27, 2023

	check.That(data.ResourceName).Key("names.#").HasValue("9"),
	check.That(data.ResourceName).Key("names.#").HasValue("31"),

New DataSource: azurerm_key_vault_certificates #19498

New DataSource: azurerm_key_vault_certificates #19498

Conversation

harshavmb commented Nov 30, 2022

katbyte left a comment

Choose a reason for hiding this comment

harshavmb commented Dec 6, 2022

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavmb commented Dec 7, 2022

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavmb commented Dec 16, 2022

katbyte left a comment

Choose a reason for hiding this comment

StefanSchoof commented Jan 20, 2023

harshavmb commented Jan 23, 2023

mbfrahry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavmb commented Jan 25, 2023

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbfrahry left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 27, 2023

github-actions bot commented Feb 27, 2023

New DataSource: `azurerm_key_vault_certificates` #19498

New DataSource: `azurerm_key_vault_certificates` #19498