Skip to content

Commit 891f930

Browse files
0msokolowski00msokolowski0
committed
feat: add validation to prevent simultaneous organization and cluster scopes in role bindings
1 parent 220d383 commit 891f930

2 files changed

Lines changed: 144 additions & 228 deletions

File tree

‎castai/resource_enterprise_group.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,15 +1006,17 @@ func resourceEnterpriseGroupUpdate(ctx context.Context, data *schema.ResourceDat
10061006
orgID, _ := scope[FieldEnterpriseGroupScopeOrganization].(string)
10071007
clusterID, _ := scope[FieldEnterpriseGroupScopeCluster].(string)
10081008

1009+
if orgID != "" && clusterID != "" {
1010+
return diag.FromErr(fmt.Errorf("scope cannot have both 'organization' and 'cluster' set simultaneously"))
1011+
}
1012+
10091013
if orgID != "" {
10101014
rbScopes = append(rbScopes, organization_management.Scope{
10111015
Organization: &organization_management.OrganizationScope{
10121016
Id: orgID,
10131017
},
10141018
})
1015-
}
1016-
1017-
if clusterID != "" {
1019+
} else if clusterID != "" {
10181020
rbScopes = append(rbScopes, organization_management.Scope{
10191021
Cluster: &organization_management.ClusterScope{
10201022
Id: clusterID,

‎castai/resource_enterprise_group_test.go‎

Lines changed: 139 additions & 225 deletions
Original file line numberDiff line numberDiff line change
@@ -836,230 +836,98 @@ func TestResourceEnterpriseGroupReadContext(t *testing.T) {
836836
})
837837
}
838838

839-
// func TestResourceEnterpriseGroupDeleteContext(t *testing.T) {
840-
// t.Run("when API successfully deletes groups then clear state", func(t *testing.T) {
841-
// t.Parallel()
842-
// r := require.New(t)
843-
// mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
844-
845-
// ctx := context.Background()
846-
// provider := &ProviderConfig{
847-
// organizationManagementClient: mockClient,
848-
// }
849-
850-
// enterpriseID := uuid.NewString()
851-
// organizationID1 := "e" + uuid.NewString()
852-
// organizationID2 := "f" + uuid.NewString()
853-
// groupID1 := "bbbb1111-1111-1111-1111-111111111111"
854-
// groupID2 := "aaaa2222-2222-2222-2222-222222222222"
855-
856-
// // Expected delete request
857-
// expectedRequest := organization_management.BatchDeleteEnterpriseGroupsRequest{
858-
// EnterpriseId: enterpriseID,
859-
// Requests: []organization_management.BatchDeleteEnterpriseGroupsRequestDeleteGroupRequest{
860-
// {
861-
// Id: groupID1,
862-
// OrganizationId: organizationID1,
863-
// },
864-
// {
865-
// Id: groupID2,
866-
// OrganizationId: organizationID2,
867-
// },
868-
// },
869-
// }
870-
871-
// mockClient.EXPECT().
872-
// EnterpriseAPIBatchDeleteEnterpriseGroupsWithResponse(gomock.Any(), enterpriseID, expectedRequest).
873-
// Return(&organization_management.EnterpriseAPIBatchDeleteEnterpriseGroupsResponse{
874-
// HTTPResponse: &http.Response{StatusCode: http.StatusOK},
875-
// }, nil)
876-
877-
// // State with 2 groups
878-
// stateValue := cty.ObjectVal(map[string]cty.Value{
879-
// FieldEnterpriseGroupsGroups: cty.ListVal([]cty.Value{
880-
// cty.ObjectVal(map[string]cty.Value{
881-
// FieldEnterpriseGroupsGroup: cty.ListVal([]cty.Value{
882-
// cty.ObjectVal(map[string]cty.Value{
883-
// FieldEnterpriseGroupID: cty.StringVal(groupID1),
884-
// FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID1),
885-
// FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
886-
// }),
887-
// }),
888-
// }),
889-
// cty.ObjectVal(map[string]cty.Value{
890-
// FieldEnterpriseGroupsGroup: cty.ListVal([]cty.Value{
891-
// cty.ObjectVal(map[string]cty.Value{
892-
// FieldEnterpriseGroupID: cty.StringVal(groupID2),
893-
// FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID2),
894-
// FieldEnterpriseGroupName: cty.StringVal("security-team"),
895-
// }),
896-
// }),
897-
// }),
898-
// }),
899-
// })
900-
// state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
901-
// state.ID = enterpriseID
902-
903-
// resource := resourceEnterpriseGroup()
904-
// data := resource.Data(state)
905-
906-
// result := resource.DeleteContext(ctx, data, provider)
907-
908-
// r.Nil(result)
909-
// r.False(result.HasError())
910-
// r.Empty(data.Id(), "Resource ID should be cleared after successful delete")
911-
// })
912-
913-
// t.Run("when enterprise ID is empty then return error", func(t *testing.T) {
914-
// t.Parallel()
915-
// r := require.New(t)
916-
// mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
917-
918-
// ctx := context.Background()
919-
// provider := &ProviderConfig{
920-
// organizationManagementClient: mockClient,
921-
// }
922-
923-
// // State with no enterprise ID
924-
// stateValue := cty.ObjectVal(map[string]cty.Value{
925-
// FieldEnterpriseGroupsGroups: cty.ListValEmpty(cty.Object(map[string]cty.Type{})),
926-
// })
927-
// state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
928-
// state.ID = "" // Empty enterprise ID
929-
930-
// resource := resourceEnterpriseGroup()
931-
// data := resource.Data(state)
932-
933-
// result := resource.DeleteContext(ctx, data, provider)
934-
935-
// r.True(result.HasError())
936-
// r.Contains(result[0].Summary, "enterprise ID is not set")
937-
// })
938-
939-
// t.Run("when group missing ID then return state corruption error", func(t *testing.T) {
940-
// t.Parallel()
941-
// r := require.New(t)
942-
// mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
943-
944-
// ctx := context.Background()
945-
// provider := &ProviderConfig{
946-
// organizationManagementClient: mockClient,
947-
// }
948-
949-
// enterpriseID := uuid.NewString()
950-
// organizationID1 := "e" + uuid.NewString()
951-
952-
// // State with group missing ID
953-
// stateValue := cty.ObjectVal(map[string]cty.Value{
954-
// FieldEnterpriseGroupsGroups: cty.ListVal([]cty.Value{
955-
// cty.ObjectVal(map[string]cty.Value{
956-
// FieldEnterpriseGroupsGroup: cty.ListVal([]cty.Value{
957-
// cty.ObjectVal(map[string]cty.Value{
958-
// FieldEnterpriseGroupID: cty.StringVal(""), // Empty ID
959-
// FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID1),
960-
// FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
961-
// }),
962-
// }),
963-
// }),
964-
// }),
965-
// })
966-
// state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
967-
// state.ID = enterpriseID
968-
969-
// resource := resourceEnterpriseGroup()
970-
// data := resource.Data(state)
971-
972-
// result := resource.DeleteContext(ctx, data, provider)
973-
974-
// r.True(result.HasError())
975-
// r.Contains(result[0].Summary, "group in state is missing valid ID - this indicates state corruption")
976-
// })
977-
978-
// t.Run("when group missing organization_id then return state corruption error", func(t *testing.T) {
979-
// t.Parallel()
980-
// r := require.New(t)
981-
// mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
982-
983-
// ctx := context.Background()
984-
// provider := &ProviderConfig{
985-
// organizationManagementClient: mockClient,
986-
// }
987-
988-
// enterpriseID := uuid.NewString()
989-
// groupID1 := "bbbb1111-1111-1111-1111-111111111111"
990-
991-
// // State with group missing organization_id
992-
// stateValue := cty.ObjectVal(map[string]cty.Value{
993-
// FieldEnterpriseGroupsGroups: cty.ListVal([]cty.Value{
994-
// cty.ObjectVal(map[string]cty.Value{
995-
// FieldEnterpriseGroupsGroup: cty.ListVal([]cty.Value{
996-
// cty.ObjectVal(map[string]cty.Value{
997-
// FieldEnterpriseGroupID: cty.StringVal(groupID1),
998-
// FieldEnterpriseGroupOrganizationID: cty.StringVal(""), // Empty organization ID
999-
// FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
1000-
// }),
1001-
// }),
1002-
// }),
1003-
// }),
1004-
// })
1005-
// state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
1006-
// state.ID = enterpriseID
1007-
1008-
// resource := resourceEnterpriseGroup()
1009-
// data := resource.Data(state)
1010-
1011-
// result := resource.DeleteContext(ctx, data, provider)
1012-
1013-
// r.True(result.HasError())
1014-
// r.Contains(result[0].Summary, fmt.Sprintf("group %s in state is missing valid organization_id - this indicates state corruption", groupID1))
1015-
// })
1016-
1017-
// t.Run("when API call fails then return error", func(t *testing.T) {
1018-
// t.Parallel()
1019-
// r := require.New(t)
1020-
// mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
1021-
1022-
// ctx := context.Background()
1023-
// provider := &ProviderConfig{
1024-
// organizationManagementClient: mockClient,
1025-
// }
1026-
1027-
// enterpriseID := uuid.NewString()
1028-
// organizationID1 := "e" + uuid.NewString()
1029-
// groupID1 := "bbbb1111-1111-1111-1111-111111111111"
1030-
1031-
// mockClient.EXPECT().
1032-
// EnterpriseAPIBatchDeleteEnterpriseGroupsWithResponse(gomock.Any(), enterpriseID, gomock.Any()).
1033-
// Return(nil, errors.New("network error"))
1034-
1035-
// // State with 1 group
1036-
// stateValue := cty.ObjectVal(map[string]cty.Value{
1037-
// FieldEnterpriseGroupsGroups: cty.ListVal([]cty.Value{
1038-
// cty.ObjectVal(map[string]cty.Value{
1039-
// FieldEnterpriseGroupsGroup: cty.ListVal([]cty.Value{
1040-
// cty.ObjectVal(map[string]cty.Value{
1041-
// FieldEnterpriseGroupID: cty.StringVal(groupID1),
1042-
// FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID1),
1043-
// FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
1044-
// }),
1045-
// }),
1046-
// }),
1047-
// }),
1048-
// })
1049-
// state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
1050-
// state.ID = enterpriseID
1051-
1052-
// resource := resourceEnterpriseGroup()
1053-
// data := resource.Data(state)
1054-
1055-
// result := resource.DeleteContext(ctx, data, provider)
1056-
1057-
// r.True(result.HasError())
1058-
// r.Contains(result[0].Summary, "calling batch delete enterprise groups")
1059-
// r.Contains(result[0].Summary, "network error")
1060-
// r.NotEmpty(data.Id(), "Resource ID should not be cleared when delete fails")
1061-
// })
1062-
// }
839+
func TestResourceEnterpriseGroupDeleteContext(t *testing.T) {
840+
t.Parallel()
841+
842+
t.Run("when API call fails then return error", func(t *testing.T) {
843+
t.Parallel()
844+
r := require.New(t)
845+
mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
846+
847+
ctx := context.Background()
848+
provider := &ProviderConfig{
849+
organizationManagementClient: mockClient,
850+
}
851+
852+
enterpriseID := uuid.NewString()
853+
organizationID := uuid.NewString()
854+
groupID := uuid.NewString()
855+
856+
mockClient.EXPECT().
857+
EnterpriseAPIBatchDeleteEnterpriseGroupsWithResponse(gomock.Any(), enterpriseID, gomock.Any()).
858+
Return(nil, errors.New("network error"))
859+
860+
// State with 1 group
861+
stateValue := cty.ObjectVal(map[string]cty.Value{
862+
FieldEnterpriseGroupEnterpriseID: cty.StringVal(enterpriseID),
863+
FieldEnterpriseGroupID: cty.StringVal(groupID),
864+
FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID),
865+
FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
866+
})
867+
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
868+
state.ID = enterpriseID
869+
870+
resource := resourceEnterpriseGroup()
871+
data := resource.Data(state)
872+
873+
result := resource.DeleteContext(ctx, data, provider)
874+
875+
r.True(result.HasError())
876+
r.Equal("calling batch delete enterprise groups: network error", result[0].Summary)
877+
r.NotEmpty(data.Id(), "Resource ID should not be cleared when delete fails")
878+
})
879+
880+
t.Run("when API successfully deletes groups then clear state", func(t *testing.T) {
881+
t.Parallel()
882+
r := require.New(t)
883+
mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
884+
885+
ctx := context.Background()
886+
provider := &ProviderConfig{
887+
organizationManagementClient: mockClient,
888+
}
889+
890+
enterpriseID := uuid.NewString()
891+
organizationID := uuid.NewString()
892+
groupID := uuid.NewString()
893+
894+
// Expected delete request
895+
expectedRequest := organization_management.BatchDeleteEnterpriseGroupsRequest{
896+
EnterpriseId: enterpriseID,
897+
Requests: []organization_management.BatchDeleteEnterpriseGroupsRequestDeleteGroupRequest{
898+
{
899+
Id: groupID,
900+
OrganizationId: organizationID,
901+
},
902+
},
903+
}
904+
905+
mockClient.EXPECT().
906+
EnterpriseAPIBatchDeleteEnterpriseGroupsWithResponse(gomock.Any(), enterpriseID, expectedRequest).
907+
Return(&organization_management.EnterpriseAPIBatchDeleteEnterpriseGroupsResponse{
908+
HTTPResponse: &http.Response{StatusCode: http.StatusOK},
909+
}, nil)
910+
911+
// State with 2 groups
912+
stateValue := cty.ObjectVal(map[string]cty.Value{
913+
FieldEnterpriseGroupEnterpriseID: cty.StringVal(enterpriseID),
914+
FieldEnterpriseGroupID: cty.StringVal(groupID),
915+
FieldEnterpriseGroupOrganizationID: cty.StringVal(organizationID),
916+
FieldEnterpriseGroupName: cty.StringVal("engineering-team"),
917+
})
918+
state := terraform.NewInstanceStateShimmedFromValue(stateValue, 0)
919+
state.ID = enterpriseID
920+
921+
resource := resourceEnterpriseGroup()
922+
data := resource.Data(state)
923+
924+
result := resource.DeleteContext(ctx, data, provider)
925+
926+
r.Nil(result)
927+
r.False(result.HasError())
928+
r.Empty(data.Id(), "Resource ID should be cleared after successful delete")
929+
})
930+
}
1063931

1064932
func TestResourceEnterpriseGroupUpdateContext(t *testing.T) {
1065933
t.Parallel()
@@ -1069,7 +937,53 @@ func TestResourceEnterpriseGroupUpdateContext(t *testing.T) {
1069937
existingGroupID := uuid.NewString()
1070938
ctx := context.Background()
1071939

1072-
// TODO: test both scopes
940+
t.Run("when scopes include both organization and cluster scopes, return error", func(t *testing.T) {
941+
t.Parallel()
942+
r := require.New(t)
943+
mockClient := mockOrganizationManagement.NewMockClientWithResponsesInterface(gomock.NewController(t))
944+
945+
provider := &ProviderConfig{
946+
organizationManagementClient: mockClient,
947+
}
948+
949+
resource := resourceEnterpriseGroup()
950+
951+
diff := map[string]any{
952+
FieldEnterpriseGroupEnterpriseID: enterpriseID,
953+
FieldEnterpriseGroupOrganizationID: organizationID,
954+
FieldEnterpriseGroupName: "engineering-team",
955+
FieldEnterpriseGroupDescription: "Engineering team group",
956+
957+
FieldEnterpriseGroupRoleBindings: []any{
958+
map[string]any{
959+
FieldEnterpriseGroupRoleBinding: []any{
960+
map[string]any{
961+
FieldEnterpriseGroupRoleBindingName: "engineering-viewer",
962+
FieldEnterpriseGroupRoleBindingRoleID: "role-1",
963+
FieldEnterpriseGroupRoleBindingScopes: []any{
964+
map[string]any{
965+
FieldEnterpriseGroupScope: []any{
966+
map[string]any{
967+
FieldEnterpriseGroupScopeCluster: uuid.NewString(),
968+
FieldEnterpriseGroupScopeOrganization: uuid.NewString(),
969+
},
970+
},
971+
},
972+
},
973+
},
974+
},
975+
},
976+
},
977+
}
978+
data := schema.TestResourceDataRaw(t, resource.Schema, diff)
979+
data.SetId(existingGroupID)
980+
981+
result := resource.UpdateContext(ctx, data, provider)
982+
r.NotNil(result)
983+
r.True(result.HasError())
984+
r.Len(result, 1)
985+
r.Equal("scope cannot have both 'organization' and 'cluster' set simultaneously", result[0].Summary)
986+
})
1073987

1074988
t.Run("when there are changes then update resource", func(t *testing.T) {
1075989
t.Parallel()

0 commit comments

Comments
 (0)