WIP: feat: implement helm chart v2 #1909

utkuozdemir · 2025-11-21T13:32:40Z

Add a brand new, more powerful helm chart.

Closes #1884.

Copilot

Pull Request Overview

This PR implements a new Helm chart (v2) for deploying Omni, along with supporting code changes to enable configuration validation skipping and better debugging capabilities. The changes introduce a comprehensive Kubernetes deployment configuration with ingress, services, persistence, and security settings.

Key Changes:

Added skipValidation parameter to config initialization to support Helm chart templating
Introduced --print-config-and-exit flag for debugging configuration issues
Created complete Helm chart v2 with deployment, services, ingress, and persistence resources

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
internal/pkg/config/config.go	Added skipValidation parameter to Init function; updated field documentation (contains errors)
internal/pkg/config/config_test.go	Updated test to include new skipValidation parameter in Init call
internal/backend/server.go	Added debug logging for registered gRPC services
cmd/omni/pkg/app/app.go	Updated PrepareConfig to accept skipValidation parameter
cmd/omni/cmd/cmd.go	Added print-config-and-exit flag, refactored logger setup, added YAML import (incorrect path)
deploy/helm/omni-v2/Chart.yaml	New Helm chart metadata with version 2.0.0
deploy/helm/omni-v2/values.yaml	Comprehensive configuration values for Omni deployment
deploy/helm/omni-v2/.helmignore	Standard Helm ignore patterns
deploy/helm/omni-v2/templates/_helpers.tpl	Helm template helpers for naming and labels
deploy/helm/omni-v2/templates/deployment.yaml	Main deployment resource with volume mounts (has volume mount issue)
deploy/helm/omni-v2/templates/configmap.yaml	ConfigMap for Omni configuration (uses non-existent function)
deploy/helm/omni-v2/templates/secret.yaml	Secret template for environment variables (creates empty secrets)
deploy/helm/omni-v2/templates/serviceaccount.yaml	ServiceAccount resource definition
deploy/helm/omni-v2/templates/service.yaml	Main service for Omni endpoints
deploy/helm/omni-v2/templates/service-wireguard.yaml	Separate service for WireGuard with NodePort
deploy/helm/omni-v2/templates/pvc.yaml	PersistentVolumeClaim for data storage
deploy/helm/omni-v2/templates/ingress-ui.yaml	Ingress for UI access
deploy/helm/omni-v2/templates/ingress-grpc.yaml	Ingress for gRPC services
deploy/helm/omni-v2/templates/ingress-siderolink-api.yaml	Ingress for Siderolink API
deploy/helm/omni-v2/templates/ingress-k8s-proxy.yaml	Ingress for Kubernetes proxy (incorrect port reference)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

deploy/helm/omni-v2/templates/configmap.yaml

Copilot · 2025-11-21T13:43:29Z

deploy/helm/omni-v2/values.yaml

+    name: default
+  services:
+    api:
+      # -- REQUIRED: The URL used to access Omni UI and the API. When ingress is used, it must match the ingress.main config (host, TLS, port).


The comment references "ingress.main config" but the actual ingress configuration is at "ingress.ui", not "ingress.main". This should say "ingress.ui config" instead.

Suggested change

# -- REQUIRED: The URL used to access Omni UI and the API. When ingress is used, it must match the ingress.main config (host, TLS, port).

# -- REQUIRED: The URL used to access Omni UI and the API. When ingress is used, it must match the ingress.ui config (host, TLS, port).

deploy/helm/omni-v2/templates/deployment.yaml

deploy/helm/omni-v2/templates/ingress-k8s-proxy.yaml

deploy/helm/omni-v2/Chart.yaml

internal/pkg/config/config.go

deploy/helm/omni-v2/values.yaml

deploy/helm/omni-v2/templates/secret.yaml

Add a brand new, more powerful helm chart. Signed-off-by: Utku Ozdemir <[email protected]>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-21T14:22:07Z

cmd/omni/cmd/cmd.go

 	"github.com/spf13/cobra"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
+	"go.yaml.in/yaml/v4"


The import path go.yaml.in/yaml/v4 appears unusual. The standard YAML library for Go uses the import path gopkg.in/yaml.v3. Please verify this is the correct package and that go.yaml.in/yaml/v4 is a valid, maintained library. If this is experimental or an RC version (v4.0.0-rc.2 as seen in go.mod), consider using the stable gopkg.in/yaml.v3 instead.

Suggested change

"go.yaml.in/yaml/v4"

"gopkg.in/yaml.v3"

Copilot · 2025-11-21T14:22:07Z

deploy/helm/omni-v2/values.yaml

+      endpoint: https://siderolink.omni.example.com # TODO: probably needs to be advertisedURL instead
+      # advertisedURL: ""


This TODO comment should be resolved before merging. Either confirm that endpoint is correct and remove the TODO, or change the field to advertisedURL as suggested.

Suggested change

endpoint: https://siderolink.omni.example.com # TODO: probably needs to be advertisedURL instead

# advertisedURL: ""

advertisedURL: https://siderolink.omni.example.com

Copilot · 2025-11-21T14:22:08Z

deploy/helm/omni-v2/templates/service.yaml

+spec:
+  type: {{ .Values.service.main.type }}
+  {{- with .Values.service.main.clusterIP }}
+  clusterIP: {{ . | quote}}


Missing space before the closing braces in the template expression.

Suggested change

clusterIP: {{ . | quote}}

clusterIP: {{ . | quote }}

Copilot · 2025-11-21T14:22:08Z

deploy/helm/omni-v2/templates/service-wireguard.yaml

+spec:
+  type: {{ .Values.service.wireguard.type }}
+  {{- with .Values.service.wireguard.clusterIP }}
+  clusterIP: {{ . | quote}}


Missing space before the closing braces in the template expression.

Suggested change

clusterIP: {{ . | quote}}

clusterIP: {{ . | quote }}

Copilot · 2025-11-21T14:22:09Z

deploy/helm/omni-v2/templates/ingress-grpc.yaml

+          - path: omni.resources.ResourceService
+            pathType: ImplementationSpecific
+            backend:
+              service:
+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: oidc.OIDCService
+            pathType: ImplementationSpecific
+            backend:
+              service:
+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: management.ManagementService
+            pathType: ImplementationSpecific
+            backend:
+              service:
+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: auth.AuthService
+            pathType: ImplementationSpecific
+            backend:
+              service:
+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: cosi.resource.State


The path value should start with a forward slash. gRPC service paths in Kubernetes Ingress should be formatted as /omni.resources.ResourceService (with a leading slash) for proper routing. Without the leading slash, this will likely fail to match incoming requests.

Suggested change

- path: omni.resources.ResourceService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: oidc.OIDCService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: management.ManagementService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: auth.AuthService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: cosi.resource.State

- path: /omni.resources.ResourceService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: /oidc.OIDCService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: /management.ManagementService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: /auth.AuthService

pathType: ImplementationSpecific

backend:

service:

name: {{ include "omni-v2.fullname" $ }}

port:

number: {{ .Values.service.main.omniPort }}

- path: /cosi.resource.State

Copilot · 2025-11-21T14:22:09Z

deploy/helm/omni-v2/templates/ingress-grpc.yaml

+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: auth.AuthService


The path value should start with a forward slash. Change to /auth.AuthService for proper routing.

Suggested change

- path: auth.AuthService

- path: /auth.AuthService

Copilot · 2025-11-21T14:22:10Z

deploy/helm/omni-v2/templates/service-wireguard.yaml

+  ports:
+    - name: wireguard
+      {{- with .Values.service.wireguard.nodePort }}
+      nodePort: {{ .}}


Missing space before the closing braces in the template expression.

Suggested change

nodePort: {{ .}}

nodePort: {{ . }}

Copilot · 2025-11-21T14:22:10Z

deploy/helm/omni-v2/templates/ingress-grpc.yaml

+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: management.ManagementService


The path value should start with a forward slash. Change to /management.ManagementService for proper routing.

Suggested change

- path: management.ManagementService

- path: /management.ManagementService

Copilot · 2025-11-21T14:22:10Z

deploy/helm/omni-v2/templates/ingress-grpc.yaml

+                name: {{ include "omni-v2.fullname" $ }}
+                port:
+                  number: {{ .Values.service.main.omniPort }}
+          - path: cosi.resource.State


The path value should start with a forward slash. Change to /cosi.resource.State for proper routing.

Suggested change

- path: cosi.resource.State

- path: /cosi.resource.State

Copilot · 2025-11-21T14:22:11Z

deploy/helm/omni-v2/values.yaml

+    siderolink:
+      wireGuard:
+        # endpoint: 0.0.0.0:50180
+        advertisedEndpoint: IP_HERE:30180


The placeholder IP_HERE should be replaced with a more descriptive comment or example. Consider using a comment like # advertisedEndpoint: 1.2.3.4:30180 or providing clearer documentation about what IP address should be used here (e.g., "your cluster's external IP").

Suggested change

advertisedEndpoint: IP_HERE:30180

# advertisedEndpoint: 1.2.3.4:30180 # Replace with your cluster's external IP and port

utkuozdemir requested a review from Copilot November 21, 2025 13:32

utkuozdemir self-assigned this Nov 21, 2025

Copilot started reviewing on behalf of utkuozdemir November 21, 2025 13:33 View session

Copilot finished reviewing on behalf of utkuozdemir November 21, 2025 13:35

utkuozdemir mentioned this pull request Nov 21, 2025

Update the helm chart to be more robust. #1607

Open

Copilot AI reviewed Nov 21, 2025

View reviewed changes

utkuozdemir force-pushed the feat/helm-chart-v2 branch 2 times, most recently from 53bbf42 to 1a00a6c Compare November 21, 2025 14:08

feat: implement helm chart v2

b076a40

Add a brand new, more powerful helm chart. Signed-off-by: Utku Ozdemir <[email protected]>

utkuozdemir force-pushed the feat/helm-chart-v2 branch from 1a00a6c to b076a40 Compare November 21, 2025 14:09

utkuozdemir requested a review from Copilot November 21, 2025 14:10

Copilot started reviewing on behalf of utkuozdemir November 21, 2025 14:11 View session

Copilot finished reviewing on behalf of utkuozdemir November 21, 2025 14:14

Copilot AI reviewed Nov 21, 2025

View reviewed changes

	# -- REQUIRED: The URL used to access Omni UI and the API. When ingress is used, it must match the ingress.main config (host, TLS, port).
	# -- REQUIRED: The URL used to access Omni UI and the API. When ingress is used, it must match the ingress.ui config (host, TLS, port).

		endpoint: https://siderolink.omni.example.com # TODO: probably needs to be advertisedURL instead
		# advertisedURL: ""

	endpoint: https://siderolink.omni.example.com # TODO: probably needs to be advertisedURL instead
	# advertisedURL: ""
	advertisedURL: https://siderolink.omni.example.com

	- path: management.ManagementService
	- path: /management.ManagementService

	advertisedEndpoint: IP_HERE:30180
	# advertisedEndpoint: 1.2.3.4:30180 # Replace with your cluster's external IP and port

WIP: feat: implement helm chart v2 #1909

Are you sure you want to change the base?

WIP: feat: implement helm chart v2 #1909

Uh oh!

Conversation

utkuozdemir commented Nov 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant