Review and improve, add documentation

2026-01-15 17:21:46 +01:00 · 2026-01-14 08:59:27 +01:00
parent 71b75eea0e
commit 6cf59043a3
1 changed files with 86 additions and 42 deletions
--- a/internal/repository/jobQuery.go
+++ b/internal/repository/jobQuery.go
@@ -2,6 +2,10 @@
 // All rights reserved. This file is part of cc-backend.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
+
+// Package repository provides job query functionality with filtering, pagination,
+// and security controls. This file contains the main query builders and security
+// checks for job retrieval operations.
 package repository

 import (
@@ -19,6 +23,22 @@ import (
 	sq "github.com/Masterminds/squirrel"
 )

+const (
+	// Default initial capacity for job result slices
+	defaultJobsCapacity = 50
+)
+
+// QueryJobs retrieves jobs from the database with optional filtering, pagination,
+// and sorting. Security controls are automatically applied based on the user context.
+//
+// Parameters:
+//   - ctx: Context containing user authentication information
+//   - filters: Optional job filters (cluster, state, user, time ranges, etc.)
+//   - page: Optional pagination parameters (page number and items per page)
+//   - order: Optional sorting specification (column or footprint field)
+//
+// Returns a slice of jobs matching the criteria, or an error if the query fails.
+// The function enforces role-based access control through SecurityCheck.
 func (r *JobRepository) QueryJobs(
 	ctx context.Context,
 	filters []*model.JobFilter,
@@ -33,18 +53,16 @@ func (r *JobRepository) QueryJobs(
 	if order != nil {
 		field := toSnakeCase(order.Field)
 		if order.Type == "col" {
-			// "col": Fixed column name query
 			switch order.Order {
 			case model.SortDirectionEnumAsc:
 				query = query.OrderBy(fmt.Sprintf("job.%s ASC", field))
 			case model.SortDirectionEnumDesc:
 				query = query.OrderBy(fmt.Sprintf("job.%s DESC", field))
 			default:
-				return nil, errors.New("REPOSITORY/QUERY > invalid sorting order for column")
+				return nil, errors.New("invalid sorting order for column")
 			}
 		} else {
-			// "foot": Order by footprint JSON field values
-			// Verify and Search Only in Valid Jsons
+			// Order by footprint JSON field values
 			query = query.Where("JSON_VALID(meta_data)")
 			switch order.Order {
 			case model.SortDirectionEnumAsc:
@@ -52,7 +70,7 @@ func (r *JobRepository) QueryJobs(
 			case model.SortDirectionEnumDesc:
 				query = query.OrderBy(fmt.Sprintf("JSON_EXTRACT(footprint, \"$.%s\") DESC", field))
 			default:
-				return nil, errors.New("REPOSITORY/QUERY > invalid sorting order for footprint")
+				return nil, errors.New("invalid sorting order for footprint")
 			}
 		}
 	}
@@ -69,29 +87,35 @@ func (r *JobRepository) QueryJobs(
 	rows, err := query.RunWith(r.stmtCache).Query()
 	if err != nil {
 		queryString, queryVars, _ := query.ToSql()
-		cclog.Errorf("Error while running query '%s' %v: %v", queryString, queryVars, err)
-		return nil, err
+		return nil, fmt.Errorf("query failed [%s] %v: %w", queryString, queryVars, err)
 	}
+	defer rows.Close()

-	jobs := make([]*schema.Job, 0, 50)
+	jobs := make([]*schema.Job, 0, defaultJobsCapacity)
 	for rows.Next() {
 		job, err := scanJob(rows)
 		if err != nil {
-			rows.Close()
-			cclog.Warn("Error while scanning rows (Jobs)")
-			return nil, err
+			cclog.Warnf("Error scanning job row: %v", err)
+			return nil, fmt.Errorf("failed to scan job row: %w", err)
 		}
 		jobs = append(jobs, job)
 	}

+	if err := rows.Err(); err != nil {
+		return nil, fmt.Errorf("error iterating job rows: %w", err)
+	}
+
 	return jobs, nil
 }

+// CountJobs returns the total number of jobs matching the given filters.
+// Security controls are automatically applied based on the user context.
+// Uses DISTINCT count to handle tag filters correctly (jobs may appear multiple
+// times when joined with the tag table).
 func (r *JobRepository) CountJobs(
 	ctx context.Context,
 	filters []*model.JobFilter,
 ) (int, error) {
-	// DISTICT count for tags filters, does not affect other queries
 	query, qerr := SecurityCheck(ctx, sq.Select("count(DISTINCT job.id)").From("job"))
 	if qerr != nil {
 		return 0, qerr
@@ -103,12 +127,22 @@ func (r *JobRepository) CountJobs(

 	var count int
 	if err := query.RunWith(r.DB).Scan(&count); err != nil {
-		return 0, err
+		return 0, fmt.Errorf("failed to count jobs: %w", err)
 	}

 	return count, nil
 }

+// SecurityCheckWithUser applies role-based access control filters to a job query
+// based on the provided user's roles and permissions.
+//
+// Access rules by role:
+//   - API role (exclusive): Full access to all jobs
+//   - Admin/Support roles: Full access to all jobs
+//   - Manager role: Access to jobs in managed projects plus own jobs
+//   - User role: Access only to own jobs
+//
+// Returns an error if the user is nil or has no recognized roles.
 func SecurityCheckWithUser(user *schema.User, query sq.SelectBuilder) (sq.SelectBuilder, error) {
 	if user == nil {
 		var qnil sq.SelectBuilder
@@ -116,32 +150,35 @@ func SecurityCheckWithUser(user *schema.User, query sq.SelectBuilder) (sq.Select
 	}

 	switch {
-	case len(user.Roles) == 1 && user.HasRole(schema.RoleApi): // API-User : All jobs
+	case len(user.Roles) == 1 && user.HasRole(schema.RoleApi):
 		return query, nil
-	case user.HasAnyRole([]schema.Role{schema.RoleAdmin, schema.RoleSupport}): // Admin & Support : All jobs
+	case user.HasAnyRole([]schema.Role{schema.RoleAdmin, schema.RoleSupport}):
 		return query, nil
-	case user.HasRole(schema.RoleManager): // Manager : Add filter for managed projects' jobs only + personal jobs
+	case user.HasRole(schema.RoleManager):
 		if len(user.Projects) != 0 {
 			return query.Where(sq.Or{sq.Eq{"job.project": user.Projects}, sq.Eq{"job.hpc_user": user.Username}}), nil
-		} else {
-			cclog.Debugf("Manager-User '%s' has no defined projects to lookup! Query only personal jobs ...", user.Username)
-			return query.Where("job.hpc_user = ?", user.Username), nil
 		}
-	case user.HasRole(schema.RoleUser): // User : Only personal jobs
+		cclog.Debugf("Manager '%s' has no assigned projects, restricting to personal jobs", user.Username)
 		return query.Where("job.hpc_user = ?", user.Username), nil
-	default: // No known Role, return error
+	case user.HasRole(schema.RoleUser):
+		return query.Where("job.hpc_user = ?", user.Username), nil
+	default:
 		var qnil sq.SelectBuilder
 		return qnil, fmt.Errorf("user has no or unknown roles")
 	}
 }

+// SecurityCheck extracts the user from the context and applies role-based access
+// control filters to the query. This is a convenience wrapper around SecurityCheckWithUser.
 func SecurityCheck(ctx context.Context, query sq.SelectBuilder) (sq.SelectBuilder, error) {
 	user := GetUserFromContext(ctx)
-
 	return SecurityCheckWithUser(user, query)
 }

-// Build a sq.SelectBuilder out of a schema.JobFilter.
+// BuildWhereClause constructs SQL WHERE conditions from a JobFilter and applies
+// them to the query. Supports filtering by job properties (cluster, state, user),
+// time ranges, resource usage, tags, and JSON field searches in meta_data,
+// footprint, and resources columns.
 func BuildWhereClause(filter *model.JobFilter, query sq.SelectBuilder) sq.SelectBuilder {
 	// Primary Key
 	if filter.DbID != nil {
@@ -205,23 +242,24 @@ func BuildWhereClause(filter *model.JobFilter, query sq.SelectBuilder) sq.Select
 	// Queries Within JSONs
 	if filter.MetricStats != nil {
 		for _, ms := range filter.MetricStats {
-			query = buildFloatJsonCondition(ms.MetricName, ms.Range, query)
+			query = buildFloatJSONCondition(ms.MetricName, ms.Range, query)
 		}
 	}
 	if filter.Node != nil {
-		query = buildResourceJsonCondition("hostname", filter.Node, query)
+		query = buildResourceJSONCondition("hostname", filter.Node, query)
 	}
 	if filter.JobName != nil {
-		query = buildMetaJsonCondition("jobName", filter.JobName, query)
+		query = buildMetaJSONCondition("jobName", filter.JobName, query)
 	}
 	if filter.Schedule != nil {
 		interactiveJobname := "interactive"
-		if *filter.Schedule == "interactive" {
+		switch *filter.Schedule {
+		case "interactive":
 			iFilter := model.StringInput{Eq: &interactiveJobname}
-			query = buildMetaJsonCondition("jobName", &iFilter, query)
-		} else if *filter.Schedule == "batch" {
+			query = buildMetaJSONCondition("jobName", &iFilter, query)
+		case "batch":
 			sFilter := model.StringInput{Neq: &interactiveJobname}
-			query = buildMetaJsonCondition("jobName", &sFilter, query)
+			query = buildMetaJSONCondition("jobName", &sFilter, query)
 		}
 	}

@@ -235,14 +273,18 @@ func BuildWhereClause(filter *model.JobFilter, query sq.SelectBuilder) sq.Select
 	return query
 }

+// buildIntCondition creates a BETWEEN clause for integer range filters.
 func buildIntCondition(field string, cond *config.IntRange, query sq.SelectBuilder) sq.SelectBuilder {
 	return query.Where(field+" BETWEEN ? AND ?", cond.From, cond.To)
 }

+// buildFloatCondition creates a BETWEEN clause for float range filters.
 func buildFloatCondition(field string, cond *model.FloatRange, query sq.SelectBuilder) sq.SelectBuilder {
 	return query.Where(field+" BETWEEN ? AND ?", cond.From, cond.To)
 }

+// buildTimeCondition creates time range filters supporting absolute timestamps,
+// relative time ranges (last6h, last24h, last7d, last30d), or open-ended ranges.
 func buildTimeCondition(field string, cond *config.TimeRange, query sq.SelectBuilder) sq.SelectBuilder {
 	if cond.From != nil && cond.To != nil {
 		return query.Where(field+" BETWEEN ? AND ?", cond.From.Unix(), cond.To.Unix())
@@ -272,12 +314,14 @@ func buildTimeCondition(field string, cond *config.TimeRange, query sq.SelectBui
 	}
 }

-func buildFloatJsonCondition(condName string, condRange *model.FloatRange, query sq.SelectBuilder) sq.SelectBuilder {
-	// Verify and Search Only in Valid Jsons
+// buildFloatJSONCondition creates a filter on a numeric field within the footprint JSON column.
+func buildFloatJSONCondition(condName string, condRange *model.FloatRange, query sq.SelectBuilder) sq.SelectBuilder {
 	query = query.Where("JSON_VALID(footprint)")
 	return query.Where("JSON_EXTRACT(footprint, \"$."+condName+"\") BETWEEN ? AND ?", condRange.From, condRange.To)
 }

+// buildStringCondition creates filters for string fields supporting equality,
+// inequality, prefix, suffix, substring, and IN list matching.
 func buildStringCondition(field string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder {
 	if cond.Eq != nil {
 		return query.Where(field+" = ?", *cond.Eq)
@@ -302,10 +346,9 @@ func buildStringCondition(field string, cond *model.StringInput, query sq.Select
 	return query
 }

-func buildMetaJsonCondition(jsonField string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder {
-	// Verify and Search Only in Valid Jsons
+// buildMetaJSONCondition creates filters on fields within the meta_data JSON column.
+func buildMetaJSONCondition(jsonField string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder {
 	query = query.Where("JSON_VALID(meta_data)")
-	// add "AND" Sql query Block for field match
 	if cond.Eq != nil {
 		return query.Where("JSON_EXTRACT(meta_data, \"$."+jsonField+"\") = ?", *cond.Eq)
 	}
@@ -324,10 +367,10 @@ func buildMetaJsonCondition(jsonField string, cond *model.StringInput, query sq.
 	return query
 }

-func buildResourceJsonCondition(jsonField string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder {
-	// Verify and Search Only in Valid Jsons
+// buildResourceJSONCondition creates filters on fields within the resources JSON array column.
+// Uses json_each to search within array elements.
+func buildResourceJSONCondition(jsonField string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder {
 	query = query.Where("JSON_VALID(resources)")
-	// add "AND" Sql query Block for field match
 	if cond.Eq != nil {
 		return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\") = ?)", *cond.Eq)
 	}
@@ -351,15 +394,16 @@ var (
 	matchAllCap   = regexp.MustCompile("([a-z0-9])([A-Z])")
 )

+// toSnakeCase converts camelCase strings to snake_case for SQL column names.
+// Includes security checks to prevent SQL injection attempts.
+// Panics if potentially dangerous characters are detected.
 func toSnakeCase(str string) string {
 	for _, c := range str {
-		if c == '\'' || c == '\\' {
-			cclog.Panic("toSnakeCase() attack vector!")
+		if c == '\'' || c == '\\' || c == '"' || c == ';' || c == '-' || c == ' ' {
+			cclog.Panicf("toSnakeCase: potentially dangerous character detected in input: %q", str)
 		}
 	}

-	str = strings.ReplaceAll(str, "'", "")
-	str = strings.ReplaceAll(str, "\\", "")
 	snake := matchFirstCap.ReplaceAllString(str, "${1}_${2}")
 	snake = matchAllCap.ReplaceAllString(snake, "${1}_${2}")
 	return strings.ToLower(snake)