Skip to content

fix(flowise): align template standards#666

Open
mberlofa wants to merge 3 commits into
mainfrom
fix/flowise-template-standards
Open

fix(flowise): align template standards#666
mberlofa wants to merge 3 commits into
mainfrom
fix/flowise-template-standards

Conversation

@mberlofa

@mberlofa mberlofa commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move Flowise template validation into a centralized helper and add validation unit coverage
  • expose startup delay controls for the main server and queue workers
  • make the queue+s3 CI fixture deterministic by avoiding first-boot migration races

Validation

  • helm unittest charts/flowise
  • helm lint --strict charts/flowise
  • make template-standards-check CHART=flowise
  • make standards-check CHART=flowise
  • make standards-guard CHART=flowise
  • make validate-chart CHART=flowise TIMEOUT=900: FULLY VALIDATED (17 layers)
  • make site-sync-check CHART=flowise
  • make release-check REPO=charts
  • make attribution-check REPO=charts

Site PR: helmforgedev/site#344
Issue: #633

Summary by CodeRabbit

  • New Features
    • Added configurable startup delays for the main Flowise server and queue-mode worker pods.
    • Reduced the default number of Flowise replicas for the S3 queue setup to 1.
  • Bug Fixes
    • Strengthened chart validation to prevent unsupported combinations of architecture, database/Redis, storage, replica sizing, and S3 settings.
    • Prevented user-supplied pod labels from overriding key selector labels.
  • Tests
    • Added/updated Helm tests to cover configurable startup delays and validation failures.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Standards Check (GR-079) — PASS

Every changed chart fully passes standards-check.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 937d92f7-778c-4a5a-8fcc-d7746a0547e7

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0ab8d and 328bb5c.

📒 Files selected for processing (8)
  • charts/flowise/ci/queue-s3.yaml
  • charts/flowise/templates/_helpers.tpl
  • charts/flowise/templates/validate.yaml
  • charts/flowise/tests/deployment_test.yaml
  • charts/flowise/tests/validation_test.yaml
  • charts/flowise/tests/worker-deployment_test.yaml
  • charts/flowise/values.schema.json
  • charts/flowise/values.yaml
✅ Files skipped from review due to trivial changes (1)
  • charts/flowise/ci/queue-s3.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
  • charts/flowise/tests/deployment_test.yaml
  • charts/flowise/tests/worker-deployment_test.yaml
  • charts/flowise/tests/validation_test.yaml
  • charts/flowise/values.yaml
  • charts/flowise/templates/_helpers.tpl

📝 Walkthrough

Walkthrough

This PR adds configurable startup delays for Flowise main and worker containers, then centralizes chart validation into a shared Helm template with new validation tests and a CI replica adjustment.

Changes

Flowise chart startup delay and validation refactor

Layer / File(s) Summary
Configurable startup delays
charts/flowise/values.yaml, charts/flowise/values.schema.json, charts/flowise/templates/_helpers.tpl, charts/flowise/tests/deployment_test.yaml, charts/flowise/tests/worker-deployment_test.yaml
Adds flowise.startupDelaySeconds and queue.worker.startupDelaySeconds, updates command templates to use those values, and adds tests for the rendered startup commands.
Shared validation template
charts/flowise/templates/_helpers.tpl, charts/flowise/templates/validate.yaml, charts/flowise/tests/validation_test.yaml, charts/flowise/ci/queue-s3.yaml
Moves validation into flowise.validate, makes validate.yaml include it, adds validation tests for the chart constraints, and changes the CI queue-s3 replica count to 1.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HelmRender
  participant validate.yaml
  participant flowise.validate
  HelmRender->>validate.yaml: render chart
  validate.yaml->>flowise.validate: include shared validation
  flowise.validate->>HelmRender: fail on invalid values
Loading
🚥 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 is concise and broadly matches the chart templating and validation-focused changes.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/flowise-template-standards

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

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🟢 Security Scan: flowise

Framework Score
MITRE + NSA + SOC2 81.818184%

✅ Security posture acceptable.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38319a6137

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread charts/flowise/templates/_helpers.tpl

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
charts/flowise/values.schema.json (1)

57-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Queue schema now partially documents worker fields.

Only worker.startupDelaySeconds gets schema coverage; sibling fields like queue.name, worker.replicaCount, concurrency, removeOnAge/Count, podLabels, podAnnotations, extraEnv, resources remain undocumented. Since this PR's goal is aligning with template standards, consider completing the queue/worker schema in one pass for consistency.

🤖 Prompt for 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.

In `@charts/flowise/values.schema.json` around lines 57 - 72, The queue schema is
only partially covered, leaving several worker-related settings undocumented.
Update the queue section in values.schema.json by expanding the queue and worker
object schemas to include the missing fields such as queue.name,
worker.replicaCount, concurrency, removeOnAge/removeOnCount, podLabels,
podAnnotations, extraEnv, and resources. Keep the schema definitions aligned
with the existing worker.startupDelaySeconds entry and the chart’s template
conventions so the full queue/worker configuration is validated consistently.
🤖 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.

Nitpick comments:
In `@charts/flowise/values.schema.json`:
- Around line 57-72: The queue schema is only partially covered, leaving several
worker-related settings undocumented. Update the queue section in
values.schema.json by expanding the queue and worker object schemas to include
the missing fields such as queue.name, worker.replicaCount, concurrency,
removeOnAge/removeOnCount, podLabels, podAnnotations, extraEnv, and resources.
Keep the schema definitions aligned with the existing worker.startupDelaySeconds
entry and the chart’s template conventions so the full queue/worker
configuration is validated consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 733939ab-8614-46a5-ae8d-f875ee70a89f

📥 Commits

Reviewing files that changed from the base of the PR and between 870b4c7 and 38319a6.

📒 Files selected for processing (8)
  • charts/flowise/ci/queue-s3.yaml
  • charts/flowise/templates/_helpers.tpl
  • charts/flowise/templates/validate.yaml
  • charts/flowise/tests/deployment_test.yaml
  • charts/flowise/tests/validation_test.yaml
  • charts/flowise/tests/worker-deployment_test.yaml
  • charts/flowise/values.schema.json
  • charts/flowise/values.yaml

@mberlofa mberlofa force-pushed the fix/flowise-template-standards branch from 084e3a2 to 6e0ab8d Compare July 4, 2026 13:17
@mberlofa mberlofa force-pushed the fix/flowise-template-standards branch from 6e0ab8d to 328bb5c Compare July 5, 2026 20:21
@mberlofa

mberlofa commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining CodeRabbit review-summary item for the queue schema.

Changes:

  • added schema coverage for queue.name
  • expanded queue.worker schema for replicaCount, concurrency, removeOnAge, removeOnCount, podLabels, podAnnotations, extraEnv, and resources

Validation:

  • make validate-chart CHART=flowise passed after rebasing on current origin/main
  • Result: flowise: FULLY VALIDATED (17 layers), including k3d GR-027 for ci/queue-s3.yaml
  • make release-check REPO=charts passed with only the expected post-merge release confirmation warning
  • make attribution-check REPO=charts passed
  • git diff --check passed

Note: this CodeRabbit item was posted in the review summary/body, not as an active review thread, so there is no thread ID to reply to or resolve for this specific item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant