Add downward-node-labels propagation to Kibana, Logstash and Elastic … by orangecola · Pull Request #9367 · elastic/cloud-on-k8s

orangecola · 2026-04-17T03:06:36Z

I attest that I have reviewed the output

cla-checker-service · 2026-04-17T03:06:40Z

💚 CLA has been signed

prodsecmachine · 2026-04-17T03:06:48Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

…Agent CRDs Mirrors the existing Elasticsearch behavior: users can set the `eck.k8s.elastic.co/downward-node-labels` annotation on the CR with a comma-separated list of node labels, and the operator copies those labels as annotations on the managed Pods. Pods can then read them via the Downward API. The parser is extracted to a pure-Go `pkg/utils/nodelabels` package to avoid an apis->controllers import cycle, and a shared `pkg/controller/common/nodelabels` helper exposes `AnnotatePods` for the three new controllers to reuse.

barkbay

Elasticsearch runs a prepare-fs init container before the main Elasticsearch container. That init script waits until the operator has written the requested node-derived keys onto the Pod’s metadata.annotations (it polls the downward API annotations file), so the main container does not start with those annotations missing.

This change adds the same operator patch behavior for Kibana, Logstash, and Elastic Agent but not that startup gate, so anything that consumes those annotations at first container start (for example valueFrom.fieldRef on metadata.annotations[...]) can still race on first start compared to Elasticsearch.

Address PR feedback: Elasticsearch's prepare-fs init container waits until the operator has patched the expected node-derived keys onto the Pod's metadata.annotations (it polls the downward-API annotations file) before the main ES container starts. The previous change for Kibana, Logstash and Elastic Agent added the same operator patch behavior but not that startup gate, so anything that consumes those annotations at first container start (for example valueFrom.fieldRef on metadata.annotations[...]) could still race on first start. This commit adds a shared wait-for-annotations init container that polls the downward-API annotations file and only lets the main container start once all expected annotations are present. The init container is added to the Pod template for Kibana, Logstash and Elastic Agent when downward node labels are configured on the CR.

orangecola · 2026-04-20T15:06:31Z

Have added to the startup gate for Kibana, Logstash, and Elastic Agent

orangecola · 2026-05-11T22:00:41Z

@barkbay bump

barkbay · 2026-05-26T05:04:00Z

@barkbay bump

I'll do a review by the end of the week.

barkbay

Any reason to exclude Beat, APM Server, Maps or the Package Registry ?

barkbay

I think we need to reuse this validation:

cloud-on-k8s/pkg/controller/elasticsearch/validation/validations.go

Line 118 in d0d9521

    
           func validNodeLabels(proposed esv1.Elasticsearch, exposedNodeLabels NodeLabels) field.ErrorList {

(and update https://github.com/elastic/cloud-on-k8s/blob/d0d9521002a053c4e77bad390678c1d23ccd8b6b/docs/reference/eck-configuration-flags.md)

barkbay

Is it worth extracting an interface? For example this one in pkg/controller/common/nodelabels/nodelabels.go :

// AnnotationTarget is implemented by ECK custom resources whose managed Pods should have
// Kubernetes node labels copied to their annotations via AnnotatePods. Any ECK CR that already
// implements metav1.Object and GetIdentityLabels satisfies this interface once it exposes a
// DownwardNodeLabels accessor.
type AnnotationTarget interface {
	metav1.Object
	// DownwardNodeLabels returns the node labels expected to be copied as annotations on the
	// Pods managed by the resource. An empty result disables node-label propagation.
	DownwardNodeLabels() []string
	// GetIdentityLabels returns the label set identifying Pods managed by the resource.
	GetIdentityLabels() map[string]string
}

// AnnotatePods copies the expected node labels as annotations on all Pods managed by the given
// target. Missing node labels are reported as errors but do not stop the reconciliation of
// other Pods. The call is a no-op when the target has no downward node labels configured.
func AnnotatePods(ctx context.Context, c k8s.Client, t AnnotationTarget) *reconciler.Results {
	span, ctx := apm.StartSpan(ctx, "annotate_pods_with_node_labels", tracing.SpanTypeApp)
	defer span.End()
	results := reconciler.NewResult(ctx)
	expectedLabels := t.DownwardNodeLabels()
	if len(expectedLabels) == 0 {
		return results
	}
	pods, err := k8s.PodsMatchingLabels(c, t.GetNamespace(), t.GetIdentityLabels())
	if err != nil {
		return results.WithError(err)
	}
	for _, pod := range pods {
		results.WithError(annotatePod(ctx, c, pod, expectedLabels, t.GetName()))
	}
	return results
}

And then from the drivers:

	// Patch the Pods to add the expected node labels as annotations. Record the error, if any, but do not stop the
	// reconciliation loop as we don't want to prevent other updates from being applied.
	statefulSetResults.WithResults(nodelabels.AnnotatePods(params.Context, params.Client, &params.Logstash))

- Extract an AnnotationTarget interface in pkg/controller/common/nodelabels so the driver call collapses to AnnotatePods(ctx, client, &resource), with each CR satisfying the interface via metav1.Object, GetIdentityLabels and DownwardNodeLabels. - Reuse the Elasticsearch annotation-policy validation across CRDs. Move NodeLabels and NewExposedNodeLabels into the shared nodelabels package and add ValidateAnnotation. Wire a WithNodeLabelsValidation webhook wrapper so Kibana/Agent/APM/Beat/Maps/Package Registry validators enforce the exposed-node-labels policy, and add the same check to each controller's validate() to cover the webhook-disabled path. Update docs/reference/eck-configuration-flags.md. - Extend support to Beat, APM Server, Elastic Maps Server and Elastic Package Registry: add DownwardNodeLabels/HasDownwardNodeLabels accessors, propagate annotations to managed Pods, include the WaitFor init container and downward-API volume on the Pod template, and roll Pods on annotation changes via the config hash.

orangecola · 2026-05-27T05:11:28Z

Any reason to exclude Beat, APM Server, Maps or the Package Registry?
extended support to all four. Each CR now has DownwardNodeLabels() / HasDownwardNodeLabels() / DownwardNodeLabelsAnnotation, drivers call nodelabels.AnnotatePods, pod templates add the WaitForAnnotationsInitContainer + downward-API volume when labels are configured, and the config hash mixes the annotation value so changes roll the Pods.

Reuse validNodeLabels and update the configuration docs
Moved NodeLabels and NewExposedNodeLabels into pkg/controller/common/nodelabels and added a shared ValidateAnnotation helper. esvalidation.NodeLabels is now a type alias so existing imports keep working. The non-Elasticsearch webhooks are wrapped with a new commonwebhook.WithNodeLabelsValidation generic helper at registration so they all enforce the same exposed-node-labels policy, and the same check runs in each controller's validate() to cover the webhook-disabled path (matching the Elasticsearch behavior). docs/reference/eck-configuration-flags.md now lists all the CRDs the flag applies to.

Extract an AnnotationTarget interface
added the interface you sketched in pkg/controller/common/nodelabels/nodelabels.go. Each driver call collapses to nodelabels.AnnotatePods(ctx, client, &resource), and any ECK CR that already implements metav1.Object and GetIdentityLabels satisfies the interface once it exposes DownwardNodeLabels().

…-node-labels-WZI3d # Conflicts: # cmd/manager/validation.go # pkg/controller/agent/controller.go # pkg/controller/agent/driver.go

pkoutsovasilis

Thanks for the contribution @orangecola! I did my pass on this PR and left some comments. Please while you address my comments merge main, so I can invoke a full run of our CI including the latest code that sits on main.

pkoutsovasilis · 2026-06-24T11:53:16Z

buildkite test this

- Move DownwardNodeLabelsAnnotation and parsing helpers to pkg/apis/common/v1 as the canonical definition; drop the per-CRD re-exports and delete pkg/utils/nodelabels. - Remove the ES-specific volume dependency from the common nodelabels init container; derive the annotations-file path via commonvolume. - Align the wait script grep with the Elasticsearch prepare-fs script (BRE). - Migrate the Elasticsearch driver to nodelabels.AnnotatePods and delete the duplicated shared/node_labels.go implementation. - Introduce per-type RegisterWebhook for APM Server, Beat, Kibana, Maps and Package Registry so a single validation function enforces the exposed-node-labels policy on both the webhook and reconciler paths; remove the WithNodeLabelsValidation wrapper. - Call WithInitContainerDefaults unconditionally in the Agent pod template. - Fix gofmt formatting in nodelabels_test.go.

…-node-labels-WZI3d # Conflicts: # pkg/controller/apmserver/deployment.go

pkoutsovasilis · 2026-06-26T04:31:27Z

buildkite test this -f p=gke,E2E_TAGS=e2e -m s=8.19.16,s=9.5.0-SNAPSHOT

BK link

pkoutsovasilis

Thanks for working through my comments @orangecola , I did a second pass and left some new ones. More importantly, I did some manual testing and I have found that apm-server container image doesn't have grep installed so this approach will never work. Even worse, for some image variants of apm-server, namely wolfi, not even a shell is available within the image. For that reason I have two requests:

Drop apm server support from this PR
We need some e2e tests to validate that the downward-node-labels mechanism is working and thus make sure that future component image changes won't break it (We can handle this in a follow-up PR)

pkoutsovasilis · 2026-06-26T05:00:27Z

 	DisableUpgradePredicatesAnnotation = "eck.k8s.elastic.co/disable-upgrade-predicates"
 	// DownwardNodeLabelsAnnotation holds an optional list of expected node labels to be set as annotations on the Elasticsearch Pods.
-	DownwardNodeLabelsAnnotation = "eck.k8s.elastic.co/downward-node-labels"
+	DownwardNodeLabelsAnnotation = commonv1.DownwardNodeLabelsAnnotation


Redundant re-export, DownwardNodeLabelsAnnotation now leaves in common api so it can be imported directly

pkoutsovasilis · 2026-06-26T05:06:25Z

+	// Changes to the downward-node-labels annotation must roll the APM Server Pods so the new annotations
+	// are re-applied on scheduling.
+	if as.HasDownwardNodeLabels() {
+		_, _ = configHash.Write([]byte(as.Annotations[commonv1.DownwardNodeLabelsAnnotation]))


Rreordering the annotation (e.g. "annotation-b,annotation-a" → "annotation-a,annotation-b") produces a different hash and triggers a spurious pod restart. I would propose here and in all other controllers that follow the same pattern to use []byte(strings.Join(resource.DownwardNodeLabels(), ",")) instead of []byte(resource.Annotations[commonv1.DownwardNodeLabelsAnnotation])

pkoutsovasilis · 2026-06-26T05:07:19Z

+	}
+	ulog.FromContext(ctx).Info(
+		"Setting Pod annotations from node labels",
+		"namespace", pod.Namespace,


namespace is injected into the context logger at pkg/controller/common/controller.go:49 via NewReconciliationContext, so here it is duplicated

pkoutsovasilis · 2026-06-26T05:15:01Z

+func ValidateAnnotation(annotations map[string]string, exposedNodeLabels NodeLabels) field.ErrorList {
+	var errs field.ErrorList
+	for _, nodeLabel := range commonv1.DownwardNodeLabelsFromAnnotations(annotations) {
+		if exposedNodeLabels.IsAllowed(nodeLabel) {
+			continue
+		}
+		errs = append(errs, field.Invalid(
+			field.NewPath("metadata").Child("annotations", commonv1.DownwardNodeLabelsAnnotation),
+			nodeLabel,
+			NotAllowedNodesLabelMsg,
+		))
+	}
+	return errs
+}


let's add exposed_test.go with our expectations of this func:

annotation set, no exposed-node-labels configured → validation error

annotation set, label matches the policy → no error

annotation set, label does not match the policy → validation error

no annotation set → no error (no-op)

pkoutsovasilis · 2026-06-26T05:20:29Z

+func (DownwardAPI) AnnotationsFilePath() string {
+	return fmt.Sprintf("%s/%s", volume.DownwardAPIMountPath, volume.AnnotationsFile)
+}


I understand that what I am about to say predates this PR, but now that more controllers (not only the elasticsearch one) utilise the DownwardAPI I believe that DownwardAPIVolumeName and DownwardAPIMountPath consts need to migrate in this package and not be defined in elasticsearch/volume

pkoutsovasilis · 2026-06-26T05:23:28Z

 package validation

 import (
-	"fmt"
-	"regexp"
+	commonnodelabels "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/nodelabels"
 )

-type NodeLabels []*regexp.Regexp
+// NodeLabels is an alias for the shared exposed-node-labels policy. It is kept here so that
+// existing callers can continue to import esvalidation.NodeLabels.
+type NodeLabels = commonnodelabels.NodeLabels

+// NewExposedNodeLabels delegates to the shared exposed-node-labels constructor.
 func NewExposedNodeLabels(exposedNodeLabels []string) (NodeLabels, error) {
-	if len(exposedNodeLabels) == 0 {
-		return nil, nil
-	}
-	compiledNodeLabels := make([]*regexp.Regexp, len(exposedNodeLabels))
-	for i, exposedNodeLabel := range exposedNodeLabels {
-		r, err := regexp.Compile(exposedNodeLabel)
-		if err != nil {
-			return nil, fmt.Errorf("exposed node label \"%s\" cannot be compiled as a regular expression: %w", exposedNodeLabel, err)
-		}
-		compiledNodeLabels[i] = r
-	}
-	return compiledNodeLabels, nil
-}
-
-func (n NodeLabels) IsAllowed(nodeLabel string) bool {
-	for _, r := range n {
-		if r.MatchString(nodeLabel) {
-			return true
-		}
-	}
-	return false
+	return commonnodelabels.NewExposedNodeLabels(exposedNodeLabels)


do we need this file? can't we use directly the commonnodelabels package?

pkoutsovasilis · 2026-06-26T06:19:08Z

+	if ems.HasDownwardNodeLabels() {
+		downwardAPIVolume := commonnodelabels.DownwardAPIVolume()
+		waitInit, err := commonnodelabels.WaitForAnnotationsInitContainer(ems.DownwardNodeLabels())
+		if err != nil {
+			return corev1.PodTemplateSpec{}, err
+		}
+		builder = builder.
+			WithVolumes(downwardAPIVolume.Volume()).
+			WithInitContainers(waitInit)
+	}


WithInitContainerDefaults() is called way above, which means that the Wait init container won't inherit the image

pkoutsovasilis · 2026-06-26T06:19:31Z

+	if epr.HasDownwardNodeLabels() {
+		downwardAPIVolume := commonnodelabels.DownwardAPIVolume()
+		waitInit, err := commonnodelabels.WaitForAnnotationsInitContainer(epr.DownwardNodeLabels())
+		if err != nil {
+			return corev1.PodTemplateSpec{}, err
+		}
+		builder = builder.
+			WithVolumes(downwardAPIVolume.Volume()).
+			WithInitContainers(waitInit)
+	}


WithInitContainerDefaults() is called way above, which means that the Wait init container won't inherit the image

pkoutsovasilis · 2026-06-26T06:36:19Z

+	commonnodelabels "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/nodelabels"
 )

-type NodeLabels []*regexp.Regexp
+// NodeLabels is an alias for the shared exposed-node-labels policy. It is kept here so that
+// existing callers can continue to import esvalidation.NodeLabels.
+type NodeLabels = commonnodelabels.NodeLabels

+// NewExposedNodeLabels delegates to the shared exposed-node-labels constructor.
 func NewExposedNodeLabels(exposedNodeLabels []string) (NodeLabels, error) {
-	if len(exposedNodeLabels) == 0 {
-		return nil, nil
-	}
-	compiledNodeLabels := make([]*regexp.Regexp, len(exposedNodeLabels))
-	for i, exposedNodeLabel := range exposedNodeLabels {
-		r, err := regexp.Compile(exposedNodeLabel)
-		if err != nil {
-			return nil, fmt.Errorf("exposed node label \"%s\" cannot be compiled as a regular expression: %w", exposedNodeLabel, err)
-		}
-		compiledNodeLabels[i] = r
-	}
-	return compiledNodeLabels, nil
-}
-
-func (n NodeLabels) IsAllowed(nodeLabel string) bool {
-	for _, r := range n {
-		if r.MatchString(nodeLabel) {
-			return true
-		}
-	}
-	return false
+	return commonnodelabels.NewExposedNodeLabels(exposedNodeLabels)


do we need this file at all?

pkoutsovasilis · 2026-06-26T07:25:31Z

Hi again @orangecola 👋 thank you for your patience with the review process and for working through all the feedback. After discussing the latest findings internally, we've realized that supporting this feature properly requires a deeper refactoring - specifically around the init container approach and its silent dependency on the component images having a shell and certain utilities available (which breaks for lean/wolfi variants). Thus we'll take it from here and complete the extra work required. We are really grateful and appreciate the effort you put into this! 🙇

orangecola requested a review from a team as a code owner April 17, 2026 03:06

botelastic Bot added the triage label Apr 17, 2026

orangecola force-pushed the claude/add-downward-node-labels-WZI3d branch from fbff3ff to 679c049 Compare April 17, 2026 03:17

barkbay reviewed Apr 20, 2026

View reviewed changes

Merge branch 'main' into claude/add-downward-node-labels-WZI3d

800a871

orangecola requested a review from barkbay April 27, 2026 15:20

barkbay reviewed May 26, 2026

View reviewed changes

barkbay added the >feature Adds or discusses adding a feature to the product label May 26, 2026

botelastic Bot removed the triage label May 26, 2026

barkbay added >enhancement Enhancement of existing functionality and removed >feature Adds or discusses adding a feature to the product labels May 26, 2026

Merge remote-tracking branch 'upstream/main' into claude/add-downward…

f5dda3f

…-node-labels-WZI3d # Conflicts: # cmd/manager/validation.go # pkg/controller/agent/controller.go # pkg/controller/agent/driver.go

pkoutsovasilis reviewed Jun 24, 2026

View reviewed changes

orangecola added 2 commits June 25, 2026 19:51

Merge remote-tracking branch 'upstream/main' into claude/add-downward…

afde375

…-node-labels-WZI3d # Conflicts: # pkg/controller/apmserver/deployment.go

pkoutsovasilis reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

orangecola commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-checker-service Bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prodsecmachine commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

orangecola commented Apr 20, 2026

Uh oh!

orangecola commented May 11, 2026

Uh oh!

barkbay commented May 26, 2026

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

orangecola commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkoutsovasilis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pkoutsovasilis commented Jun 24, 2026

Uh oh!

pkoutsovasilis commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkoutsovasilis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkoutsovasilis commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

orangecola commented Apr 17, 2026 •

edited

Loading

cla-checker-service Bot commented Apr 17, 2026 •

edited

Loading

prodsecmachine commented Apr 17, 2026 •

edited

Loading

barkbay left a comment •

edited

Loading

orangecola commented May 27, 2026 •

edited

Loading

pkoutsovasilis commented Jun 26, 2026 •

edited

Loading

pkoutsovasilis left a comment •

edited

Loading