From 22fe3eabf70b837e5fc97250950a255c0c93cef3 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 15 Apr 2024 17:03:22 +0800 Subject: [PATCH 1/7] chore: remove unused parameter --- internal/sync/rolesmapping.go | 11 ++++++++--- internal/sync/sync.go | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go index b431479..aa25be5 100644 --- a/internal/sync/rolesmapping.go +++ b/internal/sync/rolesmapping.go @@ -116,9 +116,14 @@ func filterRolesMapping(rolesmapping map[string]opensearch.RoleMapping, // syncRolesmapping reconciles Opensearch rolesmapping with Lagoon keycloak // groups. -func syncRolesMapping(ctx context.Context, log *zap.Logger, groups []keycloak.Group, - projectNames map[int]string, roles map[string]opensearch.Role, - o OpensearchService, dryRun bool) { +func syncRolesMapping( + ctx context.Context, + log *zap.Logger, + groups []keycloak.Group, + roles map[string]opensearch.Role, + o OpensearchService, + dryRun bool, +) { // get rolesmapping from Opensearch existing, err := o.RolesMapping(ctx) if err != nil { diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 5f1f1c4..f0ec20e 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -106,7 +106,7 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService, case "roles": syncRoles(ctx, log, groups, projectNames, roles, o, dryRun) case "rolesmapping": - syncRolesMapping(ctx, log, groups, projectNames, roles, o, dryRun) + syncRolesMapping(ctx, log, groups, roles, o, dryRun) case "indexpatterns": syncIndexPatterns(ctx, log, groupsSansGlobal, projectNames, o, d, dryRun, legacyIndexPatternDelimiter) From cf935fd1e2bad8966354d7ead3d9b0c9dca69a4d Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 15 Apr 2024 17:04:03 +0800 Subject: [PATCH 2/7] fix: remove isLagoonGroup() function The lagoon-projects attribute is deprecated as of Lagoon v2.18. See https://github.com/uselagoon/lagoon/pull/3612 for details. --- internal/sync/indexpatterns.go | 2 +- internal/sync/roles.go | 22 ++++++++-------------- internal/sync/rolesmapping.go | 6 +++--- internal/sync/tenants.go | 8 +++++--- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/internal/sync/indexpatterns.go b/internal/sync/indexpatterns.go index cbde848..3ecbeb6 100644 --- a/internal/sync/indexpatterns.go +++ b/internal/sync/indexpatterns.go @@ -162,7 +162,7 @@ func generateIndexPatterns(log *zap.Logger, groups []keycloak.Group, var patterns []string var err error for _, group := range groups { - if !isLagoonGroup(group) || isProjectGroup(log, group) { + if isProjectGroup(log, group) { continue } patterns, err = generateIndexPatternsForGroup(log, group, projectNames, diff --git a/internal/sync/roles.go b/internal/sync/roles.go index b90d383..ba7da7e 100644 --- a/internal/sync/roles.go +++ b/internal/sync/roles.go @@ -52,15 +52,6 @@ func isProjectGroup(log *zap.Logger, group keycloak.Group) bool { return true } -// isLagoonGroup inspects the given group to determine if it is a Lagoon group. -// -// All Lagoon groups (project groups and regular groups) have a lagoon-projects -// attribute, which is checked by this function. -func isLagoonGroup(group keycloak.Group) bool { - _, ok := group.Attributes["lagoon-projects"] - return ok -} - // isInt returns true if the given string looks like a base-10 integer. func isInt(s string) bool { _, err := strconv.Atoi(s) @@ -193,10 +184,13 @@ func generateRegularGroupRole(log *zap.Logger, projectNames map[int]string, // generateRoles returns a slice of roles generated from the given slice of // keycloak Groups. // -// Any groups which are not recognized as either project groups or regular -// Lagoon groups are ignored. -func generateRoles(log *zap.Logger, groups []keycloak.Group, - projectNames map[int]string) map[string]opensearch.Role { +// Any groups which are not recognized as project groups are assumed to be +// Lagoon groups. +func generateRoles( + log *zap.Logger, + groups []keycloak.Group, + projectNames map[int]string, +) map[string]opensearch.Role { roles := map[string]opensearch.Role{} var name string var role *opensearch.Role @@ -209,7 +203,7 @@ func generateRoles(log *zap.Logger, groups []keycloak.Group, zap.String("group name", group.Name), zap.Error(err)) continue } - } else if isLagoonGroup(group) { + } else { name, role, err = generateRegularGroupRole(log, projectNames, group) if err != nil { log.Warn("couldn't generate role for regular group", diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go index aa25be5..bdaf9e4 100644 --- a/internal/sync/rolesmapping.go +++ b/internal/sync/rolesmapping.go @@ -57,8 +57,8 @@ func calculateRoleMappingDiff( // generateRolesMapping returns a slice of rolesmapping generated from the // given slice of keycloak Groups. // -// Any groups which are not recognized as either project groups or regular -// Lagoon groups are ignored. +// 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 { rolesmapping := map[string]opensearch.RoleMapping{} @@ -79,7 +79,7 @@ func generateRolesMapping(log *zap.Logger, Users: []string{}, }, } - } else if isLagoonGroup(group) { + } else { rolesmapping[group.Name] = opensearch.RoleMapping{ RoleMappingPermissions: opensearch.RoleMappingPermissions{ BackendRoles: []string{group.Name}, diff --git a/internal/sync/tenants.go b/internal/sync/tenants.go index b6d24e5..82466ff 100644 --- a/internal/sync/tenants.go +++ b/internal/sync/tenants.go @@ -54,11 +54,13 @@ func calculateTenantDiff(existing, required map[string]opensearch.Tenant) ( // // Only regular Lagoon groups are associated with a tenant, so project groups // are ignored. -func generateTenants(log *zap.Logger, - groups []keycloak.Group) map[string]opensearch.Tenant { +func generateTenants( + log *zap.Logger, + groups []keycloak.Group, +) map[string]opensearch.Tenant { tenants := map[string]opensearch.Tenant{} for _, group := range groups { - if !isLagoonGroup(group) || isProjectGroup(log, group) { + if isProjectGroup(log, group) { continue } tenants[group.Name] = opensearch.Tenant{ From 7d4f5651c942c41b6cf6b509cf1ec3bbf1a92f2d Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 15 Apr 2024 18:07:09 +0800 Subject: [PATCH 3/7] feat: add function to get group project IDs from Lagoon API DB --- internal/lagoondb/client.go | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/internal/lagoondb/client.go b/internal/lagoondb/client.go index d5ae232..198b108 100644 --- a/internal/lagoondb/client.go +++ b/internal/lagoondb/client.go @@ -1,3 +1,4 @@ +// Package lagoondb implements a client for the Lagoon API database. package lagoondb import ( @@ -20,6 +21,13 @@ type Project struct { Name string `db:"name"` } +// groupProjectMapping maps Lagoon group ID to project ID. +// This type is only used for database unmarshalling. +type groupProjectMapping struct { + GroupID string `db:"group_id"` + ProjectID int `db:"project_id"` +} + // ErrNoResult is returned by client methods if there is no result. var ErrNoResult = errors.New("no rows in result set") @@ -36,8 +44,7 @@ func NewClient(ctx context.Context, dsn string) (*Client, error) { return &Client{db: db}, nil } -// Projects returns the Environment associated with the given -// Namespace name (on Openshift this is the project name). +// Projects returns a slice of all Projects in the Lagoon API DB. func (c *Client) Projects(ctx context.Context) ([]Project, error) { // run query var projects []Project @@ -52,3 +59,28 @@ func (c *Client) Projects(ctx context.Context) ([]Project, error) { } return projects, nil } + +// GroupProjectsMap returns a map of Group (UU)IDs to Project IDs. +// This denotes Project Group membership in Lagoon. +func (c *Client) GroupProjectsMap( + ctx context.Context, +) (map[string][]int, error) { + var gpms []groupProjectMapping + err := c.db.SelectContext(ctx, &gpms, ` + SELECT group_id, project_id + FROM kc_group_projects`) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNoResult + } + return nil, err + } + groupProjectsMap := map[string][]int{} + // no need to check for duplicates here since the table has: + // UNIQUE KEY `group_project` (`group_id`,`project_id`) + for _, gpm := range gpms { + groupProjectsMap[gpm.GroupID] = + append(groupProjectsMap[gpm.GroupID], gpm.ProjectID) + } + return groupProjectsMap, nil +} From ad0f049cce45c581d14067f7b08e331a766d746e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 15 Apr 2024 18:08:28 +0800 Subject: [PATCH 4/7] feat: switch to Lagoon API DB for project group membership --- internal/sync/indexpatterns.go | 52 +++++++++--- internal/sync/indexpatterns_test.go | 54 ++++++------- internal/sync/roles.go | 118 ++++++++++++++-------------- internal/sync/roles_test.go | 21 ++--- internal/sync/rolesmapping.go | 12 ++- internal/sync/sync.go | 14 +++- 6 files changed, 153 insertions(+), 118 deletions(-) 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: From 174bb667f42bde2f1caeb41ace146b166cb2638e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 19 Apr 2024 20:10:23 +0800 Subject: [PATCH 5/7] fix: ignore Keycloak groups which are not Lagoon groups If the Keycloak Group ID doesn't appear in the Lagoon groups-projects mapping table then there's no way to know which regex to use for the role generation, so there's no way to create an Opensearch role. --- internal/sync/indexpatterns.go | 2 +- internal/sync/roles.go | 17 ++++++++++++++--- internal/sync/rolesmapping.go | 6 +++--- internal/sync/sync.go | 2 +- internal/sync/tenants.go | 15 +++++++++++---- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/internal/sync/indexpatterns.go b/internal/sync/indexpatterns.go index 7db8fa6..345c722 100644 --- a/internal/sync/indexpatterns.go +++ b/internal/sync/indexpatterns.go @@ -179,7 +179,7 @@ func generateIndexPatterns( var patterns []string var err error for _, group := range groups { - if isProjectGroup(log, group) { + if !isLagoonGroup(group, groupProjectsMap) || isProjectGroup(log, group) { continue } patterns, err = generateIndexPatternsForGroup(log, group, projectNames, diff --git a/internal/sync/roles.go b/internal/sync/roles.go index e7af613..a1162fb 100644 --- a/internal/sync/roles.go +++ b/internal/sync/roles.go @@ -58,6 +58,17 @@ func isProjectGroup(log *zap.Logger, group keycloak.Group) bool { return true } +// isLagoonGroup inspects the given group to determine if it is a Lagoon group. +// +// It checks if the group ID appears in the groupProjectsMap. +func isLagoonGroup( + group keycloak.Group, + groupProjectsMap map[string][]int, +) bool { + _, ok := groupProjectsMap[group.ID] + return ok +} + // projectGroupRoleName generates the name of a project group role from the // ID of the group's project. func projectGroupRoleName( @@ -175,8 +186,8 @@ func generateRegularGroupRole( // generateRoles returns a slice of roles generated from the given slice of // keycloak Groups. // -// Any groups which are not recognized as project groups are assumed to be -// Lagoon groups. +// Any groups which are not recognized as either project groups or regular +// Lagoon groups are ignored. func generateRoles( log *zap.Logger, groups []keycloak.Group, @@ -195,7 +206,7 @@ func generateRoles( zap.String("group name", group.Name), zap.Error(err)) continue } - } else { + } else if isLagoonGroup(group, groupProjectsMap) { name, role, err = generateRegularGroupRole(log, group, projectNames, groupProjectsMap) if err != nil { diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go index a216dc6..cebcf6f 100644 --- a/internal/sync/rolesmapping.go +++ b/internal/sync/rolesmapping.go @@ -57,8 +57,8 @@ func calculateRoleMappingDiff( // generateRolesMapping returns a slice of rolesmapping generated from the // given slice of keycloak Groups. // -// Any groups which are not recognized as project groups are assumed to be -// Lagoon groups. +// Any groups which are not recognized as either project groups or regular +// Lagoon groups are ignored. func generateRolesMapping( log *zap.Logger, groups []keycloak.Group, @@ -82,7 +82,7 @@ func generateRolesMapping( Users: []string{}, }, } - } else { + } else if isLagoonGroup(group, groupProjectsMap) { rolesmapping[group.Name] = opensearch.RoleMapping{ RoleMappingPermissions: opensearch.RoleMappingPermissions{ BackendRoles: []string{group.Name}, diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 9e37325..667ef92 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -108,7 +108,7 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService, for _, object := range objects { switch object { case "tenants": - syncTenants(ctx, log, groupsSansGlobal, o, dryRun) + syncTenants(ctx, log, groupsSansGlobal, groupProjectsMap, o, dryRun) case "roles": syncRoles(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun) case "rolesmapping": diff --git a/internal/sync/tenants.go b/internal/sync/tenants.go index 82466ff..a50a4bb 100644 --- a/internal/sync/tenants.go +++ b/internal/sync/tenants.go @@ -57,10 +57,11 @@ func calculateTenantDiff(existing, required map[string]opensearch.Tenant) ( func generateTenants( log *zap.Logger, groups []keycloak.Group, + groupProjectsMap map[string][]int, ) map[string]opensearch.Tenant { tenants := map[string]opensearch.Tenant{} for _, group := range groups { - if isProjectGroup(log, group) { + if !isLagoonGroup(group, groupProjectsMap) || isProjectGroup(log, group) { continue } tenants[group.Name] = opensearch.Tenant{ @@ -90,8 +91,14 @@ func filterTenants( } // syncTenants reconciles Opensearch tenants with Lagoon keycloak groups. -func syncTenants(ctx context.Context, log *zap.Logger, groups []keycloak.Group, - o OpensearchService, dryRun bool) { +func syncTenants( + ctx context.Context, + log *zap.Logger, + groups []keycloak.Group, + groupProjectsMap map[string][]int, + o OpensearchService, + dryRun bool, +) { // get tenants from Opensearch existing, err := o.Tenants(ctx) if err != nil { @@ -101,7 +108,7 @@ func syncTenants(ctx context.Context, log *zap.Logger, groups []keycloak.Group, // ignore non-lagoon tenants existing = filterTenants(existing) // generate the tenants required by Lagoon - required := generateTenants(log, groups) + required := generateTenants(log, groups, groupProjectsMap) // calculate tenants to add/remove toCreate, toDelete := calculateTenantDiff(existing, required) for _, name := range toDelete { From f5468dae65c7548601c7b831e74a55f3e51a8976 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 19 Apr 2024 20:15:33 +0800 Subject: [PATCH 6/7] fix: nil pointer deref --- internal/sync/roles.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/sync/roles.go b/internal/sync/roles.go index a1162fb..36957db 100644 --- a/internal/sync/roles.go +++ b/internal/sync/roles.go @@ -215,7 +215,9 @@ func generateRoles( continue } } - roles[name] = *role + if role != nil { + roles[name] = *role + } } return roles } From 30844f08043500e372b02ee93e037ca633964c02 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 19 Apr 2024 20:25:57 +0800 Subject: [PATCH 7/7] fix: exit gracefully on SIGTERM --- cmd/lagoon-opensearch-sync/sync.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/lagoon-opensearch-sync/sync.go b/cmd/lagoon-opensearch-sync/sync.go index fb47e08..4451cd6 100644 --- a/cmd/lagoon-opensearch-sync/sync.go +++ b/cmd/lagoon-opensearch-sync/sync.go @@ -86,13 +86,17 @@ func (cmd *SyncCmd) Run(log *zap.Logger) error { return nil } // continue running in a loop - tick := time.NewTicker(cmd.Period) - for range tick.C { - err = sync.Sync(ctx, log, l, k, o, d, cmd.DryRun, cmd.Objects, - cmd.LegacyIndexPatternDelimiter) - if err != nil { - return err + ticker := time.NewTicker(cmd.Period) + for { + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + err = sync.Sync(ctx, log, l, k, o, d, cmd.DryRun, cmd.Objects, + cmd.LegacyIndexPatternDelimiter) + if err != nil { + return err + } } } - return nil }