Skip to content

fix(postgres): accept integer values for postgresql.parameters in schema - #2687

Merged
myasnikovdaniil merged 5 commits into
mainfrom
fix/postgres-params-schema-int-or-string
May 22, 2026
Merged

myasnikovdaniil merged 5 commits into
mainfrom
fix/postgres-params-schema-int-or-string

Conversation

@IvanHunters

@IvanHunters IvanHunters commented May 19, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

PostgreSQL server parameters like max_connections are natural integers,
but the chart's schema declared them as {type: string} only. When users
supply bare integers (e.g. max_connections: 100) — either directly in
Application CR specs or via the UI — Helm rejects the values at schema
validation time before template rendering, causing HelmRelease failures.

The Helm template already handles both types via {{ $value | toString | quote }},
so the fix is to teach the schema to accept both.

Approach

The schema files (values.schema.json and the embedded openAPISchema
in postgres-rd/cozyrds/postgres.yaml) are generated from values.yaml
annotations via cozyvalues-gen. Hand-editing them does not survive
make generate — same regeneration trap that caught #2466 (issue #2597).

cozyvalues-gen v1.4.0 (cozystack/cozyvalues-gen#25) adds an
intOrString type alias for map values that emits
additionalProperties.anyOf: [integer, string] plus the Kubernetes
x-kubernetes-int-or-string: true marker.

Changes

  • packages/apps/postgres/values.yaml: annotation flipped from
    @field {map[string]string} to @field {map[string]intOrString},
    description updated to note integer values are accepted.
  • Regenerated artifacts: values.schema.json, embedded openAPISchema
    in packages/system/postgres-rd/cozyrds/postgres.yaml,
    packages/apps/postgres/README.md, and api/apps/v1alpha1/postgresql/types.go
    (Parameters is now map[string]intstr.IntOrString).
  • packages/apps/postgres/templates/db.yaml: $dangerousParams list
    extended with archive_cleanup_command and recovery_end_command
    (same RCE surface as archive_command / restore_command).

Verification

  • helm template with max_connections: 200 (int) and max_connections: "200"
    (string) produce identical render output.
  • Dangerous-param block list rejects archive_cleanup_command and
    recovery_end_command with the expected SECURITY: error.
  • make test in packages/apps/postgres/: 13/13 pass.
  • make generate is idempotent — schema files don't drift on regen.

Release note

fix(postgres): accept integer values for postgresql.parameters to prevent HelmRelease failures when parameters are supplied as unquoted integers; also block archive_cleanup_command and recovery_end_command (RCE surface).

Summary by CodeRabbit

  • New Features

    • PostgreSQL parameters now accept integer or string values; integers are automatically coerced to strings.
  • Security

    • Enhanced parameter filtering blocks additional dangerous settings: archive_cleanup_command and recovery_end_command.
  • Documentation

    • Updated docs and charts to describe int-or-string parameter support and revised blocked-parameter guidance.

Review Change Stack

@github-actions github-actions Bot added area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) kind/bug Categorizes issue or PR as related to a bug size/XS This PR changes 0-9 lines, ignoring generated files labels May 19, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request relaxes the schema validation for PostgreSQL parameters in the Helm chart. By allowing both string and integer types, it aligns the schema validation with the existing template logic that already handles type conversion, ensuring that users can provide numeric values without triggering validation failures.

Highlights

  • Schema Validation Update: Updated values.schema.json to allow integer values for postgresql.parameters, preventing validation errors for numeric configurations.
  • Documentation Refinement: Updated the parameter description to clarify that while strings are preferred, integers are now supported and automatically converted.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@coderabbitai

coderabbitai Bot commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR broadens postgresql.parameters to accept integers or strings (integers coerced to strings) and aligns types/schemas/docs across values.schema.json, values.yaml, the CRD, and ApplicationDefinition. The Helm template's dangerous-parameter list is expanded to block archive_cleanup_command and recovery_end_command.

Changes

PostgreSQL Parameters Migration

Layer / File(s) Summary
Schema & types: enable int-or-string for postgresql.parameters
api/apps/v1alpha1/postgresql/types.go, packages/apps/postgres/values.schema.json, packages/system/postgres-rd/cozyrds/postgres.yaml, packages/apps/postgres/values.yaml, packages/apps/postgres/README.md, .github/workflows/pre-commit.yml
Change PostgreSQL.Parameters Go field to map[string]intstr.IntOrString; update postgresql.parameters schema to allow integer or string via anyOf with x-kubernetes-int-or-string; update docs and values type annotation to map[string:intOrString and document integer→string coercion; bump cozyvalues-gen tool to v1.4.0.
Security: expand dangerous parameters list
packages/apps/postgres/templates/db.yaml, packages/apps/postgres/values.yaml
Helm template dangerousParams and SECURITY examples now also block archive_cleanup_command and recovery_end_command in addition to existing command-style blocked parameters.

Sequence Diagram(s)

sequenceDiagram
  participant User as Chart Values
  participant Helm as Helm Template (db.yaml)
  participant Validator as Template Guard (dangerousParams)
  User->>Helm: supplies `.Values.postgresql.parameters` (string|int)
  Helm->>Validator: check parameters map
  Validator-->>Helm: fail if blocked param present
  Helm-->>User: render/abort based on validation
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Poem

🐰 I nibble on schemas, tidy and spry,

Numbers now welcome where strings used to lie.
When commands try to sneak in the night,
Our guard frowns and blocks them outright.
Safe stacks hop onward, snug and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: enabling integer values for postgresql.parameters in the schema alongside the existing string type support.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/postgres-params-schema-int-or-string

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

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the PostgreSQL application schema to allow integer values for server parameters in addition to strings. However, the feedback points out that manual edits to values.schema.json are discouraged as it is a generated file, and such changes will be overwritten. Additionally, there is a mismatch with the underlying Go types which would lead to runtime unmarshaling errors if an integer is provided. The reviewer suggests updating the type annotations in values.yaml and regenerating the schema to ensure consistency.

Comment on lines 140 to 150
"description": "PostgreSQL server parameters. String values are preferred (quote numbers: \"100\"); integer values are also accepted and converted to strings automatically. BLOCKED (enable arbitrary code execution): archive_command, restore_command, ssl_passphrase_command, dynamic_library_path, local_preload_libraries, session_preload_libraries, shared_preload_libraries. Do NOT override CloudNativePG-managed parameters: archive_mode, primary_conninfo, wal_level, max_replication_slots.",
"type": "object",
"default": {
"max_connections": "100"
},
"additionalProperties": {
"type": "string"
"anyOf": [
{"type": "string"},
{"type": "integer"}
]
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While the intent to allow integer values for PostgreSQL parameters is correct, modifying values.schema.json directly is problematic for several reasons:

  1. Generated File: According to the repository style guide (line 72), values.schema.json is a generated file that is overwritten when running make generate. Any manual changes, including the updated description and the anyOf logic, will be lost in the next build.
  2. Go Type Mismatch: The corresponding Go type for this field in api/apps/v1alpha1/postgresql/types.go is currently map[string]string (line 132). If the schema is relaxed to allow integers but the Go struct remains strictly typed to strings, the Cozystack controller will fail to unmarshal the Postgres resource whenever an integer is provided, leading to a runtime error (e.g., json: cannot unmarshal number into Go struct field PostgreSQL.parameters of type string).
  3. Metadata Stripping: The style guide also notes that description fields are stripped from the schema during regeneration.

Recommendation:
You should instead update the type annotations in packages/apps/postgres/values.yaml (specifically the ## @field annotation for parameters at line 92) to a type that supports both strings and integers (such as map[string]any or a union type if supported by the generator) and then run make generate. This will ensure that both the JSON schema and the Go types are updated consistently and persist through future builds.

References
  1. The style guide states that values.schema.json is a generated file and that manual changes to descriptions or custom annotations will be stripped or overwritten. (link)

@github-actions github-actions Bot added size/S This PR changes 10-29 lines, ignoring generated files and removed size/XS This PR changes 0-9 lines, ignoring generated files labels May 19, 2026
@IvanHunters
IvanHunters force-pushed the fix/postgres-params-schema-int-or-string branch from f445b11 to a5040e4 Compare May 19, 2026 12:54
@github-actions github-actions Bot added size/XS This PR changes 0-9 lines, ignoring generated files and removed size/S This PR changes 10-29 lines, ignoring generated files labels May 19, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/system/postgres-rd/cozyrds/postgres.yaml`:
- Line 11: Generated CRD/chart schema in
packages/system/postgres-rd/cozyrds/postgres.yaml is out-of-date; run the
project generation step to update artifacts, then commit the regenerated
file(s). Specifically, run the repository generation command (make generate) to
refresh the generated schema that feeds postgres.yaml, verify the resulting
postgres.yaml changes are included, and re-run the pre-commit/checks to ensure
no further diffs remain before pushing the commit.
- Line 11: The description for postgresql.parameters is out of sync with the
schema: additionalProperties.anyOf now allows integers in addition to strings
but the text still says "All values must be strings (quote numbers: \"100\")".
Update the postgresql.parameters description to reflect that parameter values
may be strings or integers (remove the "quote numbers" instruction) and keep the
security/block list text unchanged; ensure the description specifically
references postgresql.parameters and additionalProperties.anyOf so reviewers can
verify the wording matches the schema.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bf58e731-079e-454c-b3f2-8975dffb849b

📥 Commits

Reviewing files that changed from the base of the PR and between f445b11 and 2d16e45.

📒 Files selected for processing (4)
  • packages/apps/postgres/templates/db.yaml
  • packages/apps/postgres/values.schema.json
  • packages/apps/postgres/values.yaml
  • packages/system/postgres-rd/cozyrds/postgres.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/apps/postgres/values.yaml
  • packages/apps/postgres/values.schema.json

Comment thread packages/system/postgres-rd/cozyrds/postgres.yaml Outdated
@github-actions github-actions Bot added size/S This PR changes 10-29 lines, ignoring generated files and removed size/XS This PR changes 0-9 lines, ignoring generated files labels May 19, 2026
@IvanHunters

Copy link
Copy Markdown
Collaborator Author

Repro: after running make generate -C packages/apps/postgres/ on this branch, both the anyOf in values.schema.json and in the embedded openAPISchema of postgres-rd/cozyrds/postgres.yaml revert to {type: string}. The description field also reverts to "All values must be strings (quote numbers: "100")" because values.yaml's description is the regeneration source.

This is the same regeneration trap that #2597 flags about #2466's original 6ec761f → 53caea3 revert. The pre-commit hook run-make-generate would catch this if it were exercised on this branch.

Fix has to land in the generator first: cozystack/cozyvalues-gen#25 (reopened) adds an intOrString alias for map values, producing additionalProperties:{anyOf:[{type:integer},{type:string}], x-kubernetes-int-or-string: true} from a @field {map[string]intOrString} annotation. After that ships, this PR's schema change becomes a 1-line edit in values.yaml annotation + make generate, and survives further regenerations.

The archive_cleanup_command / recovery_end_command additions to the template's $dangerousParams list are not affected by regeneration and can stand on their own — happy to split that out into a separate PR if it helps unblock #2597 partially while the generator change ships.

@github-actions github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files and removed size/S This PR changes 10-29 lines, ignoring generated files labels May 21, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/values.yaml`:
- Line 92: Update the CI step that installs cozyvalues-gen (referenced in
.github/workflows/pre-commit.yml) to a release that supports intOrString (so it
recognizes "intOrString"/x-kubernetes-int-or-string/intstr.IntOrString), then
locally run `make generate` for the packages/apps/postgres target and commit the
regenerated artifacts (this will update values.yaml-derived generated files to
match the new generator). Ensure the workflow change pins the newer
cozyvalues-gen version and the committed outputs reflect the regenerated files
for the postgres package.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dcbdef5e-d3b9-43e2-b296-73277cee7496

📥 Commits

Reviewing files that changed from the base of the PR and between 2d16e45 and 90902bf.

📒 Files selected for processing (5)
  • api/apps/v1alpha1/postgresql/types.go
  • packages/apps/postgres/README.md
  • packages/apps/postgres/values.schema.json
  • packages/apps/postgres/values.yaml
  • packages/system/postgres-rd/cozyrds/postgres.yaml
✅ Files skipped from review due to trivial changes (1)
  • packages/apps/postgres/README.md

Comment thread packages/apps/postgres/values.yaml
@myasnikovdaniil

Copy link
Copy Markdown
Contributor

Picked this up — the path you outlined worked cleanly.

cozyvalues-gen v1.4.0 shipped today with the intOrString alias (cozystack/cozyvalues-gen#25). Pushed 90902bf which flips values.yaml to @field {map[string]intOrString} and regenerates everything from there. Diff is the source annotation + four regenerated artifacts (values.schema.json, embedded openAPISchema in postgres-rd/cozyrds/postgres.yaml, README, and api/apps/v1alpha1/postgresql/types.go now uses intstr.IntOrString). Survives make generate — verified locally before pushing.

Bonus: the regenerated additionalProperties now also carries x-kubernetes-int-or-string: true, which the hand-patch was missing. That's the marker apiserver looks at when deciding whether to round-trip integers as integers vs. coerce to string, and matches the shape resources.cpu/memory/size already use in the same file.

Verified end-to-end:

  • helm template with max_connections: 200 (int) and max_connections: "200" (string) render to identical output (template's toString | quote still does the work)
  • archive_cleanup_command / recovery_end_command still rejected by the $dangerousParams check (kept as a single PR per your offer to split — happy to split if review prefers, but the release note already covers both)
  • make test in packages/apps/postgres/: 13/13 green

IvanHunters and others added 3 commits May 21, 2026 16:04
The template already coerces all parameter values to strings via
'toString | quote', but the JSON schema had 'additionalProperties: {type: string}'
which caused Helm schema validation to fail before the template ran
when users passed integer values (e.g. max_connections: 100).

Accept both string and integer types in additionalProperties so that
existing configurations with unquoted numbers continue to work.

Signed-off-by: IvanHunters <[email protected]>
Add archive_cleanup_command and recovery_end_command to the dangerous
parameters blocklist in db.yaml. Both are PostgreSQL shell-exec hooks
identical in threat model to archive_command/restore_command and must
not be settable by tenants.

Also update the BLOCKED list in values.schema.json, values.yaml, and
the embedded openAPISchema in cozyrds/postgres.yaml to reflect the
two new blocked parameters.

Additionally update the embedded openAPISchema in cozyrds/postgres.yaml
to allow integer values for postgresql.parameters (anyOf string|integer),
matching the fix already applied to values.schema.json.

Signed-off-by: IvanHunters <[email protected]>
…erator

Ivan's original schema edits were correct in intent but lived in the
generated artifacts (values.schema.json and the embedded openAPISchema
in postgres-rd/cozyrds/postgres.yaml). `make generate` reverts them
because values.yaml's `@field {map[string]string}` annotation is the
source of truth -- same regeneration trap that caught #2466 (issue #2597).

cozyvalues-gen v1.4.0 (cozystack/cozyvalues-gen#25, merged 2026-05-21)
adds an `intOrString` type alias for map values that emits
`additionalProperties.anyOf: [integer, string]` plus
`x-kubernetes-int-or-string: true`. Move the change to source by
flipping the annotation, then regenerate:

- packages/apps/postgres/values.yaml: `@field {map[string]intOrString}`
  with updated description noting integer values are accepted.
- packages/apps/postgres/values.schema.json: regenerated -- now carries
  anyOf + x-kubernetes-int-or-string: true.
- packages/system/postgres-rd/cozyrds/postgres.yaml: embedded
  openAPISchema regenerated to match.
- packages/apps/postgres/README.md: regenerated type column.
- api/apps/v1alpha1/postgresql/types.go: Parameters is now
  `map[string]intstr.IntOrString`.

The result survives future regenerations. The template's
`{{ \$value | toString | quote }}` already coerces both inputs to
quoted strings, verified end-to-end with helm template against both
`max_connections: 100` (int) and `max_connections: "100"` (string) --
identical render output. Dangerous-param block list still rejects
archive_cleanup_command and recovery_end_command additions from the
prior commit.

Co-Authored-By: IvanHunters <[email protected]>
Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
v1.4.0 adds the `intOrString` type alias used by the postgres package's
`postgresql.parameters` schema in this PR. CI must install v1.4.0 so the
"Verify generated code is up to date" check regenerates the same schema
shape (anyOf + x-kubernetes-int-or-string: true) rather than reverting
to {type: string}.

Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
Companion regen for the type change in the previous commit. The
deepcopy generator emits a fresh import for intstr and updates the
inner make() call to match the new map value type. Caught by the
codegen-drift check on CI.

Generated by `make generate` at repo root (hack/update-codegen.sh).

Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
@myasnikovdaniil
myasnikovdaniil force-pushed the fix/postgres-params-schema-int-or-string branch from 2cbb87f to 26db323 Compare May 21, 2026 11:26
@myasnikovdaniil
myasnikovdaniil merged commit e841688 into main May 22, 2026
10 checks passed
@myasnikovdaniil
myasnikovdaniil deleted the fix/postgres-params-schema-int-or-string branch May 22, 2026 03:58
@myasnikovdaniil myasnikovdaniil added the kind/backport Categorizes issue or PR as requiring a backport to the current release line label May 22, 2026
@github-actions

Copy link
Copy Markdown

myasnikovdaniil added a commit that referenced this pull request May 22, 2026
…resql.parameters in schema (#2715)

# Description
Backport of #2687 to `release-1.4`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) kind/backport Categorizes issue or PR as requiring a backport to the current release line kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants