feat(api): add Helm chart for docspec API v3.0.1#6
Conversation
📝 WalkthroughWalkthroughAdds a new Helm chart for the Changes
Sequence DiagramsequenceDiagram
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
5fb4669 to
ef591f1
Compare
ef591f1 to
2a1b9d5
Compare
There was a problem hiding this comment.
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/apiHelm chart (Deployment/Service/Ingress/HPA/PDB/NetworkPolicy + helpers/values/notes). - Adds a CI “parent” chart under
ci/apiintended to exercise the newapichart. - 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.
There was a problem hiding this comment.
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 | 🟠 MajorMake the usage examples follow the configured access mode.
The ingress branch hardcodes
/conversion, and thecurlexamples always point tolocalhost:4000. That makes the install notes wrong whenever ingress is enabled or.Values.ingress.pathis 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
⛔ Files ignored due to path filters (1)
charts/api/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/release-please.yml.release-please-manifest.jsoncharts/api/Chart.yamlcharts/api/templates/NOTES.txtcharts/api/templates/_helpers.tplcharts/api/templates/deployment.yamlcharts/api/templates/hpa.yamlcharts/api/templates/ingress.yamlcharts/api/templates/networkpolicy.yamlcharts/api/templates/pdb.yamlcharts/api/templates/service.yamlcharts/api/templates/serviceaccount.yamlcharts/api/values.yamlci/api/Chart.yamlci/api/templates/test-render.yamlci/api/values.yamlrelease-please-config.json
2a1b9d5 to
2253a85
Compare
|
Addressed all feedback: @StephanMeijer comments (lines 54, 58): Fixed quoting — both applied ✅ @copilot comments:
All changes squashed into 2253a85. |
aa576ea to
4f54c00
Compare
There was a problem hiding this comment.
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.
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
4f54c00 to
7c32a63
Compare
|
Addressed latest Copilot feedback in 7c32a63:
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
charts/api/templates/networkpolicy.yaml (1)
21-27:⚠️ Potential issue | 🟠 MajorDNS-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/UDPis still allowed. Please add ato: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: TCPIn 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
⛔ Files ignored due to path filters (1)
charts/api/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.github/workflows/lint.yaml.github/workflows/release-please.yml.github/workflows/test.yaml.release-please-manifest.jsoncharts/api/Chart.yamlcharts/api/templates/NOTES.txtcharts/api/templates/_helpers.tplcharts/api/templates/deployment.yamlcharts/api/templates/hpa.yamlcharts/api/templates/ingress.yamlcharts/api/templates/networkpolicy.yamlcharts/api/templates/pdb.yamlcharts/api/templates/service.yamlcharts/api/templates/serviceaccount.yamlcharts/api/values.yamlci/api/Chart.yamlci/api/templates/test-render.yamlci/api/values.yamlrelease-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
Summary
ghcr.io/docspecio/api:3.0.1)commonlibrary chart (ghcr.io/docspec/common)Chart Features
/healthValues Schema (Bitnami-aligned)
containerPorts.http— decoupled from service portresourcesPreset/resources— separate preset + override (Bitnami pattern)image.digest— digest-based pinning supportnetworkPolicy.allowExternal/allowExternalEgress— Bitnami NetworkPolicy patternpodDisruptionBudget.minAvailable/maxUnavailableCI/CD
ci/api/exercises all featuresrelease-please-config.jsonupdated withcharts/apicomponent.github/workflows/release-please.ymlrefactored: dynamicpaths_releasedloop (no hardcoded chart names)Security
runAsNonRoot,readOnlyRootFilesystem,drop: [ALL],seccompProfile: RuntimeDefaultQuick Start
Summary by CodeRabbit
New Features
Chores