Merge pull request #40 from uselagoon/ignore-unlabelled-groups

fix: strictly validate regular Lagoon groups from Keycloak

internal/sync/indexpatterns.go

@@ -121,14 +121,17 @@ func generateIndexPatternsForGroup(log *zap.Logger, group keycloak.Group,
 
 // generateIndexPatterns returns a map of index patterns required by Lagoon
 // logging.
+//
+// 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) map[string]map[string]bool {
 	indexPatterns := map[string]map[string]bool{}
 	var patterns []string
 	var err error
 	for _, group := range groups {
-		if isProjectGroup(log, group) {
-			continue // project groups don't get any index patterns
+		if !isLagoonGroup(group) || isProjectGroup(log, group) {
+			continue
 		}
 		patterns, err = generateIndexPatternsForGroup(log, group, projectNames)
 		if err != nil {

internal/sync/roles.go

@@ -52,6 +52,15 @@ 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)
@@ -178,22 +187,24 @@ 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 {
 	roles := map[string]opensearch.Role{}
 	var name string
 	var role *opensearch.Role
 	var err error
 	for _, group := range groups {
-		// figure out if this is a regular group or project group
 		if isProjectGroup(log, group) {
 			name, role, err = generateProjectGroupRole(group)
 			if err != nil {
 				log.Warn("couldn't generate role for project group",
 					zap.String("group name", group.Name), zap.Error(err))
 				continue
 			}
-		} else {
+		} else if isLagoonGroup(group) {
 			name, role, err = generateRegularGroupRole(log, projectNames, group)
 			if err != nil {
 				log.Warn("couldn't generate role for regular group",

internal/sync/rolesmapping.go

@@ -56,6 +56,9 @@ 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.
 func generateRolesMapping(log *zap.Logger,
 	groups []keycloak.Group) map[string]opensearch.RoleMapping {
 	rolesmapping := map[string]opensearch.RoleMapping{}
@@ -76,7 +79,7 @@ func generateRolesMapping(log *zap.Logger,
 					Users:           []string{},
 				},
 			}
-		} else {
+		} else if isLagoonGroup(group) {
 			rolesmapping[group.Name] = opensearch.RoleMapping{
 				RoleMappingPermissions: opensearch.RoleMappingPermissions{
 					BackendRoles:    []string{group.Name},

internal/sync/sync.go

@@ -1,3 +1,4 @@
+// Package sync implements synchronization of state from Lagoon to Opensearch.
 package sync
 
 import (

internal/sync/tenants.go

@@ -51,12 +51,14 @@ func calculateTenantDiff(existing, required map[string]opensearch.Tenant) (
 
 // generateTenants returns a slice of tenants generated from the given slice of
 // keycloak Groups.
+//
+// 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 {
 	tenants := map[string]opensearch.Tenant{}
 	for _, group := range groups {
-		// we only need tenants for regular groups, not project groups
-		if isProjectGroup(log, group) {
+		if !isLagoonGroup(group) || isProjectGroup(log, group) {
 			continue
 		}
 		tenants[group.Name] = opensearch.Tenant{