Skip to content
Open
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
16 changes: 16 additions & 0 deletions bundle/manifests/external-dns-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ spec:
- --operator-namespace=$(OPERATOR_NAMESPACE)
- --operand-namespace=$(OPERATOR_NAMESPACE)
- --externaldns-image=$(RELATED_IMAGE_EXTERNAL_DNS)
- --kube-rbac-proxy-image=$(RELATED_IMAGE_KUBE_RBAC_PROXY)
- --trusted-ca-configmap=$(TRUSTED_CA_CONFIGMAP_NAME)
- --leader-elect
- --webhook-disable-http2
Expand All @@ -480,6 +481,8 @@ spec:
fieldPath: metadata.namespace
- name: RELATED_IMAGE_EXTERNAL_DNS
value: quay.io/external-dns-operator/external-dns:latest
- name: RELATED_IMAGE_KUBE_RBAC_PROXY
value: quay.io/openshift/origin-kube-rbac-proxy:latest
Comment thread
Thealisyed marked this conversation as resolved.
- name: TRUSTED_CA_CONFIGMAP_NAME
image: quay.io/openshift/origin-external-dns-operator:latest
name: external-dns-operator
Expand Down Expand Up @@ -557,6 +560,7 @@ spec:
- configmaps
- secrets
- serviceaccounts
- services
verbs:
- create
- delete
Expand Down Expand Up @@ -585,6 +589,18 @@ spec:
- patch
- update
- watch
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- coordination.k8s.io
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,15 @@ rules:
- get
- watch
- list
- apiGroups:
- authentication.k8s.io
resources:
- tokenreviews
verbs:
- create
- apiGroups:
- authorization.k8s.io
resources:
- subjectaccessreviews
verbs:
- create
3 changes: 3 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ spec:
- --operator-namespace=$(OPERATOR_NAMESPACE)
- --operand-namespace=$(OPERATOR_NAMESPACE)
- --externaldns-image=$(RELATED_IMAGE_EXTERNAL_DNS)
- --kube-rbac-proxy-image=$(RELATED_IMAGE_KUBE_RBAC_PROXY)
- --trusted-ca-configmap=$(TRUSTED_CA_CONFIGMAP_NAME)
- --leader-elect
- --webhook-disable-http2
Expand All @@ -49,6 +50,8 @@ spec:
# Use "latest" floating tag to avoid problems with the prunning of older mirorred images.
# Ref: https://issues.redhat.com/browse/OCPBUGS-57339.
value: quay.io/external-dns-operator/external-dns:latest
- name: RELATED_IMAGE_KUBE_RBAC_PROXY
value: quay.io/openshift/origin-kube-rbac-proxy:latest
- name: TRUSTED_CA_CONFIGMAP_NAME
securityContext:
capabilities:
Expand Down
12 changes: 12 additions & 0 deletions config/rbac/operand_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,15 @@ rules:
- get
- watch
- list
- apiGroups:
- authentication.k8s.io
resources:
- tokenreviews
verbs:
- create
- apiGroups:
- authorization.k8s.io
resources:
- subjectaccessreviews
verbs:
- create
Comment thread
Thealisyed marked this conversation as resolved.
13 changes: 13 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ rules:
- configmaps
- secrets
- serviceaccounts
- services
verbs:
- create
- delete
Expand Down Expand Up @@ -101,3 +102,15 @@ rules:
- patch
- update
- watch
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
15 changes: 15 additions & 0 deletions docs/openshift.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,18 @@ Use the following convenience script to secure communication between the API and
```bash
$ ./hack/add-serving-cert.sh --namespace external-dns-operator --service webhook-service --webhook validating-webhook-configuration --secret webhook-server-cert
```

## Operand metrics

Each ExternalDNS operand deployment includes a [kube-rbac-proxy](https://github.com/brancz/kube-rbac-proxy) sidecar per zone container to expose metrics over HTTPS. A `Service` and `ServiceMonitor` are created per `ExternalDNS` CR for Prometheus discovery.

### Multi-zone metric differentiation

When an `ExternalDNS` instance manages multiple zones, one ExternalDNS container runs per zone — each exposing the same metric names. Metrics are kept separate (not combined or deduplicated) through the following mechanism:

- Each zone container binds its metrics to a distinct localhost port (`:7979`, `:7980`, etc.).
- Each kube-rbac-proxy sidecar proxies one of those ports on a distinct secure port (`:9091`, `:9092`, etc.).
- The `ServiceMonitor` creates one endpoint entry per port.
- Prometheus assigns a unique `instance` label (`pod_ip:port`) to each scrape target.

This means metrics from different zones within the same `ExternalDNS` instance are differentiated by port via the `instance` label. Metrics from different `ExternalDNS` instances are differentiated by pod or deployment name.
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func main() {
flag.StringVar(&opCfg.OperatorNamespace, "operator-namespace", operatorconfig.DefaultOperatorNamespace, "The namespace that the operator is running in.")
flag.StringVar(&opCfg.OperandNamespace, "operand-namespace", operatorconfig.DefaultOperandNamespace, "The namespace that ExternalDNS containers should run in.")
flag.StringVar(&opCfg.ExternalDNSImage, "externaldns-image", operatorconfig.DefaultExternalDNSImage, "The container image used for running ExternalDNS.")
flag.StringVar(&opCfg.KubeRBACProxyImage, "kube-rbac-proxy-image", operatorconfig.DefaultKubeRBACProxyImage, "The container image used for the kube-rbac-proxy sidecar that exposes ExternalDNS operand metrics.")
flag.StringVar(&opCfg.CertDir, "cert-dir", operatorconfig.DefaultCertDir, "The directory for keys and certificates for serving the webhook.")
flag.StringVar(&opCfg.TrustedCAConfigMapName, "trusted-ca-configmap", operatorconfig.DefaultTrustedCAConfigMapName, "The name of the config map containing TLS CA(s) which should be trusted by ExternalDNS containers. PEM encoded file under \"ca-bundle.crt\" key is expected.")
flag.BoolVar(&opCfg.EnableWebhook, "enable-webhook", operatorconfig.DefaultEnableWebhook, "Enable the validating webhook server. Defaults to true.")
Expand Down
5 changes: 5 additions & 0 deletions pkg/operator/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (

const (
DefaultExternalDNSImage = "quay.io/external-dns-operator/external-dns:latest"
DefaultKubeRBACProxyImage = "quay.io/openshift/origin-kube-rbac-proxy:latest"
DefaultMetricsAddr = "127.0.0.1:8080"
DefaultOperatorNamespace = "external-dns-operator"
DefaultOperandNamespace = "external-dns"
Expand All @@ -59,6 +60,10 @@ type Config struct {
// by the operator.
ExternalDNSImage string

// KubeRBACProxyImage is the kube-rbac-proxy image for the metrics sidecar container
// in the ExternalDNS operand deployment.
KubeRBACProxyImage string

// MetricsBindAddress is the TCP address that the operator should bind to for
// serving prometheus metrics. It can be set to "0" to disable the metrics serving.
MetricsBindAddress string
Expand Down
22 changes: 21 additions & 1 deletion pkg/operator/controller/externaldns/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -50,6 +51,8 @@ type Config struct {
Namespace string
// Image is the ExternalDNS image to use.
Image string
// KubeRBACProxyImage is the kube-rbac-proxy image for the metrics sidecar.
KubeRBACProxyImage string
// OperatorNamespace is the namespace in which this operator is deployed.
OperatorNamespace string
// IsOpenShift is the flag which instructs the operator that it runs in OpenShift.
Expand Down Expand Up @@ -102,6 +105,16 @@ func New(mgr manager.Manager, cfg Config) (controller.Controller, error) {
return nil, err
}

if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.Service{}, handler.EnqueueRequestForOwner(operatorScheme, operatorRESTMapper, &operatorv1beta1.ExternalDNS{}, handler.OnlyControllerOwner()))); err != nil {
return nil, err
}

smInformer := &unstructured.Unstructured{}
smInformer.SetGroupVersionKind(serviceMonitorGVK)
if err := c.Watch(source.Kind[client.Object](operatorCache, smInformer, handler.EnqueueRequestForOwner(operatorScheme, operatorRESTMapper, &operatorv1beta1.ExternalDNS{}, handler.OnlyControllerOwner()))); err != nil {
return nil, err
}

Comment thread
Thealisyed marked this conversation as resolved.
// secret replicated by the credentials controller
// needs to trigger the reconciliation of the corresponding ExternalDNS
// because of the annotation with the secret's hash in the operand deployment
Expand Down Expand Up @@ -207,11 +220,18 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
trustCAConfigMap = configMap
}

_, currentDeployment, err := r.ensureExternalDNSDeployment(ctx, r.config.Namespace, r.config.Image, sa, credSecret, trustCAConfigMap, externalDNS)
_, currentDeployment, err := r.ensureExternalDNSDeployment(ctx, r.config.Namespace, r.config.Image, r.config.KubeRBACProxyImage, sa, credSecret, trustCAConfigMap, externalDNS)
if err != nil {
return reconcile.Result{}, fmt.Errorf("failed to ensure externalDNS deployment: %w", err)
}

if err := r.ensureExternalDNSMetricsService(ctx, r.config.Namespace, externalDNS); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to ensure externalDNS metrics service: %w", err)
}
if err := r.ensureExternalDNSServiceMonitor(ctx, r.config.Namespace, externalDNS); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to ensure externalDNS service monitor: %w", err)
}

if err := r.updateExternalDNSStatus(ctx, externalDNS, currentDeployment, true); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to update externalDNS custom resource %s: %w", externalDNS.Name, err)
}
Expand Down
58 changes: 47 additions & 11 deletions pkg/operator/controller/externaldns/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ var sourceStringTable = map[operatorv1beta1.ExternalDNSSourceType]string{
type deploymentConfig struct {
namespace string
image string
kubeRBACProxyImage string
serviceAccount *corev1.ServiceAccount
externalDNS *operatorv1beta1.ExternalDNS
isOpenShift bool
Expand All @@ -89,7 +90,7 @@ type deploymentConfig struct {

// ensureExternalDNSDeployment ensures that the externalDNS deployment exists.
// Returns a Boolean value indicating whether the deployment exists, a pointer to the deployment, and an error when relevant.
func (r *reconciler) ensureExternalDNSDeployment(ctx context.Context, namespace, image string, serviceAccount *corev1.ServiceAccount, credSecret *corev1.Secret, trustCAConfigMap *corev1.ConfigMap, externalDNS *operatorv1beta1.ExternalDNS) (bool, *appsv1.Deployment, error) {
func (r *reconciler) ensureExternalDNSDeployment(ctx context.Context, namespace, image, kubeRBACProxyImage string, serviceAccount *corev1.ServiceAccount, credSecret *corev1.Secret, trustCAConfigMap *corev1.ConfigMap, externalDNS *operatorv1beta1.ExternalDNS) (bool, *appsv1.Deployment, error) {
nsName := types.NamespacedName{Namespace: namespace, Name: controller.ExternalDNSResourceName(externalDNS)}

// build credentials secret's hash
Expand All @@ -109,16 +110,17 @@ func (r *reconciler) ensureExternalDNSDeployment(ctx context.Context, namespace,
}

desired, err := desiredExternalDNSDeployment(&deploymentConfig{
namespace,
image,
serviceAccount,
externalDNS,
r.config.IsOpenShift,
r.config.PlatformStatus,
credSecret.Name,
credSecretHash,
trustCAConfigMapName,
trustCAConfigMapHash,
namespace: namespace,
image: image,
kubeRBACProxyImage: kubeRBACProxyImage,
serviceAccount: serviceAccount,
externalDNS: externalDNS,
isOpenShift: r.config.IsOpenShift,
platformStatus: r.config.PlatformStatus,
secret: credSecret.Name,
secretHash: credSecretHash,
trustedCAConfigMapName: trustCAConfigMapName,
trustedCAConfigMapHash: trustCAConfigMapHash,
})
if err != nil {
return false, nil, fmt.Errorf("failed to build externalDNS deployment: %w", err)
Expand Down Expand Up @@ -296,6 +298,15 @@ func desiredExternalDNSDeployment(cfg *deploymentConfig) (*appsv1.Deployment, er
depl.Spec.Template.Spec.Containers = append(depl.Spec.Template.Spec.Containers, *container)
}
}
if cfg.kubeRBACProxyImage != "" {
for i := 0; i < cbld.counter; i++ {
proxyContainer := kubeRBACProxyContainer(cfg.kubeRBACProxyImage, i)
depl.Spec.Template.Spec.Containers = append(depl.Spec.Template.Spec.Containers, proxyContainer)
}
certVolume := metricsCertVolume(controller.ExternalDNSMetricsSecretName(cfg.externalDNS))
depl.Spec.Template.Spec.Volumes = append(depl.Spec.Template.Spec.Volumes, certVolume)
}
Comment on lines +301 to +308
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when the proxy image is unset.

Lines 301-307 silently skip the kube-rbac-proxy sidecars, but the reconcile path now always creates the metrics Service and ServiceMonitor. That leaves broken scrape targets instead of a clear config error. Either make the proxy unconditional here as well, or return an error when cfg.kubeRBACProxyImage is empty.

Proposed fix
-	if cfg.kubeRBACProxyImage != "" {
-		for i := 0; i < cbld.counter; i++ {
-			proxyContainer := kubeRBACProxyContainer(cfg.kubeRBACProxyImage, i)
-			depl.Spec.Template.Spec.Containers = append(depl.Spec.Template.Spec.Containers, proxyContainer)
-		}
-		certVolume := metricsCertVolume(controller.ExternalDNSMetricsSecretName(cfg.externalDNS))
-		depl.Spec.Template.Spec.Volumes = append(depl.Spec.Template.Spec.Volumes, certVolume)
-	}
+	if cfg.kubeRBACProxyImage == "" {
+		return nil, fmt.Errorf("kube-rbac-proxy image must be configured")
+	}
+	for i := 0; i < cbld.counter; i++ {
+		proxyContainer := kubeRBACProxyContainer(cfg.kubeRBACProxyImage, i)
+		depl.Spec.Template.Spec.Containers = append(depl.Spec.Template.Spec.Containers, proxyContainer)
+	}
+	certVolume := metricsCertVolume(controller.ExternalDNSMetricsSecretName(cfg.externalDNS))
+	depl.Spec.Template.Spec.Volumes = append(depl.Spec.Template.Spec.Volumes, certVolume)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/operator/controller/externaldns/deployment.go` around lines 301 - 308,
The code currently skips adding kube-rbac-proxy sidecars when
cfg.kubeRBACProxyImage == "" but still creates metrics Service/ServiceMonitor;
change this to fail fast: check cfg.kubeRBACProxyImage at the start of the
deployment creation/reconcile path and return an error if it's empty (e.g.,
fmt.Errorf("kubeRBACProxyImage must be set")), instead of silently omitting the
proxy; update the code surrounding the kubeRBACProxyContainer(...) and
metricsCertVolume(controller.ExternalDNSMetricsSecretName(...)) calls so they
only run after the non-empty validation and ensure the calling reconcile loop
surfaces/handles the returned error.


return depl, nil
}

Expand Down Expand Up @@ -418,6 +429,10 @@ func externalDNSContainersChanged(current, expected, updated *appsv1.Deployment)
updated.Spec.Template.Spec.Containers[currCont.Index].SecurityContext = updatedContext
changed = true
}
if !equalContainerPorts(currCont.Ports, expCont.Ports) {
updated.Spec.Template.Spec.Containers[currCont.Index].Ports = expCont.Ports
changed = true
}
} else {
// expected container is not present - add it
updated.Spec.Template.Spec.Containers = append(updated.Spec.Template.Spec.Containers, expCont.Container)
Expand Down Expand Up @@ -701,6 +716,27 @@ func securityContextChanged(current, updated, desired *corev1.SecurityContext) (
return changed, updated
}

// equalContainerPorts returns true if 2 container port slices have the same content.
func equalContainerPorts(current, expected []corev1.ContainerPort) bool {
if len(current) != len(expected) {
return false
}
currentMap := map[string]corev1.ContainerPort{}
for _, p := range current {
currentMap[p.Name] = p
}
for _, ep := range expected {
cp, found := currentMap[ep.Name]
if !found {
return false
}
if cp.ContainerPort != ep.ContainerPort || cp.Protocol != ep.Protocol {
return false
}
}
return true
}

func equalBoolPtr(current, desired *bool) bool {
if desired == nil {
return true
Expand Down
20 changes: 10 additions & 10 deletions pkg/operator/controller/externaldns/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3845,15 +3845,15 @@ func TestDesiredExternalDNSDeployment(t *testing.T) {
}
}()
depl, err := desiredExternalDNSDeployment(&deploymentConfig{
test.OperandNamespace,
test.OperandImage,
serviceAccount,
tc.inputExternalDNS,
tc.inputIsOpenShift,
tc.inputPlatformStatus,
tc.inputSecretName,
testSecretHash,
tc.inputTrustedCAConfigMapName, "",
namespace: test.OperandNamespace,
image: test.OperandImage,
serviceAccount: serviceAccount,
externalDNS: tc.inputExternalDNS,
isOpenShift: tc.inputIsOpenShift,
platformStatus: tc.inputPlatformStatus,
secret: tc.inputSecretName,
secretHash: testSecretHash,
trustedCAConfigMapName: tc.inputTrustedCAConfigMapName,
})
if err != nil {
t.Errorf("expected no error from calling desiredExternalDNSDeployment, but received %v", err)
Expand Down Expand Up @@ -5999,7 +5999,7 @@ func TestEnsureExternalDNSDeployment(t *testing.T) {
log: zap.New(zap.UseDevMode(true)),
}

gotExist, gotDepl, err := r.ensureExternalDNSDeployment(context.TODO(), test.OperandNamespace, test.OperandImage, serviceAccount, tc.credSecret, tc.trustCAConfigMap, &tc.extDNS)
gotExist, gotDepl, err := r.ensureExternalDNSDeployment(context.TODO(), test.OperandNamespace, test.OperandImage, "", serviceAccount, tc.credSecret, tc.trustCAConfigMap, &tc.extDNS)
if err != nil {
if !tc.errExpected {
t.Fatalf("unexpected error received: %v", err)
Expand Down
Loading