Skip to content
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.

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

Do not risk exposing unauthenticated webhook port on container #4691

Merged
merged 2 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions charts/external-dns/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed `provider.webhook.resources` behavior to correctly leverage resource limits ([#4560](https://github.com/kubernetes-sigs/external-dns/pull/4560))
- Fixed `provider.webhook.imagePullPolicy` behavior to correctly leverage pull policy ([#4643](https://github.com/kubernetes-sigs/external-dns/pull/4643)) _@kimsondrup_
- Add correct webhook metric port to `Service` and `ServiceMonitor` ([#4643](https://github.com/kubernetes-sigs/external-dns/pull/4643)) _@kimsondrup_
- No longer require the unauthenticated webhook provider port to be exposed for health probes ([#4691](https://github.com/kubernetes-sigs/external-dns/pull/4691)) _@kimsondrup_ _@hatrx_

## [v1.14.5] - 2023-06-10

Expand Down
2 changes: 1 addition & 1 deletion charts/external-dns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ If `namespaced` is set to `true`, please ensure that `sources` my only contains
| provider.webhook.readinessProbe | object | See _values.yaml_ | [Readiness probe](https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/) configuration for the `webhook` container. |
| provider.webhook.resources | object | `{}` | [Resources](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) for the `webhook` container. |
| provider.webhook.securityContext | object | See _values.yaml_ | [Pod security context](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) for the `webhook` container. |
| provider.webhook.service.metricsPort | int | `8080` | Webhook metrics port for the service. |
| provider.webhook.service.port | int | `8080` | Webhook exposed HTTP port for the service. |
| provider.webhook.serviceMonitor | object | See _values.yaml_ | Optional [Service Monitor](https://prometheus-operator.dev/docs/operator/design/#servicemonitor) configuration for the `webhook` container. |
| rbac.additionalPermissions | list | `[]` | Additional rules to add to the `ClusterRole`. |
| rbac.create | bool | `true` | If `true`, create a `ClusterRole` & `ClusterRoleBinding` with access to the Kubernetes API. |
Expand Down
3 changes: 0 additions & 3 deletions charts/external-dns/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,6 @@ spec:
{{- end }}
ports:
- name: http-webhook
kimsondrup marked this conversation as resolved.
Show resolved Hide resolved
protocol: TCP
containerPort: 8888
- name: http-wh-metrics
protocol: TCP
containerPort: 8080
livenessProbe:
Expand Down
6 changes: 3 additions & 3 deletions charts/external-dns/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ spec:
protocol: TCP
{{- if eq $providerName "webhook" }}
{{- with .Values.provider.webhook.service }}
- name: http-wh-metrics
port: {{ .metricsPort }}
targetPort: http-wh-metrics
- name: http-webhook
port: {{ .port }}
targetPort: http-webhook
protocol: TCP
{{- end }}
{{- end }}
2 changes: 1 addition & 1 deletion charts/external-dns/templates/servicemonitor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ spec:
{{- end }}
{{- if eq $providerName "webhook" }}
{{- with .Values.provider.webhook.serviceMonitor }}
- port: http-wh-metrics
- port: http-webhook
path: /metrics
{{- with .interval }}
interval: {{ . }}
Expand Down
4 changes: 2 additions & 2 deletions charts/external-dns/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,8 @@ provider:
failureThreshold: 6
successThreshold: 1
service:
# -- Webhook metrics port for the service.
metricsPort: 8080
# -- Webhook exposed HTTP port for the service.
port: 8080
# -- Optional [Service Monitor](https://prometheus-operator.dev/docs/operator/design/#servicemonitor) configuration for the `webhook` container.
# @default -- See _values.yaml_
serviceMonitor:
Expand Down
26 changes: 17 additions & 9 deletions docs/tutorials/webhook-provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,32 @@ Providers implementing the HTTP API have to keep in sync with changes to the JSO

The following table represents the methods to implement mapped to their HTTP method and route.

| Provider method | HTTP Method | Route |
| --- | --- | --- |
| Records | GET | /records |
| AdjustEndpoints | POST | /adjustendpoints |
| ApplyChanges | POST | /records |
| K8s probe | GET | /healthz |

### Provider endpoints

| Provider method | HTTP Method | Route | Description |
| --------------- | ----------- | ---------------- | ---------------------------------------- |
| Negotiate | GET | / | Negotiate `DomainFilter` |
| Records | GET | /records | Get records |
| AdjustEndpoints | POST | /adjustendpoints | Provider specific adjustments of records |
| ApplyChanges | POST | /records | Apply record |

ExternalDNS will also make requests to the `/` endpoint for negotiation and for deserialization of the `DomainFilter`.

The server needs to respond to those requests by reading the `Accept` header and responding with a corresponding `Content-Type` header specifying the supported media type format and version.

The default recommended port is 8888, and should listen only on localhost (ie: only accessible for k8s probes and external-dns).
The default recommended port for the provider endpoints is `8888`, and should listen only on `localhost` (ie: only accessible for external-dns).

**NOTE**: only `5xx` responses will be retried and only `20x` will be considered as successful. All status codes different from those will be considered a failure on ExternalDNS's side.

## Metrics support
### Exposed endpoints

| Provider method | HTTP Method | Route | Description |
| --------------- | ----------- | -------- | -------------------------------------------------------------------------------------------- |
| K8s probe | GET | /healthz | Used by `livenessProbe` and `readinessProbe` |
| Open Metrics | GET | /metrics | Optional endpoint to expose [Open Metrics](https://github.com/OpenObservability/OpenMetrics) |

The metrics should listen ":8080" on `/metrics` following [Open Metrics](https://github.com/OpenObservability/OpenMetrics) format.
The default recommended port for the exposed endpoints is `8080`, and it should be bound to all interfaces (`0.0.0.0`)

## Custom Annotations

Expand Down
Loading