Skip to content

Commit b3b5261

Browse files
GH-2356 | Normalize Workflow Permission Expansion For okta_admin_role_custom (#2902)
1 parent c10e91e commit b3b5261

5 files changed

Lines changed: 547 additions & 3 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
resource "okta_admin_role_custom" "test" {
2+
label = "testAcc_replace_with_uuid"
3+
description = "workflow permission alias migration"
4+
permissions = ["okta.workflows.flows.read"]
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
resource "okta_admin_role_custom" "test" {
2+
label = "testAcc_replace_with_uuid"
3+
description = "workflow permission alias migration"
4+
permissions = ["okta.workflows.read"]
5+
}

okta/services/idaas/resource_okta_admin_role_custom.go

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"strings"
78

89
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
910
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -135,7 +136,13 @@ func resourceAdminRoleCustomRead(ctx context.Context, d *schema.ResourceData, me
135136
if err != nil {
136137
return diag.Errorf("failed to list permissions for custom admin role: %v", err)
137138
}
138-
_ = d.Set("permissions", flattenPermissions(perms.Permissions))
139+
// The Okta API can return both the legacy workflow permission labels
140+
// (okta.workflows.invoke / okta.workflows.read) and their newer aliases
141+
// (okta.workflows.flows.invoke / okta.workflows.flows.read). Keep whichever
142+
// form is already recorded in state so we don't introduce perpetual drift.
143+
statePermissions := utils.ConvertInterfaceToStringSetNullable(d.Get("permissions"))
144+
apiPermissions := reconcileWorkflowPermissions(statePermissions, perms.Permissions)
145+
_ = d.Set("permissions", flattenPermissions(apiPermissions))
139146
return nil
140147
}
141148

@@ -155,8 +162,33 @@ func resourceAdminRoleCustomUpdate(ctx context.Context, d *schema.ResourceData,
155162
oldSet := oldPermissions.(*schema.Set)
156163
newSet := newPermissions.(*schema.Set)
157164

158-
permissionsToAdd := utils.ConvertInterfaceArrToStringArr(newSet.Difference(oldSet).List())
159-
permissionsToRemove := utils.ConvertInterfaceArrToStringArr(oldSet.Difference(newSet).List())
165+
// okta.workflows.flows.* is an alias of okta.workflows.* for the same
166+
// underlying permission. Diffing on the raw labels reports an alias swap
167+
// (e.g. okta.workflows.read -> okta.workflows.flows.read) as an add + a
168+
// remove that cancel out, deleting the permission and forcing a second
169+
// apply. Diff on the de-aliased label so a swap is a no-op.
170+
canonical := func(p string) string {
171+
return strings.Replace(p, "okta.workflows.flows.", "okta.workflows.", 1)
172+
}
173+
oldByCanon := map[string]string{}
174+
for _, p := range utils.ConvertInterfaceArrToStringArr(oldSet.List()) {
175+
oldByCanon[canonical(p)] = p
176+
}
177+
178+
var permissionsToAdd, permissionsToRemove []string
179+
newCanon := map[string]bool{}
180+
for _, p := range utils.ConvertInterfaceArrToStringArr(newSet.List()) {
181+
c := canonical(p)
182+
newCanon[c] = true
183+
if _, ok := oldByCanon[c]; !ok {
184+
permissionsToAdd = append(permissionsToAdd, p)
185+
}
186+
}
187+
for c, p := range oldByCanon {
188+
if !newCanon[c] {
189+
permissionsToRemove = append(permissionsToRemove, p)
190+
}
191+
}
160192

161193
err := addCustomRolePermissions(ctx, client, d.Id(), permissionsToAdd)
162194
if err != nil {
@@ -202,6 +234,46 @@ func flattenPermissions(permissions []*sdk.Permission) interface{} {
202234
return schema.NewSet(schema.HashString, arr)
203235
}
204236

237+
// reconcileWorkflowPermissions removes a redundant workflow permission alias
238+
// from the API response when state already tracks the other form. The Okta API
239+
// exposes 2 variations of the same workflow permission
240+
//
241+
// okta.workflows.invoke <-> okta.workflows.flows.invoke
242+
// okta.workflows.read <-> okta.workflows.flows.read
243+
func reconcileWorkflowPermissions(statePermissions []string, apiPermissions []*sdk.Permission) []*sdk.Permission {
244+
inState := make(map[string]bool, len(statePermissions))
245+
for _, p := range statePermissions {
246+
inState[p] = true
247+
}
248+
249+
existingToNew := map[string]string{
250+
"okta.workflows.invoke": "okta.workflows.flows.invoke",
251+
"okta.workflows.read": "okta.workflows.flows.read",
252+
}
253+
254+
discard := make(map[string]bool)
255+
for legacy, modern := range existingToNew {
256+
switch {
257+
case inState[legacy]:
258+
discard[modern] = true // state uses the legacy label, discard the newer alias from the API response
259+
case inState[modern]:
260+
discard[legacy] = true // state uses the newer alias, discard the legacy label from the API response
261+
}
262+
}
263+
if len(discard) == 0 {
264+
return apiPermissions
265+
}
266+
267+
filtered := make([]*sdk.Permission, 0, len(apiPermissions))
268+
for _, p := range apiPermissions {
269+
if p != nil && discard[p.Label] {
270+
continue
271+
}
272+
filtered = append(filtered, p)
273+
}
274+
return filtered
275+
}
276+
205277
func addCustomRolePermissions(ctx context.Context, client *sdk.APISupplement, roleIdOrLabel string, permissions []string) error {
206278
for _, permission := range permissions {
207279
_, _, err := client.AddCustomRolePermission(ctx, roleIdOrLabel, permission)

okta/services/idaas/resource_okta_admin_role_custom_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,46 @@ func TestAccResourceOktaAdminRoleCustom_crud(t *testing.T) {
4343
})
4444
}
4545

46+
func TestAccResourceOktaAdminRoleCustom_workflowPermissionAliasMigration(t *testing.T) {
47+
mgr := newFixtureManager("resources", resources.OktaIDaaSAdminRoleCustom, t.Name())
48+
legacy := mgr.GetFixtures("workflow_permission_legacy.tf", t)
49+
alias := mgr.GetFixtures("workflow_permission_alias.tf", t)
50+
resourceName := fmt.Sprintf("%s.test", resources.OktaIDaaSAdminRoleCustom)
51+
acctest.OktaResourceTest(
52+
t, resource.TestCase{
53+
PreCheck: acctest.AccPreCheck(t),
54+
ErrorCheck: testAccErrorChecks(t),
55+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactoriesForTestAcc(t),
56+
CheckDestroy: checkResourceDestroy(resources.OktaIDaaSAdminRoleCustom, doesAdminRoleCustomExist),
57+
Steps: []resource.TestStep{
58+
{
59+
Config: legacy,
60+
Check: resource.ComposeTestCheckFunc(
61+
resource.TestCheckResourceAttr(resourceName, "permissions.#", "1"),
62+
resource.TestCheckTypeSetElemAttr(resourceName, "permissions.*", "okta.workflows.read"),
63+
),
64+
},
65+
{
66+
// Swap to the alias label. Post-apply the framework runs a
67+
// refresh + plan and fails on a non-empty plan, which is
68+
// exactly the regression: the old logic deleted the
69+
// permission here, requiring a second apply to re-add it.
70+
Config: alias,
71+
Check: resource.ComposeTestCheckFunc(
72+
resource.TestCheckResourceAttr(resourceName, "permissions.#", "1"),
73+
resource.TestCheckTypeSetElemAttr(resourceName, "permissions.*", "okta.workflows.flows.read"),
74+
),
75+
},
76+
{
77+
// Explicit no-drift assertion after the migration.
78+
Config: alias,
79+
PlanOnly: true,
80+
ExpectNonEmptyPlan: false,
81+
},
82+
},
83+
})
84+
}
85+
4686
func doesAdminRoleCustomExist(id string) (bool, error) {
4787
client := iDaaSAPIClientForTestUtil.OktaSDKSupplementClient()
4888
_, response, err := client.GetCustomRole(context.Background(), id)

0 commit comments

Comments
 (0)