Skip to content

feat(api): add Helm chart for docspec API v3.0.1#6

Merged
StephanMeijer merged 1 commit into
mainfrom
feat/docspec-chart
Apr 28, 2026
Merged

feat(api): add Helm chart for docspec API v3.0.1#6
StephanMeijer merged 1 commit into
mainfrom
feat/docspec-chart

Conversation

@StephanMeijer
Copy link
Copy Markdown
Contributor

@StephanMeijer StephanMeijer commented Apr 28, 2026

Summary

  • Add production-ready Helm chart for the docspec document conversion API (ghcr.io/docspecio/api:3.0.1)
  • Depends on the common library chart (ghcr.io/docspec/common)
  • Refactor release-please workflow to support multi-chart publishing

Chart Features

Feature Status Default
Deployment Restricted PSS, probes on /health
Service ClusterIP, port 4000
ServiceAccount Enabled
Ingress Disabled
HPA Disabled
PodDisruptionBudget Enabled
NetworkPolicy Enabled (Bitnami-style, DNS egress only)

Values Schema (Bitnami-aligned)

  • containerPorts.http — decoupled from service port
  • resourcesPreset / resources — separate preset + override (Bitnami pattern)
  • image.digest — digest-based pinning support
  • networkPolicy.allowExternal / allowExternalEgress — Bitnami NetworkPolicy pattern
  • podDisruptionBudget.minAvailable / maxUnavailable

CI/CD

  • CI test chart at ci/api/ exercises all features
  • release-please-config.json updated with charts/api component
  • .github/workflows/release-please.yml refactored: dynamic paths_released loop (no hardcoded chart names)

Security

  • Restricted Pod Security Standard: runAsNonRoot, readOnlyRootFilesystem, drop: [ALL], seccompProfile: RuntimeDefault
  • Runs as UID 1000 (matches upstream image)
  • NetworkPolicy: DNS-only egress by default

Quick Start

helm dependency update charts/api
helm install docspec charts/api --set ingress.enabled=true --set ingress.hostname=api.example.com

Summary by CodeRabbit

  • New Features

    • Added a new API service Helm chart with full Kubernetes manifests (Deployment, Service, Ingress, HPA, NetworkPolicy, PDB) and deployment notes for local/testing access.
    • Release automation now publishes multiple charts dynamically and gates publishing on successful workflow runs.
  • Chores

    • CI and lint workflows expanded to validate and template the new chart and authenticate with the registry.
    • Release config updated to include the new chart.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

Adds a new Helm chart for the api component and updates the release pipeline to publish multiple charts by paths (outputs: releases_created, paths_released), switching publishing to iterate released chart paths and push OCI packages to GHCR.

Changes

Cohort / File(s) Summary
Release Pipeline
.github/workflows/release-please.yml, .release-please-manifest.json, release-please-config.json
Gated release-please trigger to successful push on main. Replaced chart-specific outputs with releases_created/paths_released. Publish job now iterates paths_released, checks out head_sha, updates dependencies and publishes each chart to oci://ghcr.io/docspec. Added charts/api to manifest and release-please config.
API Helm Chart
charts/api/Chart.yaml, charts/api/values.yaml, charts/api/templates/_helpers.tpl, charts/api/templates/deployment.yaml, charts/api/templates/hpa.yaml, charts/api/templates/ingress.yaml, charts/api/templates/networkpolicy.yaml, charts/api/templates/pdb.yaml, charts/api/templates/service.yaml, charts/api/templates/serviceaccount.yaml, charts/api/templates/NOTES.txt
New complete Helm chart for api with metadata, values, helpers, and templates for Deployment, Service, Ingress, HPA, NetworkPolicy, PDB, ServiceAccount, and NOTES; uses common chart as dependency and OCI image registry for dependency resolution.
API Chart CI/Test
ci/api/Chart.yaml, ci/api/templates/test-render.yaml, ci/api/values.yaml
Adds a CI/test chart and values for rendering api in CI, plus a test ConfigMap template to validate templating.
Workflows: Lint & Test
.github/workflows/lint.yaml, .github/workflows/test.yaml
Added contents/packages read permissions, GHCR login, helm dependency update for charts/api and ci/api, and extended lint/templating steps to include the new api chart and CI scenarios.

Sequence Diagram

sequenceDiagram
    participant GH as "GitHub Actions"
    participant RP as "release-please"
    participant Git as "Git Repo"
    participant PKG as "Packaging/Publish"
    participant GHCR as "GHCR"

    GH->>RP: workflow_run (push to main, success)
    RP->>Git: inspect commits / changed packages
    Git-->>RP: return paths_released (e.g., charts/common, charts/api)
    RP-->>GH: set outputs releases_created & paths_released

    GH->>PKG: for each path in paths_released
    PKG->>PKG: helm dependency update <path>
    PKG->>PKG: helm package <path>
    PKG->>GHCR: push OCI artifact to oci://ghcr.io/docspec
    GHCR-->>PKG: publish confirmation
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 A tiny chart hops into place,

tags turn paths into a race,
Packages roll and pitchers sing,
OCI bells now, hoppity-ting,
Pipelines stitch charts into space. ✨

🚥 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 pull request title clearly and concisely summarizes the main change: adding a new Helm chart for the docspec API v3.0.1, which is the primary objective evidenced throughout the changeset.
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 feat/docspec-chart

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

Comment thread .github/workflows/release-please.yml Outdated
Comment thread .github/workflows/release-please.yml
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new production Helm chart for the docspec API (v3.0.1) and updates release automation to support publishing multiple charts from the repository.

Changes:

  • Introduces charts/api Helm chart (Deployment/Service/Ingress/HPA/PDB/NetworkPolicy + helpers/values/notes).
  • Adds a CI “parent” chart under ci/api intended to exercise the new api chart.
  • Extends release-please config/manifest and refactors the publish workflow to push all released charts dynamically.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
release-please-config.json Adds charts/api as a release-please package/component.
.release-please-manifest.json Tracks initial version for charts/api.
.github/workflows/release-please.yml Publishes multiple charts based on paths_released.
charts/api/Chart.yaml Defines the new API chart metadata and dependency on common.
charts/api/Chart.lock Locks common dependency version for the API chart.
charts/api/values.yaml Default values for the API chart (security context, probes, NP, PDB, etc.).
charts/api/templates/_helpers.tpl Helper wrappers around common library templates + api.image.
charts/api/templates/deployment.yaml API Deployment with restricted security context, probes, env, resources preset.
charts/api/templates/service.yaml ClusterIP service exposing HTTP.
charts/api/templates/serviceaccount.yaml Optional ServiceAccount for the workload.
charts/api/templates/ingress.yaml Optional Ingress support.
charts/api/templates/hpa.yaml Optional HPA support.
charts/api/templates/pdb.yaml Optional PodDisruptionBudget support.
charts/api/templates/networkpolicy.yaml Optional NetworkPolicy with DNS-only egress by default.
charts/api/templates/NOTES.txt Post-install usage instructions.
ci/api/Chart.yaml Adds a CI test chart that depends on charts/api.
ci/api/values.yaml CI values intended to exercise chart features.
ci/api/templates/test-render.yaml CI render template that calls api helper templates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ci/api/Chart.yaml
Comment thread release-please-config.json
Comment thread charts/api/templates/pdb.yaml
Comment thread .github/workflows/release-please.yml
Comment thread charts/api/templates/serviceaccount.yaml
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
charts/api/templates/NOTES.txt (1)

1-18: ⚠️ Potential issue | 🟠 Major

Make the usage examples follow the configured access mode.

The ingress branch hardcodes /conversion, and the curl examples always point to localhost:4000. That makes the install notes wrong whenever ingress is enabled or .Values.ingress.path is customized.

♻️ Proposed fix
 Get the API URL:
 {{- if .Values.ingress.enabled }}
-  http{{ if .Values.ingress.tls }}s{{ end }}://{{ .Values.ingress.hostname }}/conversion
+  http{{ if .Values.ingress.tls }}s{{ end }}://{{ .Values.ingress.hostname }}{{ .Values.ingress.path }}
 {{- else }}
   kubectl port-forward svc/{{ include "api.names.fullname" . }} 4000:{{ .Values.service.port }} -n {{ include "api.names.namespace" . }}
   Then: http://localhost:4000/conversion
 {{- end }}

 Health check:
