Open
Conversation
…, serviceaccount)
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Helm chart "aisix" with chart metadata, values, helpers, Kubernetes templates (Deployment, Services, Ingresses, HPA, ServiceAccount, ConfigMap, Secret), NOTES, .helmignore, and a README entry linking the chart from the repo root. Changes
Sequence Diagram(s)sequenceDiagram
participant Operator as "Operator (helm)"
participant Chart as "Helm Chart (aisix)"
participant Kube as "Kubernetes API"
participant Pod as "AISIX Pod"
participant Etcd as "etcd (bundled/external)"
Operator->>Chart: helm install/upgrade
Chart->>Kube: render & apply manifests (ServiceAccount, Services, ConfigMap, Secret, Deployment, Ingress, HPA)
Kube-->>Chart: acknowledge resources
Kube->>Pod: schedule & start pod(s)
Pod->>Kube: mount ConfigMap/Secret, become ready
Pod->>Etcd: connect using configured hosts/scheme
Etcd-->>Pod: respond to requests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new charts/aisix Helm chart to deploy AISIX AI Gateway on Kubernetes, including optional bundled etcd and standard Kubernetes resources (Deployment/Services/Ingress/HPA/SA).
Changes:
- Introduces a complete AISIX Helm chart (Chart.yaml/values.yaml/templates/README/NOTES) with proxy + admin endpoints.
- Adds bitnami/etcd as an optional dependency (vendored
.tgz+ Chart.lock). - Updates the repo root README to list the new chart.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/aisix/Chart.yaml | Defines the AISIX chart metadata and etcd dependency. |
| charts/aisix/Chart.lock | Locks the etcd dependency version/digest. |
| charts/aisix/.helmignore | Helm ignore patterns for chart packaging. |
| charts/aisix/README.md | Chart usage and configuration documentation. |
| charts/aisix/values.yaml | Default configuration for AISIX, services, ingress, autoscaling, and etcd subchart. |
| charts/aisix/templates/_helpers.tpl | Naming/label helpers and etcd host construction helper. |
| charts/aisix/templates/configmap.yaml | Renders AISIX config file into a ConfigMap. |
| charts/aisix/templates/deployment.yaml | Deploys AISIX pod(s) and mounts config/env/ports. |
| charts/aisix/templates/service-proxy.yaml | Exposes proxy port (3000) via configurable Service. |
| charts/aisix/templates/service-admin.yaml | Exposes admin port (3001) via Service. |
| charts/aisix/templates/ingress.yaml | Optional Ingress for the proxy service. |
| charts/aisix/templates/ingress-admin.yaml | Optional Ingress for the admin service. |
| charts/aisix/templates/hpa.yaml | Optional HorizontalPodAutoscaler. |
| charts/aisix/templates/serviceaccount.yaml | Optional ServiceAccount. |
| charts/aisix/templates/NOTES.txt | Post-install instructions for accessing proxy/admin endpoints. |
| charts/aisix/charts/etcd-8.7.7.tgz | Vendored etcd subchart archive. |
| README.md | Adds AISIX to the list of available charts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
charts/aisix/README.md (1)
16-18: Prefer non-CLI secret flow in primary install examples.Passing admin keys through
--setis easy to leak via shell history/process inspection. Recommend making the existing-Secret path the primary example, and if retaining CLI examples, use--set-stringwith a brief caution.Also applies to: 61-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/README.md` around lines 16 - 18, Change the primary install example to use the existing-Secret flow instead of passing secrets on the CLI: show how to reference an existing Kubernetes Secret for deployment.admin.adminKey (e.g., point to the existing Secret key name and secretKeyRef) as the top example, and if you keep the CLI variant that uses deployment.admin.adminKey[0].key, switch it to using --set-string and add a one-line caution about shell-history/process-list leakage; update the examples around the same area (including the other occurrences at lines 61-64) so they consistently prefer the Secret-based approach and include the --set-string + caution only as an alternative.charts/aisix/templates/ingress.yaml (1)
13-17: AddingressClassNamesupport for better cluster compatibility.Without an explicit class, some controllers may ignore this Ingress depending on cluster defaults.
Suggested patch
spec: + {{- with .Values.ingress.ingressClassName }} + ingressClassName: {{ . | quote }} + {{- end }} {{- if .Values.ingress.tls }} tls: {{- toYaml .Values.ingress.tls | nindent 4 }} {{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/ingress.yaml` around lines 13 - 17, The Ingress manifest currently only injects TLS and lacks support for an explicit ingress class; update the Ingress spec to conditionally set spec.ingressClassName from a value like .Values.ingress.className when provided. Modify the template around the spec (near the existing .Values.ingress.tls block in charts/aisix/templates/ingress.yaml) to include a conditional that emits "ingressClassName: {{ .Values.ingress.className }}" only when .Values.ingress.className is set, preserving indentation and not changing the existing TLS logic.charts/aisix/templates/ingress-admin.yaml (1)
13-17: MirroringressClassNamesupport in admin Ingress.For consistency and controller compatibility, add class selection here as well.
Suggested patch
spec: + {{- with .Values.adminIngress.ingressClassName }} + ingressClassName: {{ . | quote }} + {{- end }} {{- if .Values.adminIngress.tls }} tls: {{- toYaml .Values.adminIngress.tls | nindent 4 }} {{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/ingress-admin.yaml` around lines 13 - 17, Add support for ingressClassName in the admin Ingress spec so it mirrors the main Ingress behavior; update the ingress-admin.yaml template to conditionally emit the spec.ingressClassName when .Values.adminIngress.ingressClassName is set (use the same conditional pattern as the tls block and proper indentation via nindent), referencing .Values.adminIngress.ingressClassName and the template name ingress-admin.yaml to locate the place to insert the field while keeping the existing tls handling intact.charts/aisix/templates/NOTES.txt (1)
1-3: Pod listing may match pods from other releases.The label selector only uses
app.kubernetes.io/name, which could match pods from different Helm releases of the same chart. For consistency with the pod selection on lines 18 and 29, include both labels.Proposed fix
AISIX has been installed. Check its status by running: - kubectl --namespace {{ .Release.Namespace }} get pods -l "app.kubernetes.io/name={{ include "aisix.name" . }}" + kubectl --namespace {{ .Release.Namespace }} get pods -l "app.kubernetes.io/name={{ include "aisix.name" . }},app.kubernetes.io/instance={{ .Release.Name }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/NOTES.txt` around lines 1 - 3, The kubectl pod selector in NOTES.txt uses only app.kubernetes.io/name and may match pods from other releases; update the selector to include both labels used elsewhere (the chart name and the release instance) by changing the -l selector to include app.kubernetes.io/name={{ include "aisix.name" . }} and app.kubernetes.io/instance={{ .Release.Name }} so it targets only pods from this release.charts/aisix/templates/deployment.yaml (2)
76-77: Consider makingRUST_LOGconfigurable.The log level is hardcoded to
"info". Making this configurable via values would allow users to adjust logging verbosity for debugging.Proposed change
In
values.yaml:# -- Rust log level (e.g., info, debug, warn, error) rustLogLevel: "info"In
deployment.yaml:- name: RUST_LOG - value: "info" + value: {{ .Values.rustLogLevel | default "info" | quote }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/deployment.yaml` around lines 76 - 77, Make RUST_LOG configurable by adding a new values entry rustLogLevel (default "info") in values.yaml and update the deployment template so the RUST_LOG env var uses that value (e.g., reference .Values.rustLogLevel and use a Helm default fallback to "info" if absent); change the env var setting for RUST_LOG in deployment.yaml to read from the Helm value instead of the hardcoded "info" so users can override log level at deploy time.
35-49: Consider providing secure default security contexts.Trivy flags that the container lacks
readOnlyRootFilesystemand uses default security context. While the chart allows customization via.Values.securityContextand.Values.podSecurityContext, providing secure defaults would improve the out-of-box security posture.Proposed secure defaults in values.yaml
podSecurityContext: runAsNonRoot: true seccompProfile: type: RuntimeDefault securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true capabilities: drop: - ALLIf the AISIX container requires a writable filesystem, set
readOnlyRootFilesystem: falseor add anemptyDirvolume for writable paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/deployment.yaml` around lines 35 - 49, The deployment lacks secure default security contexts; update the chart by adding safe defaults in values.yaml for podSecurityContext (e.g., runAsNonRoot: true, seccompProfile.type: RuntimeDefault) and for securityContext (e.g., allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, capabilities.drop: [ALL]) and ensure the template continues to honor .Values.podSecurityContext and .Values.securityContext (using Helm's default/merge behavior) so these defaults apply when the user doesn't override them; if the container truly needs writable paths, document setting readOnlyRootFilesystem: false or mount an emptyDir for those paths.charts/aisix/values.yaml (1)
168-187: Consider noting that etcd RBAC is disabled by default.The etcd subchart has
auth.rbac.create: falsewhich means no authentication is required to access etcd. While this simplifies initial setup, it may be a security concern in multi-tenant clusters. Consider adding a comment noting this should be enabled for production deployments.Proposed documentation improvement
etcd: # -- Install etcd as a subchart. Set false to use an external etcd. enabled: true image: repository: bitnami/etcd auth: rbac: + # -- Enable RBAC authentication for etcd. Recommended for production. create: false rootPassword: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/values.yaml` around lines 168 - 187, The values.yaml currently sets etcd.auth.rbac.create: false which disables etcd RBAC by default; update the etcd section (etcd, auth.rbac.create) to include a short inline comment above or beside that key stating that RBAC is disabled by default and recommending enabling etcd RBAC (set auth.rbac.create: true and configure auth.tls/existingSecret) for production or multi-tenant clusters to avoid unauthenticated access.
🤖 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/aisix/README.md`:
- Line 3: The README sentence "A Helm chart for
[AISIX](https://github.com/api7/aisix) — an open source, high-performance AI
Gateway and LLM proxy built in Rust." uses "open source" and should be
hyphenated; update that sentence to read "open-source" (i.e., change "an open
source" to "an open-source") so the README text uses standard hyphenation.
In `@charts/aisix/templates/hpa.yaml`:
- Around line 2-36: The HPA template conditionally emits autoscaling/v2beta1
when .Values.autoscaling.version != "v2", which produces an incompatible metrics
schema; update the HorizontalPodAutoscaler template (the apiVersion block in the
HPA template that checks .Values.autoscaling.version) to always emit apiVersion:
autoscaling/v2 and remove the conditional branch for v2beta1, and also remove
the .Values.autoscaling.version option from values.yaml (or set it as fixed to
"v2") so the chart no longer exposes or depends on the deprecated v2beta1 path.
In `@charts/aisix/values.yaml`:
- Around line 74-77: The chart defines Values.extraEnvVarsSecret but the
Deployment template only consumes extraEnvVarsCM; update the Deployment template
(the container spec that references extraEnvVarsCM/extraEnvVars) to also
conditionally include an envFrom entry for a secret when
.Values.extraEnvVarsSecret is set (e.g. add a conditional block that appends -
secretRef: name: {{ .Values.extraEnvVarsSecret }} to the container.envFrom so
both ConfigMap and Secret can be specified together), ensuring it safely handles
empty values and preserves existing extraEnvVars and extraEnvVarsCM behavior.
---
Nitpick comments:
In `@charts/aisix/README.md`:
- Around line 16-18: Change the primary install example to use the
existing-Secret flow instead of passing secrets on the CLI: show how to
reference an existing Kubernetes Secret for deployment.admin.adminKey (e.g.,
point to the existing Secret key name and secretKeyRef) as the top example, and
if you keep the CLI variant that uses deployment.admin.adminKey[0].key, switch
it to using --set-string and add a one-line caution about
shell-history/process-list leakage; update the examples around the same area
(including the other occurrences at lines 61-64) so they consistently prefer the
Secret-based approach and include the --set-string + caution only as an
alternative.
In `@charts/aisix/templates/deployment.yaml`:
- Around line 76-77: Make RUST_LOG configurable by adding a new values entry
rustLogLevel (default "info") in values.yaml and update the deployment template
so the RUST_LOG env var uses that value (e.g., reference .Values.rustLogLevel
and use a Helm default fallback to "info" if absent); change the env var setting
for RUST_LOG in deployment.yaml to read from the Helm value instead of the
hardcoded "info" so users can override log level at deploy time.
- Around line 35-49: The deployment lacks secure default security contexts;
update the chart by adding safe defaults in values.yaml for podSecurityContext
(e.g., runAsNonRoot: true, seccompProfile.type: RuntimeDefault) and for
securityContext (e.g., allowPrivilegeEscalation: false, readOnlyRootFilesystem:
true, capabilities.drop: [ALL]) and ensure the template continues to honor
.Values.podSecurityContext and .Values.securityContext (using Helm's
default/merge behavior) so these defaults apply when the user doesn't override
them; if the container truly needs writable paths, document setting
readOnlyRootFilesystem: false or mount an emptyDir for those paths.
In `@charts/aisix/templates/ingress-admin.yaml`:
- Around line 13-17: Add support for ingressClassName in the admin Ingress spec
so it mirrors the main Ingress behavior; update the ingress-admin.yaml template
to conditionally emit the spec.ingressClassName when
.Values.adminIngress.ingressClassName is set (use the same conditional pattern
as the tls block and proper indentation via nindent), referencing
.Values.adminIngress.ingressClassName and the template name ingress-admin.yaml
to locate the place to insert the field while keeping the existing tls handling
intact.
In `@charts/aisix/templates/ingress.yaml`:
- Around line 13-17: The Ingress manifest currently only injects TLS and lacks
support for an explicit ingress class; update the Ingress spec to conditionally
set spec.ingressClassName from a value like .Values.ingress.className when
provided. Modify the template around the spec (near the existing
.Values.ingress.tls block in charts/aisix/templates/ingress.yaml) to include a
conditional that emits "ingressClassName: {{ .Values.ingress.className }}" only
when .Values.ingress.className is set, preserving indentation and not changing
the existing TLS logic.
In `@charts/aisix/templates/NOTES.txt`:
- Around line 1-3: The kubectl pod selector in NOTES.txt uses only
app.kubernetes.io/name and may match pods from other releases; update the
selector to include both labels used elsewhere (the chart name and the release
instance) by changing the -l selector to include app.kubernetes.io/name={{
include "aisix.name" . }} and app.kubernetes.io/instance={{ .Release.Name }} so
it targets only pods from this release.
In `@charts/aisix/values.yaml`:
- Around line 168-187: The values.yaml currently sets etcd.auth.rbac.create:
false which disables etcd RBAC by default; update the etcd section (etcd,
auth.rbac.create) to include a short inline comment above or beside that key
stating that RBAC is disabled by default and recommending enabling etcd RBAC
(set auth.rbac.create: true and configure auth.tls/existingSecret) for
production or multi-tenant clusters to avoid unauthenticated access.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32a909d1-9c36-4fb1-9e8d-02355d5bc46c
⛔ Files ignored due to path filters (1)
charts/aisix/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
README.mdcharts/aisix/.helmignorecharts/aisix/Chart.yamlcharts/aisix/README.mdcharts/aisix/charts/etcd-8.7.7.tgzcharts/aisix/templates/NOTES.txtcharts/aisix/templates/_helpers.tplcharts/aisix/templates/configmap.yamlcharts/aisix/templates/deployment.yamlcharts/aisix/templates/hpa.yamlcharts/aisix/templates/ingress-admin.yamlcharts/aisix/templates/ingress.yamlcharts/aisix/templates/service-admin.yamlcharts/aisix/templates/service-proxy.yamlcharts/aisix/templates/serviceaccount.yamlcharts/aisix/values.yaml
…lt, replicaCount 3, full comments)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
charts/aisix/README.md (1)
3-3:⚠️ Potential issue | 🟡 MinorUse “open-source” hyphenation.
Line 3 should use the compound adjective form: “an open-source, high-performance AI Gateway...”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/README.md` at line 3, The phrase "an open source, high-performance AI Gateway..." should use the compound adjective hyphenation "open-source"; update the README sentence that currently reads "an open source, high-performance AI Gateway and LLM proxy built in Rust" to "an open-source, high-performance AI Gateway and LLM proxy built in Rust" so the compound modifier is correctly hyphenated.
🤖 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/aisix/README.md`:
- Around line 16-17: The helm install examples use unquoted --set arguments with
angle-bracket placeholders and array index syntax (e.g., the command using --set
deployment.admin.adminKey[0].key=<your-admin-key>); update those README lines to
wrap the entire --set key=value argument in quotes (for example "--set
deployment.admin.adminKey[0].key=<your-admin-key>") and ensure placeholders with
angle brackets are preserved inside the quotes so the shell won’t treat '<' as
redirection or '[0]' as a glob pattern; apply the same quoting to any other
--set usages (such as where a strong key is set) so copy-paste works reliably.
---
Duplicate comments:
In `@charts/aisix/README.md`:
- Line 3: The phrase "an open source, high-performance AI Gateway..." should use
the compound adjective hyphenation "open-source"; update the README sentence
that currently reads "an open source, high-performance AI Gateway and LLM proxy
built in Rust" to "an open-source, high-performance AI Gateway and LLM proxy
built in Rust" so the compound modifier is correctly hyphenated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d55233ef-fa53-4e11-8aab-64e54a93d7f1
📒 Files selected for processing (3)
charts/aisix/Chart.yamlcharts/aisix/README.mdcharts/aisix/values.yaml
✅ Files skipped from review due to trivial changes (2)
- charts/aisix/Chart.yaml
- charts/aisix/values.yaml
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Helm chart (charts/aisix/) to deploy AISIX AI Gateway on Kubernetes, including optional bundled etcd, core workload/service templates, and user-facing documentation/notes.
Changes:
- Introduces AISIX chart metadata, default values, and helper templates for naming/labels and etcd host construction.
- Adds Kubernetes manifests for Deployment, ConfigMap-rendered app config, Services (proxy/admin), Ingresses, ServiceAccount, and HPA.
- Updates the repository root README to list the new chart.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/aisix/values.yaml | Defines chart defaults for image, services, AISIX config, ingress, autoscaling, and optional etcd subchart. |
| charts/aisix/templates/serviceaccount.yaml | Optional ServiceAccount creation and metadata wiring. |
| charts/aisix/templates/service-proxy.yaml | Proxy Service (port 3000) with configurable type/NodePort settings. |
| charts/aisix/templates/service-admin.yaml | Admin Service (port 3001) with configurable type/annotations. |
| charts/aisix/templates/ingress.yaml | Proxy Ingress template. |
| charts/aisix/templates/ingress-admin.yaml | Admin Ingress template. |
| charts/aisix/templates/hpa.yaml | HorizontalPodAutoscaler template for scaling AISIX Deployment. |
| charts/aisix/templates/deployment.yaml | AISIX Deployment: config mount, ports, env vars, probes/resources, scheduling knobs. |
| charts/aisix/templates/configmap.yaml | Renders /etc/aisix/config.yaml from Helm values, including Secret-backed admin key placeholder support. |
| charts/aisix/templates/_helpers.tpl | Name/label helpers and etcd host URL construction (subchart vs external). |
| charts/aisix/templates/NOTES.txt | Post-install instructions for discovering proxy/admin URLs. |
| charts/aisix/charts/etcd-8.7.7.tgz | Vendored etcd dependency chart package. |
| charts/aisix/README.md | Chart usage docs and configuration table. |
| charts/aisix/Chart.yaml | Chart metadata and etcd dependency declaration. |
| charts/aisix/Chart.lock | Locked dependency versions/digest for reproducible builds. |
| charts/aisix/.helmignore | Helm packaging ignore patterns for the chart. |
| README.md | Adds AISIX chart link to the repository chart list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s with enabled switch
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
charts/aisix/templates/ingress.yaml (1)
13-17: Consider addingingressClassNamesupport.The Ingress template uses
networking.k8s.io/v1but doesn't supportspec.ingressClassName, which is the recommended way to specify the ingress controller in Kubernetes 1.18+. The annotation-based approach (kubernetes.io/ingress.class) is deprecated.♻️ Proposed fix to add ingressClassName support
spec: + {{- if .Values.gateway.ingress.className }} + ingressClassName: {{ .Values.gateway.ingress.className }} + {{- end }} {{- if .Values.gateway.ingress.tls }} tls: {{- toYaml .Values.gateway.ingress.tls | nindent 4 }} {{- end }}And add to
values.yamlundergateway.ingress:# -- Ingress class name className: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/ingress.yaml` around lines 13 - 17, Add support for ingressClassName in the Ingress template by reading a new value at .Values.gateway.ingress.className and, when non-empty, set spec.ingressClassName to that value in charts/aisix/templates/ingress.yaml; update the template to prefer spec.ingressClassName over deprecated annotation usage (kubernetes.io/ingress.class) and ensure to add the corresponding values gateway.ingress.className: "" in values.yaml so chart consumers can configure the ingress controller class.charts/aisix/templates/configmap.yaml (1)
27-28: Consider making TLS configurable.The proxy TLS setting is hardcoded to
false. Consider exposing this as a configurable value for users who want to enable TLS termination at the application level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/configmap.yaml` around lines 27 - 28, The tls.enabled value is hardcoded to false; make it configurable by wiring it to a Helm values entry (e.g., use .Values.proxy.tls.enabled) so users can toggle TLS from values.yaml, add the corresponding default (false) and comment in values.yaml, and ensure any code reading tls.enabled (the tls block) handles the boolean value correctly when templates render.charts/aisix/templates/deployment.yaml (1)
36-49: Consider providing secure defaults forsecurityContext.Trivy flags that the deployment uses default security context which allows root privileges (KSV-0118) and doesn't set
readOnlyRootFilesystem(KSV-0014). While the template correctly supports user-provided security contexts, the empty defaults invalues.yamlmean users must explicitly configure them.Consider providing secure defaults:
🔐 Proposed secure defaults in values.yaml
podSecurityContext: runAsNonRoot: true seccompProfile: type: RuntimeDefault securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true runAsNonRoot: true capabilities: drop: - ALLVerify that AISIX can run with
readOnlyRootFilesystem: true. If it needs to write to specific paths, addemptyDirvolumes for those locations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/deployment.yaml` around lines 36 - 49, Add secure defaults for podSecurityContext and container securityContext in values.yaml (e.g., runAsNonRoot, seccompProfile RuntimeDefault, allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, drop ALL capabilities) so the deployment template (deployment.yaml) uses safe defaults when .Values.podSecurityContext or .Values.securityContext are empty; update any container-specific settings (the aisix container and any initContainers) to ensure they can run with readOnlyRootFilesystem by provisioning emptyDir volumes for writable paths or adjusting paths in the container, and include a short verification note that AISIX runs with readOnlyRootFilesystem:true or list the required writable mounts.
🤖 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/aisix/templates/configmap.yaml`:
- Around line 29-30: The admin listen line (server.admin.listen: "{{
.Values.admin.ip }}:{{ .Values.admin.containerPort }}") is always rendered even
when .Values.admin.enabled is false; wrap the entire admin block (the "admin:"
key plus the listen line) in a Helm conditional using .Values.admin.enabled
(e.g., {{- if .Values.admin.enabled }} ... {{- end }}), preserving YAML
indentation so the block is omitted when admin.enabled is false and included
when true.
In `@charts/aisix/values.yaml`:
- Around line 92-96: The default adminKey value ("changeme") in values.yaml is
insecure; update documentation and templates to force or surface a non-default
secret: add a clear warning in the README and helm chart README about changing
adminKey, remove the literal "changeme" default or replace it with an
empty/placeholder value, and enforce a check in the Helm template (e.g., in
configmap.yaml or deployment templates that reference deployment.admin.adminKey
or adminKey) using Helm's required/fail logic or by validating that (index
.Values.deployment.admin.adminKey 0).key is not "changeme"; optionally implement
generating a random key at install time if adminKey is unset and no
existingSecret is provided.
- Around line 127-129: The values.yaml currently sets hosts -> host: aisix.local
with paths: [] which yields an Ingress with no routing rules when
gateway.ingress.enabled is true; change the default paths array under hosts (the
paths key) to include a sensible route (for example "/") so the generated
Ingress has at least one rule; update the hosts entry where host: aisix.local
and paths are defined to provide a non-empty array (e.g., ["/"]) and ensure any
templating that renders gateway.ingress.enabled uses that paths value.
---
Nitpick comments:
In `@charts/aisix/templates/configmap.yaml`:
- Around line 27-28: The tls.enabled value is hardcoded to false; make it
configurable by wiring it to a Helm values entry (e.g., use
.Values.proxy.tls.enabled) so users can toggle TLS from values.yaml, add the
corresponding default (false) and comment in values.yaml, and ensure any code
reading tls.enabled (the tls block) handles the boolean value correctly when
templates render.
In `@charts/aisix/templates/deployment.yaml`:
- Around line 36-49: Add secure defaults for podSecurityContext and container
securityContext in values.yaml (e.g., runAsNonRoot, seccompProfile
RuntimeDefault, allowPrivilegeEscalation: false, readOnlyRootFilesystem: true,
drop ALL capabilities) so the deployment template (deployment.yaml) uses safe
defaults when .Values.podSecurityContext or .Values.securityContext are empty;
update any container-specific settings (the aisix container and any
initContainers) to ensure they can run with readOnlyRootFilesystem by
provisioning emptyDir volumes for writable paths or adjusting paths in the
container, and include a short verification note that AISIX runs with
readOnlyRootFilesystem:true or list the required writable mounts.
In `@charts/aisix/templates/ingress.yaml`:
- Around line 13-17: Add support for ingressClassName in the Ingress template by
reading a new value at .Values.gateway.ingress.className and, when non-empty,
set spec.ingressClassName to that value in charts/aisix/templates/ingress.yaml;
update the template to prefer spec.ingressClassName over deprecated annotation
usage (kubernetes.io/ingress.class) and ensure to add the corresponding values
gateway.ingress.className: "" in values.yaml so chart consumers can configure
the ingress controller class.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5015454-9f56-45af-8c90-5fe2616be19b
📒 Files selected for processing (8)
charts/aisix/templates/NOTES.txtcharts/aisix/templates/configmap.yamlcharts/aisix/templates/deployment.yamlcharts/aisix/templates/ingress-admin.yamlcharts/aisix/templates/ingress.yamlcharts/aisix/templates/service-admin.yamlcharts/aisix/templates/service-proxy.yamlcharts/aisix/values.yaml
✅ Files skipped from review due to trivial changes (1)
- charts/aisix/templates/NOTES.txt
- hpa.yaml: use correct v2beta1 metrics schema (targetAverageUtilization) to match v2 schema split, matching gateway chart pattern
- deployment.yaml: fix imagePullSecrets to emit name: wrapper; wire up extraEnvVarsSecret in envFrom; always inject AISIX_ADMIN_KEY from Secret
- configmap.yaml: always use {{AISIX_ADMIN_KEY}} placeholder instead of embedding key in ConfigMap
- secret.yaml: new template to create internal Secret from adminKey when existingSecret is not set
- values.yaml: default ingress paths to ["/"] (was []); improve etcd RBAC comment; clarify adminKey comment
- README.md: fix hyphenation, table row format, stale field names, etcd.enabled default, prefer Secret-based install example
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new charts/aisix/ Helm chart to deploy AISIX AI Gateway on Kubernetes, including optional etcd dependency packaging and standard Kubernetes resources (Deployment/Services/Ingress/HPA) consistent with the existing charts structure in this repo.
Changes:
- Introduces a complete AISIX Helm chart (values, helpers, Deployment, ConfigMap, Secret, Services, Ingresses, HPA, ServiceAccount, NOTES).
- Adds bitnami/etcd dependency metadata and vendors the packaged subchart (
etcd-8.7.7.tgz) plusChart.lock. - Updates the repo root
README.mdto link to the new chart documentation.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/aisix/Chart.yaml | Defines the AISIX chart metadata and declares the etcd dependency. |
| charts/aisix/Chart.lock | Locks the etcd dependency version and digest for reproducible builds. |
| charts/aisix/values.yaml | Provides default configuration for image, services, admin key handling, autoscaling, and etcd options. |
| charts/aisix/README.md | Documents installation and key configuration options for the AISIX chart. |
| charts/aisix/.helmignore | Adds standard Helm packaging ignore patterns. |
| charts/aisix/templates/_helpers.tpl | Adds naming/label helpers and logic to construct etcd endpoints. |
| charts/aisix/templates/configmap.yaml | Renders AISIX config.yaml (etcd + admin key placeholder + listeners). |
| charts/aisix/templates/deployment.yaml | Deploys AISIX with config mount, ports, env vars (including admin key), and optional extras. |
| charts/aisix/templates/secret.yaml | Creates an internal Secret for the admin key when not using an existing Secret. |
| charts/aisix/templates/service-proxy.yaml | Exposes the proxy port (3000) with configurable Service type and options. |
| charts/aisix/templates/service-admin.yaml | Exposes the admin port (3001) as a separate Service (when enabled). |
| charts/aisix/templates/serviceaccount.yaml | Optionally creates a ServiceAccount for the AISIX Deployment. |
| charts/aisix/templates/ingress.yaml | Optionally exposes the proxy Service via Ingress. |
| charts/aisix/templates/ingress-admin.yaml | Optionally exposes the admin Service via Ingress (when admin is enabled). |
| charts/aisix/templates/hpa.yaml | Adds optional HorizontalPodAutoscaler support. |
| charts/aisix/templates/NOTES.txt | Provides post-install access instructions for proxy/admin endpoints. |
| charts/aisix/charts/etcd-8.7.7.tgz | Vendors the bitnami/etcd subchart artifact for offline/reproducible installs. |
| README.md | Adds AISIX chart link to the repository chart index. |
💡 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
♻️ Duplicate comments (3)
charts/aisix/templates/hpa.yaml (1)
2-2:⚠️ Potential issue | 🟠 MajorDrop
autoscaling/v2beta1rendering path and standardize onautoscaling/v2.This template still supports
v2beta1via.Values.autoscaling.version, which can produce invalid manifests on modern clusters and keeps two schemas in sync unnecessarily.💡 Proposed fix
-apiVersion: autoscaling/{{ .Values.autoscaling.version }} +apiVersion: autoscaling/v2 @@ - {{- if eq .Values.autoscaling.version "v2" }} {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} - type: Resource resource: name: cpu target: type: Utilization averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} {{- end }} @@ - {{- else }} - {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} - - type: Resource - resource: - name: cpu - targetAverageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} - {{- end }} - {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} - - type: Resource - resource: - name: memory - targetAverageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} - {{- end }} - {{- end }}Kubernetes deprecation guide: when was autoscaling/v2beta1 removed, and what HPA apiVersion should Helm charts target for Kubernetes 1.25+?Also applies to: 17-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/hpa.yaml` at line 2, The HPA template should stop rendering a configurable `.Values.autoscaling.version` that can yield deprecated `autoscaling/v2beta1`; change the manifest to always use the stable apiVersion `autoscaling/v2` by replacing the `apiVersion: autoscaling/{{ .Values.autoscaling.version }}` usage in the HPA template (`hpa.yaml`) and remove or ignore `.Values.autoscaling.version` for this chart so we don't produce `v2beta1` manifests for modern clusters.charts/aisix/templates/configmap.yaml (1)
25-26:⚠️ Potential issue | 🟠 MajorMake
server.adminconditional onadmin.enabled.The admin block is always rendered in config, even when admin exposure is disabled elsewhere. That creates behavior/config mismatch and can keep admin listener active unintentionally.
💡 Proposed fix
server: proxy: listen: "{{ .Values.gateway.ip }}:{{ .Values.gateway.containerPort }}" tls: enabled: false + {{- if .Values.admin.enabled }} admin: listen: "{{ .Values.admin.ip }}:{{ .Values.admin.containerPort }}" + {{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/templates/configmap.yaml` around lines 25 - 26, The configMap currently always renders the server.admin block; change the template in charts/aisix/templates/configmap.yaml to conditionally include the admin listener only when .Values.admin.enabled is true by wrapping the admin block that references .Values.admin.ip and .Values.admin.containerPort in a Helm if (.Values.admin.enabled) ... end block so the server.admin key is omitted entirely when admin.enabled is false.charts/aisix/values.yaml (1)
95-96:⚠️ Potential issue | 🟠 MajorDo not ship a production-unsafe default admin key.
Using
"changeme"as a default is risky and easy to miss during install.💡 Proposed fix
adminKey: - - key: "changeme" + - key: ""Then enforce non-empty key in template when
existingSecretis not provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/aisix/values.yaml` around lines 95 - 96, Remove the production-unsafe default ("changeme") from the adminKey value block so charts do not ship with a hard-coded credential; make adminKey empty or unset by default and document that consumers must supply it, and update the chart template logic that consumes adminKey (the template that currently checks existingSecret) to enforce a non-empty adminKey when existingSecret is not provided (use Helm template validation/required checks in the template that references adminKey and existingSecret so installation fails with a clear message if neither a secret nor a valid adminKey is supplied).
🤖 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/aisix/templates/secret.yaml`:
- Line 1: The Secret template currently only creates the admin Secret when
.Values.admin.enabled is true, but the Deployment unconditionally references
AISIX_ADMIN_KEY; change the condition in charts/aisix/templates/secret.yaml so
the Secret is generated whenever there is no existing secret (i.e., base the if
solely on .Values.deployment.admin.existingSecret or remove the
.Values.admin.enabled check), or alternatively make the Deployment stop
referencing the Secret when admin is disabled—ensure the Secret creation logic
(secret.yaml) and the Deployment reference (deployment.yaml AISIX_ADMIN_KEY) are
consistent so Pods never reference a missing Secret.
In `@charts/aisix/values.yaml`:
- Line 168: The Helm chart defaults are inconsistent: etcd.enabled is false
while etcd.external.host remains the placeholder "http://etcd.host:2379", which
yields a broken install; update values.yaml so either set etcd.enabled: true by
default or replace etcd.external.host with a realistic endpoint (or empty) and
add a template validation that fails fast when the placeholder is unchanged
(reference the values keys etcd.enabled and etcd.external.host and the templates
that render etcd endpoints) so users must explicitly provide a valid external
host if embedded etcd is disabled.
---
Duplicate comments:
In `@charts/aisix/templates/configmap.yaml`:
- Around line 25-26: The configMap currently always renders the server.admin
block; change the template in charts/aisix/templates/configmap.yaml to
conditionally include the admin listener only when .Values.admin.enabled is true
by wrapping the admin block that references .Values.admin.ip and
.Values.admin.containerPort in a Helm if (.Values.admin.enabled) ... end block
so the server.admin key is omitted entirely when admin.enabled is false.
In `@charts/aisix/templates/hpa.yaml`:
- Line 2: The HPA template should stop rendering a configurable
`.Values.autoscaling.version` that can yield deprecated `autoscaling/v2beta1`;
change the manifest to always use the stable apiVersion `autoscaling/v2` by
replacing the `apiVersion: autoscaling/{{ .Values.autoscaling.version }}` usage
in the HPA template (`hpa.yaml`) and remove or ignore
`.Values.autoscaling.version` for this chart so we don't produce `v2beta1`
manifests for modern clusters.
In `@charts/aisix/values.yaml`:
- Around line 95-96: Remove the production-unsafe default ("changeme") from the
adminKey value block so charts do not ship with a hard-coded credential; make
adminKey empty or unset by default and document that consumers must supply it,
and update the chart template logic that consumes adminKey (the template that
currently checks existingSecret) to enforce a non-empty adminKey when
existingSecret is not provided (use Helm template validation/required checks in
the template that references adminKey and existingSecret so installation fails
with a clear message if neither a secret nor a valid adminKey is supplied).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3e84fcc-012f-42bc-8f29-6f1bec8b1ea6
📒 Files selected for processing (6)
charts/aisix/README.mdcharts/aisix/templates/configmap.yamlcharts/aisix/templates/deployment.yamlcharts/aisix/templates/hpa.yamlcharts/aisix/templates/secret.yamlcharts/aisix/values.yaml
✅ Files skipped from review due to trivial changes (1)
- charts/aisix/README.md
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new charts/aisix Helm chart to deploy AISIX AI Gateway on Kubernetes, including optional etcd dependency wiring and standard Kubernetes resources (Deployment/Services/Ingress/HPA), plus documentation and repo index updates.
Changes:
- Introduces a full AISIX Helm chart with configurable gateway/admin exposure, ConfigMap-based app config, and optional HPA/Ingress.
- Adds admin key management via chart-created Secret or existing Secret reference.
- Vendors bitnami/etcd as a conditional dependency and updates top-level README to list the new chart.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/aisix/values.yaml | Defines chart configuration defaults for AISIX, services, ingress, autoscaling, admin key, and etcd. |
| charts/aisix/templates/serviceaccount.yaml | Optional ServiceAccount template. |
| charts/aisix/templates/service-proxy.yaml | Service for gateway/proxy port exposure. |
| charts/aisix/templates/service-admin.yaml | Optional Service for admin port exposure. |
| charts/aisix/templates/secret.yaml | Renders admin-key Secret when not using an existing Secret. |
| charts/aisix/templates/ingress.yaml | Optional Ingress for the gateway/proxy service. |
| charts/aisix/templates/ingress-admin.yaml | Optional Ingress for the admin service. |
| charts/aisix/templates/hpa.yaml | Optional HPA template driven by values. |
| charts/aisix/templates/deployment.yaml | AISIX Deployment wiring image, ports, env, volumes, and config checksum. |
| charts/aisix/templates/configmap.yaml | Renders /etc/aisix/config.yaml with etcd/admin/server settings. |
| charts/aisix/templates/_helpers.tpl | Common naming/label helpers and etcd host construction logic. |
| charts/aisix/templates/NOTES.txt | Post-install instructions for accessing gateway/admin endpoints. |
| charts/aisix/charts/etcd-8.7.7.tgz | Vendored etcd dependency chart package. |
| charts/aisix/README.md.gotmpl | README template for values table generation. |
| charts/aisix/README.md | Generated README including values reference. |
| charts/aisix/Chart.yaml | Chart metadata and etcd dependency declaration. |
| charts/aisix/Chart.lock | Locked dependency version/digest for reproducible installs. |
| charts/aisix/.helmignore | Helm packaging ignore rules. |
| README.md | Adds AISIX chart to the repository’s chart index list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove v2beta1 HPA support (removed in K8s 1.26), always use autoscaling/v2 - Guard Ingress rules against empty paths to avoid invalid Ingress objects - Decouple Secret creation from admin.enabled so AISIX_ADMIN_KEY env var always resolves - Add template validation when adminKey list is empty - Add checksum/secret annotation to trigger Pod rollout on admin key rotation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Helm chart under charts/aisix/ to deploy AISIX AI Gateway on Kubernetes, including optional etcd dependency wiring and standard Kubernetes resources (Deployment/Services/Ingress/HPA).
Changes:
- Introduces the
aisixchart metadata (Chart.yaml,values.yaml,README.md) and vendors the bitnami/etcd dependency. - Adds core manifests: Deployment, ConfigMap/Secret for app config + admin key, proxy/admin Services, optional Ingresses, optional HPA, and ServiceAccount.
- Updates the repository root
README.mdto list the new chart.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/aisix/values.yaml | Default configuration surface for AISIX, services, admin key, and etcd subchart options |
| charts/aisix/templates/serviceaccount.yaml | Optional ServiceAccount manifest |
| charts/aisix/templates/service-proxy.yaml | Gateway (proxy) Service (port 3000) |
| charts/aisix/templates/service-admin.yaml | Admin Service (port 3001) gated by admin.enabled |
| charts/aisix/templates/secret.yaml | Secret generation for admin key when not using an existing Secret |
| charts/aisix/templates/ingress.yaml | Optional Ingress for the gateway service |
| charts/aisix/templates/ingress-admin.yaml | Optional Ingress for the admin service |
| charts/aisix/templates/hpa.yaml | Optional HorizontalPodAutoscaler |
| charts/aisix/templates/deployment.yaml | AISIX Deployment wiring env/config/ports and checksums |
| charts/aisix/templates/configmap.yaml | Renders AISIX config.yaml including etcd/admin/server blocks |
| charts/aisix/templates/_helpers.tpl | Naming/labels helpers + etcd host construction helper |
| charts/aisix/templates/NOTES.txt | Post-install instructions and access hints |
| charts/aisix/charts/etcd-8.7.7.tgz | Vendored etcd dependency chart package |
| charts/aisix/README.md | Chart documentation and values table |
| charts/aisix/Chart.yaml | Chart metadata + etcd dependency declaration |
| charts/aisix/Chart.lock | Dependency lockfile for etcd |
| charts/aisix/.helmignore | Helm ignore patterns |
| README.md | Adds aisix to the list of available charts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change deployment.admin.adminKey from list to single string; fail if empty and existingSecret is not set - Gate server.admin.listen config on admin.enabled so disabling the admin service actually suppresses the listener - Add ingressClassName support to ingress.yaml and ingress-admin.yaml, mirroring the semverCompare pattern used in charts/gateway Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
charts/aisix/Helm chart for deploying AISIX AI Gateway on KubernetesChart Structure
How to Test
helm dependency update charts/aisix helm lint charts/aisix helm template test-aisix charts/aisix --set 'deployment.admin.adminKey[0].key=testkey'Quick Install
helm install my-aisix ./charts/aisix \ --set 'deployment.admin.adminKey[0].key=your-strong-key'Summary by CodeRabbit
New Features
Documentation