feat(azure): add explicit_max_ttl for azure roles by gsantos-hc · Pull Request #2438 · hashicorp/terraform-provider-vault

gsantos-hc · 2025-03-25T13:00:04Z

Description

Update the azure_secret_backend_role resource to add support for explicit_max_ttl for Azure Secrets roles. This field was introduced as part of Azure Secrets plugin v0.20.0, incorporated into Vault 1.18.0 and later.

Closes #2434.

Checklist

Added CHANGELOG entry (only for user-facing changes)
Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

# Running against Vault 1.16.17+ent
$ make testacc TESTARGS='-run=TestAzureSecretBackendRole_AzureRoles' TEST_PATH='./vault/...'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -run=TestAzureSecretBackendRole_AzureRoles -timeout 30m ./vault/...
ok      github.com/hashicorp/terraform-provider-vault/vault     3.977s

# Running against Vault 1.19.0+ent, after cleaning test cache
$ make testacc TESTARGS='-run=TestAzureSecretBackendRole_AzureRoles' TEST_PATH='./vault/...'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -run=TestAzureSecretBackendRole_AzureRoles -timeout 30m ./vault/...
ok      github.com/hashicorp/terraform-provider-vault/vault     3.636s

IMPORTANT NOTE: I applied the patch below to work around the acceptance test failing on main:

Acceptance test patch

diff --git a/vault/resource_azure_secret_backend_role_test.go b/vault/resource_azure_secret_backend_role_test.go
index 69652e60..1a938280 100644
--- a/vault/resource_azure_secret_backend_role_test.go
+++ b/vault/resource_azure_secret_backend_role_test.go
@@ -17,6 +17,8 @@ import (
 	"github.com/hashicorp/terraform-provider-vault/testutil"
 )
 
+const azReaderRoleId string = "acdd72a7-3385-48ef-bd42-f606fba81ae7"
+
 func TestAzureSecretBackendRole_AzureRoles(t *testing.T) {
 	subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
 	if subscriptionID == "" {
@@ -31,15 +33,18 @@ func TestAzureSecretBackendRole_AzureRoles(t *testing.T) {
 	path := acctest.RandomWithPrefix("tf-test-azure")
 	role := acctest.RandomWithPrefix("tf-test-azure-role")
 
+	azRoleId := fmt.Sprintf(
+		"/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s",
+		subscriptionID, azReaderRoleId)
+
 	azureRoleInitialCheckFuncs := []resource.TestCheckFunc{
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "role", role+"-azure-roles"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "description", "Test for Vault Provider"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "ttl", "300"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "max_ttl", "600"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.#", "1"),
-		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_name", "Reader"),
+		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_id", azRoleId),
 		resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.scope"),
-		resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.role_id"),
 	}
 
 	azureRoleUpdatedCheckFuncs := []resource.TestCheckFunc{
@@ -48,9 +53,8 @@ func TestAzureSecretBackendRole_AzureRoles(t *testing.T) {
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "ttl", "600"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "max_ttl", "900"),
 		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.#", "1"),
-		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_name", "Reader"),
+		resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_id", azRoleId),
 		resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.scope"),
-		resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.role_id"),
 	}
 
 	isVaultVersion116 := provider.IsAPISupported(testProvider.Meta(), provider.VaultVersion116)
@@ -101,9 +105,8 @@ func TestAzureSecretBackendRole_AzureRoles(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "description", "Test for Vault Provider"),
 					resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "permanently_delete", "false"),
 					resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.#", "1"),
-					resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_name", "Reader"),
+					resource.TestCheckResourceAttr(resourceName+".test_azure_roles", "azure_roles.0.role_id", azRoleId),
 					resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.scope"),
-					resource.TestCheckResourceAttrSet(resourceName+".test_azure_roles", "azure_roles.0.role_id"),
 				),
 			},
 		},
@@ -222,11 +225,11 @@ resource "vault_azure_secret_backend_role" "test_azure_roles" {
  tags             = ["team:engineering"]
 
  azure_roles {
-   role_name = "Reader"
-   scope =  "/subscriptions/%[1]s/resourceGroups/%[7]s"
+   role_id = "/subscriptions/%[1]s/providers/Microsoft.Authorization/roleDefinitions/%[8]s"
+   scope   = "/subscriptions/%[1]s/resourceGroups/%[7]s"
  }
 }
-`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup)
+`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup, azReaderRoleId)
 }
 
 func testAzureSecretBackendRoleInitialConfig_azureGroups(subscriptionID string, tenantID string, clientID string, clientSecret string, path string, role string, resourceGroup string) string {
@@ -274,11 +277,11 @@ resource "vault_azure_secret_backend_role" "test_azure_roles" {
   tags       	   = ["environment:development","project:vault_testing"]
 
   azure_roles {
-    role_name = "Reader"
-    scope =  "/subscriptions/%[1]s/resourceGroups/%[7]s"
+    role_id = "/subscriptions/%[1]s/providers/Microsoft.Authorization/roleDefinitions/%[8]s"
+    scope   = "/subscriptions/%[1]s/resourceGroups/%[7]s"
   }
 }
-`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup)
+`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup, azReaderRoleId)
 }
 
 func testAzureSecretBackendRole_updatedAzureGroups(subscriptionID string, tenantID string, clientID string, clientSecret string, path string, role string, resourceGroup string) string {
@@ -322,13 +325,15 @@ resource "vault_azure_secret_backend_role" "test_azure_roles" {
   max_ttl          = 600
   explicit_max_ttl = 2592000
   description      = "Test for Vault Provider"
+  sign_in_audience = "AzureADMultipleOrgs"
+  tags       	   = ["environment:development","project:vault_testing"]
 
   azure_roles {
-    role_name = "Reader"
-    scope =  "/subscriptions/%[1]s/resourceGroups/%[7]s"
+    role_id = "/subscriptions/%[1]s/providers/Microsoft.Authorization/roleDefinitions/%[8]s"
+    scope   = "/subscriptions/%[1]s/resourceGroups/%[7]s"
   }
 }
-`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup)
+`, subscriptionID, tenantID, clientID, clientSecret, path, role, resourceGroup, azReaderRoleId)
 }
 
 func testAzureSecretBackendRolePermanentlyDelete_azureGroups(subscriptionID string, tenantID string, clientID string, clientSecret string, path string, role string, resourceGroup string) string {

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Change introduced in hashicorp#2438

gsantos-hc · 2025-03-25T13:54:43Z

Unrelated to this PR, but the Vault API is returning the role's GUID (acdd72a7-3385-48ef-bd42-f606fba81ae7) rather than its name (Reader), which causes existing acceptance tests to fail.

I think this may be the result of an Azure API change, as I'm seeing the same behavior for both Vault 1.16 and 1.19.

vinay-gopalan · 2025-03-25T17:27:45Z

Unrelated to this PR, but the Vault API is returning the role's GUID (acdd72a7-3385-48ef-bd42-f606fba81ae7) rather than its name (Reader), which causes existing acceptance tests to fail.

I think this may be the result of an Azure API change, as I'm seeing the same behavior for both Vault 1.16 and 1.19.

Hi @gsantos-hc, we recently merged a fix for this issue to the Azure Secrets engine here: hashicorp/vault-plugin-secrets-azure#233

This reverts the GUID / Role name to the original behavior from 1.15.5 and should resolve the issue you're seeing above

Add support for configuring `explicit_max_ttl` for Azure Secrets roles. This field was released as part of Azure Secrets plugin v0.20.0, incorporated into Vault 1.18.0 and later. Note that the field is configured as an integer to align with how other resources throughout the provider handle TTL fields. Configuring it to a seconds-based integer avoids Terraform flagging drifts between the seconds returned from Vault's API and the Terraform string-based configuration.

Change introduced in hashicorp#2438

gsantos-hc · 2025-03-25T18:16:50Z

Force-pushed to:

Fix original commit's acceptance tests, which were missing the Terraform configuration bits
Fix merge conflict for changelog
Add website documentation (force-pushing wasn't needed for this one)

gsantos-hc · 2025-03-25T18:19:48Z

@Zlaticanin When you get a chance, would you be able to review? I added some callouts throughout the PR already.

Per feedback in hashicorp#2438, revert the explicit_max_ttl field in the `azure_secret_backend_role` resource to a string type for internal consistency within the resource. Fix deletion acceptance test code.

vinay-gopalan

Looking great! Had a couple small comments, should be good to get in soon after; thanks for working on this!

Invert when tests for `explicit_max_ttl`, `tags`, and `sign_in_audience` run. Previously, these tests were run only when the API did _not_ support the attributes, which was a likely oversight. Addresses feedback in hashicorp#2438.

Addresses feedback in hashicorp#2438.

vinay-gopalan

Thanks for the PR and your patience with the feedback! LGTM

gsantos-hc requested a review from a team as a code owner March 25, 2025 13:00

gsantos-hc requested a review from digivava March 25, 2025 13:00

gsantos-hc commented Mar 25, 2025

View reviewed changes

Comment thread vault/resource_azure_secret_backend_role.go Outdated

Comment thread vault/resource_azure_secret_backend_role.go

gsantos-hc added a commit to gsantos-hc/terraform-provider-vault that referenced this pull request Mar 25, 2025

docs(changelog): explicit max ttl for azure secrets

5a03519

Change introduced in hashicorp#2438

gsantos-hc marked this pull request as draft March 25, 2025 13:34

gsantos-hc added 3 commits March 25, 2025 14:14

docs(changelog): explicit max ttl for azure secrets

6aefbc5

Change introduced in hashicorp#2438

docs(website): add explicit_max_ttl for azure secret roles

3ae75de

gsantos-hc force-pushed the feat/azure-secret-explicit-ttl branch from 5a03519 to 3ae75de Compare March 25, 2025 18:15

gsantos-hc commented Mar 25, 2025

View reviewed changes

Comment thread vault/resource_azure_secret_backend_role_test.go Outdated

gsantos-hc marked this pull request as ready for review March 25, 2025 18:19

gsantos-hc requested review from Zlaticanin and removed request for digivava March 26, 2025 00:31

fix(azure): revert explicit_max_ttl to string

7eaaf74

Per feedback in hashicorp#2438, revert the explicit_max_ttl field in the `azure_secret_backend_role` resource to a string type for internal consistency within the resource. Fix deletion acceptance test code.

Zlaticanin requested a review from vinay-gopalan April 1, 2025 16:18

vinay-gopalan reviewed Apr 1, 2025

View reviewed changes

Comment thread website/docs/r/azure_secret_backend_role.html.md Outdated

Comment thread vault/resource_azure_secret_backend_role_test.go Outdated

gsantos-hc added 2 commits April 2, 2025 09:26

test(azure): run tests when supported by api

43484da

Invert when tests for `explicit_max_ttl`, `tags`, and `sign_in_audience` run. Previously, these tests were run only when the API did _not_ support the attributes, which was a likely oversight. Addresses feedback in hashicorp#2438.

docs(azure): add reqd vault version to explicit_max_ttl

a0b7db8

Addresses feedback in hashicorp#2438.

vinay-gopalan approved these changes Apr 2, 2025

View reviewed changes

vinay-gopalan added this to the 4.8.0 milestone Apr 2, 2025

vinay-gopalan merged commit 6017d29 into hashicorp:main Apr 2, 2025

gsantos-hc deleted the feat/azure-secret-explicit-ttl branch April 2, 2025 17:04

Navigation Menu

Uh oh!

feat(azure): add explicit_max_ttl for azure roles - #2438

feat(azure): add explicit_max_ttl for azure roles#2438
vinay-gopalan merged 6 commits into
hashicorp:mainfrom
gsantos-hc:feat/azure-secret-explicit-ttl

gsantos-hc commented Mar 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

vinay-gopalan commented Mar 25, 2025

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

vinay-gopalan left a comment

Uh oh!

Uh oh!

Uh oh!

vinay-gopalan left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gsantos-hc commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Output from acceptance testing:

Community Note

Uh oh!

Uh oh!

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

vinay-gopalan commented Mar 25, 2025

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

Uh oh!

gsantos-hc commented Mar 25, 2025

Uh oh!

vinay-gopalan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vinay-gopalan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gsantos-hc commented Mar 25, 2025 •

edited

Loading