diff --git a/internal/sync/indexpatterns.go b/internal/sync/indexpatterns.go index 3ecbeb6..7db8fa6 100644 --- a/internal/sync/indexpatterns.go +++ b/internal/sync/indexpatterns.go @@ -123,17 +123,29 @@ func calculateIndexPatternDiff(log *zap.Logger, // generateIndexPatternsForGroup returns a slice of index patterns for all the // projects associated with the given group. -func generateIndexPatternsForGroup(log *zap.Logger, group keycloak.Group, - projectNames map[int]string, legacyDelimiter bool) ([]string, error) { - pids, err := projectIDsForGroup(group) - if err != nil { - return nil, fmt.Errorf("couldn't get project IDs for group: %v", err) +func generateIndexPatternsForGroup( + log *zap.Logger, + group keycloak.Group, + projectNames map[int]string, + groupProjectsMap map[string][]int, + legacyDelimiter bool, +) ([]string, error) { + pids, ok := groupProjectsMap[group.ID] + if !ok { + return nil, fmt.Errorf("missing project group ID %s in groupProjectsMap", + group.ID) } var indexPatterns []string for _, pid := range pids { name, ok := projectNames[pid] if !ok { - log.Debug("invalid project ID in lagoon-projects group attribute", + // If you see this warning it means that a project ID appears in the + // kc_group_projects table that does not appear in the projects table in + // the Lagoon API DB. + // This is likely a bug in Lagoon causing loss of referential integrity, + // as there is no foreign key constraint to enforce valid project IDs in + // the group mapping. + log.Warn("invalid project ID when generating index patterns", zap.Int("projectID", pid)) continue } @@ -156,8 +168,13 @@ func generateIndexPatternsForGroup(log *zap.Logger, group keycloak.Group, // // Only regular Lagoon groups are associated with a tenant (which is where // index patterns are placed), so project groups are ignored. -func generateIndexPatterns(log *zap.Logger, groups []keycloak.Group, - projectNames map[int]string, legacyDelimiter bool) map[string]map[string]bool { +func generateIndexPatterns( + log *zap.Logger, + groups []keycloak.Group, + projectNames map[int]string, + groupProjectsMap map[string][]int, + legacyDelimiter bool, +) map[string]map[string]bool { indexPatterns := map[string]map[string]bool{} var patterns []string var err error @@ -166,7 +183,7 @@ func generateIndexPatterns(log *zap.Logger, groups []keycloak.Group, continue } patterns, err = generateIndexPatternsForGroup(log, group, projectNames, - legacyDelimiter) + groupProjectsMap, legacyDelimiter) if err != nil { log.Warn("couldn't generate index patterns for group", zap.String("group", group.Name), zap.Error(err)) @@ -191,9 +208,17 @@ func generateIndexPatterns(log *zap.Logger, groups []keycloak.Group, // syncIndexPatterns reconciles Opensearch Dashboards index patterns with // Lagoon logging requirements. -func syncIndexPatterns(ctx context.Context, log *zap.Logger, - groups []keycloak.Group, projectNames map[int]string, o OpensearchService, - d DashboardsService, dryRun, legacyDelimiter bool) { +func syncIndexPatterns( + ctx context.Context, + log *zap.Logger, + groups []keycloak.Group, + projectNames map[int]string, + groupProjectsMap map[string][]int, + o OpensearchService, + d DashboardsService, + dryRun, + legacyDelimiter bool, +) { // get index patterns from Opensearch existing, err := o.IndexPatterns(ctx) if err != nil { @@ -201,7 +226,8 @@ func syncIndexPatterns(ctx context.Context, log *zap.Logger, return } // generate the index patterns required by Lagoon - required := generateIndexPatterns(log, groups, projectNames, legacyDelimiter) + required := generateIndexPatterns(log, groups, projectNames, + groupProjectsMap, legacyDelimiter) // calculate index templates to add/remove toCreate, toDelete := calculateIndexPatternDiff(log, existing, required) for tenant, patternIDMap := range toDelete { diff --git a/internal/sync/indexpatterns_test.go b/internal/sync/indexpatterns_test.go index 7e6e6db..7f5b643 100644 --- a/internal/sync/indexpatterns_test.go +++ b/internal/sync/indexpatterns_test.go @@ -11,8 +11,9 @@ import ( ) type generateIndexPatternsForGroupInput struct { - group keycloak.Group - projectNames map[int]string + group keycloak.Group + projectNames map[int]string + groupProjectsMap map[string][]int } type generateIndexPatternsForGroupOutput struct { @@ -31,10 +32,6 @@ func TestGenerateIndexPatternsForGroup(t *testing.T) { ID: "f6697da3-016a-43cd-ba9f-3f5b91b45302", GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "drupal-example", - Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"drupal-example":[31,34,35]}`}, - "lagoon-projects": {`31,34,35`}, - }, }, }, projectNames: map[int]string{ @@ -43,6 +40,9 @@ func TestGenerateIndexPatternsForGroup(t *testing.T) { 35: "drupal10-prerelease", 36: "delta-backend", }, + groupProjectsMap: map[string][]int{ + "f6697da3-016a-43cd-ba9f-3f5b91b45302": {31, 34, 35}, + }, }, expect: generateIndexPatternsForGroupOutput{ indexPatterns: []string{ @@ -71,10 +71,6 @@ func TestGenerateIndexPatternsForGroup(t *testing.T) { ID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "drupal-example2", - Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"drupal-example":[31,35,44]}`}, - "lagoon-projects": {`31,35,44`}, - }, }, }, projectNames: map[int]string{ @@ -83,6 +79,9 @@ func TestGenerateIndexPatternsForGroup(t *testing.T) { 35: "drupal10-prerelease", 36: "delta-backend", }, + groupProjectsMap: map[string][]int{ + "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee": {31, 35, 44}, + }, }, expect: generateIndexPatternsForGroupOutput{ indexPatterns: []string{ @@ -106,7 +105,7 @@ func TestGenerateIndexPatternsForGroup(t *testing.T) { for name, tc := range testCases { t.Run(name, func(tt *testing.T) { indexPatterns, err := sync.GenerateIndexPatternsForGroup(log, tc.input.group, - tc.input.projectNames, false) + tc.input.projectNames, tc.input.groupProjectsMap, false) if (err == nil && tc.expect.err != nil) || (err != nil && tc.expect.err == nil) { tt.Fatalf("got err:\n%v\nexpected err:\n%v\n", err, tc.expect.err) @@ -376,9 +375,10 @@ func TestCalculateIndexPatternDiff(t *testing.T) { func TestGenerateIndexPatterns(t *testing.T) { type generateIndexPatternsInput struct { - groups []keycloak.Group - projectNames map[int]string - legacyDelimiter bool + groups []keycloak.Group + projectNames map[int]string + groupProjectsMap map[string][]int + legacyDelimiter bool } var testCases = map[string]struct { input generateIndexPatternsInput @@ -391,10 +391,6 @@ func TestGenerateIndexPatterns(t *testing.T) { ID: "08fef83d-cde7-43a5-8bd2-a18cf440214a", GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "foocorp", - Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"foocorp":[3133,34435]}`}, - "lagoon-projects": {`3133,34435`}, - }, }, }, { @@ -402,9 +398,7 @@ func TestGenerateIndexPatterns(t *testing.T) { GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "project-drupal12-base", Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"project-drupal12-base":[34435]}`}, - "lagoon-projects": {`34435`}, - "type": {`project-default-group`}, + "type": {`project-default-group`}, }, }, }, @@ -412,6 +406,10 @@ func TestGenerateIndexPatterns(t *testing.T) { projectNames: map[int]string{ 34435: "drupal12-base", }, + groupProjectsMap: map[string][]int{ + "08fef83d-cde7-43a5-8bd2-a18cf440214a": {3133, 34435}, + "9f92af94-a7ee-4759-83bb-2b983bd30142": {34435}, + }, legacyDelimiter: false, }, expect: map[string]map[string]bool{ @@ -446,10 +444,6 @@ func TestGenerateIndexPatterns(t *testing.T) { ID: "08fef83d-cde7-43a5-8bd2-a18cf440214a", GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "foocorp", - Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"foocorp":[3133,34435]}`}, - "lagoon-projects": {`3133,34435`}, - }, }, }, { @@ -457,9 +451,7 @@ func TestGenerateIndexPatterns(t *testing.T) { GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "project-drupal12-base", Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"project-drupal12-base":[34435]}`}, - "lagoon-projects": {`34435`}, - "type": {`project-default-group`}, + "type": {`project-default-group`}, }, }, }, @@ -467,6 +459,10 @@ func TestGenerateIndexPatterns(t *testing.T) { projectNames: map[int]string{ 34435: "drupal12-base", }, + groupProjectsMap: map[string][]int{ + "08fef83d-cde7-43a5-8bd2-a18cf440214a": {3133, 34435}, + "9f92af94-a7ee-4759-83bb-2b983bd30142": {34435}, + }, legacyDelimiter: true, }, expect: map[string]map[string]bool{ @@ -499,7 +495,7 @@ func TestGenerateIndexPatterns(t *testing.T) { for name, tc := range testCases { t.Run(name, func(tt *testing.T) { indexPatterns := sync.GenerateIndexPatterns( - log, tc.input.groups, tc.input.projectNames, + log, tc.input.groups, tc.input.projectNames, tc.input.groupProjectsMap, tc.input.legacyDelimiter) if !reflect.DeepEqual(indexPatterns, tc.expect) { tt.Fatalf("got:\n%v\nexpected:\n%v\n", indexPatterns, tc.expect) diff --git a/internal/sync/roles.go b/internal/sync/roles.go index ba7da7e..e7af613 100644 --- a/internal/sync/roles.go +++ b/internal/sync/roles.go @@ -1,11 +1,8 @@ package sync import ( - "bytes" "context" - "encoding/json" "fmt" - "strconv" "strings" "github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak" @@ -16,13 +13,22 @@ import ( // generateIndexPermissionPatterns returns a slice of index pattern strings // in regular expressions format generated from the given slice of project IDs. // https://www.elastic.co/guide/en/elasticsearch/reference/7.10/defining-roles.html#roles-indices-priv -func generateIndexPermissionPatterns(log *zap.Logger, pids []int, - projectNames map[int]string) []string { +func generateIndexPermissionPatterns( + log *zap.Logger, + pids []int, + projectNames map[int]string, +) []string { var patterns []string for _, pid := range pids { name, ok := projectNames[pid] if !ok { - log.Debug("invalid project ID in lagoon-projects group attribute", + // If you see this warning it means that a project ID appears in the + // kc_group_projects table that does not appear in the projects table in + // the Lagoon API DB. + // This is likely a bug in Lagoon causing loss of referential integrity, + // as there is no foreign key constraint to enforce valid project IDs in + // the group mapping. + log.Warn("invalid project ID when generating index permission patterns", zap.Int("projectID", pid)) continue } @@ -52,30 +58,35 @@ func isProjectGroup(log *zap.Logger, group keycloak.Group) bool { return true } -// isInt returns true if the given string looks like a base-10 integer. -func isInt(s string) bool { - _, err := strconv.Atoi(s) - return err == nil -} - // projectGroupRoleName generates the name of a project group role from the -// lagoon-projects attribute on a keycloak group. -func projectGroupRoleName(group keycloak.Group) (string, error) { - pAttr, ok := group.Attributes["lagoon-projects"] +// ID of the group's project. +func projectGroupRoleName( + group keycloak.Group, + groupProjectsMap map[string][]int, +) (string, error) { + projectIDs, ok := groupProjectsMap[group.ID] if !ok { - return "", fmt.Errorf("missing lagoon-projects attribute") + return "", fmt.Errorf("missing project group ID %s in groupProjectsMap", + group.ID) + } + if len(projectIDs) != 1 { + return "", fmt.Errorf("too many projects in group ID %s: %d", group.ID, + len(projectIDs)) } - if len(pAttr) != 1 || !isInt(pAttr[0]) { - return "", fmt.Errorf("invalid lagoon-projects attribute") + if projectIDs[0] < 0 { + return "", fmt.Errorf("invalid project ID in group ID %s: %d", group.ID, + projectIDs[0]) } - return fmt.Sprintf("p%s", pAttr[0]), nil + return fmt.Sprintf("p%d", projectIDs[0]), nil } // generateProjectGroupRole constructs an opensearch.Role from the given // keycloak group corresponding to a Lagoon project group. -func generateProjectGroupRole(group keycloak.Group) ( - string, *opensearch.Role, error) { - name, err := projectGroupRoleName(group) +func generateProjectGroupRole( + group keycloak.Group, + groupProjectsMap map[string][]int, +) (string, *opensearch.Role, error) { + name, err := projectGroupRoleName(group, groupProjectsMap) if err != nil { return "", nil, fmt.Errorf("couldn't generate project group role name: %v", err) @@ -108,40 +119,20 @@ func generateProjectGroupRole(group keycloak.Group) ( }, nil } -// projectIDsForGroup parses the lagoon-projects attribute on the given group -// and returns a slice of project IDs. -func projectIDsForGroup(group keycloak.Group) ([]int, error) { - // get lagoon-projects attribute - lpa, ok := group.Attributes["lagoon-projects"] - if !ok { - return nil, fmt.Errorf("missing lagoon-projects attribute") - } - if len(lpa) != 1 { - return nil, fmt.Errorf("invalid lagoon-projects attribute") - } - // get the project IDs - var buf bytes.Buffer - if _, err := fmt.Fprintf(&buf, "[%s]", lpa[0]); err != nil { - return nil, - fmt.Errorf("couldn't format lagoon-projects attribute: %v", err) - } - var pids []int - if err := json.Unmarshal(buf.Bytes(), &pids); err != nil { - return nil, - fmt.Errorf("couldn't unmarshal lagoon-projects attribute: %v", err) - } - return pids, nil -} - // generateRegularGroupRole constructs an opensearch.Role from the given // keycloak group corresponding to a Lagoon group. -func generateRegularGroupRole(log *zap.Logger, projectNames map[int]string, - group keycloak.Group) (string, *opensearch.Role, error) { - pids, err := projectIDsForGroup(group) - if err != nil { - return "", nil, fmt.Errorf("couldn't get project IDs for group: %v", err) +func generateRegularGroupRole( + log *zap.Logger, + group keycloak.Group, + projectNames map[int]string, + groupProjectsMap map[string][]int, +) (string, *opensearch.Role, error) { + pids, ok := groupProjectsMap[group.ID] + if !ok { + return "", nil, fmt.Errorf("missing project group ID %s in groupProjectsMap", + group.ID) } - // calculate index patterns from lagoon-projects attribute + // calculate index patterns from project IDs indexPatterns := generateIndexPermissionPatterns(log, pids, projectNames) // the Opensearch API is picky about the structure of create group requests, // so ensure that the index_permissions field is only set if there are any @@ -190,6 +181,7 @@ func generateRoles( log *zap.Logger, groups []keycloak.Group, projectNames map[int]string, + groupProjectsMap map[string][]int, ) map[string]opensearch.Role { roles := map[string]opensearch.Role{} var name string @@ -197,14 +189,15 @@ func generateRoles( var err error for _, group := range groups { if isProjectGroup(log, group) { - name, role, err = generateProjectGroupRole(group) + name, role, err = generateProjectGroupRole(group, groupProjectsMap) if err != nil { log.Warn("couldn't generate role for project group", zap.String("group name", group.Name), zap.Error(err)) continue } } else { - name, role, err = generateRegularGroupRole(log, projectNames, group) + name, role, err = + generateRegularGroupRole(log, group, projectNames, groupProjectsMap) if err != nil { log.Warn("couldn't generate role for regular group", zap.String("group name", group.Name), zap.Error(err)) @@ -261,13 +254,20 @@ func filterRoles( } // syncRoles reconciles Opensearch roles with Lagoon keycloak and projects. -func syncRoles(ctx context.Context, log *zap.Logger, groups []keycloak.Group, - projectNames map[int]string, roles map[string]opensearch.Role, - o OpensearchService, dryRun bool) { +func syncRoles( + ctx context.Context, + log *zap.Logger, + groups []keycloak.Group, + projectNames map[int]string, + roles map[string]opensearch.Role, + groupProjectsMap map[string][]int, + o OpensearchService, + dryRun bool, +) { // ignore non-lagoon roles existing := filterRoles(roles) // generate the roles required by Lagoon - required := generateRoles(log, groups, projectNames) + required := generateRoles(log, groups, projectNames, groupProjectsMap) // calculate roles to add/remove toCreate, toDelete := calculateRoleDiff(existing, required) var err error diff --git a/internal/sync/roles_test.go b/internal/sync/roles_test.go index be19228..938e13a 100644 --- a/internal/sync/roles_test.go +++ b/internal/sync/roles_test.go @@ -59,8 +59,9 @@ func TestGenerateIndexPermissionPatterns(t *testing.T) { func TestGenerateRoles(t *testing.T) { type generateRolesInput struct { - groups []keycloak.Group - projectNames map[int]string + groups []keycloak.Group + projectNames map[int]string + groupProjectsMap map[string][]int } type generateRolesOutput struct { roles map[string]opensearch.Role @@ -76,10 +77,6 @@ func TestGenerateRoles(t *testing.T) { ID: "f6697da3-016a-43cd-ba9f-3f5b91b45302", GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "drupal-example", - Attributes: map[string][]string{ - "group-lagoon-project-ids": {`{"drupal-example":[31,36,34,25,35]}`}, - "lagoon-projects": {`31,36,34,25,35`}, - }, }, }, }, @@ -89,6 +86,9 @@ func TestGenerateRoles(t *testing.T) { 35: "drupal10-prerelease", 36: "delta-backend", }, + groupProjectsMap: map[string][]int{ + "f6697da3-016a-43cd-ba9f-3f5b91b45302": {31, 36, 34, 25, 35}, + }, }, expect: generateRolesOutput{ roles: map[string]opensearch.Role{ @@ -132,8 +132,7 @@ func TestGenerateRoles(t *testing.T) { GroupUpdateRepresentation: keycloak.GroupUpdateRepresentation{ Name: "project-beta-ui", Attributes: map[string][]string{ - "lagoon-projects": {`27`}, - "type": {`project-default-group`}, + "type": {`project-default-group`}, }, }, }, @@ -143,6 +142,9 @@ func TestGenerateRoles(t *testing.T) { 27: "beta-ui", 48: "somelongprojectname", }, + groupProjectsMap: map[string][]int{ + "3fc60c90-b72d-4704-8a57-80438adac98d": {27}, + }, }, expect: generateRolesOutput{ roles: map[string]opensearch.Role{ @@ -175,7 +177,8 @@ func TestGenerateRoles(t *testing.T) { log := zap.Must(zap.NewDevelopment(zap.AddStacktrace(zap.ErrorLevel))) for name, tc := range testCases { t.Run(name, func(tt *testing.T) { - roles := sync.GenerateRoles(log, tc.input.groups, tc.input.projectNames) + roles := sync.GenerateRoles( + log, tc.input.groups, tc.input.projectNames, tc.input.groupProjectsMap) assert.Equal(tt, tc.expect.roles, roles, "roles") }) } diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go index bdaf9e4..a216dc6 100644 --- a/internal/sync/rolesmapping.go +++ b/internal/sync/rolesmapping.go @@ -59,13 +59,16 @@ func calculateRoleMappingDiff( // // Any groups which are not recognized as project groups are assumed to be // Lagoon groups. -func generateRolesMapping(log *zap.Logger, - groups []keycloak.Group) map[string]opensearch.RoleMapping { +func generateRolesMapping( + log *zap.Logger, + groups []keycloak.Group, + groupProjectsMap map[string][]int, +) map[string]opensearch.RoleMapping { rolesmapping := map[string]opensearch.RoleMapping{} for _, group := range groups { // figure out if this is a regular group or project group if isProjectGroup(log, group) { - name, err := projectGroupRoleName(group) + name, err := projectGroupRoleName(group, groupProjectsMap) if err != nil { log.Warn("couldn't generate project group role name", zap.Error(err), zap.String("group name", group.Name)) @@ -121,6 +124,7 @@ func syncRolesMapping( log *zap.Logger, groups []keycloak.Group, roles map[string]opensearch.Role, + groupProjectsMap map[string][]int, o OpensearchService, dryRun bool, ) { @@ -133,7 +137,7 @@ func syncRolesMapping( // ignore non-lagoon rolesmapping existing = filterRolesMapping(existing, roles) // generate the rolesmapping required by Lagoon - required := generateRolesMapping(log, groups) + required := generateRolesMapping(log, groups, groupProjectsMap) // calculate rolesmapping to add/remove toCreate, toDelete := calculateRoleMappingDiff(existing, required) for _, name := range toDelete { diff --git a/internal/sync/sync.go b/internal/sync/sync.go index f0ec20e..9e37325 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -21,6 +21,7 @@ type KeycloakService interface { // LagoonDBService defines the Lagoon database service interface. type LagoonDBService interface { Projects(context.Context) ([]lagoondb.Project, error) + GroupProjectsMap(context.Context) (map[string][]int, error) } // OpensearchService defines the Opensearch service interface. @@ -60,6 +61,11 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService, if err != nil { return fmt.Errorf("couldn't get projects: %v", err) } + // get group project membership map from Lagoon + groupProjectsMap, err := l.GroupProjectsMap(ctx) + if err != nil { + return fmt.Errorf("couldn't get group projects map: %v", err) + } // https://github.com/uselagoon/lagoon/blob/ // 7dd4eb3b695bd507f25de5d7ea49d6601a229b87/services/api/src/resources/ // group/opendistroSecurity.ts#L31-L34 @@ -104,12 +110,12 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService, case "tenants": syncTenants(ctx, log, groupsSansGlobal, o, dryRun) case "roles": - syncRoles(ctx, log, groups, projectNames, roles, o, dryRun) + syncRoles(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun) case "rolesmapping": - syncRolesMapping(ctx, log, groups, roles, o, dryRun) + syncRolesMapping(ctx, log, groups, roles, groupProjectsMap, o, dryRun) case "indexpatterns": - syncIndexPatterns(ctx, log, groupsSansGlobal, projectNames, o, d, dryRun, - legacyIndexPatternDelimiter) + syncIndexPatterns(ctx, log, groupsSansGlobal, projectNames, groupProjectsMap, + o, d, dryRun, legacyIndexPatternDelimiter) case "indextemplates": syncIndexTemplates(ctx, log, o, dryRun) default: