Skip to content

refactor: Remove nginx proxy and use Ingress directly #5

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

Open
alexnuttinck wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −69
Open

refactor: Remove nginx proxy and use Ingress directly #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd4dc6e
refactor: remove nginx proxy container and use Ingress directly
alexnuttinck Dec 11, 2025
fc8b080
[DEL] unused scripts
alexnuttinck Dec 11, 2025
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 Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: v2
name: odoo
description: An opiniated Helm Chart for deploying Odoo
type: application
version: 0.2.7
version: 0.3.0
appVersion: "16.0"
sources:
- https://github.com/odoo/odoo
Expand Down
48 changes: 0 additions & 48 deletions templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,6 @@
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "..fullname" . }}-nginx-conf
data:
nginx.conf: |-
upstream odoo {
server 0.0.0.0:8069;
}
upstream odoochat {
server 0.0.0.0:8072;
}
map $http_upgrade $connection_upgrade {
default upgrade;
'' close;
}
server {
listen 80;

client_max_body_size 0;
proxy_set_header Host $http_host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-SSL on;
proxy_set_header X-Forwarded-Protocol ssl;
proxy_set_header X-Forwarded-Proto https;

location /websocket {
proxy_pass http://odoochat;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection $connection_upgrade;
proxy_set_header X-Forwarded-Host $host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Real-IP $remote_addr;
}

location / {
proxy_redirect off;
proxy_pass http://odoo;
proxy_set_header X-Forwarded-Host $host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Real-IP $remote_addr;
}
gzip_types text/css text/scss text/plain text/xml application/xml application/json application/javascript;
gzip on;
}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "..fullname" . }}-default-conf
data:
Expand Down
14 changes: 0 additions & 14 deletions templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,6 @@ spec:
- name: {{ .Values.image.pullSecret }}
{{- end }}
containers:
- name: {{ include "..fullname" . }}-proxy
image: nginx
imagePullPolicy: {{ .Values.image.pullPolicy }}
ports:
- name: nginx-http
containerPort: {{ .Values.service.port }}
protocol: TCP
volumeMounts:
- name: {{ include "..fullname" . }}-nginx-conf
mountPath: /etc/nginx/conf.d/default.conf
subPath: nginx.conf
- name: {{ include "..fullname" . }}-service
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The container name includes '-service' suffix which may be confusing now that the nginx proxy is removed. Consider renaming to just use the fullname template without the suffix for clarity.

Suggested change
- name: {{ include "..fullname" . }}-service
- name: {{ include "..fullname" . }}

Copilot uses AI. Check for mistakes.
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
{{- if .Values.odoo.update.enabled }}
Expand Down Expand Up @@ -97,9 +86,6 @@ spec:
startupProbe:
{{- toYaml .Values.startupProbe | nindent 12 }}
volumes:
- name: {{ include "..fullname" . }}-nginx-conf
configMap:
name: "{{ include "..fullname" . }}-nginx-conf"
- name: {{ include "..fullname" . }}-odoo-conf
secret:
secretName: "{{ include "..fullname" . }}-odoo-conf"
Expand Down
2 changes: 1 addition & 1 deletion templates/ingress.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ spec:
port:
number: 80
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The hardcoded port number 80 is inconsistent with the service port change to 8069. This port reference should be updated to match the new service port.

Suggested change
number: 80
number: {{ $svcPort }}

Copilot uses AI. Check for mistakes.
{{- else }}
name: {{ $fullName }}-nginx
name: {{ $fullName }}
port:
number: {{ $svcPort }}
{{- end}}
Expand Down
6 changes: 3 additions & 3 deletions templates/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
apiVersion: v1
kind: Service
metadata:
name: {{ include "..fullname" . }}-nginx
name: {{ include "..fullname" . }}
labels:
{{- include "..labels" . | nindent 4 }}
spec:
type: {{ .Values.service.type }}
ports:
- name: nginx-http
- name: odoo-http
port: {{ .Values.service.port }}
targetPort: nginx-http
targetPort: odoo-http
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The targetPort references 'odoo-http' but there is no corresponding containerPort with this name defined in the deployment. The Odoo container needs to define a port with name 'odoo-http' and containerPort 8069.

Copilot uses AI. Check for mistakes.
protocol: TCP
selector:
{{- include "..selectorLabels" . | nindent 4 }}
Expand Down
12 changes: 10 additions & 2 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ image:
repository: odoo
# Pull policy can be set to Always, IfNotPresent, or Never
# ref: https://kubernetes.io/docs/concepts/containers/images/#updating-images
# Will set the policy for Odoo and Nginx
pullPolicy: IfNotPresent
# Overrides the image tag whose default is the chart appVersion.
tag: "16.0"
Expand Down Expand Up @@ -146,7 +145,7 @@ securityContext:

service:
type: ClusterIP
port: 80
port: 8069

ingress:
enabled: false
Expand All @@ -155,6 +154,15 @@ ingress:
cert-manager.io/cluster-issuer: letsencrypt
nginx.ingress.kubernetes.io/backend-protocol: HTTP
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The proxy-body-size annotation is set to '0' which disables size limits. Consider documenting this in a comment, as it allows unlimited upload sizes which may have security or performance implications.

Suggested change
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
# Setting proxy-body-size to "0" disables upload size limits.
# This allows unlimited upload sizes, which may have security or performance implications.
# Ensure this is intentional. To set a limit, use e.g. "50m" for 50 megabytes.

Copilot uses AI. Check for mistakes.
nginx.ingress.kubernetes.io/proxy-body-size: "0"
nginx.ingress.kubernetes.io/proxy-connect-timeout: "300"
nginx.ingress.kubernetes.io/proxy-send-timeout: "300"
nginx.ingress.kubernetes.io/proxy-read-timeout: "300"
nginx.ingress.kubernetes.io/configuration-snippet: |
proxy_set_header X-Forwarded-Host $http_host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Real-IP $remote_addr;
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The configuration snippet doesn't include WebSocket upgrade headers that were present in the nginx ConfigMap. Consider documenting whether WebSocket support (previously handled via /websocket location in nginx) is now managed differently or if additional configuration is needed.

Suggested change
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection "upgrade";

Copilot uses AI. Check for mistakes.
hosts:
- host: odoo.example.com
paths:
Expand Down