perf(import): do not check for existing grant ID (#8164)

# Which Problems Are Solved

Improve the performance of the `admin/v1/import` API endpoint.
Specifaclly the import of large amount of project grants.

# How the Problems Are Solved

`AddProjectGrantWithID` and `AddProjectGrantMember` methods of
`Commands` used to get the current state of the Writemodel to check if
the current GrantID or the combination of GrantID & UserID wasn't
already used. However, the Added events already have protection against
duplication by the `UniqueConstaint` methods.

The queries become very slow when there is a great amount of project
grants. Because all the events are pushed to the aggregate ID of the
project, we had to obtain all related project events, including events
of grantIDs we do not care about. This O(n) duration for bached import
jobs adding many organization granted to a single project.

This change removes the unnecesary state query to improve performance.

# Additional Changes

- Add integration tests for import

# Additional Context

- reported internally
This commit is contained in:
Tim Möhlmann 2024-06-20 16:31:58 +03:00 committed by GitHub
parent 4101e1cd49
commit 669ac6bda2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 529 additions and 70 deletions

2
go.mod
View File

@ -179,7 +179,7 @@ require (
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/muesli/clusters v0.0.0-20200529215643-2700303c1762 // indirect
github.com/muesli/kmeans v0.3.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
github.com/prometheus/client_golang v1.19.1
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.54.0 // indirect

View File

@ -0,0 +1,492 @@
//go:build integration
package admin_test
import (
"testing"
"time"
"github.com/brianvoe/gofakeit/v6"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/zitadel/zitadel/internal/integration"
"github.com/zitadel/zitadel/pkg/grpc/admin"
"github.com/zitadel/zitadel/pkg/grpc/management"
v1 "github.com/zitadel/zitadel/pkg/grpc/v1"
)
func TestServer_ImportData(t *testing.T) {
orgIDs := generateIDs(10)
projectIDs := generateIDs(10)
userIDs := generateIDs(10)
grantIDs := generateIDs(10)
tests := []struct {
name string
req *admin.ImportDataRequest
want *admin.ImportDataResponse
wantErr bool
}{
{
name: "success",
req: &admin.ImportDataRequest{
Data: &admin.ImportDataRequest_DataOrgs{
DataOrgs: &admin.ImportDataOrg{
Orgs: []*admin.DataOrg{
{
OrgId: orgIDs[0],
Org: &management.AddOrgRequest{
Name: gofakeit.ProductName(),
},
Projects: []*v1.DataProject{
{
ProjectId: projectIDs[0],
Project: &management.AddProjectRequest{
Name: gofakeit.AppName(),
ProjectRoleAssertion: true,
},
},
{
ProjectId: projectIDs[1],
Project: &management.AddProjectRequest{
Name: gofakeit.AppName(),
ProjectRoleAssertion: false,
},
},
},
ProjectRoles: []*management.AddProjectRoleRequest{
{
ProjectId: projectIDs[0],
RoleKey: "role1",
DisplayName: "role1",
},
{
ProjectId: projectIDs[0],
RoleKey: "role2",
DisplayName: "role2",
},
{
ProjectId: projectIDs[1],
RoleKey: "role3",
DisplayName: "role3",
},
{
ProjectId: projectIDs[1],
RoleKey: "role4",
DisplayName: "role4",
},
},
HumanUsers: []*v1.DataHumanUser{
{
UserId: userIDs[0],
User: &management.ImportHumanUserRequest{
UserName: gofakeit.Username(),
Profile: &management.ImportHumanUserRequest_Profile{
FirstName: gofakeit.FirstName(),
LastName: gofakeit.LastName(),
DisplayName: gofakeit.Username(),
PreferredLanguage: gofakeit.LanguageBCP(),
},
Email: &management.ImportHumanUserRequest_Email{
Email: gofakeit.Email(),
IsEmailVerified: true,
},
},
},
{
UserId: userIDs[1],
User: &management.ImportHumanUserRequest{
UserName: gofakeit.Username(),
Profile: &management.ImportHumanUserRequest_Profile{
FirstName: gofakeit.FirstName(),
LastName: gofakeit.LastName(),
DisplayName: gofakeit.Username(),
PreferredLanguage: gofakeit.LanguageBCP(),
},
Email: &management.ImportHumanUserRequest_Email{
Email: gofakeit.Email(),
IsEmailVerified: true,
},
},
},
},
ProjectGrants: []*v1.DataProjectGrant{
{
GrantId: grantIDs[0],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[0],
GrantedOrgId: orgIDs[1],
RoleKeys: []string{"role1", "role2"},
},
},
{
GrantId: grantIDs[1],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[1],
GrantedOrgId: orgIDs[1],
RoleKeys: []string{"role3", "role4"},
},
},
{
GrantId: grantIDs[2],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[0],
GrantedOrgId: orgIDs[2],
RoleKeys: []string{"role1", "role2"},
},
},
{
GrantId: grantIDs[3],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[1],
GrantedOrgId: orgIDs[2],
RoleKeys: []string{"role3", "role4"},
},
},
},
},
{
OrgId: orgIDs[1],
Org: &management.AddOrgRequest{
Name: gofakeit.ProductName(),
},
UserGrants: []*management.AddUserGrantRequest{
{
UserId: userIDs[0],
ProjectId: projectIDs[0],
ProjectGrantId: grantIDs[0],
},
{
UserId: userIDs[0],
ProjectId: projectIDs[1],
ProjectGrantId: grantIDs[1],
},
},
},
{
OrgId: orgIDs[2],
Org: &management.AddOrgRequest{
Name: gofakeit.ProductName(),
},
UserGrants: []*management.AddUserGrantRequest{
{
UserId: userIDs[1],
ProjectId: projectIDs[0],
ProjectGrantId: grantIDs[2],
},
{
UserId: userIDs[1],
ProjectId: projectIDs[1],
ProjectGrantId: grantIDs[3],
},
},
},
},
},
},
Timeout: time.Minute.String(),
},
want: &admin.ImportDataResponse{
Success: &admin.ImportDataSuccess{
Orgs: []*admin.ImportDataSuccessOrg{
{
OrgId: orgIDs[0],
ProjectIds: projectIDs[0:2],
ProjectRoles: []string{
projectIDs[0] + "_role1",
projectIDs[0] + "_role2",
projectIDs[1] + "_role3",
projectIDs[1] + "_role4",
},
HumanUserIds: userIDs[0:2],
ProjectGrants: []*admin.ImportDataSuccessProjectGrant{
{
GrantId: grantIDs[0],
ProjectId: projectIDs[0],
OrgId: orgIDs[1],
},
{
GrantId: grantIDs[1],
ProjectId: projectIDs[1],
OrgId: orgIDs[1],
},
{
GrantId: grantIDs[2],
ProjectId: projectIDs[0],
OrgId: orgIDs[2],
},
{
GrantId: grantIDs[3],
ProjectId: projectIDs[1],
OrgId: orgIDs[2],
},
},
},
{
OrgId: orgIDs[1],
UserGrants: []*admin.ImportDataSuccessUserGrant{
{
ProjectId: projectIDs[0],
UserId: userIDs[0],
},
{
UserId: userIDs[0],
ProjectId: projectIDs[1],
},
},
},
{
OrgId: orgIDs[2],
UserGrants: []*admin.ImportDataSuccessUserGrant{
{
ProjectId: projectIDs[0],
UserId: userIDs[1],
},
{
UserId: userIDs[1],
ProjectId: projectIDs[1],
},
},
},
},
},
},
},
{
name: "duplicate project grant error",
req: &admin.ImportDataRequest{
Data: &admin.ImportDataRequest_DataOrgs{
DataOrgs: &admin.ImportDataOrg{
Orgs: []*admin.DataOrg{
{
OrgId: orgIDs[3],
Org: &management.AddOrgRequest{
Name: gofakeit.ProductName(),
},
Projects: []*v1.DataProject{
{
ProjectId: projectIDs[2],
Project: &management.AddProjectRequest{
Name: gofakeit.AppName(),
ProjectRoleAssertion: true,
},
},
{
ProjectId: projectIDs[3],
Project: &management.AddProjectRequest{
Name: gofakeit.AppName(),
ProjectRoleAssertion: false,
},
},
},
ProjectRoles: []*management.AddProjectRoleRequest{
{
ProjectId: projectIDs[2],
RoleKey: "role1",
DisplayName: "role1",
},
{
ProjectId: projectIDs[2],
RoleKey: "role2",
DisplayName: "role2",
},
{
ProjectId: projectIDs[3],
RoleKey: "role3",
DisplayName: "role3",
},
{
ProjectId: projectIDs[3],
RoleKey: "role4",
DisplayName: "role4",
},
},
ProjectGrants: []*v1.DataProjectGrant{
{
GrantId: grantIDs[4],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[2],
GrantedOrgId: orgIDs[4],
RoleKeys: []string{"role1", "role2"},
},
},
{
GrantId: grantIDs[4],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[2],
GrantedOrgId: orgIDs[4],
RoleKeys: []string{"role1", "role2"},
},
},
},
},
},
},
},
Timeout: time.Minute.String(),
},
want: &admin.ImportDataResponse{
Errors: []*admin.ImportDataError{
{
Type: "project_grant",
Id: orgIDs[3] + "_" + projectIDs[2] + "_" + orgIDs[4],
Message: "ID=V3-DKcYh Message=Errors.Project.Grant.AlreadyExists Parent=(ERROR: duplicate key value violates unique constraint \"unique_constraints_pkey\" (SQLSTATE 23505))",
},
},
Success: &admin.ImportDataSuccess{
Orgs: []*admin.ImportDataSuccessOrg{
{
OrgId: orgIDs[3],
ProjectIds: projectIDs[2:4],
ProjectRoles: []string{
projectIDs[2] + "_role1",
projectIDs[2] + "_role2",
projectIDs[3] + "_role3",
projectIDs[3] + "_role4",
},
ProjectGrants: []*admin.ImportDataSuccessProjectGrant{
{
GrantId: grantIDs[4],
ProjectId: projectIDs[2],
OrgId: orgIDs[4],
},
},
},
},
},
},
},
{
name: "duplicate project grant member error",
req: &admin.ImportDataRequest{
Data: &admin.ImportDataRequest_DataOrgs{
DataOrgs: &admin.ImportDataOrg{
Orgs: []*admin.DataOrg{
{
OrgId: orgIDs[5],
Org: &management.AddOrgRequest{
Name: gofakeit.ProductName(),
},
Projects: []*v1.DataProject{
{
ProjectId: projectIDs[4],
Project: &management.AddProjectRequest{
Name: gofakeit.AppName(),
ProjectRoleAssertion: true,
},
},
},
ProjectRoles: []*management.AddProjectRoleRequest{
{
ProjectId: projectIDs[4],
RoleKey: "role1",
DisplayName: "role1",
},
},
HumanUsers: []*v1.DataHumanUser{
{
UserId: userIDs[2],
User: &management.ImportHumanUserRequest{
UserName: gofakeit.Username(),
Profile: &management.ImportHumanUserRequest_Profile{
FirstName: gofakeit.FirstName(),
LastName: gofakeit.LastName(),
DisplayName: gofakeit.Username(),
PreferredLanguage: gofakeit.LanguageBCP(),
},
Email: &management.ImportHumanUserRequest_Email{
Email: gofakeit.Email(),
IsEmailVerified: true,
},
},
},
},
ProjectGrants: []*v1.DataProjectGrant{
{
GrantId: grantIDs[5],
ProjectGrant: &management.AddProjectGrantRequest{
ProjectId: projectIDs[4],
GrantedOrgId: orgIDs[6],
RoleKeys: []string{"role1", "role2"},
},
},
},
ProjectGrantMembers: []*management.AddProjectGrantMemberRequest{
{
ProjectId: projectIDs[4],
GrantId: grantIDs[5],
UserId: userIDs[2],
Roles: []string{"PROJECT_GRANT_OWNER"},
},
{
ProjectId: projectIDs[4],
GrantId: grantIDs[5],
UserId: userIDs[2],
Roles: []string{"PROJECT_GRANT_OWNER"},
},
},
},
},
},
},
Timeout: time.Minute.String(),
},
want: &admin.ImportDataResponse{
Errors: []*admin.ImportDataError{
{
Type: "project_grant_member",
Id: orgIDs[5] + "_" + projectIDs[4] + "_" + grantIDs[5] + "_" + userIDs[2],
Message: "ID=V3-DKcYh Message=Errors.Project.Member.AlreadyExists Parent=(ERROR: duplicate key value violates unique constraint \"unique_constraints_pkey\" (SQLSTATE 23505))",
},
},
Success: &admin.ImportDataSuccess{
Orgs: []*admin.ImportDataSuccessOrg{
{
OrgId: orgIDs[5],
ProjectIds: projectIDs[4:5],
ProjectRoles: []string{
projectIDs[4] + "_role1",
},
HumanUserIds: userIDs[2:3],
ProjectGrants: []*admin.ImportDataSuccessProjectGrant{
{
GrantId: grantIDs[5],
ProjectId: projectIDs[4],
OrgId: orgIDs[6],
},
},
ProjectGrantMembers: []*admin.ImportDataSuccessProjectGrantMember{
{
ProjectId: projectIDs[4],
GrantId: grantIDs[5],
UserId: userIDs[2],
},
},
},
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := Client.ImportData(AdminCTX, tt.req)
if tt.wantErr {
assert.Error(t, err)
return
}
require.NoError(t, err)
integration.EqualProto(t, tt.want, got)
})
}
}
func generateIDs(n int) []string {
ids := make([]string, n)
for i := range ids {
ids[i] = uuid.NewString()
}
return ids
}

View File

@ -17,14 +17,6 @@ func (c *Commands) AddProjectGrantWithID(ctx context.Context, grant *domain.Proj
ctx, span := tracing.NewSpan(ctx)
defer func() { span.EndWithError(err) }()
existingMember, err := c.projectGrantWriteModelByID(ctx, grantID, grant.AggregateID, resourceOwner)
if err != nil && !zerrors.IsNotFound(err) {
return nil, err
}
if existingMember != nil && existingMember.State != domain.ProjectGrantStateUnspecified {
return nil, zerrors.ThrowInvalidArgument(nil, "PROJECT-2b8fs", "Errors.Project.Grant.AlreadyExisting")
}
return c.addProjectGrantWithID(ctx, grant, grantID, resourceOwner)
}

View File

@ -26,13 +26,6 @@ func (c *Commands) AddProjectGrantMember(ctx context.Context, member *domain.Pro
return nil, err
}
addedMember := NewProjectGrantMemberWriteModel(member.AggregateID, member.UserID, member.GrantID)
err = c.eventstore.FilterToQueryReducer(ctx, addedMember)
if err != nil {
return nil, err
}
if addedMember.State == domain.MemberStateActive {
return nil, zerrors.ThrowAlreadyExists(nil, "PROJECT-16dVN", "Errors.Project.Member.AlreadyExists")
}
projectAgg := ProjectAggregateFromWriteModel(&addedMember.WriteModel)
pushedEvents, err := c.eventstore.Push(
ctx,

View File

@ -104,58 +104,6 @@ func TestCommandSide_AddProjectGrantMember(t *testing.T) {
err: zerrors.IsPreconditionFailed,
},
},
{
name: "member already exists, precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username1",
"firstname1",
"lastname1",
"nickname1",
"displayname1",
language.German,
domain.GenderMale,
"email1",
true,
),
),
),
expectFilter(
eventFromEventPusher(
project.NewProjectGrantMemberAddedEvent(context.Background(),
&project.NewAggregate("project1", "org1").Aggregate,
"user1",
"projectgrant1",
),
),
),
),
zitadelRoles: []authz.RoleMapping{
{
Role: "PROJECT_GRANT_OWNER",
},
},
},
args: args{
ctx: context.Background(),
member: &domain.ProjectGrantMember{
ObjectRoot: models.ObjectRoot{
AggregateID: "project1",
},
GrantID: "projectgrant1",
UserID: "user1",
Roles: []string{"PROJECT_GRANT_OWNER"},
},
},
res: res{
err: zerrors.IsErrorAlreadyExists,
},
},
{
name: "member add uniqueconstraint err, already exists",
fields: fields{
@ -177,7 +125,6 @@ func TestCommandSide_AddProjectGrantMember(t *testing.T) {
),
),
),
expectFilter(),
expectPushFailed(zerrors.ThrowAlreadyExists(nil, "ERROR", "internal"),
project.NewProjectGrantMemberAddedEvent(context.Background(),
&project.NewAggregate("project1", "").Aggregate,
@ -229,7 +176,6 @@ func TestCommandSide_AddProjectGrantMember(t *testing.T) {
),
),
),
expectFilter(),
expectPush(
project.NewProjectGrantMemberAddedEvent(context.Background(),
&project.NewAggregate("project1", "").Aggregate,

View File

@ -4,7 +4,10 @@ import (
"testing"
"time"
"github.com/pmezard/go-difflib/difflib"
"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"
object "github.com/zitadel/zitadel/pkg/grpc/object/v2beta"
@ -71,3 +74,36 @@ func AssertListDetails[D ListDetailsMsg](t testing.TB, expected, actual D) {
assert.WithinRange(t, gotCD, wantCD.Add(-time.Minute), wantCD.Add(time.Minute))
}
}
// EqualProto is inspired by [assert.Equal], only that it tests equality of a proto message.
// A message diff is printed on the error test log if the messages are not equal.
//
// As [assert.Equal] is based on reflection, comparing 2 proto messages sometimes fails,
// due to their internal state.
// Expected messages are usually with a vanilla state, eg only exported fields contain data.
// Actual messages obtained from the gRPC client had unexported fields with data.
// This makes them hard to compare.
func EqualProto(t testing.TB, expected, actual proto.Message) bool {
t.Helper()
if proto.Equal(expected, actual) {
return true
}
t.Errorf("Proto messages not equal: %s", diffProto(expected, actual))
return false
}
func diffProto(expected, actual proto.Message) string {
diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
A: difflib.SplitLines(protojson.Format(expected)),
B: difflib.SplitLines(protojson.Format(actual)),
FromFile: "Expected",
FromDate: "",
ToFile: "Actual",
ToDate: "",
Context: 1,
})
if err != nil {
panic(err)
}
return "\n\nDiff:\n" + diff
}