-  curl http://localhost:4000/health
+{{- if .Values.ingress.enabled }}
+  curl http{{ if .Values.ingress.tls }}s{{ end }}://{{ .Values.ingress.hostname }}/health
+{{- else }}
+  curl http://localhost:4000/health
+{{- end }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/api/templates/NOTES.txt` around lines 1 - 18, Update the NOTES.txt
usage examples to respect configured access mode and path: when
.Values.ingress.enabled is true, construct the URL using http{{ if
.Values.ingress.tls }}s{{ end }}://{{ .Values.ingress.hostname }}{{-
.Values.ingress.path }} (or default to /conversion) instead of hardcoding
/conversion, and update the displayed curl examples to use that ingress URL;
when ingress is disabled keep the kubectl port-forward example but build the
local URL using the forwarded port (4000) and include the same path variable
(.Values.ingress.path or default) so the health and conversion curl commands
reference the correct path and host/port based on the values used in the
template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@charts/api/templates/networkpolicy.yaml`:
- Around line 17-29: The egress rule currently allowing ports 53/TCP/UDP is too
broad because it permits any destination on port 53; update the egress block in
networkpolicy.yaml (the section gated by
.Values.networkPolicy.allowExternalEgress and .Values.networkPolicy.extraEgress)
to restrict traffic to only cluster DNS endpoints by adding an explicit "to"
selector targeting the cluster DNS pods/service (e.g., match labels or namespace
for kube-dns/coredns) or call the chart's DNS-only helper if available (reuse a
helper like dnsOnly or similar in common templates) so egress rules include both
ports and a to: clause limiting destinations to DNS pods/services.

In `@charts/api/values.yaml`:
- Around line 74-77: podDisruptionBudget is enabled but minAvailable and
maxUnavailable are empty strings so charts/api/templates/pdb.yaml renders no
valid threshold and the install fails; update the values.yaml
podDisruptionBudget block to provide a valid default (e.g. set minAvailable: 1
or maxUnavailable: "1") or disable the PDB by default (set enabled: false) so
the template renders a valid PDB; change the
podDisruptionBudget.enabled/minAvailable/maxUnavailable entries accordingly.

---

Outside diff comments:
In `@charts/api/templates/NOTES.txt`:
- Around line 1-18: Update the NOTES.txt usage examples to respect configured
access mode and path: when .Values.ingress.enabled is true, construct the URL
using http{{ if .Values.ingress.tls }}s{{ end }}://{{ .Values.ingress.hostname
}}{{- .Values.ingress.path }} (or default to /conversion) instead of hardcoding
/conversion, and update the displayed curl examples to use that ingress URL;
when ingress is disabled keep the kubectl port-forward example but build the
local URL using the forwarded port (4000) and include the same path variable
(.Values.ingress.path or default) so the health and conversion curl commands
reference the correct path and host/port based on the values used in the
template.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91ff8ac5-5b79-492b-b987-a218aff7408c

📥 Commits

Reviewing files that changed from the base of the PR and between e22fe32 and 5fb4669.

⛔ Files ignored due to path filters (1)
  • charts/api/Chart.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • .github/workflows/release-please.yml
  • .release-please-manifest.json
  • charts/api/Chart.yaml
  • charts/api/templates/NOTES.txt
  • charts/api/templates/_helpers.tpl
  • charts/api/templates/deployment.yaml
  • charts/api/templates/hpa.yaml
  • charts/api/templates/ingress.yaml
  • charts/api/templates/networkpolicy.yaml
  • charts/api/templates/pdb.yaml
  • charts/api/templates/service.yaml
  • charts/api/templates/serviceaccount.yaml
  • charts/api/values.yaml
  • ci/api/Chart.yaml
  • ci/api/templates/test-render.yaml
  • ci/api/values.yaml
  • release-please-config.json

Comment thread charts/api/templates/networkpolicy.yaml
Comment thread charts/api/values.yaml Outdated
@StephanMeijer
Copy link
Copy Markdown
Contributor Author

Addressed all feedback:

@StephanMeijer comments (lines 54, 58): Fixed quoting — both applied ✅

@copilot comments:

  1. CI coverage for ci/api/: Updated lint.yaml and test.yaml to include api chart ✅
  2. changelog-path: release-please auto-creates the CHANGELOG on first release — no manual file needed. Keeping as-is.
  3. PDB defaults: Set maxUnavailable: 1 as default ✅
  4. Checkout ref: Added ref: ${{ github.event.workflow_run.head_sha }} to pin checkout ✅
  5. ServiceAccount annotations: This follows Bitnami pattern — serviceAccount.annotations is intentionally separate from commonAnnotations (SA-specific annotations like IAM role ARNs vs resource-wide metadata). Keeping as-is.

All changes squashed into 2253a85.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ci/api/values.yaml
Comment thread .github/workflows/release-please.yml Outdated
Comment thread charts/api/templates/NOTES.txt Outdated
Comment thread .github/workflows/release-please.yml
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
@StephanMeijer
Copy link
Copy Markdown
Contributor Author

Addressed latest Copilot feedback in 7c32a63:

  1. CI values PDB conflict (ci/api/values.yaml:49): Added maxUnavailable: "" to override default ✅
  2. helm push glob quoting (release-please.yml:60): Removed quotes, added --destination . for explicit output dir ✅
  3. workflow_run guard (release-please.yml:14): Added github.event.workflow_run.event == 'push' && head_branch == 'main' guard ✅
  4. NOTES.txt hardcoded port: Now uses .Values.service.port dynamically ✅

@StephanMeijer StephanMeijer requested a review from Copilot April 28, 2026 15:45
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
charts/api/templates/networkpolicy.yaml (1)

21-27: ⚠️ Potential issue | 🟠 Major

DNS-only egress is still too broad without destination selectors.

Line 21–25 restricts ports but not destinations, so traffic to any endpoint on 53/TCP,53/UDP is still allowed. Please add a to: selector for cluster DNS endpoints (or reuse the common-chart DNS helper if available).

Proposed fix
-    - ports:
+    - to:
+        - namespaceSelector:
+            matchLabels:
+              kubernetes.io/metadata.name: kube-system
+          podSelector:
+            matchExpressions:
+              - key: k8s-app
+                operator: In
+                values: ["kube-dns", "coredns"]
+      ports:
         - port: 53
           protocol: UDP
         - port: 53
           protocol: TCP
In Kubernetes NetworkPolicy, if an egress rule specifies only ports (like TCP/UDP 53) and omits `to`, does it allow those ports to any destination?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/api/templates/networkpolicy.yaml` around lines 21 - 27, The egress
rule in the network policy currently restricts only the ports (53 TCP/UDP) but
lacks a destination selector, which allows traffic to those ports on any
endpoint. To fix this, modify the egress rule in the networkpolicy.yaml template
by adding a `to:` field that restricts egress traffic to the cluster DNS
endpoints using the relevant selector (or reuse the common-chart DNS helper if
available). This ensures that only DNS traffic to the cluster's DNS servers is
allowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@charts/api/templates/networkpolicy.yaml`:
- Around line 21-27: The egress rule in the network policy currently restricts
only the ports (53 TCP/UDP) but lacks a destination selector, which allows
traffic to those ports on any endpoint. To fix this, modify the egress rule in
the networkpolicy.yaml template by adding a `to:` field that restricts egress
traffic to the cluster DNS endpoints using the relevant selector (or reuse the
common-chart DNS helper if available). This ensures that only DNS traffic to the
cluster's DNS servers is allowed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 35daa502-3c7e-46dc-8ad2-040330e1f0c3

📥 Commits

Reviewing files that changed from the base of the PR and between 5fb4669 and 7c32a63.

⛔ Files ignored due to path filters (1)
  • charts/api/Chart.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • .github/workflows/lint.yaml
  • .github/workflows/release-please.yml
  • .github/workflows/test.yaml
  • .release-please-manifest.json
  • charts/api/Chart.yaml
  • charts/api/templates/NOTES.txt
  • charts/api/templates/_helpers.tpl
  • charts/api/templates/deployment.yaml
  • charts/api/templates/hpa.yaml
  • charts/api/templates/ingress.yaml
  • charts/api/templates/networkpolicy.yaml
  • charts/api/templates/pdb.yaml
  • charts/api/templates/service.yaml
  • charts/api/templates/serviceaccount.yaml
  • charts/api/values.yaml
  • ci/api/Chart.yaml
  • ci/api/templates/test-render.yaml
  • ci/api/values.yaml
  • release-please-config.json
✅ Files skipped from review due to trivial changes (7)
  • release-please-config.json
  • ci/api/Chart.yaml
  • ci/api/templates/test-render.yaml
  • charts/api/Chart.yaml
  • .release-please-manifest.json
  • charts/api/templates/NOTES.txt
  • charts/api/templates/pdb.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
  • charts/api/templates/hpa.yaml
  • charts/api/templates/_helpers.tpl
  • charts/api/templates/deployment.yaml
  • charts/api/templates/service.yaml
  • ci/api/values.yaml
  • charts/api/values.yaml
  • charts/api/templates/ingress.yaml

@StephanMeijer StephanMeijer merged commit a83280c into main Apr 28, 2026
5 checks passed
This was referenced Apr 28, 2026
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