Helm: The insecure webhook (if running on 8080) is exposed through hard coded ports

[NickLarsenNZ]

What happened:

As far as I understand, there is no authentication mechanism when using webhooks.
If that is true, then I believe the webhook port should not be exposed in the cluster (and the webhook can then listen on 127.0.0.1 or ::1).

While the helm-chart allows for adding additional containers (through extraContainers), ports are not configurable through provider.webhook

What you expected to happen:

1. I expect to be able to restrict webhook access containers in the pod (namely external-dns).
2. I expect to be able to run healthchecks (livenessProbe/readinessProbe) on the webhook sidecar, which means exposing its endpoint without exposing the webhook endpoint.

How to reproduce it (as minimally and precisely as possible):

values.yaml

extraArgs:
  ## must override the default value with port 8888 with port 8080 because this is hard-coded in the helm chart
  - --webhook-provider-url=http://localhost:8080

provider:
  name: webhook
  webhook:
    image:
      repository: ghcr.io/ionos-cloud/external-dns-ionos-webhook
      tag: v0.6.3
    env:
    - name: IONOS_API_KEY
      valueFrom:
        secretKeyRef:
          name: ionos-credentials
          key: api-key
    - name: DRY_RUN
      value: "true"
    # NOTE: this is exposed due to the hard-coded http-webhook port
    - name: SERVER_PORT
      value: "8080"
    # NOTE: This feature is not yet available. 
    # It arrives in https://github.com/ionos-cloud/external-dns-ionos-webhook/pull/77 which depends on this issue being resolved.
    #
    # Try to expose /health endpoint to kubelet. But it is not exposed due to hard-coded ports
    - name: HEALTH_PORT
      value: "8081"
    livenessProbe:
      httpGet:
        path: /health
        port: http-health
    readinessProbe:
      httpGet:
        path: /health
        port: http-health
    # Ideally, I could expose the health endpoint like so:
    # ports:
    # - name: http-health
    #   protocol: TCP
    #   containerPort: 8081

Anything else we need to know?:

I believe the solution to this to be:

1. Remove the hard-coded webhook port from the helm-chart.
2. Allow the webhook sidecar ports to be customised through helm values, so that healthchecks can still be exposed.
   • Ideally they should run on a separate port so the webhook endpoint can remain locked down.

Environment:

• External-DNS version (use external-dns --version):
• DNS provider: webhook (https://github.com/ionos-cloud/external-dns-ionos-webhook)
• Others:

[ivankatliarchuk]

/help

[k8s-ci-robot]

@ivankatliarchuk:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mloiseleur]

One can see in webhook documentation:

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

For the exposed port, used for metrics and probes, it says:

The default recommended port for the exposed endpoints is 8080, and it should be bound to all interfaces (0.0.0.0)

The default parameters in CLI follow those guidelines. It says on --webhook-provider-url:

The URL of the remote endpoint to call for the webhook provider (default: http://localhost:8888/)

AFAIK, the chart follows those guidelines: it's using the exposed port for metrics and probes.

Does that address your concerns ?

[mcoakley]

I can add a little experience context when using the current chart Deployment and Service templates. For the external-dns-provider-adgaurd webhook provider, the livenessProbe and readinessProbe will fail. Because the Deployment chart hard codes one a single port to be available for the provide container one of the ports (8888 & 8080) will not be accessible - in my case the 8080 which makes all probes fail. Exposing port 8080 of the provider via the Service seems to be insecure and assumes that only a single port is being defined.

I modified the chart locally to remove the inclusion of ports 8080 or 8888 from the Service and made the Deployment template render all ports defined in the provider Helm Chart values.

The only path I can currently think of to keep backward compatibility (let the Chart work as it does today) and allow for more customization (as my chart variant does) is to include one or more boolean flags that would tell the chart templates to use the current path or allow the chart values to override that implementation. If this is acceptable, I can update the chart code and submit a PR.