fix: create roles and rolesmapping based on project IDs and names only

Previously lagoon-opensearch-sync was creating project roles and rolesmapping by iterating over groups, including for project groups. This logic was based on the incorrect assumption that projects and project groups (AKA project default groups) are a 1:1 mapping. In reality, project groups can have multiple project "members". So the new logic ignores the project groups and just uses project IDs and names for roles and rolesmapping. This matches the logic used in the custom Keycloak mapper Lagoon uses to grant roles to Opensearch users.
uselagoon · Jan 17, 2025 · c205f15 · c205f15
1 parent 164bb8e
commit c205f15
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 67 deletions.
diff --git a/internal/sync/roles.go b/internal/sync/roles.go
@@ -3,7 +3,6 @@ package sync
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak"
 	"github.com/uselagoon/lagoon-opensearch-sync/internal/opensearch"
@@ -69,40 +68,13 @@ func isLagoonGroup(
 	return ok
 }
 
-// projectGroupRoleName generates the name of a project group role from the
-// 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 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 projectIDs[0] < 0 {
-		return "", fmt.Errorf("invalid project ID in group ID %s: %d", group.ID,
-			projectIDs[0])
-	}
-	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,
-	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)
-	}
-	return name, &opensearch.Role{
+// generateProjectRole constructs an opensearch.Role from the given
+// project ID and project name.
+func generateProjectRole(
+	id int,
+	name string,
+) (string, *opensearch.Role) {
+	return fmt.Sprintf("p%d", id), &opensearch.Role{
 		RolePermissions: opensearch.RolePermissions{
 			// use an empty slice instead of omitting this entirely because the
 			// Opensearch API errors if this field is omitted.
@@ -115,8 +87,7 @@ func generateProjectGroupRole(
 					},
 					IndexPatterns: []string{
 						fmt.Sprintf(
-							`/^(application|container|lagoon|router)-logs-%s-_-.+/`,
-							strings.TrimPrefix(group.Name, "project-")),
+							`/^(application|container|lagoon|router)-logs-%s-_-.+/`, name),
 					},
 				},
 			},
@@ -127,7 +98,7 @@ func generateProjectGroupRole(
 				},
 			},
 		},
-	}, nil
+	}
 }
 
 // generateRegularGroupRole constructs an opensearch.Role from the given
@@ -184,10 +155,12 @@ func generateRegularGroupRole(
 }
 
 // generateRoles returns a slice of roles generated from the given slice of
-// keycloak Groups.
+// keycloak Groups, and the projectNames map.
 //
 // Any groups which are not recognized as either project groups or regular
 // Lagoon groups are ignored.
+//
+// All projectNames map entries generate a single role.
 func generateRoles(
 	log *zap.Logger,
 	groups []keycloak.Group,
@@ -199,14 +172,7 @@ func generateRoles(
 	var role *opensearch.Role
 	var err error
 	for _, group := range groups {
-		if isProjectGroup(log, 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 if isLagoonGroup(group, groupProjectsMap) {
+		if isLagoonGroup(group, groupProjectsMap) && !isProjectGroup(log, group) {
 			name, role, err =
 				generateRegularGroupRole(log, group, projectNames, groupProjectsMap)
 			if err != nil {
@@ -219,6 +185,10 @@ func generateRoles(
 			roles[name] = *role
 		}
 	}
+	for pid, pname := range projectNames {
+		name, role = generateProjectRole(pid, pname)
+		roles[name] = *role
+	}
 	return roles
 }
 

diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go
@@ -2,6 +2,7 @@ package sync
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak"
 	"github.com/uselagoon/lagoon-opensearch-sync/internal/opensearch"
@@ -55,34 +56,21 @@ func calculateRoleMappingDiff(
 }
 
 // generateRolesMapping returns a slice of rolesmapping generated from the
-// given slice of keycloak Groups.
+// given slice of keycloak Groups, and the projectNames map.
 //
 // Any groups which are not recognized as either project groups or regular
 // Lagoon groups are ignored.
+//
+// All projectNames map entries generate a single rolesmapping.
 func generateRolesMapping(
 	log *zap.Logger,
 	groups []keycloak.Group,
+	projectNames map[int]string,
 	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, groupProjectsMap)
-			if err != nil {
-				log.Warn("couldn't generate project group role name", zap.Error(err),
-					zap.String("group name", group.Name))
-				continue
-			}
-			rolesmapping[name] = opensearch.RoleMapping{
-				RoleMappingPermissions: opensearch.RoleMappingPermissions{
-					BackendRoles:    []string{name},
-					AndBackendRoles: []string{},
-					Hosts:           []string{},
-					Users:           []string{},
-				},
-			}
-		} else if isLagoonGroup(group, groupProjectsMap) {
+		if isLagoonGroup(group, groupProjectsMap) && !isProjectGroup(log, group) {
 			rolesmapping[group.Name] = opensearch.RoleMapping{
 				RoleMappingPermissions: opensearch.RoleMappingPermissions{
 					BackendRoles:    []string{group.Name},
@@ -93,6 +81,17 @@ func generateRolesMapping(
 			}
 		}
 	}
+	for pid := range projectNames {
+		roleName := fmt.Sprintf("p%d", pid)
+		rolesmapping[roleName] = opensearch.RoleMapping{
+			RoleMappingPermissions: opensearch.RoleMappingPermissions{
+				BackendRoles:    []string{roleName},
+				AndBackendRoles: []string{},
+				Hosts:           []string{},
+				Users:           []string{},
+			},
+		}
+	}
 	return rolesmapping
 }
 
@@ -123,6 +122,7 @@ func syncRolesMapping(
 	ctx context.Context,
 	log *zap.Logger,
 	groups []keycloak.Group,
+	projectNames map[int]string,
 	roles map[string]opensearch.Role,
 	groupProjectsMap map[string][]int,
 	o OpensearchService,
@@ -137,7 +137,7 @@ func syncRolesMapping(
 	// ignore non-lagoon rolesmapping
 	existing = filterRolesMapping(existing, roles)
 	// generate the rolesmapping required by Lagoon
-	required := generateRolesMapping(log, groups, groupProjectsMap)
+	required := generateRolesMapping(log, groups, projectNames, 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
@@ -112,7 +112,7 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService,
 		case "roles":
 			syncRoles(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun)
 		case "rolesmapping":
-			syncRolesMapping(ctx, log, groups, roles, groupProjectsMap, o, dryRun)
+			syncRolesMapping(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun)
 		case "indexpatterns":
 			syncIndexPatterns(ctx, log, groupsSansGlobal, projectNames, groupProjectsMap,
 				o, d, dryRun, legacyIndexPatternDelimiter)