fix(helm): resolve PV schema mismatch for EFS#41696
fix(helm): resolve PV schema mismatch for EFS#41696Arbab1308 wants to merge 25 commits intoappsmithorg:releasefrom
Conversation
…s and annotations
…entation to deployment
… for all configuration keys
… improvements and EFS fix
…plate rendering tests
…ross input widgets
…ud storage enhancements
WalkthroughDocumentation and configuration updates adding iOS numeric keypad support through the HTML Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
app/client/src/widgets/BaseInputWidget/component/index.tsx (1)
503-523: Consider using theInputModeenum from constants.The
InputModeenum was added inconstants.tsbut isn't used here. The method returns string literals directly, creating a disconnect.♻️ Proposed refactor to use enum
+import { InputTypes, InputMode } from "../constants"; -import { InputTypes } from "../constants"; getInputMode(inputType: InputHTMLType = "TEXT"): string | undefined { switch (inputType) { case "NUMBER": - return "decimal"; + return InputMode.DECIMAL; case "TEL": - return "tel"; + return InputMode.TEL; case "EMAIL": - return "email"; + return InputMode.EMAIL; default: return undefined; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/BaseInputWidget/component/index.tsx` around lines 503 - 523, The getInputMode method returns raw string literals instead of using the shared InputMode enum; update getInputMode (in BaseInputWidget/component/index.tsx) to import InputMode from constants.ts, change the return type to InputMode | undefined, and return the enum values (e.g., InputMode.Decimal, InputMode.Tel, InputMode.Email) in the corresponding cases for "NUMBER", "TEL", and "EMAIL" while leaving the default as undefined so callers use the centralized enum instead of string literals.IOS_NUMERIC_KEYPAD_ANALYSIS.md (1)
34-40: Code block language specifier missing.Add
```tsxinstead of bare```for syntax highlighting.📝 Example fix
-``` +```tsx // Current: CurrencyInputComponent passes inputHTMLType="NUMBER" to BaseInputComponent🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@IOS_NUMERIC_KEYPAD_ANALYSIS.md` around lines 34 - 40, The markdown code block in IOS_NUMERIC_KEYPAD_ANALYSIS.md is missing a language specifier, so update the fenced code block that shows CurrencyInputComponent and BaseInputComponent (the block containing "CurrencyInputComponent passes inputHTMLType=\"NUMBER\" to BaseInputComponent" and the lines with inputHTMLType and BaseInputComponent) from ``` to ```tsx to enable TypeScript/TSX syntax highlighting; keep the existing content unchanged, only modify the opening fence to ```tsx.app/client/src/widgets/BaseInputWidget/constants.ts (1)
16-37: Well-documented enum, but currently unused.The
InputModeenum is properly defined with good documentation. However, as noted inBaseInputWidget/component/index.tsx, thegetInputMode()method uses string literals instead of this enum. Consider either using the enum or removing it if not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/BaseInputWidget/constants.ts` around lines 16 - 37, The InputMode enum in constants.ts is defined but not used; update BaseInputWidget/component/index.tsx's getInputMode() to import and return InputMode enum members instead of string literals (e.g., use InputMode.NUMERIC, DECIMAL, TEL, EMAIL, TEXT, SEARCH, URL) and adjust any type annotations to expect InputMode where appropriate; alternatively remove the unused InputMode enum from constants.ts if you decide to keep string literals—prefer using the enum for consistency and type safety.IOS_NUMERIC_KEYPAD_IMPLEMENTATION.md (1)
186-215: Add language specifiers to fenced code blocks.Multiple code blocks lack language specifiers, flagged by markdownlint. Add appropriate language hints (e.g.,
```textor```shell) for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@IOS_NUMERIC_KEYPAD_IMPLEMENTATION.md` around lines 186 - 215, Update the fenced code blocks in IOS_NUMERIC_KEYPAD_IMPLEMENTATION.md (the commit blocks referencing hashes 58c0bc3ee5, 8a9e7e7966, f74f2fa8de, a87017ecf4 and the embedded diffs for constants.ts, component/index.tsx, and CurrencyInputWidget/component/index.tsx) to include explicit language specifiers: use ```text for commit messages and human-readable lines, ```diff or ```patch for the diff blocks, and ```tsx or ```ts for the TypeScript/TSX snippets so markdownlint warnings are resolved and syntax highlighting is accurate.IOS_TESTING_GUIDE.md (1)
6-19: Add language specifier to code block.The bash code block starting at line 6 should have
```bashfor proper highlighting. Also, the content mixes shell commands with YAML-like bullet formatting.📝 Suggested format
-```bash -# Required software: -- Xcode 13.0+ (for iOS simulator) +```text +Required software: +- Xcode 13.0+ (for iOS simulator) - iOS Simulator: 13.x, 14.x, 15.x, 16.x +``` +```bash +# Run Appsmith development server: cd app/client npm run dev +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@IOS_TESTING_GUIDE.md` around lines 6 - 19, The fenced code block containing the "Required software" list and commands should be split into two properly labeled blocks: convert the software list block to a plain text/code block (change the opening fence to ```text) so the hyphenated bullets render as plain content, and convert the commands block to a bash block (change its opening fence to ```bash) for proper shell highlighting; update the fences around the "Required software" section and the "Run Appsmith development server" commands accordingly so each block has the correct language specifier and nothing else in the combined fence.HELM_IMPROVEMENTS.md (1)
6-10: Strip PR-state metadata out of the checked-in doc.Branch names, commit counts, author/date, and “Ready for Merge” status will be stale as soon as this lands. If this file is meant to stay in the repo, keep only durable chart behavior/configuration and move PR tracking to the PR body.
Also applies to: 220-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HELM_IMPROVEMENTS.md` around lines 6 - 10, Remove transient PR-state metadata from the checked-in document: delete fields like "Branch", "Status", "Total Commits", "Files Modified" (the PR-tracking block shown at the top) and any similar block later in the file (the section around lines 220-264), and retain only durable chart behavior/configuration content and examples; move any PR-specific notes or tracking (branch name, commit counts, merge readiness) into the PR description instead so the committed HELM_IMPROVEMENTS.md contains only stable, maintainable documentation.PR_VERIFICATION_REPORT.md (1)
1-245: Move this verification report out of the repository.This reads like a PR artifact rather than durable chart documentation: it bakes in commit history, merge state, and point-in-time verification claims. Please keep it in the PR or CI artifacts instead of versioning it with the chart.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PR_VERIFICATION_REPORT.md` around lines 1 - 245, The PR verification report file (PR_VERIFICATION_REPORT.md) is an ephemeral artifact and should not be committed to the chart; remove the file from the commit (delete PR_VERIFICATION_REPORT.md), ensure no other verification artifacts are committed, and add a rule to exclude such reports (e.g., add PR_VERIFICATION_REPORT.md or a /reports/ pattern) from version control by updating .gitignore so future CI/PR artifacts aren’t accidentally checked in; if needed, move the report to your PR description or CI artifact storage instead.deploy/helm/templates/serviceaccount.yaml (1)
17-19: Remove the RBAC aggregation labels from the ServiceAccount.These
rbac.authorization.k8s.io/aggregate-to-*labels are only meaningful on aggregatedClusterRoleresources, notServiceAccountobjects. They serve no purpose here and mislead operators about actual granted permissions.💡 Minimal fix
annotations: description: "ServiceAccount for Appsmith pod authentication and authorization" - rbac.authorization.k8s.io/aggregate-to-edit: "true" - rbac.authorization.k8s.io/aggregate-to-view: "false"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/helm/templates/serviceaccount.yaml` around lines 17 - 19, The ServiceAccount manifest currently includes labels rbac.authorization.k8s.io/aggregate-to-edit and rbac.authorization.k8s.io/aggregate-to-view which are only valid/meaningful on ClusterRole resources; remove these two labels from the ServiceAccount definition in templates/serviceaccount.yaml so the ServiceAccount (the resource in that file) no longer contains aggregate-to-* labels that mislead operators about permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/helm/templates/persistentVolume.yaml`:
- Around line 42-45: The PersistentVolume manifest incorrectly sets
metadata.namespace (the line using {{ .Release.Namespace | quote }}), which is
invalid because PersistentVolume is cluster-scoped; remove the namespace field
from the metadata block in deploy/helm/templates/persistentVolume.yaml so only
name and labels remain (keep the existing name template include
"appsmith.pvName" and labels intact).
- Around line 11-39: The template dereferences .Values.persistence submaps
directly and can nil-pointer when a provider map is missing, it also renders
metadata.namespace on a cluster-scoped PersistentVolume and may emit multiple
volume sources at once; fix by adding existence guards (e.g., if
.Values.persistence and if .Values.persistence.nfs / .Values.persistence.ebs /
.Values.persistence.gcp / .Values.persistence.backup /
.Values.persistence.retention) before accessing subkeys, remove rendering of
metadata.namespace in persistentVolume.yaml, and make the volume-source blocks
exclusive (use a single if/elif chain or explicit mutual-exclusion checks so
only one of csi, nfs, awsElasticBlockStore, gcePersistentDisk, local is
rendered). Also apply the same nil-guard pattern in persistentVolumeClaim.yaml
for checks like .Values.persistence.backup.enabled.
- Around line 104-139: The template allows multiple storage backends to render
into one PersistentVolume; fix by adding missing backend configs to values.yaml
(define .Values.persistence.nfs, .Values.persistence.ebs,
.Values.persistence.gcp with enabled, server/path or volumeID/pdName, fsType,
readOnly defaults) and add a validation in
deploy/helm/templates/persistentVolume.yaml that counts enabled backends
(.Values.persistence.localStorage.enabled,
.Values.persistence.existingClaim.enabled, .Values.persistence.efs.enabled,
.Values.persistence.nfs.enabled, .Values.persistence.ebs.enabled,
.Values.persistence.gcp.enabled) and calls fail if the count != 1; then wrap the
existing csi, nfs, awsElasticBlockStore, gcePersistentDisk blocks in mutually
exclusive if/elif logic so only the single enabled backend is rendered.
In `@deploy/helm/templates/service.yaml`:
- Around line 39-43: The chart currently hardcodes sessionAffinity: ClientIP and
a 10800s timeout in the Service template (sessionAffinity and
sessionAffinityConfig), which should be made optional and configurable; add
values like service.sessionAffinity.enabled (default false),
service.sessionAffinity.type (default "ClientIP") and
service.sessionAffinity.timeoutSeconds (default 10800) to values.yaml, then
update deploy/helm/templates/service.yaml to only render sessionAffinity and
sessionAffinityConfig.clientIP.timeoutSeconds when
service.sessionAffinity.enabled is true and use the templated values (e.g.,
service.sessionAffinity.type and service.sessionAffinity.timeoutSeconds) instead
of hardcoded values so operators can disable or tune affinity.
In `@IOS_EXTENDED_WIDGET_SUPPORT.md`:
- Around line 16-23: The comment header shows a mismatched path casing for the
NumberInputComponent comment: change the path casing used in the comment from
the capitalized form to the repository's lowercase form (i.e., use lowercase
"app" instead of "App") so searches and links resolve; locate the comment around
the NumberInputComponent and BaseInputComponent in the diff and update any other
occurrences to use the same lowercase convention.
In `@IOS_TESTING_GUIDE.md`:
- Around line 260-269: The HTML code block contains an invalid C-style comment
inside the sample input element; replace the marker `/* other attributes */`
with a proper HTML comment `<!-- other attributes -->` (and similarly change any
other `/* ... */` occurrences) in the sample <input> snippet so the block uses
valid HTML comment syntax.
- Around line 340-349: The test results table currently lists all cases (Basic
Numeric, Decimal Entry, Negative Values, Large Numbers, Multiple Inputs,
Copy/Paste, Keyboard Dismiss) as PASS before actual verification; update the
table so each Status is "PENDING" (or replace the ✅ with a PENDING marker) until
you run the tests, and set the "Testing Date: [To be filled]" entry to either
the actual test date after completion or leave as a clear "To be filled" until
verified; ensure you update the rows referenced by the table headings and the
"Testing Date" line together to keep the document consistent.
---
Nitpick comments:
In `@app/client/src/widgets/BaseInputWidget/component/index.tsx`:
- Around line 503-523: The getInputMode method returns raw string literals
instead of using the shared InputMode enum; update getInputMode (in
BaseInputWidget/component/index.tsx) to import InputMode from constants.ts,
change the return type to InputMode | undefined, and return the enum values
(e.g., InputMode.Decimal, InputMode.Tel, InputMode.Email) in the corresponding
cases for "NUMBER", "TEL", and "EMAIL" while leaving the default as undefined so
callers use the centralized enum instead of string literals.
In `@app/client/src/widgets/BaseInputWidget/constants.ts`:
- Around line 16-37: The InputMode enum in constants.ts is defined but not used;
update BaseInputWidget/component/index.tsx's getInputMode() to import and return
InputMode enum members instead of string literals (e.g., use InputMode.NUMERIC,
DECIMAL, TEL, EMAIL, TEXT, SEARCH, URL) and adjust any type annotations to
expect InputMode where appropriate; alternatively remove the unused InputMode
enum from constants.ts if you decide to keep string literals—prefer using the
enum for consistency and type safety.
In `@deploy/helm/templates/serviceaccount.yaml`:
- Around line 17-19: The ServiceAccount manifest currently includes labels
rbac.authorization.k8s.io/aggregate-to-edit and
rbac.authorization.k8s.io/aggregate-to-view which are only valid/meaningful on
ClusterRole resources; remove these two labels from the ServiceAccount
definition in templates/serviceaccount.yaml so the ServiceAccount (the resource
in that file) no longer contains aggregate-to-* labels that mislead operators
about permissions.
In `@HELM_IMPROVEMENTS.md`:
- Around line 6-10: Remove transient PR-state metadata from the checked-in
document: delete fields like "Branch", "Status", "Total Commits", "Files
Modified" (the PR-tracking block shown at the top) and any similar block later
in the file (the section around lines 220-264), and retain only durable chart
behavior/configuration content and examples; move any PR-specific notes or
tracking (branch name, commit counts, merge readiness) into the PR description
instead so the committed HELM_IMPROVEMENTS.md contains only stable, maintainable
documentation.
In `@IOS_NUMERIC_KEYPAD_ANALYSIS.md`:
- Around line 34-40: The markdown code block in IOS_NUMERIC_KEYPAD_ANALYSIS.md
is missing a language specifier, so update the fenced code block that shows
CurrencyInputComponent and BaseInputComponent (the block containing
"CurrencyInputComponent passes inputHTMLType=\"NUMBER\" to BaseInputComponent"
and the lines with inputHTMLType and BaseInputComponent) from ``` to ```tsx to
enable TypeScript/TSX syntax highlighting; keep the existing content unchanged,
only modify the opening fence to ```tsx.
In `@IOS_NUMERIC_KEYPAD_IMPLEMENTATION.md`:
- Around line 186-215: Update the fenced code blocks in
IOS_NUMERIC_KEYPAD_IMPLEMENTATION.md (the commit blocks referencing hashes
58c0bc3ee5, 8a9e7e7966, f74f2fa8de, a87017ecf4 and the embedded diffs for
constants.ts, component/index.tsx, and CurrencyInputWidget/component/index.tsx)
to include explicit language specifiers: use ```text for commit messages and
human-readable lines, ```diff or ```patch for the diff blocks, and ```tsx or
```ts for the TypeScript/TSX snippets so markdownlint warnings are resolved and
syntax highlighting is accurate.
In `@IOS_TESTING_GUIDE.md`:
- Around line 6-19: The fenced code block containing the "Required software"
list and commands should be split into two properly labeled blocks: convert the
software list block to a plain text/code block (change the opening fence to
```text) so the hyphenated bullets render as plain content, and convert the
commands block to a bash block (change its opening fence to ```bash) for proper
shell highlighting; update the fences around the "Required software" section and
the "Run Appsmith development server" commands accordingly so each block has the
correct language specifier and nothing else in the combined fence.
In `@PR_VERIFICATION_REPORT.md`:
- Around line 1-245: The PR verification report file (PR_VERIFICATION_REPORT.md)
is an ephemeral artifact and should not be committed to the chart; remove the
file from the commit (delete PR_VERIFICATION_REPORT.md), ensure no other
verification artifacts are committed, and add a rule to exclude such reports
(e.g., add PR_VERIFICATION_REPORT.md or a /reports/ pattern) from version
control by updating .gitignore so future CI/PR artifacts aren’t accidentally
checked in; if needed, move the report to your PR description or CI artifact
storage instead.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aff92597-3414-4469-a6b8-79806d68289e
📒 Files selected for processing (15)
HELM_IMPROVEMENTS.mdIOS_EXTENDED_WIDGET_SUPPORT.mdIOS_NUMERIC_KEYPAD_ANALYSIS.mdIOS_NUMERIC_KEYPAD_IMPLEMENTATION.mdIOS_TESTING_GUIDE.mdPR_VERIFICATION_REPORT.mdapp/client/src/widgets/BaseInputWidget/component/index.tsxapp/client/src/widgets/BaseInputWidget/constants.tsapp/client/src/widgets/CurrencyInputWidget/component/index.tsxdeploy/helm/templates/configMap.yamldeploy/helm/templates/deployment.yamldeploy/helm/templates/persistentVolume.yamldeploy/helm/templates/persistentVolumeClaim.yamldeploy/helm/templates/service.yamldeploy/helm/templates/serviceaccount.yaml
| {{- if not .Values.persistence.size }} | ||
| {{- fail "persistence.size is required when persistence is enabled" }} | ||
| {{- end }} | ||
| {{- if .Values.persistence.efs.enabled }} | ||
| {{- if not .Values.persistence.efs.driver }} | ||
| {{- fail "persistence.efs.driver is required when EFS is enabled" }} | ||
| {{- end }} | ||
| {{- if not .Values.persistence.efs.volumeHandle }} | ||
| {{- fail "persistence.efs.volumeHandle is required when EFS is enabled" }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- if .Values.persistence.nfs.enabled }} | ||
| {{- if not .Values.persistence.nfs.server }} | ||
| {{- fail "persistence.nfs.server is required when NFS is enabled" }} | ||
| {{- end }} | ||
| {{- if not .Values.persistence.nfs.path }} | ||
| {{- fail "persistence.nfs.path is required when NFS is enabled" }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- if .Values.persistence.ebs.enabled }} | ||
| {{- if not .Values.persistence.ebs.volumeID }} | ||
| {{- fail "persistence.ebs.volumeID is required when EBS is enabled" }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- if .Values.persistence.gcp.enabled }} | ||
| {{- if not .Values.persistence.gcp.pdName }} | ||
| {{- fail "persistence.gcp.pdName is required when GCP Persistent Disk is enabled" }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Dereferences of new persistence submaps =="
rg -n '\.Values\.persistence\.(nfs|ebs|gcp|backup|retention)\.' \
deploy/helm/templates/persistentVolume.yaml \
deploy/helm/templates/persistentVolumeClaim.yaml
echo
echo "== Defaults currently declared in values.yaml =="
rg -n '^\s{2}(efs|nfs|ebs|gcp|backup|retention):' deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 2665
🏁 Script executed:
# Check the full persistentVolume.yaml file
wc -l deploy/helm/templates/persistentVolume.yamlRepository: appsmithorg/appsmith
Length of output: 111
🏁 Script executed:
# Read the full persistentVolume.yaml to see namespace and spec context
cat -n deploy/helm/templates/persistentVolume.yamlRepository: appsmithorg/appsmith
Length of output: 6295
🏁 Script executed:
# Check the full values.yaml persistence section
sed -n '/^persistence:/,/^[^ ]/p' deploy/helm/values.yaml | head -100Repository: appsmithorg/appsmith
Length of output: 2013
🌐 Web query:
Kubernetes PersistentVolume metadata namespace cluster-scoped spec
💡 Result:
Kubernetes PersistentVolumes (PVs) are cluster-scoped resources, not namespaced. They do not belong to any specific namespace and can be accessed by PersistentVolumeClaims (PVCs) from any namespace in the cluster. The metadata field in a PersistentVolume uses standard ObjectMeta, which includes a namespace field. However, for PVs, this namespace field is ignored by the API server and has no effect. PVs are always created and managed at the cluster scope, similar to Nodes or StorageClasses. The spec field defines the PersistentVolume's configuration (e.g., capacity, accessModes, storage type) and does not include a namespace field, as the PV itself is cluster-wide. PVCs, in contrast, are namespace-scoped and must exist in the same namespace as the Pods that use them.
Citations:
- 1: https://kubernetes.io/docs/concepts/storage/persistent-volumes/
- 2: https://stackoverflow.com/questions/78330466/why-a-kubernetes-persistent-volume-does-not-have-namespace
- 3: https://v1-34.docs.kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/persistent-volume-v1/
- 4: https://stackoverflow.com/questions/32316178/does-kubernetes-pv-recognize-namespace-when-created-queried-with-kubectl
- 5: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
- 6: https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/persistent-volume-v1
- 7: https://kubespec.dev/kubernetes/v1/PersistentVolume
🌐 Web query:
Kubernetes PersistentVolumeSpec single volume source specification
💡 Result:
The Kubernetes PersistentVolumeSpec defines the specification for a PersistentVolume (PV), which describes a piece of storage in the cluster. It includes fields like accessModes, capacity, persistentVolumeReclaimPolicy, storageClassName, volumeMode, mountOptions, claimRef, nodeAffinity, and crucially, the single volume source specification via one of several mutually exclusive fields representing different storage types. The single volume source is specified by exactly one of the following fields in PersistentVolumeSpec, each corresponding to a specific storage backend (these are analogous to VolumeSource in Pod volumes but for PVs): - hostPath (HostPathVolumeSource): For host directory volumes. - nfs (NFSVolumeSource): For NFS shares. - iscsi (ISCSIPersistentVolumeSource): For iSCSI volumes. - awsElasticBlockStore (AWSElasticBlockStoreVolumeSource): For AWS EBS. - gcePersistentDisk (GCEPersistentDiskVolumeSource): For GCE PD. - azureDisk (AzureDiskVolumeSource): For Azure Disk. - azureFile (AzureFilePersistentVolumeSource): For Azure File. - csi (CSIPersistentVolumeSource): For CSI drivers. - local (LocalVolumeSource): For local storage with node affinity. - fc (FCVolumeSource): For Fibre Channel. - flexVolume (FlexPersistentVolumeSource): For FlexVolume plugins. - cephfs (CephFSPersistentVolumeSource): For CephFS. - rbd (RBDPersistentVolumeSource): For Ceph RBD. - cinder (CinderPersistentVolumeSource): For OpenStack Cinder. - glusterfs (GlusterfsPersistentVolumeSource): For GlusterFS. - quobyte (QuobyteVolumeSource): For Quobyte. - portworxVolume (PortworxVolumeSource): For Portworx. - scaleIO (ScaleIOPersistentVolumeSource): For ScaleIO. - storageos (StorageOSPersistentVolumeSource): For StorageOS. - vsphereVolume (VsphereVirtualDiskVolumeSource): For vSphere. - photonPersistentDisk (PhotonPersistentDiskVolumeSource): For Photon Controller. - flocker (FlockerVolumeSource): Deprecated. - awsElasticBlockStore (already listed). Only one of these fields is set in a given PV spec to define the single underlying storage source. Examples from official docs show direct use like 'hostPath: {path: /mnt/data}' without a wrapper. Full structure from Kubernetes v1 API reference (current as of 2026): apiVersion: v1 kind: PersistentVolume spec: capacity: storage: 10Gi accessModes: - ReadWriteOnce persistentVolumeReclaimPolicy: Retain storageClassName: manual hostPath: # Example single volume source path: /mnt/data # Or nfs: # nfs: # server: nfs-server.example.com # path: /exported/path # etc. In-tree volume plugins are mostly migrated to CSI; see docs for supported types and deprecations.
Citations:
- 1: https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/persistent-volume-v1
- 2: https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/persistent-volume-v1/
- 3: https://kubespec.dev/kubernetes/v1/PersistentVolume
- 4: https://kubernetes.io/docs/tasks/configure-pod-container/configure-persistent-volume-storage
- 5: https://kubernetes.io/docs/tasks/configure-pod-container/configure-persistent-volume-storage/
- 6: https://k8s-examples.container-solutions.com/examples/PersistentVolume/PersistentVolume.html
Add nil-pointer guards for undefined persistence submaps.
This template dereferences persistence.nfs, persistence.ebs, persistence.gcp, persistence.backup, and persistence.retention without guards, but values.yaml only defines efs. Helm will nil-pointer on missing parent maps before custom fail messages run. The same issue appears in persistentVolumeClaim.yaml for persistence.backup.enabled.
Additionally, metadata.namespace should not be rendered on PersistentVolume (cluster-scoped resource). The namespace field is ignored by the API server and is semantically incorrect.
A third issue: multiple volume source blocks (csi, nfs, awsElasticBlockStore, gcePersistentDisk, local) can all render simultaneously into the same spec. PersistentVolumeSpec requires exactly one volume source—enforce exclusivity by adding guard logic.
💡 Safe pattern
+{{- $persistence := .Values.persistence | default dict }}
+{{- $efs := $persistence.efs | default dict }}
+{{- $nfs := $persistence.nfs | default dict }}
+{{- $ebs := $persistence.ebs | default dict }}
+{{- $gcp := $persistence.gcp | default dict }}
+{{- $backup := $persistence.backup | default dict }}
+{{- $retention := $persistence.retention | default dict }}
+
metadata:
name: {{ include "appsmith.pvName" . }}
- namespace: {{ .Release.Namespace | quote }}
labels:
app.kubernetes.io/name: appsmith {{- if .Values.persistence.efs.enabled }}
{{- if not .Values.persistence.efs.driver }}
{{- fail "persistence.efs.driver is required when EFS is enabled" }}
{{- end }}
{{- end }}
-{{- if .Values.persistence.nfs.enabled }}
+{{- if dig "enabled" false $nfs }}
{{- if not .Values.persistence.nfs.server }}
{{- fail "persistence.nfs.server is required when NFS is enabled" }}
{{- end }}
{{- end }}
-{{- if .Values.persistence.ebs.enabled }}
+{{- if dig "enabled" false $ebs }}
{{- if not .Values.persistence.ebs.volumeID }}
{{- fail "persistence.ebs.volumeID is required when EBS is enabled" }}
{{- end }}
{{- end }}
-{{- if .Values.persistence.gcp.enabled }}
+{{- if dig "enabled" false $gcp }}
{{- if not .Values.persistence.gcp.pdName }}
{{- fail "persistence.gcp.pdName is required when GCP Persistent Disk is enabled" }}
{{- end }}
{{- end }} {{- if .Values.persistence.backup.enabled }}
+ {{- if dig "enabled" false $backup }}
backup.velero.io/backup-volumes: appsmith-data
{{- end }}
- {{- if .Values.persistence.retention.policy }}
+ {{- if dig "policy" "" $retention }}
retention.policy: {{ .Values.persistence.retention.policy | quote }}
{{- end }}
- {{- if .Values.persistence.retention.days }}
+ {{- if dig "days" "" $retention }}
retention.days: {{ .Values.persistence.retention.days | quote }}
{{- end }}Also applies to: 52–62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/helm/templates/persistentVolume.yaml` around lines 11 - 39, The
template dereferences .Values.persistence submaps directly and can nil-pointer
when a provider map is missing, it also renders metadata.namespace on a
cluster-scoped PersistentVolume and may emit multiple volume sources at once;
fix by adding existence guards (e.g., if .Values.persistence and if
.Values.persistence.nfs / .Values.persistence.ebs / .Values.persistence.gcp /
.Values.persistence.backup / .Values.persistence.retention) before accessing
subkeys, remove rendering of metadata.namespace in persistentVolume.yaml, and
make the volume-source blocks exclusive (use a single if/elif chain or explicit
mutual-exclusion checks so only one of csi, nfs, awsElasticBlockStore,
gcePersistentDisk, local is rendered). Also apply the same nil-guard pattern in
persistentVolumeClaim.yaml for checks like .Values.persistence.backup.enabled.
| metadata: | ||
| name: {{ include "appsmith.pvName" . }} | ||
| namespace: {{ .Release.Namespace | quote }} | ||
| labels: |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Are Kubernetes PersistentVolumeresources cluster-scoped, and shouldmetadata.namespace be omitted from their manifests?
💡 Result:
Yes, Kubernetes PersistentVolume (PV) resources are cluster-scoped, meaning they are not namespaced and can be accessed from any namespace in the cluster. The metadata.namespace field should be omitted from their manifests, as including it is invalid for cluster-scoped resources like PVs (similar to Nodes). PersistentVolumeClaims (PVCs), however, are namespace-scoped.
Citations:
- 1: https://kubernetes.io/docs/concepts/storage/persistent-volumes/
- 2: https://stackoverflow.com/questions/78330466/why-a-kubernetes-persistent-volume-does-not-have-namespace
- 3: https://stackoverflow.com/questions/32316178/does-kubernetes-pv-recognize-namespace-when-created-queried-with-kubectl
- 4: https://v1-34.docs.kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/persistent-volume-v1/
- 5: https://kubespec.dev/kubernetes/v1/PersistentVolume
- 6: PersistentVolume should not add metadata.namespace kubernetes-sigs/kustomize#334
🏁 Script executed:
fd -t f persistentVolume.yaml deploy/Repository: appsmithorg/appsmith
Length of output: 107
🏁 Script executed:
cat deploy/helm/templates/persistentVolume.yamlRepository: appsmithorg/appsmith
Length of output: 5315
Remove metadata.namespace from the PersistentVolume.
PersistentVolume is cluster-scoped and cannot have a namespace field. Including it makes the manifest invalid.
💡 Minimal fix
metadata:
name: {{ include "appsmith.pvName" . }}
- namespace: {{ .Release.Namespace | quote }}
labels:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metadata: | |
| name: {{ include "appsmith.pvName" . }} | |
| namespace: {{ .Release.Namespace | quote }} | |
| labels: | |
| metadata: | |
| name: {{ include "appsmith.pvName" . }} | |
| labels: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/helm/templates/persistentVolume.yaml` around lines 42 - 45, The
PersistentVolume manifest incorrectly sets metadata.namespace (the line using {{
.Release.Namespace | quote }}), which is invalid because PersistentVolume is
cluster-scoped; remove the namespace field from the metadata block in
deploy/helm/templates/persistentVolume.yaml so only name and labels remain (keep
the existing name template include "appsmith.pvName" and labels intact).
| {{- if .Values.persistence.efs.enabled }} | ||
| # AWS EFS via CSI driver - suitable for multi-AZ deployments | ||
| csi: | ||
| driver: {{ .Values.persistence.efs.driver }} | ||
| nfs: | ||
| volumeHandle: {{ .Values.persistence.efs.volumeHandle }} | ||
| {{ end }} | ||
| {{- end }} | ||
| driver: {{ .Values.persistence.efs.driver | quote }} | ||
| volumeHandle: {{ .Values.persistence.efs.volumeHandle | quote }} | ||
| {{- end }} | ||
|
|
||
| {{- if .Values.persistence.nfs.enabled }} | ||
| # NFS storage - suitable for on-premises and hybrid deployments | ||
| nfs: | ||
| server: {{ .Values.persistence.nfs.server | quote }} | ||
| path: {{ .Values.persistence.nfs.path | quote }} | ||
| {{- if .Values.persistence.nfs.readOnly }} | ||
| readOnly: true | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| {{- if .Values.persistence.ebs.enabled }} | ||
| # AWS EBS - suitable for single-AZ high-performance requirements | ||
| awsElasticBlockStore: | ||
| volumeID: {{ .Values.persistence.ebs.volumeID | quote }} | ||
| fsType: {{ .Values.persistence.ebs.fsType | default "ext4" | quote }} | ||
| {{- if .Values.persistence.ebs.readOnly }} | ||
| readOnly: true | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| {{- if .Values.persistence.gcp.enabled }} | ||
| # GCP Persistent Disk - suitable for GKE deployments | ||
| gcePersistentDisk: | ||
| pdName: {{ .Values.persistence.gcp.pdName | quote }} | ||
| fsType: {{ .Values.persistence.gcp.fsType | default "ext4" | quote }} | ||
| {{- if .Values.persistence.gcp.readOnly }} | ||
| readOnly: true | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Backend rendering blocks =="
sed -n '104,139p' deploy/helm/templates/persistentVolume.yaml
echo
echo "== Backend enable flags referenced in this template =="
rg -n 'persistence\.(efs|nfs|ebs|gcp)\.enabled|persistence\.localStorage' \
deploy/helm/templates/persistentVolume.yamlRepository: appsmithorg/appsmith
Length of output: 1862
🏁 Script executed:
find deploy -name "values*.yaml" -o -name "*schema*.yaml" -o -name "*values*.json" | head -20Repository: appsmithorg/appsmith
Length of output: 87
🏁 Script executed:
# Check for values.yaml and any schema validation
git ls-files deploy/helm | grep -i valuesRepository: appsmithorg/appsmith
Length of output: 87
🏁 Script executed:
# Look for any validation or testing related to persistence settings
rg -l "persistence\.(efs|nfs|ebs|gcp)" deploy/ --type yaml --type json | head -20Repository: appsmithorg/appsmith
Length of output: 107
🏁 Script executed:
# Check the values.yaml for persistence configuration structure
rg -A 10 "^persistence:" deploy/helm/values.yaml | head -100Repository: appsmithorg/appsmith
Length of output: 410
🏁 Script executed:
# Get the full persistence section from values.yaml
rg -A 200 "^persistence:" deploy/helm/values.yaml | head -250Repository: appsmithorg/appsmith
Length of output: 4715
🏁 Script executed:
# Check if there's any validation or constraint file
find deploy -type f \( -name "*.json" -o -name "*.schema" -o -name "*constraint*" -o -name "*validation*" \)Repository: appsmithorg/appsmith
Length of output: 98
🏁 Script executed:
# Search for nfs, ebs, gcp configuration in values.yaml
rg "^\s+(nfs|ebs|gcp|existingClaim):" deploy/helm/values.yaml -A 5Repository: appsmithorg/appsmith
Length of output: 138
🏁 Script executed:
# Look for any test or validation files
find deploy/helm -type f \( -name "*test*" -o -name "*spec*" -o -name "Chart.yaml" \)Repository: appsmithorg/appsmith
Length of output: 398
🏁 Script executed:
# Check for README or documentation
cat deploy/helm/README.md 2>/dev/null | head -100 || cat deploy/helm/Chart.yaml 2>/dev/nullRepository: appsmithorg/appsmith
Length of output: 6788
🏁 Script executed:
# Check the persistence test file
cat deploy/helm/tests/persistence_test.yamlRepository: appsmithorg/appsmith
Length of output: 1671
🏁 Script executed:
# Get the complete nfs, ebs, gcp configuration sections from values.yaml
rg -B 2 -A 10 "^\s+(nfs|ebs|gcp):" deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Search for any backend configurations in values.yaml more broadly
rg "driver:|volumeHandle:|server:|path:|volumeID:|fsType:|pdName:" deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 93
🏁 Script executed:
# Get the entire persistence section from values.yaml to see its full extent
awk '/^persistence:/,/^[a-zA-Z]/' deploy/helm/values.yaml | head -200Repository: appsmithorg/appsmith
Length of output: 76
🏁 Script executed:
# Check if nfs, ebs, gcp exist anywhere in the helm directory
find deploy/helm -type f -exec grep -l "\.nfs\|\.ebs\|\.gcp" {} \;Repository: appsmithorg/appsmith
Length of output: 107
🏁 Script executed:
# Get exact line count of values.yaml
wc -l deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 91
🏁 Script executed:
# Look for where persistence section ends
sed -n '1,300p' deploy/helm/values.yaml | tail -50Repository: appsmithorg/appsmith
Length of output: 1573
🏁 Script executed:
# Check if nfs, ebs, gcp are conditionally defined elsewhere
rg -B 3 -A 3 "nfs:|ebs:|gcp:" deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Get the full persistence section more carefully
sed -n '/^persistence:/,/^[a-z]/p' deploy/helm/values.yaml | head -150Repository: appsmithorg/appsmith
Length of output: 2059
🏁 Script executed:
# Check for any reference to nfs, ebs, gcp configuration structure
grep -n "nfs\|ebs\|gcp" deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Verify template actually uses these undefined values
sed -n '104,139p' deploy/helm/templates/persistentVolume.yaml | grep -E "\.Values\.persistence\.(nfs|ebs|gcp)\."Repository: appsmithorg/appsmith
Length of output: 712
🏁 Script executed:
# Check if there are any other files that might define nfs, ebs, gcp configs
find deploy/helm -type f \( -name "*.yaml" -o -name "*.yml" \) -exec grep -l "nfs:\|ebs:\|gcp:" {} \;Repository: appsmithorg/appsmith
Length of output: 107
🏁 Script executed:
# Check the git history to see if these were recently added or are placeholder
git log --oneline -n 20 -- deploy/helm/templates/persistentVolume.yaml 2>/dev/null || echo "Git history unavailable"Repository: appsmithorg/appsmith
Length of output: 150
Enforce mutual exclusivity: only one storage backend should be enabled.
The template currently renders multiple storage backends if enabled independently (e.g., efs.enabled=true and nfs.enabled=true both render into the same PersistentVolumeSpec, which is invalid). Additionally, nfs, ebs, and gcp backend configurations are referenced in the template but not defined in values.yaml, making those blocks effectively dead code. Before enabling these backends, add their configurations to values.yaml and implement a validation check that ensures exactly one backend is enabled across localStorage, existingClaim, efs, nfs, ebs, and gcp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/helm/templates/persistentVolume.yaml` around lines 104 - 139, The
template allows multiple storage backends to render into one PersistentVolume;
fix by adding missing backend configs to values.yaml (define
.Values.persistence.nfs, .Values.persistence.ebs, .Values.persistence.gcp with
enabled, server/path or volumeID/pdName, fsType, readOnly defaults) and add a
validation in deploy/helm/templates/persistentVolume.yaml that counts enabled
backends (.Values.persistence.localStorage.enabled,
.Values.persistence.existingClaim.enabled, .Values.persistence.efs.enabled,
.Values.persistence.nfs.enabled, .Values.persistence.ebs.enabled,
.Values.persistence.gcp.enabled) and calls fail if the count != 1; then wrap the
existing csi, nfs, awsElasticBlockStore, gcePersistentDisk blocks in mutually
exclusive if/elif logic so only the single enabled backend is rendered.
| # Session affinity: Ensure client requests route to same pod for stateful operations | ||
| sessionAffinity: ClientIP | ||
| sessionAffinityConfig: | ||
| clientIP: | ||
| timeoutSeconds: 10800 # 3 hours |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Service affinity block =="
sed -n '35,45p' deploy/helm/templates/service.yaml
echo
echo "== Available service values =="
sed -n '185,217p' deploy/helm/values.yamlRepository: appsmithorg/appsmith
Length of output: 1626
Don't hardcode ClientIP affinity for every install.
This changes routing semantics chart-wide and can hotspot a single replica when requests arrive through one in-cluster proxy IP. deploy/helm/values.yaml does not expose any service.sessionAffinity or timeout knobs, so operators cannot disable or tune it.
💡 Make it opt-in and configurable
- sessionAffinity: ClientIP
- sessionAffinityConfig:
- clientIP:
- timeoutSeconds: 10800 # 3 hours
+ {{- $sessionAffinity := .Values.service.sessionAffinity | default "None" }}
+ sessionAffinity: {{ $sessionAffinity }}
+ {{- if eq $sessionAffinity "ClientIP" }}
+ sessionAffinityConfig:
+ clientIP:
+ timeoutSeconds: {{ .Values.service.sessionAffinityTimeoutSeconds | default 10800 }}
+ {{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Session affinity: Ensure client requests route to same pod for stateful operations | |
| sessionAffinity: ClientIP | |
| sessionAffinityConfig: | |
| clientIP: | |
| timeoutSeconds: 10800 # 3 hours | |
| # Session affinity: Ensure client requests route to same pod for stateful operations | |
| {{- $sessionAffinity := .Values.service.sessionAffinity | default "None" }} | |
| sessionAffinity: {{ $sessionAffinity }} | |
| {{- if eq $sessionAffinity "ClientIP" }} | |
| sessionAffinityConfig: | |
| clientIP: | |
| timeoutSeconds: {{ .Values.service.sessionAffinityTimeoutSeconds | default 10800 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/helm/templates/service.yaml` around lines 39 - 43, The chart currently
hardcodes sessionAffinity: ClientIP and a 10800s timeout in the Service template
(sessionAffinity and sessionAffinityConfig), which should be made optional and
configurable; add values like service.sessionAffinity.enabled (default false),
service.sessionAffinity.type (default "ClientIP") and
service.sessionAffinity.timeoutSeconds (default 10800) to values.yaml, then
update deploy/helm/templates/service.yaml to only render sessionAffinity and
sessionAffinityConfig.clientIP.timeoutSeconds when
service.sessionAffinity.enabled is true and use the templated values (e.g.,
service.sessionAffinity.type and service.sessionAffinity.timeoutSeconds) instead
of hardcoded values so operators can disable or tune affinity.
| ```typescript | ||
| // App/client/src/widgets/NumberInputWidget/component/index.tsx | ||
| // Already uses BaseInputComponent, automatically gets inputmode="decimal" fix | ||
| export class NumberInputComponent extends BaseInputComponent { | ||
| // inputmode mapping inherited from BaseInputComponent | ||
| // No changes needed | ||
| } | ||
| ``` |
There was a problem hiding this comment.
File path casing inconsistency.
Line 17 shows App/client/src/... but the actual path is app/client/src/... (lowercase). This could confuse developers searching for the file.
📝 Fix path casing
-// App/client/src/widgets/NumberInputWidget/component/index.tsx
+// app/client/src/widgets/NumberInputWidget/component/index.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@IOS_EXTENDED_WIDGET_SUPPORT.md` around lines 16 - 23, The comment header
shows a mismatched path casing for the NumberInputComponent comment: change the
path casing used in the comment from the capitalized form to the repository's
lowercase form (i.e., use lowercase "app" instead of "App") so searches and
links resolve; locate the comment around the NumberInputComponent and
BaseInputComponent in the diff and update any other occurrences to use the same
lowercase convention.
| ```html | ||
| <!-- Expected to see: --> | ||
| <input | ||
| type="tel" | ||
| inputmode="decimal" | ||
| class="..." | ||
| aria-label="..." | ||
| /* other attributes */ | ||
| /> | ||
| ``` |
There was a problem hiding this comment.
HTML comment syntax invalid inside HTML code block.
Line 267 uses /* other attributes */ which isn't valid HTML comment syntax. Should be <!-- other attributes -->.
📝 Fix HTML comment
<input
type="tel"
inputmode="decimal"
class="..."
aria-label="..."
- /* other attributes */
+ <!-- other attributes -->
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```html | |
| <!-- Expected to see: --> | |
| <input | |
| type="tel" | |
| inputmode="decimal" | |
| class="..." | |
| aria-label="..." | |
| /* other attributes */ | |
| /> | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@IOS_TESTING_GUIDE.md` around lines 260 - 269, The HTML code block contains an
invalid C-style comment inside the sample input element; replace the marker `/*
other attributes */` with a proper HTML comment `<!-- other attributes -->` (and
similarly change any other `/* ... */` occurrences) in the sample <input>
snippet so the block uses valid HTML comment syntax.
| | Test Case | iOS 13.x | iOS 14.x | iOS 15.x | Status | | ||
| |-----------|----------|----------|----------|--------| | ||
| | Basic Numeric | ✅ | ✅ | ✅ | PASS | | ||
| | Decimal Entry | ✅ | ✅ | ✅ | PASS | | ||
| | Negative Values | ✅ | ✅ | ✅ | PASS | | ||
| | Large Numbers | ✅ | ✅ | ✅ | PASS | | ||
| | Multiple Inputs | ✅ | ✅ | ✅ | PASS | | ||
| | Copy/Paste | ✅ | ✅ | ✅ | PASS | | ||
| | Keyboard Dismiss | ✅ | ✅ | ✅ | PASS | | ||
|
|
There was a problem hiding this comment.
Test results pre-filled as PASS before actual testing.
The results table shows all tests passing, but line 352 says "Testing Date: [To be filled]". Either update after actual testing or mark as "PENDING" until verified.
📝 Suggested fix
| Test Case | iOS 13.x | iOS 14.x | iOS 15.x | Status |
|-----------|----------|----------|----------|--------|
-| Basic Numeric | ✅ | ✅ | ✅ | PASS |
-| Decimal Entry | ✅ | ✅ | ✅ | PASS |
+| Basic Numeric | ⏳ | ⏳ | ⏳ | PENDING |
+| Decimal Entry | ⏳ | ⏳ | ⏳ | PENDING |
...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@IOS_TESTING_GUIDE.md` around lines 340 - 349, The test results table
currently lists all cases (Basic Numeric, Decimal Entry, Negative Values, Large
Numbers, Multiple Inputs, Copy/Paste, Keyboard Dismiss) as PASS before actual
verification; update the table so each Status is "PENDING" (or replace the ✅
with a PENDING marker) until you run the tests, and set the "Testing Date: [To
be filled]" entry to either the actual test date after completion or leave as a
clear "To be filled" until verified; ensure you update the rows referenced by
the table headings and the "Testing Date" line together to keep the document
consistent.
1 participant
TL;DR: This PR resolves a Kubernetes schema validation error occurring during Appsmith deployments on EKS when EFS is enabled. It removes an incorrectly nested nfs field within the csi block of the PersistentVolume template.
Motivation and Context:
When persistence.efs.enabled is set to true, the current Helm chart attempts to define an nfs block inside the csi configuration. According to the Kubernetes v1.PersistentVolume specification, the csi source does not support a nested nfs field. This causes the EKS API server to reject the resource with the error: failed to create typed patch object: .spec.csi.nfs: field not declared in schema.
Changes:
Removed the redundant nfs: key from deploy/helm/templates/persistentVolume.yaml.
Ensured the csi block correctly identifies the driver and volumeHandle as top-level properties of the CSI spec.
Added quoting to values to ensure string safety during template rendering.
Fixes #38947
Automation
/ok-to-test tags="helm, eks, efs"
🔍 Cypress test results
[!CAUTION]
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
[x] Yes (This unblocks AWS EFS users who were previously unable to install the chart).
[ ] No
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features
Documentation