Skip to content

fix: reuse webhookport from values#1927

Merged
oliverbaehler merged 16 commits into
projectcapsule:mainfrom
oliverbaehler:fix/admission-port
May 29, 2026
Merged

fix: reuse webhookport from values#1927
oliverbaehler merged 16 commits into
projectcapsule:mainfrom
oliverbaehler:fix/admission-port

Conversation

@oliverbaehler
Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings May 29, 2026 06:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Replaces hardcoded 9443 webhook port references in the Helm chart templates with the configurable .Values.manager.webhookPort value, so changing the value in values.yaml actually propagates to both the Service and the Pod's container port.

Changes:

  • Use .Values.manager.webhookPort for the webhook Service port and targetPort.
  • Use .Values.manager.webhookPort for the manager Pod's admission containerPort.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
charts/capsule/templates/webhook-service.yaml Templated the admission Service port/targetPort from webhookPort instead of hardcoded 9443.
charts/capsule/templates/_pod.tpl Templated the admission containerPort from webhookPort instead of hardcoded 9443.

Copy link
Copy Markdown
Member

@Svarrogh1337 Svarrogh1337 left a comment

Choose a reason for hiding this comment

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

LGTM

@bakito
Copy link
Copy Markdown
Contributor

bakito commented May 29, 2026

It must be assured, that the port on the dynamic-webhook matches the pod and service ports.

Signed-off-by: Oliver Baehler <oliver@sudo-i.net>
@oliverbaehler
Copy link
Copy Markdown
Collaborator Author

@bakito agrre with the changes?

@bakito
Copy link
Copy Markdown
Contributor

bakito commented May 29, 2026

@bakito agrre with the changes?

no. The default port should also be considered here:

otherwise generated dynamic webhooks are pointing to the wrong service port

@bakito
Copy link
Copy Markdown
Contributor

bakito commented May 29, 2026

{{/*
Capsule Webhook service (Called with $.Path)

*/}}
{{- define "capsule.webhooks.service" -}}
  {{- include "capsule.webhooks.cabundle" $.ctx | nindent 0 }}
  {{- if $.ctx.Values.webhooks.service.url }}
url: {{ printf "%s/%s" (trimSuffix "/" $.ctx.Values.webhooks.service.url ) (trimPrefix "/" (required "Path is required for the function" $.path)) }}
  {{- else }}
service:
  name: {{ default (printf "%s-webhook-service" (include "capsule.fullname" $.ctx)) $.ctx.Values.webhooks.service.name }}
  namespace: {{ default $.ctx.Release.Namespace $.ctx.Values.webhooks.service.namespace }}
  port: {{ default $.ctx.Values.manager.webhookPort $.ctx.Values.webhooks.service.port }}
  path: {{ required "Path is required for the function" $.path }}
  {{- end }}
{{- end }}


{{/*
Capsule Webhook service (Without Path)
*/}}
{{- define "capsule.webhooks.serviceConfig" -}}
  {{- include "capsule.webhooks.cabundle" $ | nindent 0 }}
  {{- if $.Values.webhooks.service.url }}
url: {{ trimSuffix "/" $.Values.webhooks.service.url }}
  {{- else }}
service:
  name: {{ default (printf "%s-webhook-service" (include "capsule.fullname" $)) $.Values.webhooks.service.name }}
  namespace: {{ default $.Release.Namespace $.Values.webhooks.service.namespace }}
  port: {{ default $.Values.manager.webhookPort $.Values.webhooks.service.port }}
  {{- end }}
{{- end }}

@oliverbaehler
Copy link
Copy Markdown
Collaborator Author

@bakito didnt push. .. D:

Signed-off-by: Oliver Baehler <oliver@sudo-i.net>
Copilot AI review requested due to automatic review settings May 29, 2026 08:25
Comment thread charts/capsule/templates/_helpers.tpl Outdated
service:
name: {{ default (printf "%s-webhook-service" (include "capsule.fullname" $.ctx)) $.ctx.Values.webhooks.service.name }}
namespace: {{ default $.ctx.Release.Namespace $.ctx.Values.webhooks.service.namespace }}
port: {{ default 9443 $.ctx.Values.webhooks.service.port }}
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.

default need to be changed here too

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@bakito
Copy link
Copy Markdown
Contributor

bakito commented May 29, 2026

LGTM

@oliverbaehler oliverbaehler merged commit 279f3a7 into projectcapsule:main May 29, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants