Skip to content

chore: deprecate ce helm charts and publish ee helm charts #37794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pratapaprasanna merged 5 commits into from
Dec 31, 2024
Merged

chore: deprecate ce helm charts and publish ee helm charts #37794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2fd7e52
chore: deprecate ce helm charts and move to ee helm charts
pratapaprasanna Nov 28, 2024
6534291
chore: update helm chart for appsmith-ce
pratapaprasanna Dec 31, 2024
a01009b
chore: update helm charts in ce
pratapaprasanna Dec 31, 2024
6f5c2a8
chore: update init container and add affinity labels
pratapaprasanna Dec 31, 2024
0e76b6a
chore: Bump helm chart version
pratapaprasanna Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/helm-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,6 @@ jobs:
helm dep build
helm package .
aws s3 cp s3://${{ secrets.HELM_S3_BUCKET }}/index.yaml .
helm repo index . --url https://${{ secrets.HELM_S3_BUCKET }} --merge index.yaml
helm repo index . --url https://${{ secrets.HELM_S3_BUCKET_URL }} --merge index.yaml
aws s3 cp appsmith-${{ steps.chart-version.outputs.version }}.tgz s3://${{ secrets.HELM_S3_BUCKET }}
aws s3 cp index.yaml s3://${{ secrets.HELM_S3_BUCKET }}
9 changes: 0 additions & 9 deletions deploy/helm/Chart.lock
View file
Open in desktop

This file was deleted.

32 changes: 21 additions & 11 deletions deploy/helm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,25 @@ sources:
- https://github.com/appsmithorg/appsmith
home: https://www.appsmith.com/
icon: https://assets.appsmith.com/appsmith-icon.png
version: 2.3.0
version: 3.4.6
dependencies:
- condition: redis.enabled
name: redis
version: 16.11.2
appVersion: 6.2.7
repository: https://charts.bitnami.com/bitnami
- condition: mongodb.enabled
name: mongodb
version: 12.1.16
appVersion: 6.0.10
repository: https://charts.bitnami.com/bitnami
- condition: redis.enabled
name: redis
version: 16.11.2
appVersion: 6.2.7
repository: https://charts.bitnami.com/bitnami
- condition: mongodb.enabled
name: mongodb
version: 12.1.16
appVersion: 6.0.10
repository: https://charts.bitnami.com/bitnami
- condition: postgresql.enabled
name: postgresql
version: 11.9.5
appVersion: 14.12.0
repository: https://charts.bitnami.com/bitnami
- condition: prometheus.enabled
name: prometheus
version: 25.27.0
appVersion: 2.54.1
repository: https://prometheus-community.github.io/helm-charts
Comment on lines +16 to +35

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

PostgreSQL and Prometheus versions need updates in Chart.yaml

The current versions in Chart.yaml are outdated compared to the latest stable releases:

  • PostgreSQL chart version 11.9.5 (appVersion: 14.12.0) should be updated to 16.2.2 (appVersion: 17.2.0)
  • Prometheus chart version 25.27.0 (appVersion: 2.54.1) should be updated to 26.0.0 (appVersion: v3.0.0)
🔗 Analysis chain

Dependencies configuration looks good with a suggestion

The dependencies are well-structured with specific versions and proper conditions. However, it's recommended to verify the compatibility of these versions, especially for the new PostgreSQL and Prometheus integrations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for potential version compatibility issues in Helm dependencies
echo "Checking PostgreSQL compatibility..."
curl -s https://raw.githubusercontent.com/bitnami/charts/master/bitnami/postgresql/Chart.yaml | grep -E "version:|appVersion:"

echo "Checking Prometheus compatibility..."
curl -s https://raw.githubusercontent.com/prometheus-community/helm-charts/main/charts/prometheus/Chart.yaml | grep -E "version:|appVersion:"

# Check if values.yaml contains corresponding configurations
echo "Verifying values.yaml configurations..."
rg -A 5 "postgresql:|prometheus:"

Length of output: 23365

2 changes: 2 additions & 0 deletions deploy/helm/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ To change Appsmith configurations, you can use configuration UI in application o
| `applicationConfig.APPSMITH_OAUTH2_GOOGLE_CLIENT_SECRET`| `""` |
| `applicationConfig.APPSMITH_OAUTH2_GITHUB_CLIENT_ID` | `""` |
| `applicationConfig.APPSMITH_OAUTH2_GITHUB_CLIENT_SECRET`| `""` |
| `applicationConfig.APPSMITH_OAUTH2_OIDC_CLIENT_ID` | `""` |
| `applicationConfig.APPSMITH_OAUTH2_OIDC_CLIENT_SECRET` | `""` |
| `applicationConfig.APPSMITH_CLIENT_LOG_LEVEL` | `""` |
| `applicationConfig.APPSMITH_MAIL_ENABLED` | `""` |
| `applicationConfig.APPSMITH_MAIL_HOST` | `""` |
Expand Down
21 changes: 20 additions & 1 deletion deploy/helm/templates/configMap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{{- $nameSpace := include "appsmith.namespace" . -}}
{{- $name := include "appsmith.fullname" . -}}
{{- $mongoUser := .Values.mongodb.auth.rootUser -}}
{{- $mongoPassword := .Values.mongodb.auth.rootPassword -}}
{{- $mongoServicename := .Values.mongodb.service.nameOverride -}}
{{- $postgresqlUser := .Values.postgresql.auth.username -}}
{{- $postgresqlPassword := .Values.postgresql.auth.password -}}
{{- $postgresqlDatabase := .Values.postgresql.auth.database -}}
{{- $releaseName := .Release.Name -}}
apiVersion: v1
kind: ConfigMap
Expand All @@ -18,11 +20,28 @@ data:
{{- end }}

{{- range $key, $value := .Values.applicationConfig }}
{{- if and (eq "APPSMITH_KEYCLOAK_DB_DRIVER" $key) ( not $value) }}
{{ $key }}: {{ $.Values.postgresql.enabled | ternary "postgresql" "h2" | quote }}
{{- end }}

{{- if and (eq "APPSMITH_KEYCLOAK_DB_URL" $key) ( not $value) }}
{{ $key }}: {{ $.Values.postgresql.enabled | ternary (printf "%s-postgresql.%s.svc.cluster.local:5432/%s" $releaseName $nameSpace $postgresqlDatabase) "${jboss.server.data.dir}" | quote }}
{{- end }}

{{- if and (eq "APPSMITH_KEYCLOAK_DB_USERNAME" $key) ( not $value) }}
{{ $key }}: {{ $.Values.postgresql.enabled | ternary $postgresqlUser "sa" | quote }}
{{- end }}

{{- if and (eq "APPSMITH_KEYCLOAK_DB_PASSWORD" $key) ( not $value) }}
{{ $key }}: {{ $.Values.postgresql.enabled | ternary $postgresqlPassword "sa" | quote }}
{{- end }}

{{- if and (eq "APPSMITH_REDIS_URL" $key) ( not $value) }}
{{- if $.Values.redis.enabled }}
{{ $key }}: redis://{{ $releaseName }}-redis-master.{{ $nameSpace }}.svc.cluster.local:6379
{{- end }}
{{- end }}

{{- if $value }}
{{ $key }}: {{ $value | quote }}
{{- end }}
Expand Down
88 changes: 77 additions & 11 deletions deploy/helm/templates/statefulset.yaml → deploy/helm/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
{{- $updateStrategy := .Values.updateStrategy | default dict }}
{{- $postgresuser := .Values.postgresql.auth.username }}
{{- $postgrespass := .Values.postgresql.auth.password }}
{{- $postgrespass := .Values.postgresql.auth.password }}
Comment on lines +3 to +4

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate variable declaration

The $postgrespass variable is declared twice consecutively.

 {{- $postgrespass := .Values.postgresql.auth.password }}
-{{- $postgrespass := .Values.postgresql.auth.password }}
📝 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.

Suggested change
{{- $postgrespass := .Values.postgresql.auth.password }}
{{- $postgrespass := .Values.postgresql.auth.password }}
{{- $postgrespass := .Values.postgresql.auth.password }}

{{- $releaseName := include "appsmith.fullname" . -}}
apiVersion: apps/v1
kind: StatefulSet
kind: {{ if not .Values.autoscaling.enabled }}StatefulSet{{- else }}Deployment{{- end }}
metadata:
name: {{ include "appsmith.fullname" . }}
namespace: {{ include "appsmith.namespace" . }}
labels:
{{- include "appsmith.labels" . | nindent 4 }}
spec:
{{- if not .Values.autoscaling.enabled }}
replicas: 1
serviceName: {{ include "appsmith.fullname" . }}
updateStrategy:
type: {{ .Values.strategyType }}
{{- else }}
strategy:
type: {{ .Values.strategyType | default "RollingUpdate" }}
rollingUpdate:
maxSurge: {{ dig "maxSurge" 1 $updateStrategy }}
maxUnavailable: {{ dig "maxUnavailable" "0" $updateStrategy }}
{{- end }}
selector:
matchLabels:
{{- include "appsmith.selectorLabels" . | nindent 6 }}
Expand All @@ -25,6 +37,10 @@ spec:
{{- toYaml .Values.podLabels | nindent 8 }}
{{- end }}
spec:
{{- with .Values.topologySpreadConstraints }}
topologySpreadConstraints:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- if .Values.schedulerName }}
schedulerName: {{ .Values.schedulerName | quote }}
{{- end }}
Expand Down Expand Up @@ -62,19 +78,32 @@ spec:
{{- end }}
command: ['sh', '-c', "until mongo --host appsmith-mongodb.{{.Release.Namespace}}.svc.cluster.local --eval 'db.runCommand({ping:1})' ; do echo waiting for mongo; sleep 2; done"]
{{- end }}
{{- if .Values.postgresql.enabled }}
- name: psql-init-container
{{- if ((.Values.initContainer.postgresql).image) }}
image: {{ .Values.initContainer.postgresql.image }}
{{- else}}
image: docker.io/bitnami/postgresql:14.5.0-debian-11-r21
{{- end}}
command: ['sh', '-c', "until pg_isready -U $postgresuser -d $postgresdb -h {{.Release.Name}}-postgresql.{{.Release.Namespace}}.svc.cluster.local; do echo waiting for postgresql; sleep 2; done"]
{{- end }}
Comment on lines +81 to +89

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid hardcoded image versions in init containers

The PostgreSQL init container uses a hardcoded image version. Consider moving this to values.yaml for better maintainability.

-        image: docker.io/bitnami/postgresql:14.5.0-debian-11-r21
+        image: {{ .Values.initContainer.postgresql.defaultImage | default "docker.io/bitnami/postgresql:14.5.0-debian-11-r21" }}
📝 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.

Suggested change
{{- if .Values.postgresql.enabled }}
- name: psql-init-container
{{- if ((.Values.initContainer.postgresql).image) }}
image: {{ .Values.initContainer.postgresql.image }}
{{- else}}
image: docker.io/bitnami/postgresql:14.5.0-debian-11-r21
{{- end}}
command: ['sh', '-c', "until pg_isready -U $postgresuser -d $postgresdb -h {{.Release.Name}}-postgresql.{{.Release.Namespace}}.svc.cluster.local; do echo waiting for postgresql; sleep 2; done"]
{{- end }}
{{- if .Values.postgresql.enabled }}
- name: psql-init-container
{{- if ((.Values.initContainer.postgresql).image) }}
image: {{ .Values.initContainer.postgresql.image }}
{{- else}}
image: {{ .Values.initContainer.postgresql.defaultImage | default "docker.io/bitnami/postgresql:14.5.0-debian-11-r21" }}
{{- end}}
command: ['sh', '-c', "until pg_isready -U $postgresuser -d $postgresdb -h {{.Release.Name}}-postgresql.{{.Release.Namespace}}.svc.cluster.local; do echo waiting for postgresql; sleep 2; done"]
{{- end }}

containers:
- name: {{ .Values.containerName }}
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
{{- $customImage := .Values._image | default dict }}
image: {{ dig "registry" "index.docker.io" $customImage }}/{{ dig "repository" "appsmith/appsmith-ee" $customImage }}:{{ dig "tag" (.Values.image.tag | default "latest") $customImage }}
imagePullPolicy: {{ dig "pullPolicy" "IfNotPresent" $customImage }}
Comment on lines +94 to +96

@coderabbitai coderabbitai Bot Dec 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Image pull secrets configuration required for EE repository

The deployment template expects image pull secrets to be configured via .Values.image.pullSecrets, but the default value is empty (""). Since you're using the enterprise edition repository (appsmith/appsmith-ee), you must configure the pull secrets to access the private repository.

  • Add image pull secrets configuration in your values override:
image:
  pullSecrets: "your-ee-registry-secret"
🔗 Analysis chain

Verify EE image repository access

The image configuration points to the enterprise edition repository. Ensure that proper credentials are configured for accessing the EE images.

The previous results show that imagePullSecrets exist, but I need to verify their actual configuration to ensure they're properly set up for the EE repository.

I need to verify the default configuration for image pull secrets to ensure they're properly documented and configured for the EE repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if image pull secrets are properly configured when EE repository is used
rg -l "appsmith/appsmith-ee" | xargs rg "imagePullSecrets"

Length of output: 244

Script:

#!/bin/bash
# Check the values.yaml for image pull secrets configuration
cat deploy/helm/values.yaml | grep -A 10 "imagePullSecrets"

# Check if there are any other image pull secrets configurations in the deployment template
cat deploy/helm/templates/deployment.yaml | grep -A 5 "imagePullSecrets"

Length of output: 766

Script:

#!/bin/bash
# Check the default values for image pull secrets in values.yaml
cat deploy/helm/values.yaml | grep -A 15 "^image:"

# Check if there's any documentation about image pull secrets
cat deploy/helm/README.md 2>/dev/null || cat deploy/helm/values.yaml | grep -A 2 -B 2 "pullSecrets"

Length of output: 16873

ports:
- name: http
containerPort: {{ .Values.HTTPContainerPort | default 80 }}
protocol: TCP
- name: https
containerPort: 443
protocol: TCP
- name: metrics
containerPort: {{ .Values.metrics.port }}
protocol: TCP
{{- $probes := .Values.probes | default dict }}
startupProbe:
# The `livenessProbe` and `readinessProbe` will be disabled until the `startupProbe` is successful.
Expand All @@ -100,42 +129,79 @@ spec:
volumeMounts:
- name: data
mountPath: /appsmith-stacks
{{- if .Values.customCAcert }}
- name: ca-cert
mountPath: "/appsmith-stacks/ca-certs"
{{- end }}
env:
{{- if .Values.HTTPContainerPort }}
- name: PORT
value: "{{ .Values.HTTPContainerPort }}"
{{- end }}
- name: APPSMITH_ENABLE_EMBEDDED_DB
value: "0"
- name: JGROUPS_DISCOVERY_PROTOCOL
value: kubernetes.KUBE_PING
- name: APPSMITH_HEADLESS_SVC
value: {{ include "appsmith.fullname" . }}-headless
envFrom:
- configMapRef:
name: {{ include "appsmith.fullname" . }}
{{- if .Values.secretName }}
- secretRef:
name: {{ .Values.secretName }}
{{- end }}
{{- if .Values.secrets }}
- secretRef:
name: {{ include "appsmith.fullname" . }}
{{- end }}
{{- if .Values.externalSecrets.enabled }}
- secretRef:
name: "{{ include "appsmith.fullname" . }}-external-secret"
{{- end }}
{{- if .Values.image.pullSecrets}}
imagePullSecrets:
- name: {{ .Values.image.pullSecrets }}
{{- end }}
volumes:
{{- if .Values.customCAcert }}
- name: ca-cert
configMap:
name: {{ $releaseName }}-trustedca
items:
{{- range $key, $value := .Values.customCAcert }}
- key: {{ $key }}
path: {{ $key }}.crt
{{- end }}
{{- end }}
{{- if not .Values.persistence.enabled }}
- name: data
emptyDir: {}
{{- else }}
{{- else if and (not .Values.autoscaling.enabled) (.Values.persistence.enabled) }}
volumeClaimTemplates:
- metadata:
name: data
{{- if .Values.persistence.annotations }}
annotations: {{- include "tplvalues.render" (dict "value" .Values.persistence.annotations "context" $) | nindent 10 }}
{{- if .Values.persistence.annotations}}
annotations:
{{- include "tplvalues.render" (dict "value" .Values.persistence.annotations "context" $) | nindent 10 }}
{{- end }}
spec:
accessModes:
{{- range .Values.persistence.accessModes }}
- {{ . | quote }}
{{- end }}
- ReadWriteOnce
resources:
requests:
storage: {{ .Values.persistence.size | quote }}
{{ include "storage.class" (dict "persistence" .Values.persistence "global" .Values.global) }}
{{- if .Values.persistence.volumeClaimTemplates.selector }}
selector:
{{- include "tplvalues.render" (dict "value" .Values.persistence.volumeClaimTemplates.selector "context" $) | nindent 10 }}
{{- end }}
{{ include "storage.class" (dict "persistence" .Values.persistence "global" .Values.global) | nindent 8 }}
{{- else }}
- name: data
persistentVolumeClaim:
{{- if .Values.persistence.existingClaim.enabled }}
claimName: {{ .Values.persistence.existingClaim.claimName }}
{{- else }}
claimName: {{ include "appsmith.fullname" . }}
{{- end }}
{{- end }}
18 changes: 18 additions & 0 deletions deploy/helm/templates/external-secrets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{{- if .Values.externalSecrets.enabled }}
apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
name: "{{ include "appsmith.fullname" . }}-external-secret"
namespace: {{ include "appsmith.namespace" . }}
spec:
refreshInterval: {{ .Values.externalSecrets.refreshInterval }}
secretStoreRef:
name: secretstore
kind: SecretStore
target:
name: "{{ include "appsmith.fullname" . }}-external-secret"
creationPolicy: Owner
Comment on lines +7 to +14

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix indentation for YAML compliance

The YAML indentation is inconsistent which could cause parsing issues.

Apply this diff to fix the indentation:

 spec:
   refreshInterval: {{ .Values.externalSecrets.refreshInterval }}
   secretStoreRef:
-    name: secretstore
-    kind: SecretStore
+     name: secretstore
+     kind: SecretStore
   target:
-    name: "{{ include "appsmith.fullname" . }}-external-secret"
-    creationPolicy: Owner
+     name: "{{ include "appsmith.fullname" . }}-external-secret"
+     creationPolicy: Owner
📝 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.

Suggested change
spec:
refreshInterval: {{ .Values.externalSecrets.refreshInterval }}
secretStoreRef:
name: secretstore
kind: SecretStore
target:
name: "{{ include "appsmith.fullname" . }}-external-secret"
creationPolicy: Owner
spec:
refreshInterval: {{ .Values.externalSecrets.refreshInterval }}
secretStoreRef:
name: secretstore
kind: SecretStore
target:
name: "{{ include "appsmith.fullname" . }}-external-secret"
creationPolicy: Owner
🧰 Tools
🪛 yamllint (1.35.1)

[warning] 10-10: wrong indentation: expected 5 but found 4

(indentation)

[warning] 13-13: wrong indentation: expected 5 but found 4

(indentation)

dataFrom:
- extract:
key: {{ .Values.externalSecrets.remoteSecretName }}
{{- end }}
Comment on lines +15 to +18

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Remote secret path configuration needs validation

Based on the values.yaml content, I found that remoteSecretName is documented under externalSecrets parameters but with an incorrect name. The actual parameter in values.yaml is remoteNameSecret which could cause configuration issues.

  • deploy/helm/values.yaml: Update parameter name from remoteNameSecret to remoteSecretName to match the template usage
🔗 Analysis chain

Verify secret path configuration

The remote secret path is critical for security. Ensure it's properly validated and documented in values.yaml.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if remoteSecretName is documented in values.yaml
rg -A 5 "remoteSecretName:" deploy/helm/values.yaml

# Check for any hardcoded secret paths in the codebase
rg -g '*.yaml' -g '*.yml' "key: .*secret.*"

Length of output: 97

Script:

#!/bin/bash
# Let's check the values.yaml file content directly
cat deploy/helm/values.yaml

# Check for any external secrets configuration patterns
rg -g '*.yaml' -g '*.yml' "externalSecrets:"

# Look for any documentation about external secrets
fd README.md --exec cat {}

Length of output: 108393

🧰 Tools
🪛 yamllint (1.35.1)

[warning] 17-17: wrong indentation: expected 7 but found 6

(indentation)

31 changes: 31 additions & 0 deletions deploy/helm/templates/headless-svc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
apiVersion: v1
kind: Service
metadata:
name: {{ include "appsmith.fullname" . }}-headless
namespace: {{ include "appsmith.namespace" . }}
labels:
{{- include "appsmith.labels" . | nindent 4 }}
{{- if or .Values.service.annotations .Values.commonAnnotations }}
annotations:
{{- if .Values.service.annotations }}
{{- include "tplvalues.render" ( dict "value" .Values.service.annotations "context" $) | nindent 4 }}
{{- end }}
{{- if .Values.commonAnnotations }}
{{- include "tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
{{- end }}
{{- end }}
spec:
type: ClusterIP
clusterIP: None
clusterIPs:
- None
internalTrafficPolicy: Cluster
ipFamilies:
- IPv4
ipFamilyPolicy: SingleStack
ports:
- name: http
port: 8080
targetPort: 8080
selector:
{{- include "appsmith.selectorLabels" . | nindent 4 }}
41 changes: 41 additions & 0 deletions deploy/helm/templates/hpa.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{{- if and (.Values.autoscaling.enabled) (not .Values.keda.enabled) }}
apiVersion: {{ ternary "autoscaling/v2" "autoscaling/v2beta2" (.Capabilities.APIVersions.Has "autoscaling/v2") }}
kind: HorizontalPodAutoscaler
metadata:
name: {{ include "appsmith.fullname" . }}
namespace: {{ include "appsmith.namespace" . }}
labels:
{{- include "appsmith.labels" . | nindent 4 }}
spec:
scaleTargetRef:
apiVersion: apps/v1
kind: Deployment
name: {{ include "appsmith.fullname" . }}
minReplicas: {{ .Values.autoscaling.minReplicas }}
maxReplicas: {{ .Values.autoscaling.maxReplicas }}
metrics:
{{- if .Values.autoscaling.targetCPUUtilizationPercentage }}
- type: Resource
resource:
name: cpu
{{- if .Capabilities.APIVersions.Has "autoscaling/v2"}}
target:
type: Utilization
averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }}
{{- else }}
targetAverageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }}
{{- end }}
{{- end }}
{{- if .Values.autoscaling.targetMemoryUtilizationPercentage }}
- type: Resource
resource:
name: memory
{{- if .Capabilities.APIVersions.Has "autoscaling/v2"}}
target:
type: Utilization
averageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }}
{{- else }}
targetAverageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }}
{{- end }}
{{- end }}
Comment on lines +16 to +40

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add validation constraints for autoscaling metrics

The current CPU utilization is set to 5%, which is unusually low and could lead to aggressive scaling. Add validation in values.yaml:

  • Set reasonable defaults (e.g., CPU: 50-80%, Memory: 60-90%)
  • Add comments documenting the acceptable ranges
  • Consider adding a validation helper in _helpers.tpl for these metrics
🔗 Analysis chain

Add validation for utilization percentages

The CPU and memory utilization percentages should be validated to ensure they're within reasonable bounds.

Let me check if there are any validation helpers or constraints defined in other Helm-related files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any validation helpers or value constraints in the templates
rg -A 3 "targetCPUUtilizationPercentage|targetMemoryUtilizationPercentage" deploy/helm/values.yaml

Length of output: 155

Script:

#!/bin/bash
# Check for any validation or constraint-related code in Helm templates
rg -g "deploy/helm/**" "_helpers|validation|constraint" -A 3

# Check the complete autoscaling section in values.yaml
rg -g "deploy/helm/values.yaml" "autoscaling:" -A 10

Length of output: 538

{{- end }}
28 changes: 28 additions & 0 deletions deploy/helm/templates/import.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{{- /* Fail helm chart if autoscaling is enabled and mongo is disabled */ -}}
{{- if and ( .Values.autoscaling.enabled ) ( not .Values.mongodb.enabled ) }}
{{- if and ( not .Values.applicationConfig.APPSMITH_DB_URL ) ( not .Values.applicationConfig.APPSMITH_MONGODB_URI ) }}
{{- fail "To enable autoscaling on Appsmith, MongoDB needs to be enabled or an external MongoDB needs to be configured. Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
{{- end }}
{{- end }}

{{- /* Fail helm chart if keycloak is disabled and postgresql is enabled */ -}}
{{- if and ( .Values.postgresql.enabled ) (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "1" )}}
{{- fail "Keycloak is disabled therefore postgresql is not required. Please disable postgres or to enable keycloak on Appsmith, set APPSMITH_DISABLE_EMBEDDED_KEYCLOAK to \"0\" Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
{{- end }}

{{- /* Fail helm chart if autoscaling, keycloak is enabled and postgresql is disabled */ -}}
{{- if and ( .Values.autoscaling.enabled ) ( not .Values.postgresql.enabled ) ( not .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL ) (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "0" )}}
{{- fail "To enable autoscaling on Appsmith, PostgreSQL needs to be enabled or an external PostgreSQL has to be configured. Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
{{- end }}
Comment on lines +13 to +16

@coderabbitai coderabbitai Bot Nov 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider breaking down the complex condition for better maintainability

The validation logic combines multiple conditions which might be harder to maintain.

-{{- if and ( .Values.autoscaling.enabled ) ( not .Values.postgresql.enabled ) ( not .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL ) (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "0" )}}
+{{- /* First check if autoscaling and Keycloak are enabled */ -}}
+{{- if and .Values.autoscaling.enabled (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "0") }}
+{{- /* Then verify PostgreSQL configuration */ -}}
+{{- if and (not .Values.postgresql.enabled) (not .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL) }}
{{- fail "To enable autoscaling on Appsmith, PostgreSQL needs to be enabled or an external PostgreSQL has to be configured. Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
+{{- end }}
{{- 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.

Suggested change
{{- /* Fail helm chart if autoscaling, keycloak is enabled and postgresql is disabled */ -}}
{{- if and ( .Values.autoscaling.enabled ) ( not .Values.postgresql.enabled ) ( not .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL ) (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "0" )}}
{{- fail "To enable autoscaling on Appsmith, PostgreSQL needs to be enabled or an external PostgreSQL has to be configured. Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
{{- end }}
{{- /* Fail helm chart if autoscaling, keycloak is enabled and postgresql is disabled */ -}}
{{- /* First check if autoscaling and Keycloak are enabled */ -}}
{{- if and .Values.autoscaling.enabled (eq .Values.applicationConfig.APPSMITH_DISABLE_EMBEDDED_KEYCLOAK "0") }}
{{- /* Then verify PostgreSQL configuration */ -}}
{{- if and (not .Values.postgresql.enabled) (not .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL) }}
{{- fail "To enable autoscaling on Appsmith, PostgreSQL needs to be enabled or an external PostgreSQL has to be configured. Refer: https://docs.appsmith.com/getting-started/setup/installation-guides/kubernetes#install-appsmith" }}
{{- end }}
{{- end }}


{{- /* Fail helm chart if mongodb is enabled along with APPSMITH_DB_URL in the ApplicationConfig */ -}}
{{- if ( .Values.mongodb.enabled ) }}
{{- if or ( .Values.applicationConfig.APPSMITH_DB_URL ) ( .Values.applicationConfig.APPSMITH_MONGODB_URI ) }}
{{- fail "MongoDB is enabled, but also found APPSMITH_DB_URL or APPSMITH_MONGODB_URI configured to an external instance, MongoDB needs to be disabled if using an external MongoDB instance" }}
{{- end }}
{{- end }}

{{- /* Fail helm chart if postgresql is enabled along with APPSMITH_DB_URL in the ApplicationConfig */ -}}
{{- if and ( .Values.postgresql.enabled ) ( .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_DRIVER ) ( .Values.applicationConfig.APPSMITH_KEYCLOAK_DB_URL ) }}
{{- fail "PostgreSQL is enabled, but also found APPSMITH_KEYCLOAK_DB_URL configured to an external instance, PostgreSQL needs to be disabled if using an external PostgreSQL instance" }}
{{- end }}
Loading