Skip to content

FLPATH-4757 | feat: bootstrap for cnpg-database-sp - #36

Open
NoamNakash wants to merge 2 commits into
dcm-project:mainfrom
NoamNakash:flpath-4757-bootstrap-database
Open

FLPATH-4757 | feat: bootstrap for cnpg-database-sp#36
NoamNakash wants to merge 2 commits into
dcm-project:mainfrom
NoamNakash:flpath-4757-bootstrap-database

Conversation

@NoamNakash

Copy link
Copy Markdown

Summary

Adds contents of the cnpg-database-sp to the environment-agent repository
This PR is a little long since it includes the openapi spec, which is identical to the original other than:

  1. Renamed DatabaseCPU type to DatabaseCpu
  2. Moved the metadata field from the top level of the Database type to the DatabaseSpec type
  3. Added a service_type field in the DatabaseSpec type

The changes above were made for consistency with the existing service types in the repo

This PR also adds the internal/openshift/database/{config, dcm, store, units, validate} packages to the repository for future use. the packages are currently not in use to keep this PR from becoming too long

Related Jira Issue

Type of Change

  • Bug fix
  • New feature / Service Provider (SP)
  • Refactoring / Chore

Stacked PR Status

  • No, this is a standalone PR targeting main.

Author Checklist

  • PR title follows the mandatory FLPATH-XXXX: <Clear description> format.
  • Functional code diff is under 300 lines (or split into stacked PRs). - while this PR is big, it includes a lot of identical code from other packages, and an already reviewed file which is the bulk of the lines (the database openapi spec)
  • Tests written in Ginkgo & Gomega (Mandatory E2E tests included if touching an SP).
  • Self-reviewed all code (including AI-generated parts) before requesting review.
  • Linked approved RFC proposal from dcm-project/enhancements (if introducing structural changes).

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Bootstrap CloudNativePG database service provider support

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds the database service provider OpenAPI contract and generated Go models.
• Introduces configuration, validation, labeling, storage, and unit-conversion scaffolding.
• Integrates database API generation and adds focused unit tests.
Diagram

graph TD
  MAKE["Make targets"] --> SPEC["Database OpenAPI"] --> API["Generated API"] --> PATHS["Path helper"]
  MAKE --> CODEGEN["Codegen config"] --> API
  API --> VALIDATE["Create validation"] --> STORE["Store contract"]
  HELPERS["DCM and units"] --> VALIDATE
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extract shared provider primitives
  • ➕ Reduces duplication across container and database providers
  • ➕ Prevents label, unit, and validation behavior from drifting
  • ➖ Broadens this bootstrap into a cross-provider refactor
  • ➖ Requires generic abstractions before database behavior is fully established
2. Stage API and internals separately
  • ➕ Keeps the initial contract review smaller and more focused
  • ➕ Avoids merging unused scaffolding before its integration requirements are known
  • ➖ Requires additional dependent pull requests
  • ➖ Delays availability of supporting database packages

Recommendation: Keep the self-contained bootstrap approach because it follows established provider structure without forcing a premature shared abstraction. Before relying on the scaffolding, ensure database-specific contracts and terminology replace copied container types—particularly in the repository interface—and validate CPU quantities numerically rather than comparing their string representations. Shared primitives can be extracted later once another provider confirms stable common behavior.

Files changed (19) +2216 / -1

Enhancement (9) +1827 / -0
openapi.yamlDefine the v1alpha1 database service API +618/-0

Define the v1alpha1 database service API

• Introduces health and database list, create, get, and delete operations. Defines database metadata, resources, networking, lifecycle status, pagination, and RFC 9457 errors with the repository-aligned schema layout.

api/database/v1alpha1/openapi.yaml

paths.goDiscover the database creation path from OpenAPI +19/-0

Discover the database creation path from OpenAPI

• Adds a helper that loads the embedded specification and returns its first POST operation path, avoiding a hardcoded endpoint suffix.

api/database/v1alpha1/paths.go

spec.gen.goEmbed the generated database OpenAPI specification +175/-0

Embed the generated database OpenAPI specification

• Adds generated code for decoding, caching, loading, and exposing the database OpenAPI document through kin-openapi.

api/database/v1alpha1/spec.gen.go

types.gen.goGenerate database API models and enums +750/-0

Generate database API models and enums

• Adds generated database resources, specifications, parameters, status enums, health models, and problem-detail types. Includes JSON support for schemas permitting additional properties.

api/database/v1alpha1/types.gen.go

labels.goDefine database DCM labels and selectors +45/-0

Define database DCM labels and selectors

• Adds reserved label keys, standard database instance labels, and selectors for individual or all DCM-managed database resources.

internal/openshift/database/dcm/labels.go

errors.goAdd database repository domain errors +30/-0

Add database repository domain errors

• Defines not-found, conflict, and invalid-argument errors for future database persistence operations.

internal/openshift/database/store/errors.go

repository.goScaffold the database repository interface +18/-0

Scaffold the database repository interface

• Introduces CRUD, pagination, deletion, and health-check operations for the future database backing store. The current signature still references container API models and identifiers.

internal/openshift/database/store/repository.go

convert.goAdd database resource unit conversions +56/-0

Add database resource unit conversions

• Converts database CPU bounds to Kubernetes quantities and translates memory values between API and Kubernetes unit suffixes.

internal/openshift/database/units/convert.go

validate.goValidate database creation specifications +116/-0

Validate database creation specifications

• Rejects reserved identifiers and labels, invalid memory units, and inverted CPU or memory bounds. Returns repository invalid-argument errors for the first detected failure.

internal/openshift/database/validate/validate.go

Tests (6) +307 / -0
paths_test.goVerify database POST path discovery +22/-0

Verify database POST path discovery

• Confirms the embedded specification resolves the database collection endpoint and returns no error.

api/database/v1alpha1/paths_test.go

config_test.goRegister the database configuration test suite +13/-0

Register the database configuration test suite

• Adds the Ginkgo suite entry point for database configuration tests.

internal/openshift/database/config/config_test.go

config_unit_test.goTest database configuration loading and validation +113/-0

Test database configuration loading and validation

• Covers environment overrides, defaults, shared messaging requirements, and accepted or rejected external service types.

internal/openshift/database/config/config_unit_test.go

convert_test.goTest database CPU and memory conversions +88/-0

Test database CPU and memory conversions

• Covers CPU quantities, supported memory suffixes, invalid inputs, reverse conversion, and fallback behavior.

internal/openshift/database/units/convert_test.go

validate_suite_test.goRegister the database validation test suite +13/-0

Register the database validation test suite

• Adds the Ginkgo suite entry point for database create validation.

internal/openshift/database/validate/validate_suite_test.go

validate_test.goTest database creation validation rules +58/-0

Test database creation validation rules

• Covers valid specifications, reserved identifiers, CPU bounds, malformed memory, and reserved DCM labels.

internal/openshift/database/validate/validate_test.go

Other (4) +82 / -1
MakefileIntegrate database OpenAPI generation targets +16/-1

Integrate database OpenAPI generation targets

• Adds targets for generating database models and the embedded specification. Includes database generation in aggregate API-generation and help output.

Makefile

spec.gen.cfgConfigure embedded database specification generation +5/-0

Configure embedded database specification generation

• Configures oapi-codegen to emit the embedded OpenAPI specification while retaining otherwise unreferenced definitions.

api/database/v1alpha1/spec.gen.cfg

types.gen.cfgConfigure database model generation +5/-0

Configure database model generation

• Configures oapi-codegen to generate model types without pruning unused schemas.

api/database/v1alpha1/types.gen.cfg

config.goAdd database provider environment configuration +56/-0

Add database provider environment configuration

• Defines database namespace, storage class, image catalog, external service type, and monitoring settings. Loads shared agent settings and restricts external services to NodePort or LoadBalancer.

internal/openshift/database/config/config.go

Signed-off-by: Noam Nakash <nnakash@nnakash-thinkpadp1gen7.raanaii.csb>
@NoamNakash
NoamNakash force-pushed the flpath-4757-bootstrap-database branch from 6cee74c to 1ea23d6 Compare September 9, 2026 11:46
@qodo-code-review

qodo-code-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Valid processor requests are rejected ✓ Resolved 🐞 Bug ≡ Correctness
Description
checkResources compares DatabaseCpu.Min and DatabaseCpu.Max as raw strings instead of parsed
Kubernetes resource quantities. Because ValidateCreate relies on this check, valid whole-core and
millicore ranges such as 2 to 10 or 500m to 1 are rejected, while an invalid range such as
10 to 2 can pass without a later numeric comparison correcting it.
Code

internal/openshift/database/validate/validate.go[R50-55]

+	if res.Cpu.Min > res.Cpu.Max {
+		errs = append(errs, validationError{
+			Detail:  fmt.Sprintf("cpu.min (%s) must not exceed cpu.max (%s)", res.Cpu.Min, res.Cpu.Max),
+			Pointer: ptrCPUMin,
+		})
+	}
Evidence
The OpenAPI contract permits CPU values in both whole cores and millicores, and the units package
demonstrates that these values are parsed as Kubernetes resource quantities, but checkResources
orders them using Go string comparison. ValidateCreate uses that result to accept or reject the
request, with no subsequent numeric comparison, and the added test codifies the incorrect lexical
behavior by expecting the valid range 400m through 1 to fail.

api/database/v1alpha1/openapi.yaml[325-342]
internal/openshift/database/validate/validate.go[47-55]
internal/openshift/database/validate/validate.go[104-115]
internal/openshift/database/units/convert.go[12-17]
internal/openshift/database/validate/validate_test.go[34-40]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Parse CPU minimum and maximum values as Kubernetes resource quantities instead of ordering their source strings. Treat parse errors as validation failures and compare successfully parsed bounds using `Quantity.Cmp`.

## Issue Context
The OpenAPI contract accepts both whole-core and millicore CPU values, while lexical ordering does not match numerical CPU ordering. Existing unit conversion code demonstrates how these strings are parsed; update validation so valid ranges such as `2` to `10`, `500m` to `1`, and `400m` to `1` are accepted, invalid ranges such as `10` to `2` are rejected, and tests no longer codify the incorrect lexical result.

## Fix Focus Areas
- internal/openshift/database/validate/validate.go[47-55]
- internal/openshift/database/units/convert.go[12-18]
- internal/openshift/database/validate/validate_test.go[34-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Database implementations cannot connect ✓ Resolved 🐞 Bug ≡ Correctness
Description
DatabaseRepository imports the container API and declares its create, get, and list methods with
ContainerSpec, Container, and ContainerList instead of the distinct database API types. When
this package is activated, implementations using DatabaseSpec, Database, and DatabaseList
cannot satisfy the interface, while callers are forced to exchange container resources rather than
the database resources defined by this service.
Code

internal/openshift/database/store/repository.go[R13-15]

+	Create(ctx context.Context, spec v1alpha1.ContainerSpec, id string) (*v1alpha1.Container, error)
+	Get(ctx context.Context, containerID string) (*v1alpha1.Container, error)
+	List(ctx context.Context, maxPageSize int32, pageToken string) (*v1alpha1.ContainerList, error)
Evidence
The repository imports the container API and uses container models throughout its CRUD signatures,
while the generated database API defines distinct DatabaseSpec, Database, and DatabaseList
named types. Those database types are already used by the database validation and conversion
packages, and analogous repository interfaces use their own service API package, confirming that
this repository should expose database rather than container models.

internal/openshift/database/store/repository.go[4-17]
api/database/v1alpha1/types.gen.go[153-290]
internal/openshift/database/validate/validate.go[9-12]
internal/openshift/container/store/repository.go[7-15]
internal/openshift/database/store/repository.go[7-16]
api/database/v1alpha1/types.gen.go[153-204]
api/database/v1alpha1/types.gen.go[257-290]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description

Change the database repository interface, which was copied from the container provider, to import and use the database v1alpha1 API types for create, get, and list operations. Update the method signatures and parameter names to reflect database resources rather than containers.

## Issue Context

Go interface method signatures require exact named types. The current interface imports and exposes `ContainerSpec`, `Container`, and `ContainerList`, even though the generated database package defines the distinct `DatabaseSpec`, `Database`, and `DatabaseList` models expected by this repository and used by the database validators.

## Fix Focus Areas

- internal/openshift/database/store/repository.go[7-16]
- api/database/v1alpha1/types.gen.go[153-204]
- api/database/v1alpha1/types.gen.go[257-290]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Accepted memory requests are rejected ✓ Resolved 🐞 Bug ≡ Correctness
Description
ConvertMemory recognizes only MB, GB, and TB, although both memory bounds in the database
schema also accept MiB, GiB, and TiB. A request using a documented binary-unit value for
either bound reaches checkResources, where conversion returns an unsupported-unit error and causes
ValidateCreate to fail.
Code

internal/openshift/database/units/convert.go[R20-25]

+// apiToK8s maps API memory units to Kubernetes binary units.
+var apiToK8s = map[string]string{
+	"MB": "Mi",
+	"GB": "Gi",
+	"TB": "Ti",
+}
Evidence
Both memory fields in the OpenAPI schema explicitly permit six suffixes, while the conversion map
contains only the three decimal forms and returns an error for unrecognized units. Creation
validation invokes this converter for both minimum and maximum memory values, then turns conversion
errors into invalid-argument request validation failures.

api/database/v1alpha1/openapi.yaml[344-359]
internal/openshift/database/units/convert.go[20-43]
internal/openshift/database/validate/validate.go[57-75]
internal/openshift/database/units/convert.go[20-42]
internal/openshift/database/validate/validate.go[57-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description

Extend memory conversion to support every suffix accepted by the database OpenAPI schema, including `MiB`, `GiB`, and `TiB`, and add test coverage for all accepted suffixes.

## Issue Context

The schema contract accepts decimal- and binary-suffixed memory strings, but the conversion map used by creation validation contains only decimal-suffixed forms. Kubernetes quantities use `Mi`, `Gi`, and `Ti`, so API values ending with the additional `B` require explicit normalization before parsing.

## Fix Focus Areas

- internal/openshift/database/units/convert.go[20-43]
- internal/openshift/database/units/convert_test.go[41-67]
- api/database/v1alpha1/openapi.yaml[344-359]
- internal/openshift/database/validate/validate.go[57-75]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Database path test uses legacy style ✓ Resolved 📘 Rule violation ▣ Testability
Description
TestPostPath uses testing.T failure methods instead of Ginkgo Describe/Context/It blocks
and Gomega Expect assertions. The new database API test therefore follows a separate test and
assertion structure that later cases in this file are likely to copy.
Code

api/database/v1alpha1/paths_test.go[R9-12]

+func TestPostPath(t *testing.T) {
+	got, err := v1alpha1.PostPath()
+	if err != nil {
+		t.Fatalf("PostPath() returned unexpected error: %v", err)
Evidence
Compliance rule 2788537 requires new tests to use Ginkgo v2 BDD blocks with Gomega assertions. The
added test is implemented as a standard testing.T function and calls t.Fatalf, t.Fatal, and
t.Errorf.

Rule 2788537: Use Ginkgo BDD structure with Describe/Context/It and Expect assertions
api/database/v1alpha1/paths_test.go[9-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Convert `TestPostPath` from direct `testing.T` checks to the required Ginkgo v2 and Gomega structure.

## Issue Context
Keep the same observable checks for errors, a non-empty result, and the expected database path. A suite bootstrap may remain based on `testing.T`, but the test cases must use Ginkgo blocks and Gomega assertions.

## Fix Focus Areas
- api/database/v1alpha1/paths_test.go[9-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 17 rules
Review mode: 🧠 Deep: This introduces a new public API/spec, build-generation integration, and several independent configuration, validation, conversion, labeling, and repository paths with substantial logic density and many opportunities for subtle contract defects.

Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread api/database/v1alpha1/paths_test.go Outdated
Comment thread internal/openshift/database/validate/validate.go Outdated
Comment thread internal/openshift/database/units/convert.go
Comment thread internal/openshift/database/store/repository.go Outdated
Comment thread api/database/v1alpha1/openapi.yaml
Comment thread api/database/v1alpha1/openapi.yaml Outdated
Comment thread internal/openshift/database/config/config.go
Comment thread internal/openshift/database/store/errors.go Outdated
Comment thread internal/openshift/database/validate/validate.go Outdated
@NoamNakash
NoamNakash force-pushed the flpath-4757-bootstrap-database branch from 429d129 to 7e2f596 Compare September 10, 2026 06:14
Signed-off-by: Noam Nakash <nnakash@nnakash-thinkpadp1gen7.raanaii.csb>
@NoamNakash
NoamNakash force-pushed the flpath-4757-bootstrap-database branch from 7e2f596 to 7585f19 Compare September 10, 2026 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants