Conversation
Pristine `helm create charts/ogis` output. Subsequent commits layer OGIS-specific changes on top so the divergence from upstream is easy to audit and re-apply if the template ever needs regenerating.
Description, sources, keywords, and maintainers. appVersion pinned to the current OGIS Rust binary (0.2.0). kubeVersion set to >=1.23 since the templates use autoscaling/v2 which is GA from 1.23.
- image.repository defaults to twango/ogis - containerPort hardcoded to 3000 (OGIS binary listens here regardless of service.port, which stays at 80 for the service-side mapping) - livenessProbe and readinessProbe hit /health - helm test pod hits /health
Adds `config.logLevel`, `config.defaults.{title,description,subtitle,logo}`,
`extraEnv`, and `extraEnvFrom` to values.yaml. The deployment now sources
its env from a new `ogis.env` helper which composes everything together —
single edit point when more knobs are added later.
- hmac.{enabled,value,existingSecret,existingSecretKey} in values
- templates/secret.yaml renders only when chart-managed (BYO existingSecret
skips creation) and fails fast if neither value nor existingSecret is set
- ogis.env helper adds OGIS_HMAC_SECRET via secretKeyRef
Today only `cache.backend: memory` is supported (in-process Moka). The shape leaves room to add `cache.redis` / `cache.memcached` blocks later without breaking existing values.yaml files. The `ogis.cacheEnv` helper is the single edit point for wiring backend- specific env vars when those backends land — deployment.yaml never has to change.
Optional NetworkPolicy (default off) that allows ingress from anywhere, DNS egress to kube-dns, and public-internet egress with RFC1918 + the cloud metadata IP (169.254.0.0/16) excluded. K8s-side belt to the in-process SSRF blocklist in src/image/resolver.rs — no-op on clusters without a CNI that enforces NetworkPolicy.
Optional PDB (default off) that's automatically skipped when running a single replica — disruption budgets on a 1-replica deployment block all voluntary evictions, which is almost never what you want. Renders only when podDisruptionBudget.enabled and (autoscaling.enabled || replicaCount > 1).
Quick start, common configurations (cert-manager, HMAC BYO-secret, autoscaling, NetworkPolicy, extraEnv), and a structure overview noting which files are vanilla helm-create scaffolding vs OGIS-specific additions.
Adds charts/ogis as a separate package with release-type=helm so the chart releases on its own cadence (driven by conventional commits scoped to charts/ogis/) and produces chart-vX.Y.Z tags that don't collide with the rust app's vX.Y.Z tags. Chart's appVersion stays manually managed in chart commits when pointing at a newer image release.
helm-lint runs on every push/PR alongside the other lint jobs — strict lint plus default and all-features template renders. helm-publish runs after the chart's own release-please component creates a release (gated on `chart_release_created`, independent of the rust app's release flow). Packages and pushes to oci://ghcr.io/<owner>/charts using the workflow's GITHUB_TOKEN with packages:write — no extra secrets.
In manifest mode, the root path "." is special-cased to receive every commit in the repo (src/manifest.ts: `path === ROOT_PROJECT_PATH ? commits : splitCommits[path]`). Without exclude-paths, a commit like `feat(chart): add ServiceMonitor` that only touched charts/ogis/ would bump the rust app's minor version too. Also drops the redundant `package-name: ogis` — the helm strategy auto-detects it from Chart.yaml's `name` field (src/strategies/helm.ts#getDefaultPackageName).
Contributor
📝 WalkthroughWalkthroughThis PR adds a complete Helm chart at charts/ogis (metadata, values, templates, helpers, docs, notes, and tests) and augments release automation: CI helm-lint, release-please outputs for chart metadata, and a helm-publish job that packages and pushes the released chart to GHCR. ChangesHelm Chart for OGIS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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)
⚔️ Resolve merge conflicts
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@charts/ogis/templates/deployment.yaml`:
- Around line 25-30: Add a pod-level automount override in the Deployment so the
chart honors .Values.serviceAccount.automount even when
serviceAccount.create=false: in charts/ogis/templates/deployment.yaml, under
spec.template.spec (next to imagePullSecrets and serviceAccountName which uses
include "ogis.serviceAccountName"), add automountServiceAccountToken: {{
.Values.serviceAccount.automount | default false }} (rendered as an unquoted
boolean) so the pod enforces the desired automount behavior regardless of
whether the ServiceAccount object is created by the chart.
In `@charts/ogis/templates/hpa.yaml`:
- Around line 15-31: The HPA template currently always outputs a metrics: header
while the actual metric entries are conditional, which can render an empty
invalid block; update the template around the metrics section to only emit the
entire metrics: block when at least one of
.Values.autoscaling.targetCPUUtilizationPercentage or
.Values.autoscaling.targetMemoryUtilizationPercentage is set, and add a template
check that fails (helm template/lookup error) or omits metrics entirely when
neither is configured so the HPA either receives valid metric entries or no
metrics field; locate the metrics block in charts/ogis/templates/hpa.yaml (the
lines containing metrics:, the CPU block referencing
.Values.autoscaling.targetCPUUtilizationPercentage, and the memory block
referencing .Values.autoscaling.targetMemoryUtilizationPercentage) and wrap them
in a single conditional guard that emits metrics: plus entries only when one or
both values are present, otherwise abort or omit.
In `@charts/ogis/templates/ingress.yaml`:
- Around line 31-35: The Ingress template conditionally emits pathType causing
invalid networking.k8s.io/v1 manifests; always render a pathType for each path
in the range over .paths. Update the template loop that uses "{{- range .paths
}}" and the optional "{{- with .pathType }} pathType: {{ . }} {{- end }}" so it
always outputs a pathType value (e.g., defaulting to "ImplementationSpecific"
when .pathType is empty) for each path entry to satisfy v1 Ingress requirements.
In `@charts/ogis/templates/networkpolicy.yaml`:
- Around line 12-14: The NetworkPolicy currently includes an allow-all ingress
rule via "ingress: - {}", which defeats egress-only hardening; update the
NetworkPolicy by changing the "policyTypes" field to only include "Egress" and
remove the entire "ingress" section so the manifest only declares egress
restrictions (refer to the "policyTypes" key and the "ingress" block in
networkpolicy.yaml to locate and remove it).
- Around line 27-35: The egress ipBlock currently allows all IPv6 traffic
because only an IPv4 cidr (0.0.0.0/0 with RFC1918 excepts) is defined; update
the same "to: - ipBlock:" section in networkpolicy.yaml to add a second ipBlock
entry for IPv6 with cidr: ::/0 and excepts mirroring the IPv4 list (at minimum
::1, fe80::/10, fc00::/7) so loopback, link-local and ULA ranges are excluded
for IPv6 just as they are for IPv4.
In `@charts/ogis/templates/pdb.yaml`:
- Line 1: The PDB gating currently renders whenever .Values.autoscaling.enabled
is true even if the autoscaler minReplicas is 1; update the template condition
that uses .Values.podDisruptionBudget.enabled and the autoscaling/replica checks
so it only renders when either .Values.replicaCount > 1 OR autoscaling is
enabled AND .Values.autoscaling.minReplicas > 1; specifically replace the
existing (or .Values.autoscaling.enabled (gt (int .Values.replicaCount) 1))
logic with an OR that checks (gt (int .Values.replicaCount) 1) or (and
.Values.autoscaling.enabled (gt (int .Values.autoscaling.minReplicas) 1)) so the
PDB is skipped for effectively single-replica autoscaled workloads.
In `@charts/ogis/templates/secret.yaml`:
- Line 8: The Secret name currently appends "-hmac" to the already-truncated
ogis.fullname which can exceed Kubernetes' 63-char limit; change the template so
the final string (printf "%s-hmac" (include "ogis.fullname" .)) is truncated to
63 characters (use the templating truncation function) and use that truncated
value as the name to guarantee it remains a valid DNS label; update the name
line that references include "ogis.fullname" accordingly.
🪄 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: d09ca387-fd36-41d3-b9ae-3a78ea628978
📒 Files selected for processing (18)
.github/workflows/rust.yml.release-please-manifest.jsoncharts/ogis/.helmignorecharts/ogis/Chart.yamlcharts/ogis/README.mdcharts/ogis/templates/NOTES.txtcharts/ogis/templates/_helpers.tplcharts/ogis/templates/deployment.yamlcharts/ogis/templates/hpa.yamlcharts/ogis/templates/ingress.yamlcharts/ogis/templates/networkpolicy.yamlcharts/ogis/templates/pdb.yamlcharts/ogis/templates/secret.yamlcharts/ogis/templates/service.yamlcharts/ogis/templates/serviceaccount.yamlcharts/ogis/templates/tests/test-connection.yamlcharts/ogis/values.yamlrelease-please-config.json
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class Kubernetes deployment support by introducing a Helm chart for OGIS and wiring it into the existing release automation so chart releases can be independently versioned and published to GHCR.
Changes:
- Add a new Helm chart at
charts/ogis/(scaffold + OGIS-specific templates/values for HMAC, PDB, NetworkPolicy, etc.). - Register
charts/ogisas an independentrelease-pleasecomponent (helm release-type) and add it to the release manifest. - Extend CI to lint/template the Helm chart and publish it to GHCR as an OCI artifact when a chart release is created.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| release-please-config.json | Excludes chart path from the Rust package release and adds a dedicated helm component for charts/ogis. |
| .release-please-manifest.json | Tracks independent versioning for the new charts/ogis component. |
| .github/workflows/rust.yml | Adds Helm lint/render checks and a conditional GHCR Helm publish job tied to release-please chart releases. |
| charts/ogis/Chart.yaml | Defines chart metadata, versions, and Artifact Hub annotations. |
| charts/ogis/values.yaml | Provides default values, including OGIS-specific configuration knobs (HMAC, cache extension point, NetworkPolicy, PDB). |
| charts/ogis/README.md | Documents installation and common configurations for the Helm chart. |
| charts/ogis/.helmignore | Standard Helm ignore patterns for packaging. |
| charts/ogis/templates/_helpers.tpl | Defines naming/label helpers and composes container env vars (including HMAC wiring). |
| charts/ogis/templates/deployment.yaml | Deploys the OGIS container, exposes port 3000, and wires probes/resources/env. |
| charts/ogis/templates/service.yaml | Exposes the Deployment via a Kubernetes Service. |
| charts/ogis/templates/serviceaccount.yaml | Creates an optional ServiceAccount with configurable annotations/automount. |
| charts/ogis/templates/ingress.yaml | Optional Ingress support using networking.k8s.io/v1. |
| charts/ogis/templates/hpa.yaml | Optional autoscaling via autoscaling/v2 HPA. |
| charts/ogis/templates/pdb.yaml | Optional PodDisruptionBudget to limit voluntary disruptions. |
| charts/ogis/templates/networkpolicy.yaml | Optional egress-lockdown NetworkPolicy intended to complement SSRF protections. |
| charts/ogis/templates/secret.yaml | Optional chart-managed Secret for HMAC authentication. |
| charts/ogis/templates/NOTES.txt | Post-install instructions for accessing the service. |
| charts/ogis/templates/tests/test-connection.yaml | Helm test pod to validate service connectivity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+105
to
+107
| name: {{ include "ogis.hmacSecretName" . }} | ||
| key: {{ .Values.hmac.existingSecretKey | default "secret" }} | ||
| {{- end }} |
Comment on lines
+16
to
+24
| # DNS to kube-dns | ||
| - to: | ||
| - namespaceSelector: {} | ||
| podSelector: | ||
| matchLabels: | ||
| k8s-app: kube-dns | ||
| ports: | ||
| - { protocol: UDP, port: 53 } | ||
| - { protocol: TCP, port: 53 } |
| # This is a YAML-formatted file. | ||
| # Declare variables to be passed into your templates. | ||
|
|
||
| # This will set the replicaset count more information can be found here: https://kubernetes.io/docs/concepts/workloads/controllers/replicaset/ |
| # Overrides the image tag whose default is the chart appVersion. | ||
| tag: "" | ||
|
|
||
| # This is for the secretes for pulling an image from a private repository more information can be found here: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/ |
Comment on lines
+1
to
+9
| {{- if and .Values.podDisruptionBudget.enabled (or .Values.autoscaling.enabled (gt (int .Values.replicaCount) 1)) -}} | ||
| apiVersion: policy/v1 | ||
| kind: PodDisruptionBudget | ||
| metadata: | ||
| name: {{ include "ogis.fullname" . }} | ||
| labels: | ||
| {{- include "ogis.labels" . | nindent 4 }} | ||
| spec: | ||
| minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} |
The chart-managed Secret hardcodes its key to `secret`, but the helper was also passing existingSecretKey through when no existingSecret was set — causing the Deployment to look up a non-existent key and HMAC auth to silently break. Now existingSecretKey only applies in BYO mode. Caught in PR review by Copilot.
Previous gate rendered the PDB whenever autoscaling was enabled, even with the default `minReplicas: 1` — which blocks evictions on a single-pod deployment. Now matches the README's "auto-skipped at one replica" promise. Caught in PR review by both CodeRabbit and Copilot.
Previously the toggle only applied to the chart-managed ServiceAccount object, so users reusing an existing ServiceAccount (serviceAccount.create=false) got whatever automount behavior that SA was configured with — the chart value was silently ignored. Pod-level automountServiceAccountToken takes precedence in Kubernetes, making it the reliable enforcement point. Caught in PR review by CodeRabbit.
ogis.fullname is already truncated to 63 chars; appending '-hmac' could push it past Kubernetes' 63-char DNS label limit. Re-truncate in both places that build the chart-managed secret name (secret.yaml and the hmacSecretName helper). Caught in PR review by CodeRabbit.
networking.k8s.io/v1 requires pathType on every path. Helm-create's
default uses a {{ with }} guard, so a user-supplied ingress.hosts entry
that omits pathType would render an invalid manifest. Default to
ImplementationSpecific (matching helm-create's example value) when
unset.
Caught in PR review by CodeRabbit.
If both targetCPU and targetMemory utilization are unset (e.g. user explicitly nulls the CPU default to use only a custom external metric they wire in elsewhere), the previous template rendered an empty 'metrics:' block — invalid per the HPA spec. Fail fast with a clear error instead. Caught in PR review by CodeRabbit.
Three changes per PR review: - Drop the allow-all ingress rule and set policyTypes to [Egress] only. The chart's intent is egress hardening; leaving ingress to other NetworkPolicies in the namespace is the correct boundary. - Restrict the DNS egress rule to pods in kube-system. Previously the empty namespaceSelector matched any namespace where a pod happened to carry the k8s-app=kube-dns label, which is easier to bypass. - Add IPv6 egress with the equivalent loopback / link-local / ULA exceptions to keep the SSRF mirror meaningful on dual-stack clusters. Caught in PR review by both CodeRabbit (#4, #5, #7) and Copilot (#9).
'replicaset' -> 'replica count', 'secretes' -> 'secrets'. Both inherited from the helm-create scaffold; could be upstreamed. Caught in PR review by Copilot.
Contributor
There was a problem hiding this comment.
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 `@charts/ogis/templates/ingress.yaml`:
- Around line 26-40: The template always emits a top-level rules: block even
when .Values.ingress.hosts is empty, producing an invalid Ingress; fix by
failing fast using Helm's required check before rendering rules. Add a
required(...) guard that validates .Values.ingress.hosts (e.g. assign $hosts :=
required "ingress.hosts must be set and non-empty to render Ingress"
.Values.ingress.hosts) and then iterate over $hosts for the rules block; keep
the existing references to include "ogis.fullname" and $.Values.service.port
when building each backend. Ensure the template errors out with that clear
message instead of emitting an empty rules: section.
🪄 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: 4b9d68b7-a2f9-474d-a161-a075da95f6d3
📒 Files selected for processing (8)
charts/ogis/templates/_helpers.tplcharts/ogis/templates/deployment.yamlcharts/ogis/templates/hpa.yamlcharts/ogis/templates/ingress.yamlcharts/ogis/templates/networkpolicy.yamlcharts/ogis/templates/pdb.yamlcharts/ogis/templates/secret.yamlcharts/ogis/values.yaml
✅ Files skipped from review due to trivial changes (1)
- charts/ogis/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/ogis/templates/_helpers.tpl
Comment on lines
+26
to
+40
| rules: | ||
| {{- range .Values.ingress.hosts }} | ||
| - host: {{ .host | quote }} | ||
| http: | ||
| paths: | ||
| {{- range .paths }} | ||
| - path: {{ .path }} | ||
| pathType: {{ .pathType | default "ImplementationSpecific" }} | ||
| backend: | ||
| service: | ||
| name: {{ include "ogis.fullname" $ }} | ||
| port: | ||
| number: {{ $.Values.service.port }} | ||
| {{- end }} | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Kubernetes networking.k8s.io/v1 Ingress, must at least one of spec.rulesorspec.defaultBackend be specified?
💡 Result:
Yes. For the Ingress resource in apiVersion networking.k8s.io/v1, if you do not specify spec.rules, then spec.defaultBackend must be specified. Otherwise (if spec.defaultBackend is not set), the behavior for requests that don’t match any rule is left to the ingress controller. [1][2] In other words: - Provide spec.rules: then spec.defaultBackend is optional. - Provide no spec.rules: then spec.defaultBackend is required. [1][3]
Citations:
- 1: https://kubernetes.io/docs/concepts/services-networking/ingress/
- 2: https://kubernetes.io/docs/reference/kubernetes-api/service-resources/ingress-v1
- 3: https://v1-30.docs.kubernetes.io/docs/concepts/services-networking/ingress
Fail fast when ingress.hosts is empty to avoid rendering an invalid Ingress.
At line 26, rules: is always emitted. If .Values.ingress.hosts is empty, the template produces an Ingress with no rules and no defaultBackend, which violates the Kubernetes spec requirement that at least one of these must be specified. Add a validation guard so rendering fails with a clear error message instead of producing an invalid manifest.
Suggested minimal fix
{{- if .Values.ingress.enabled -}}
+{{- if not .Values.ingress.hosts }}
+{{- fail "ingress.hosts must contain at least one host when ingress.enabled=true" }}
+{{- end }}
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:🤖 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/ogis/templates/ingress.yaml` around lines 26 - 40, The template always
emits a top-level rules: block even when .Values.ingress.hosts is empty,
producing an invalid Ingress; fix by failing fast using Helm's required check
before rendering rules. Add a required(...) guard that validates
.Values.ingress.hosts (e.g. assign $hosts := required "ingress.hosts must be set
and non-empty to render Ingress" .Values.ingress.hosts) and then iterate over
$hosts for the rules block; keep the existing references to include
"ogis.fullname" and $.Values.service.port when building each backend. Ensure the
template errors out with that clear message instead of emitting an empty rules:
section.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Helm chart at
charts/ogis/(scaffolded fromhelm create, then layered with OGIS-specific deltas) and registers it with release-please as an independent helm component publishing tooci://ghcr.io/twangodev/charts/ogis.Summary by CodeRabbit