Skip to content

Commit 6563f18

Browse files
oanatmariaoanatmaria
committed
Address comments
1 parent e7a71fe commit 6563f18

File tree

4 files changed

+274
-77
lines changed

4 files changed

+274
-77
lines changed

‎castai/resource_role_bindings.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,15 @@ func resourceRoleBindingsCreate(ctx context.Context, data *schema.ResourceData,
226226
scope := convertScopeToSDK(data)
227227
scopes := convertScopesToSDK(data)
228228

229+
if scope == nil && len(scopes) == 0 {
230+
return diag.Errorf("role binding scopes were not provided")
231+
}
232+
229233
resp, err := client.RbacServiceAPICreateRoleBindingsWithResponse(ctx, organizationID, sdk.RbacServiceAPICreateRoleBindingsJSONRequestBody{
230234
{
231235
Definition: sdk.CastaiRbacV1beta1RoleBindingDefinition{
232236
RoleId: data.Get(FieldRoleBindingsRoleID).(string),
233-
Scope: &scope,
237+
Scope: scope,
234238
Scopes: &scopes,
235239
Subjects: &subjects,
236240
},
@@ -276,10 +280,14 @@ func resourceRoleBindingsUpdate(ctx context.Context, data *schema.ResourceData,
276280
scope := convertScopeToSDK(data)
277281
scopes := convertScopesToSDK(data)
278282

283+
if scope == nil && len(scopes) == 0 {
284+
return diag.Errorf("role binding scopes were not provided")
285+
}
286+
279287
resp, err := client.RbacServiceAPIUpdateRoleBindingWithResponse(ctx, organizationID, roleBindingID, sdk.RbacServiceAPIUpdateRoleBindingJSONRequestBody{
280288
Definition: sdk.CastaiRbacV1beta1RoleBindingDefinition{
281289
RoleId: data.Get(FieldRoleBindingsRoleID).(string),
282-
Scope: &scope,
290+
Scope: scope,
283291
Scopes: &scopes,
284292
Subjects: &subjects,
285293
},
@@ -433,29 +441,29 @@ func assignRoleBindingData(roleBinding *sdk.CastaiRbacV1beta1RoleBinding, data *
433441
return nil
434442
}
435443

436-
func convertScopeToSDK(data *schema.ResourceData) sdk.CastaiRbacV1beta1Scope {
444+
func convertScopeToSDK(data *schema.ResourceData) *sdk.CastaiRbacV1beta1Scope {
437445
scopes := data.Get(FieldRoleBindingsScope).([]any)
438446
if len(scopes) == 0 {
439-
return sdk.CastaiRbacV1beta1Scope{}
447+
return nil
440448
}
441449

442450
scope := scopes[0].(map[string]any)
443451

444452
switch scope[FieldRoleBindingsScopeKind].(string) {
445453
case RoleBindingScopeKindOrganization:
446-
return sdk.CastaiRbacV1beta1Scope{
454+
return &sdk.CastaiRbacV1beta1Scope{
447455
Organization: &sdk.CastaiRbacV1beta1OrganizationScope{
448456
Id: scope[FieldRoleBindingsScopeResourceID].(string),
449457
},
450458
}
451459
case RoleBindingScopeKindCluster:
452-
return sdk.CastaiRbacV1beta1Scope{
460+
return &sdk.CastaiRbacV1beta1Scope{
453461
Cluster: &sdk.CastaiRbacV1beta1ClusterScope{
454462
Id: scope[FieldRoleBindingsScopeResourceID].(string),
455463
},
456464
}
457465
default:
458-
return sdk.CastaiRbacV1beta1Scope{}
466+
return nil
459467
}
460468
}
461469

‎castai/resource_role_bindings_test.go‎

Lines changed: 173 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/google/uuid"
1414
"github.com/hashicorp/go-cty/cty"
1515
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
16-
"github.com/samber/lo"
1716
"github.com/stretchr/testify/require"
1817

1918
"github.com/castai/terraform-provider-castai/castai/sdk"
@@ -392,14 +391,35 @@ func TestRoleBindingsUpdateContext(t *testing.T) {
392391
r.Equal(organizationID, reqOrgID)
393392
r.Equal(roleBindingID, reqRoleBindingID)
394393

395-
body := &bytes.Buffer{}
396-
err := json.NewEncoder(body).Encode(&sdk.CastaiRbacV1beta1RoleBinding{})
397-
r.NoError(err)
398-
return &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(body), Header: map[string][]string{"Content-Type": {"json"}}}, nil
394+
body := io.NopCloser(bytes.NewReader([]byte(`{
395+
"message": "Internal server error",
396+
"error": "Something went wrong on the server",
397+
"code": 500
398+
}`)))
399+
return &http.Response{StatusCode: http.StatusInternalServerError, Body: body, Header: map[string][]string{"Content-Type": {"json"}}}, nil
399400
})
400401

401402
stateValue := cty.ObjectVal(map[string]cty.Value{
402403
"organization_id": cty.StringVal(organizationID),
404+
"name": cty.StringVal("test group"),
405+
"description": cty.StringVal("test role binding description"),
406+
"role_id": cty.StringVal(uuid.NewString()),
407+
"scopes": cty.ListVal([]cty.Value{
408+
cty.ObjectVal(map[string]cty.Value{
409+
"kind": cty.StringVal("organization"),
410+
"resource_id": cty.StringVal(organizationID),
411+
}),
412+
}),
413+
"subjects": cty.ListVal([]cty.Value{
414+
cty.ObjectVal(map[string]cty.Value{
415+
"subject": cty.ListVal([]cty.Value{
416+
cty.ObjectVal(map[string]cty.Value{
417+
"kind": cty.StringVal("user"),
418+
"user_id": cty.StringVal(uuid.NewString()),
419+
}),
420+
}),
421+
}),
422+
}),
403423
})
404424
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
405425
state.ID = roleBindingID
@@ -429,47 +449,85 @@ func TestRoleBindingsUpdateContext(t *testing.T) {
429449

430450
organizationID := uuid.NewString()
431451
roleBindingID := uuid.NewString()
432-
452+
roleID := uuid.NewString()
433453
firstUserID := uuid.NewString()
434-
secondUserID := uuid.NewString()
435-
436-
body := io.NopCloser(bytes.NewReader([]byte(`{
437-
"id": "` + roleBindingID + `",
438-
"organizationId": "` + organizationID + `",
439-
"name": "test group",
440-
"description": "test role binding description changed",
441-
"definition": {
442-
"members": [
443-
{
444-
"id": "` + firstUserID + `",
445-
"email": "test-user-1@test.com"
446-
},
447-
{
448-
"id": "` + secondUserID + `",
449-
"email": "test-user-2@test.com"
450-
}
451-
]
452-
}
453-
}`)))
454454

455455
mockClient.EXPECT().
456456
RbacServiceAPIGetRoleBinding(gomock.Any(), organizationID, roleBindingID).
457-
Return(&http.Response{StatusCode: http.StatusOK, Body: body, Header: map[string][]string{"Content-Type": {"json"}}}, nil)
457+
Return(&http.Response{
458+
StatusCode: http.StatusOK,
459+
Body: io.NopCloser(bytes.NewReader([]byte(`{
460+
"id": "` + roleBindingID + `",
461+
"organizationId": "` + organizationID + `",
462+
"name": "test group",
463+
"description": "test role binding description",
464+
"definition": {
465+
"roleId": "` + roleID + `",
466+
"scope": {
467+
"organization": {
468+
"id": "` + organizationID + `"
469+
}
470+
},
471+
"subjects": [
472+
{
473+
"user": {
474+
"id": "` + firstUserID + `"
475+
}
476+
}
477+
]
478+
}
479+
}`))),
480+
Header: map[string][]string{"Content-Type": {"application/json"}},
481+
}, nil)
458482

459483
mockClient.EXPECT().
460484
RbacServiceAPIUpdateRoleBinding(gomock.Any(), organizationID, roleBindingID, gomock.Any()).
461485
DoAndReturn(func(ctx context.Context, reqOrgID string, reqRoleBindingID string, req sdk.RbacServiceAPIUpdateRoleBindingJSONRequestBody) (*http.Response, error) {
462486
r.Equal(organizationID, reqOrgID)
463487
r.Equal(roleBindingID, reqRoleBindingID)
464488

465-
body := &bytes.Buffer{}
466-
err := json.NewEncoder(body).Encode(&sdk.CastaiRbacV1beta1RoleBinding{})
467-
r.NoError(err)
468-
return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(body), Header: map[string][]string{"Content-Type": {"json"}}}, nil
489+
return &http.Response{
490+
StatusCode: http.StatusOK,
491+
Body: io.NopCloser(bytes.NewReader([]byte(`{
492+
"id": "` + roleBindingID + `",
493+
"organizationId": "` + organizationID + `",
494+
"name": "` + req.Name + `",
495+
"description": "` + *req.Description + `",
496+
"definition": {
497+
"roleId": "` + req.Definition.RoleId + `",
498+
"scope": {
499+
"organization": {
500+
"id": "` + organizationID + `"
501+
}
502+
}
503+
}
504+
}`))),
505+
Header: map[string][]string{"Content-Type": {"application/json"}},
506+
}, nil
469507
})
470508

509+
// Create a resource state with all required fields
471510
stateValue := cty.ObjectVal(map[string]cty.Value{
472511
"organization_id": cty.StringVal(organizationID),
512+
"name": cty.StringVal("test group"),
513+
"description": cty.StringVal("test role binding description"),
514+
"role_id": cty.StringVal(roleID),
515+
"scope": cty.ListVal([]cty.Value{
516+
cty.ObjectVal(map[string]cty.Value{
517+
"kind": cty.StringVal("organization"),
518+
"resource_id": cty.StringVal(organizationID),
519+
}),
520+
}),
521+
"subjects": cty.ListVal([]cty.Value{
522+
cty.ObjectVal(map[string]cty.Value{
523+
"subject": cty.ListVal([]cty.Value{
524+
cty.ObjectVal(map[string]cty.Value{
525+
"kind": cty.StringVal("user"),
526+
"user_id": cty.StringVal(firstUserID),
527+
}),
528+
}),
529+
}),
530+
}),
473531
})
474532
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
475533
state.ID = roleBindingID
@@ -500,20 +558,42 @@ func TestRoleBindingsCreateContext(t *testing.T) {
500558
}
501559

502560
organizationID := uuid.NewString()
561+
roleID := uuid.NewString()
562+
userID := uuid.NewString()
503563

504564
mockClient.EXPECT().
505565
RbacServiceAPICreateRoleBindings(gomock.Any(), organizationID, gomock.Any()).
506566
DoAndReturn(func(ctx context.Context, reqOrgID string, req sdk.RbacServiceAPICreateRoleBindingsJSONRequestBody) (*http.Response, error) {
507567
r.Equal(organizationID, reqOrgID)
508568

509-
body := &bytes.Buffer{}
510-
err := json.NewEncoder(body).Encode(&sdk.CastaiRbacV1beta1RoleBinding{})
511-
r.NoError(err)
512-
return &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(body), Header: map[string][]string{"Content-Type": {"json"}}}, nil
569+
return &http.Response{
570+
StatusCode: http.StatusInternalServerError,
571+
Body: io.NopCloser(bytes.NewReader([]byte(`{"message":"Internal server error"}`))),
572+
Header: map[string][]string{"Content-Type": {"application/json"}},
573+
}, nil
513574
})
514575

515576
stateValue := cty.ObjectVal(map[string]cty.Value{
516577
"organization_id": cty.StringVal(organizationID),
578+
"name": cty.StringVal("test role binding"),
579+
"description": cty.StringVal("test role binding description"),
580+
"role_id": cty.StringVal(roleID),
581+
"scope": cty.ListVal([]cty.Value{
582+
cty.ObjectVal(map[string]cty.Value{
583+
"kind": cty.StringVal("organization"),
584+
"resource_id": cty.StringVal(organizationID),
585+
}),
586+
}),
587+
"subjects": cty.ListVal([]cty.Value{
588+
cty.ObjectVal(map[string]cty.Value{
589+
"subject": cty.ListVal([]cty.Value{
590+
cty.ObjectVal(map[string]cty.Value{
591+
"kind": cty.StringVal("user"),
592+
"user_id": cty.StringVal(userID),
593+
}),
594+
}),
595+
}),
596+
}),
517597
})
518598
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
519599

@@ -542,53 +622,77 @@ func TestRoleBindingsCreateContext(t *testing.T) {
542622

543623
organizationID := uuid.NewString()
544624
roleBindingID := uuid.NewString()
545-
625+
roleID := uuid.NewString()
546626
firstUserID := uuid.NewString()
547-
secondUserID := uuid.NewString()
548-
549-
body := io.NopCloser(bytes.NewReader([]byte(`{
550-
"id": "` + roleBindingID + `",
551-
"organizationId": "` + organizationID + `",
552-
"name": "test role binding",
553-
"description": "test role binding description changed",
554-
"definition": {
555-
"members": [
556-
{
557-
"id": "` + firstUserID + `",
558-
"email": "test-user-1@test.com"
559-
},
560-
{
561-
"id": "` + secondUserID + `",
562-
"email": "test-user-2@test.com"
563-
}
564-
]
565-
}
566-
}`)))
567-
568-
mockClient.EXPECT().
569-
RbacServiceAPIGetRoleBinding(gomock.Any(), organizationID, roleBindingID).
570-
Return(&http.Response{StatusCode: http.StatusOK, Body: body, Header: map[string][]string{"Content-Type": {"json"}}}, nil)
571627

572628
mockClient.EXPECT().
573629
RbacServiceAPICreateRoleBindings(gomock.Any(), organizationID, gomock.Any()).
574630
DoAndReturn(func(ctx context.Context, reqOrgID string, req sdk.RbacServiceAPICreateRoleBindingsJSONRequestBody) (*http.Response, error) {
575631
r.Equal(organizationID, reqOrgID)
576632

577-
body := bytes.NewBuffer([]byte(""))
578-
err := json.NewEncoder(body).Encode(&[]sdk.CastaiRbacV1beta1RoleBinding{
579-
{
580-
Id: lo.ToPtr(roleBindingID),
581-
},
582-
})
583-
r.NoError(err)
584-
return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(body), Header: map[string][]string{"Content-Type": {"json"}}}, nil
633+
return &http.Response{
634+
StatusCode: http.StatusOK,
635+
Body: io.NopCloser(bytes.NewReader([]byte(`[{
636+
"id": "` + roleBindingID + `",
637+
"organizationId": "` + organizationID + `",
638+
"name": "test role binding",
639+
"description": "test role binding description"
640+
}]`))),
641+
Header: map[string][]string{"Content-Type": {"application/json"}},
642+
}, nil
585643
})
586644

645+
mockClient.EXPECT().
646+
RbacServiceAPIGetRoleBinding(gomock.Any(), organizationID, roleBindingID).
647+
Return(&http.Response{
648+
StatusCode: http.StatusOK,
649+
Body: io.NopCloser(bytes.NewReader([]byte(`{
650+
"id": "` + roleBindingID + `",
651+
"organizationId": "` + organizationID + `",
652+
"name": "test role binding",
653+
"description": "test role binding description",
654+
"definition": {
655+
"roleId": "` + roleID + `",
656+
"scope": {
657+
"organization": {
658+
"id": "` + organizationID + `"
659+
}
660+
},
661+
"subjects": [
662+
{
663+
"user": {
664+
"id": "` + firstUserID + `"
665+
}
666+
}
667+
]
668+
}
669+
}`))),
670+
Header: map[string][]string{"Content-Type": {"application/json"}},
671+
}, nil)
672+
587673
stateValue := cty.ObjectVal(map[string]cty.Value{
588674
"organization_id": cty.StringVal(organizationID),
675+
"name": cty.StringVal("test role binding"),
676+
"description": cty.StringVal("test role binding description"),
677+
"role_id": cty.StringVal(roleID),
678+
"scope": cty.ListVal([]cty.Value{
679+
cty.ObjectVal(map[string]cty.Value{
680+
"kind": cty.StringVal("organization"),
681+
"resource_id": cty.StringVal(organizationID),
682+
}),
683+
}),
684+
"subjects": cty.ListVal([]cty.Value{
685+
cty.ObjectVal(map[string]cty.Value{
686+
"subject": cty.ListVal([]cty.Value{
687+
cty.ObjectVal(map[string]cty.Value{
688+
"kind": cty.StringVal("user"),
689+
"user_id": cty.StringVal(firstUserID),
690+
}),
691+
}),
692+
}),
693+
}),
589694
})
590695
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
591-
state.ID = roleBindingID
592696

593697
resource := resourceRoleBindings()
594698
data := resource.Data(state)

0 commit comments

Comments
 (0)