feat: system filter options

[jmpTeixeira02]

Summary by CodeRabbit

• New Features

  • Added /systems/filterOptions endpoint that returns available filter values for systems including region, type, and key configuration name. Use this endpoint to discover valid filter options when querying systems.

• Tests

  • Added test coverage for the new endpoint.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1811404c-f25e-4cb9-b170-a951132a32d9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (7)

internal/repo/query.go (1)

309-313: Consider simplifying the Filter.Values API

Values *[]string requires callers to pre-allocate a slice and pass its address so the repo can write into it via *filter.Values = .... This is a somewhat unusual pattern in Go — it's common to return the data as the function's result instead (e.g., GetFilterOptions(...) (map[string][]string, error) or have Filter receive results by reassigning). The current shape is workable but makes call sites noisier and failure modes (nil pointer) easy to hit. Also, the struct-level doc only describes Column; a short comment on Values (input/output, non-nil required, etc.) would help consumers.

apis/cmk/cmk-ui.yaml (2)

701-724: Remove 404 response and consider a more specific operationId

• The endpoint has no path parameters and returns aggregated filter values; there is no addressable resource that can be "not found", so the "404" response mapping (Line 719-720) is misleading. Consider dropping it.
• operationId: getFilters is quite generic for an OpenAPI spec that mixes multiple resource types. Something like getSystemFilters would be more self-describing and consistent with neighbors like getAllSystems.

---

1534-1552: SystemFilters fields have no constraints

region, type, and keyConfigurationName item strings have no maxLength, unlike the equivalent fields in the System schema (region/type use maxLength: 50, keyConfigurationName uses maxLength: 255). For consistency and to bound payload size on a UI-facing endpoint, consider mirroring those limits on the item schemas.

internal/manager/system_test.go (1)

1538-1553: Guard against nil pointer dereferences and extend coverage

• assert.Len(t, *filters.Type, 2) (and the same for Region/KeyConfigurationName) panics the test if the field is unexpectedly nil. Prefer require.NotNil(t, filters.Type) before dereferencing, so a regression surfaces as a failure rather than a panic.
• Consider adding a case with a system that has no linked KeyConfiguration (i.e. KeyConfigurationID == nil) to verify the LEFT JOIN + distinct-non-null filtering in SystemManager.GetFilters still returns the system's region/type and does not include a null entry in KeyConfigurationName. This is the main edge case the new SQL path has to handle correctly.

internal/manager/system.go (2)

450-457: Return zero-value SystemFilters on error.

GetFilterOptions can fail partway through (its implementation runs a UNION ALL and scans into the provided slices), yet the method unconditionally returns a SystemFilters with pointers to types / regions / keyConfigNames. Callers that forget to check the error (or log-and-continue) will see a non-nil response with possibly partial data, which can leak through the controller as a successful payload.

Prefer the standard Go pattern:

🛠 Proposed fix

-	err := m.repo.GetFilterOptions(ctx, model.System{}, filters, *query)
-
-	return cmkapi.SystemFilters{
-		KeyConfigurationName: &keyConfigNames,
-		Region:               &regions,
-		Type:                 &types,
-	}, err
+	if err := m.repo.GetFilterOptions(ctx, model.System{}, filters, *query); err != nil {
+		return cmkapi.SystemFilters{}, errs.Wrap(ErrQuerySystemList, err)
+	}
+
+	return cmkapi.SystemFilters{
+		KeyConfigurationName: &keyConfigNames,
+		Region:               &regions,
+		Type:                 &types,
+	}, nil

Wrapping with a dedicated sentinel (similar to ErrQuerySystemList used in GetAllSystems) also keeps the controller’s error-mapping consistent with the rest of this file.

---

445-449: Use consistent table qualification for JOIN columns; avoid mixing fmt.Sprintf literals with unqualified field constants.

The filters use inconsistent column qualification: repo.TypeField and repo.RegionField are left bare, while KeyConfiguration.name is table-qualified via fmt.Sprintf. While model.KeyConfiguration doesn't currently have type or region columns (no ambiguity today), this mixed approach is fragile and harder to maintain.

Qualify all three columns consistently—either all bare or all table-qualified like:

Column: fmt.Sprintf("%s.%s", model.System{}.TableName(), repo.TypeField)

or use a structured table-qualified helper. This pattern also reduces the risk of column-name typos, since these strings are interpolated directly into SQL.

internal/repo/sql/repo.go (1)

601-609: Consider deterministic ordering of filter values.

Without ORDER BY, returned values can vary between executions. Sorting at SQL level keeps API responses stable.

♻️ Suggested adjustment

-		unionQuery := strings.Join(unionParts, " UNION ALL ")
+		unionQuery := fmt.Sprintf(
+			"SELECT column_name, value FROM (%s) AS filter_values ORDER BY column_name, value",
+			strings.Join(unionParts, " UNION ALL "),
+		)

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5114f171-55df-43ff-829c-9d2e0029469e

📥 Commits

Reviewing files that changed from the base of the PR and between 660003d and 4e5d77f.

📒 Files selected for processing (13)

• apis/cmk/cmk-ui.yaml
• internal/api/cmkapi/cmkapi.go
• internal/authz/api_endpoint_mapping.go
• internal/authz/repo/authz_repo.go
• internal/controllers/cmk/system_controller.go
• internal/controllers/cmk/system_controller_test.go
• internal/manager/system.go
• internal/manager/system_test.go
• internal/manager/tenant_test.go
• internal/repo/mock/repo.go
• internal/repo/query.go
• internal/repo/repository.go
• internal/repo/sql/repo.go