Skip to content

Commit 6cb5046

Browse files
dtoxvanilla1991dtoxvanilla1991
committed
Enhance validation for OAuth2 options and improve role resource tests for permission handling
1 parent 8db5761 commit 6cb5046

File tree

3 files changed

+65
-96
lines changed

3 files changed

+65
-96
lines changed

‎internal/provider/connection_resource.go‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,28 @@ func (r *ConnectionResource) ValidateConfig(ctx context.Context, req resource.Va
386386
if !data.Strategy.IsNull() {
387387
strategy := connections.Strategy(data.Strategy.ValueString())
388388
if strings.HasPrefix(string(strategy), "oauth2:") {
389-
// Validate options if present
390-
if data.Options != nil {
391-
if err := data.Options.Validate(); err != nil {
392-
resp.Diagnostics.AddError(
393-
"Invalid Options Configuration",
394-
err.Error(),
395-
)
396-
}
389+
if data.Options == nil || data.Options.IsEmpty() {
390+
resp.Diagnostics.AddError(
391+
"Missing OAuth2 Options",
392+
"options must be set for OAuth2 connections and must include both client_id and client_secret.",
393+
)
394+
return
395+
}
396+
397+
if err := data.Options.Validate(); err != nil {
398+
resp.Diagnostics.AddError(
399+
"Invalid Options Configuration",
400+
err.Error(),
401+
)
402+
return
403+
}
404+
} else {
405+
if data.Options != nil && !data.Options.IsEmpty() {
406+
resp.Diagnostics.AddError(
407+
"Unsupported Options",
408+
"options is only supported for OAuth2 connection strategies.",
409+
)
410+
return
397411
}
398412
}
399413
}

‎internal/provider/role_resource.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,9 @@ func (r *RoleResource) Create(ctx context.Context, req resource.CreateRequest, r
171171

172172
// Helper function to sort permissions without modifying original.
173173
func sortPermissions(permissions []string) []string {
174+
if permissions == nil {
175+
return nil
176+
}
174177
sorted := make([]string, len(permissions))
175178
copy(sorted, permissions)
176179
sort.Strings(sorted)
@@ -410,5 +413,9 @@ func (r *RoleResource) ImportState(ctx context.Context, req resource.ImportState
410413
return
411414
}
412415

413-
resp.State.Set(ctx, &state)
416+
diags := resp.State.Set(ctx, &state)
417+
resp.Diagnostics.Append(diags...)
418+
if resp.Diagnostics.HasError() {
419+
return
420+
}
414421
}

‎internal/provider/role_resource_test.go‎

Lines changed: 35 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package provider
22

33
import (
44
"fmt"
5+
"strings"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
89
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
9-
"github.com/hashicorp/terraform-plugin-testing/terraform"
1010
)
1111

1212
func TestAccRoleResource(t *testing.T) {
@@ -54,63 +54,23 @@ func TestAccRoleResource_PermissionOrdering(t *testing.T) {
5454
permission1ID := acctest.RandomWithPrefix("tfacc-perm1")
5555
permission2ID := acctest.RandomWithPrefix("tfacc-perm2")
5656

57-
var permission1ResourceID, permission2ResourceID string
58-
5957
resource.Test(t, resource.TestCase{
6058
PreCheck: func() { testAccPreCheck(t) },
6159
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
6260
Steps: []resource.TestStep{
63-
// Create first permission.
64-
{
65-
Config: testAccPermissionResourceConfig("test-permission-1", permission1ID, "Test permission 1"),
66-
Check: resource.ComposeAggregateTestCheckFunc(
67-
resource.TestCheckResourceAttrSet("kinde_permission.test", "id"),
68-
func(s *terraform.State) error {
69-
rs, ok := s.RootModule().Resources["kinde_permission.test"]
70-
if !ok {
71-
return fmt.Errorf("permission1 not found")
72-
}
73-
permission1ResourceID = rs.Primary.ID
74-
return nil
75-
},
76-
),
77-
},
78-
// Create second permission.
61+
// Create both permissions and the role with permissions in one order.
7962
{
80-
Config: testAccPermissionResourceConfig("test-permission-2", permission2ID, "Test permission 2"),
81-
Check: resource.ComposeAggregateTestCheckFunc(
82-
resource.TestCheckResourceAttrSet("kinde_permission.test", "id"),
83-
func(s *terraform.State) error {
84-
rs, ok := s.RootModule().Resources["kinde_permission.test"]
85-
if !ok {
86-
return fmt.Errorf("permission2 not found")
87-
}
88-
permission2ResourceID = rs.Primary.ID
89-
return nil
90-
},
91-
),
92-
},
93-
// Create role with permissions in one order.
94-
{
95-
Config: testAccRoleResourceConfig_WithPermissions(testID, testID, []string{
96-
permission1ResourceID,
97-
permission2ResourceID,
98-
}),
63+
Config: testAccRoleWithPermissionResourcesConfig(testID, permission1ID, permission2ID, []string{"kinde_permission.perm1.id", "kinde_permission.perm2.id"}),
9964
Check: resource.ComposeAggregateTestCheckFunc(
10065
resource.TestCheckResourceAttr("kinde_role.test", "name", testID),
10166
resource.TestCheckResourceAttr("kinde_role.test", "key", testID),
10267
resource.TestCheckResourceAttr("kinde_role.test", "description", "Test role with permissions"),
10368
resource.TestCheckResourceAttr("kinde_role.test", "permissions.#", "2"),
104-
resource.TestCheckTypeSetElemAttr("kinde_role.test", "permissions.*", permission1ResourceID),
105-
resource.TestCheckTypeSetElemAttr("kinde_role.test", "permissions.*", permission2ResourceID),
10669
),
10770
},
10871
// Update with same permissions in different order - should not trigger a change.
10972
{
110-
Config: testAccRoleResourceConfig_WithPermissions(testID, testID, []string{
111-
permission2ResourceID,
112-
permission1ResourceID,
113-
}),
73+
Config: testAccRoleWithPermissionResourcesConfig(testID, permission1ID, permission2ID, []string{"kinde_permission.perm2.id", "kinde_permission.perm1.id"}),
11474
PlanOnly: true,
11575
},
11676
// Remove all permissions.
@@ -136,68 +96,28 @@ func TestAccRoleResource_RemovePermissions(t *testing.T) {
13696
permission1ID := acctest.RandomWithPrefix("tfacc-perm1")
13797
permission2ID := acctest.RandomWithPrefix("tfacc-perm2")
13898

139-
var permission1ResourceID, permission2ResourceID string
140-
14199
resource.Test(t, resource.TestCase{
142100
PreCheck: func() { testAccPreCheck(t) },
143101
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
144102
Steps: []resource.TestStep{
145-
// Create first permission.
103+
// Create both permissions and the role with two permissions.
146104
{
147-
Config: testAccPermissionResourceConfig("test-permission-1", permission1ID, "Test permission 1"),
148-
Check: resource.ComposeAggregateTestCheckFunc(
149-
resource.TestCheckResourceAttrSet("kinde_permission.test", "id"),
150-
func(s *terraform.State) error {
151-
rs, ok := s.RootModule().Resources["kinde_permission.test"]
152-
if !ok {
153-
return fmt.Errorf("permission1 not found")
154-
}
155-
permission1ResourceID = rs.Primary.ID
156-
return nil
157-
},
158-
),
159-
},
160-
// Create second permission.
161-
{
162-
Config: testAccPermissionResourceConfig("test-permission-2", permission2ID, "Test permission 2"),
163-
Check: resource.ComposeAggregateTestCheckFunc(
164-
resource.TestCheckResourceAttrSet("kinde_permission.test", "id"),
165-
func(s *terraform.State) error {
166-
rs, ok := s.RootModule().Resources["kinde_permission.test"]
167-
if !ok {
168-
return fmt.Errorf("permission2 not found")
169-
}
170-
permission2ResourceID = rs.Primary.ID
171-
return nil
172-
},
173-
),
174-
},
175-
// Create role with two permissions.
176-
{
177-
Config: testAccRoleResourceConfig_WithPermissions(testID, testID, []string{
178-
permission1ResourceID,
179-
permission2ResourceID,
180-
}),
105+
Config: testAccRoleWithPermissionResourcesConfig(testID, permission1ID, permission2ID, []string{"kinde_permission.perm1.id", "kinde_permission.perm2.id"}),
181106
Check: resource.ComposeAggregateTestCheckFunc(
182107
resource.TestCheckResourceAttr("kinde_role.test", "name", testID),
183108
resource.TestCheckResourceAttr("kinde_role.test", "key", testID),
184109
resource.TestCheckResourceAttr("kinde_role.test", "description", "Test role with permissions"),
185110
resource.TestCheckResourceAttr("kinde_role.test", "permissions.#", "2"),
186-
resource.TestCheckTypeSetElemAttr("kinde_role.test", "permissions.*", permission1ResourceID),
187-
resource.TestCheckTypeSetElemAttr("kinde_role.test", "permissions.*", permission2ResourceID),
188111
),
189112
},
190113
// Remove one permission.
191114
{
192-
Config: testAccRoleResourceConfig_WithPermissions(testID, testID, []string{
193-
permission1ResourceID,
194-
}),
115+
Config: testAccRoleWithPermissionResourcesConfig(testID, permission1ID, permission2ID, []string{"kinde_permission.perm1.id"}),
195116
Check: resource.ComposeAggregateTestCheckFunc(
196117
resource.TestCheckResourceAttr("kinde_role.test", "name", testID),
197118
resource.TestCheckResourceAttr("kinde_role.test", "key", testID),
198119
resource.TestCheckResourceAttr("kinde_role.test", "description", "Test role with permissions"),
199120
resource.TestCheckResourceAttr("kinde_role.test", "permissions.#", "1"),
200-
resource.TestCheckTypeSetElemAttr("kinde_role.test", "permissions.*", permission1ResourceID),
201121
),
202122
},
203123
// Remove all permissions.
@@ -264,3 +184,31 @@ resource "kinde_role" "test" {
264184
}
265185
`, name, key, description, permissionsStr)
266186
}
187+
188+
func testAccRoleWithPermissionResourcesConfig(roleName, permission1Key, permission2Key string, permissionRefs []string) string {
189+
permissionsStr := "[]"
190+
if len(permissionRefs) > 0 {
191+
permissionsStr = "[" + strings.Join(permissionRefs, ", ") + "]"
192+
}
193+
194+
return fmt.Sprintf(`
195+
resource "kinde_permission" "perm1" {
196+
name = "test-permission-1"
197+
key = %q
198+
description = "Test permission 1"
199+
}
200+
201+
resource "kinde_permission" "perm2" {
202+
name = "test-permission-2"
203+
key = %q
204+
description = "Test permission 2"
205+
}
206+
207+
resource "kinde_role" "test" {
208+
name = %q
209+
key = %q
210+
description = "Test role with permissions"
211+
permissions = %s
212+
}
213+
`, permission1Key, permission2Key, roleName, roleName, permissionsStr)
214+
}

0 commit comments

Comments
 (0)