Skip to content

fix(agent): apply pod-security labels from namespaceLabels again #5687

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.

Sign up for GitHub

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

Merged
p-se merged 1 commit into from
Sep 3, 2026
+56 −50
Merged

fix(agent): apply pod-security labels from namespaceLabels again #5687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 11 additions & 26 deletions internal/cmd/agent/deployer/deployer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func (d *Deployer) setNamespaceLabelsAndAnnotations(ctx context.Context, bd *fle
if desiredLabels == nil {
desiredLabels = make(map[string]string)
}
addLabelsFromOptions(log.FromContext(ctx), desiredLabels, bd.Spec.Options.NamespaceLabels)
addLabelsFromOptions(desiredLabels, bd.Spec.Options.NamespaceLabels)
}
desiredAnnotations := maps.Clone(ns.Annotations)
if bd.Spec.Options.NamespaceAnnotations != nil {
Expand Down Expand Up @@ -328,36 +328,21 @@ func fetchNamespace(ctx context.Context, c client.Client, releaseID string) (*co
return ns, nil
}

const podSecurityLabelPrefix = "pod-security.kubernetes.io/"

// addLabelsFromOptions updates nsLabels to contain labels from optLabels, while preserving
// the `kubernetes.io/metadata.name` label added by Kubernetes when creating the namespace
// and any existing `pod-security.kubernetes.io/*` labels. Labels with the
// `pod-security.kubernetes.io/` prefix in optLabels are ignored.
// addLabelsFromOptions updates nsLabels to contain the labels from optLabels, while preserving
// the `kubernetes.io/metadata.name` label added by Kubernetes when creating the namespace.
//
// This filtering is intentionally unconditional and independent of the
// service-account impersonation used for the namespace patch: it must also hold
// for deployments that run as the agent (no service account pinned), where
// there is no downstream RBAC gating at all. It is the only safeguard against a
// bundle escalating pod-security enforcement on its target namespace. To set
// pod-security labels on a namespace, declare them on the Namespace resource in
// the bundle instead.
func addLabelsFromOptions(logger logr.Logger, nsLabels map[string]string, optLabels map[string]string) {
for k, v := range optLabels {
if strings.HasPrefix(k, podSecurityLabelPrefix) {
logger.V(1).Info("Ignoring label from options", "label", k)
continue
}
nsLabels[k] = v
}
// Security-sensitive labels such as `pod-security.kubernetes.io/*` are not
// filtered here. The namespace is patched as the deployment's service account
// (see namespaceClient), so which labels a bundle may set on its target
// namespace is gated by the downstream RBAC of that account. Deployments that
// resolve to no service account still run as the agent, so restricting them
// requires pinning a service account, either in the bundle or through a Policy.
func addLabelsFromOptions(nsLabels map[string]string, optLabels map[string]string) {
maps.Copy(nsLabels, optLabels)
Comment on lines +340 to +341

@p-se p-se Sep 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#5550 came from operators, not from tenants. The complaint is that an operator writes pod-security labels in namespaceLabels and the agent silently drops them. Those deployments are exactly the ones in the d.client fallback: no fleet-default service account ships with the agent chart, so out of the box nothing is pinned. Restricting pod-security labels to the impersonated path would keep ignoring them for every one of the people who reported this, and silently at that.

The tenant path is the one that's already handled. Since #5351 the namespace is patched as the deployment's service account, so whether a bundle may touch its target namespace at all is decided by that account's downstream RBAC. That's a coarser gate than the prefix filter, since RBAC scopes the namespace and not individual labels, but it's the same gate Kubernetes itself uses for PSA, and it's the one operators control through Policy and allowedServiceAccounts. The filter is redundant there.

On the fallback path being an escalation: there's no service account in that path, so every manifest in the bundle is already applied with the agent's cluster-admin credentials. You're right that PodSecurity admission still applies to cluster-admin, so relabeling a namespace is a genuine capability gain, but the filter never prevented it. The same actor can point the bundle at a new namespace (Fleet creates it, and with no cluster-wide AdmissionConfiguration default it has no enforcement at all), ship a Namespace object carrying the labels, or adopt an existing namespace with helm.takeOwnership. All three go through the same credentials and the same API call, and none of them were ever filtered. This was the narrowest of four doors.

Where you have a point is the pruning. Dropping the exemption means a bundle that declares one unrelated label removes an admin-set enforce=restricted without ever naming a pod-security key. That's #4564's clobber bug reaching the labels where it matters most, and it's a surprise rather than a deliberate act. ⚠️ But it's fixed by #5460 which rewrites the pruning to only remove what Fleet actually set. Without it, there is merit in keeping the pruning exemption in this PR.


// Delete labels not defined in the options.
// Keep the `kubernetes.io/metadata.name` label as it is added by kubernetes when creating the namespace.
// Keep pod-security.kubernetes.io/ labels as they are managed by cluster administrators.
for k := range nsLabels {
if strings.HasPrefix(k, podSecurityLabelPrefix) {
continue
}
if _, ok := optLabels[k]; k != corev1.LabelMetadataName && !ok {
delete(nsLabels, k)
}
Expand Down
69 changes: 45 additions & 24 deletions internal/cmd/agent/deployer/deployer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (

fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"

"github.com/go-logr/logr"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -306,8 +304,8 @@ func TestSetNamespaceLabelsAndAnnotationsError(t *testing.T) {
// TestSetNamespaceLabelsAndAnnotations_NoUpdateWhenAlreadyCorrect verifies that
// updateNamespace is not called when the namespace already reflects the desired state.
// This guards against the broken reflect.DeepEqual check that compared raw option
// labels to ns.Labels; ns.Labels always includes kubernetes.io/metadata.name and
// may include preserved pod-security labels, so a direct equality check never holds.
// labels to ns.Labels; ns.Labels always includes kubernetes.io/metadata.name, so a
// direct equality check never holds.
func TestSetNamespaceLabelsAndAnnotations_NoUpdateWhenAlreadyCorrect(t *testing.T) {
bd := &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
Options: fleet.BundleDeploymentOptions{
Expand Down Expand Up @@ -348,13 +346,13 @@ func TestSetNamespaceLabelsAndAnnotations_NoUpdateWhenAlreadyCorrect(t *testing.
}
}

func TestAddLabelsFromOptions_PodSecurityLabelsFiltered(t *testing.T) {
func TestAddLabelsFromOptions_PodSecurityLabels(t *testing.T) {
tests := map[string]struct {
nsLabels map[string]string
optLabels map[string]string
expectedLabels map[string]string
}{
"pod-security.kubernetes.io labels in optLabels are not applied to namespace": {
"pod-security.kubernetes.io labels in optLabels are applied to the namespace": {
nsLabels: map[string]string{"kubernetes.io/metadata.name": "ns"},
optLabels: map[string]string{
"pod-security.kubernetes.io/enforce": "privileged",
Expand All @@ -363,27 +361,41 @@ func TestAddLabelsFromOptions_PodSecurityLabelsFiltered(t *testing.T) {
"safe-label": "value",
},
expectedLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"safe-label": "value",
"kubernetes.io/metadata.name": "ns",
"pod-security.kubernetes.io/enforce": "privileged",
"pod-security.kubernetes.io/audit": "privileged",
"pod-security.kubernetes.io/warn": "privileged",
"safe-label": "value",
},
},
"existing pod-security.kubernetes.io labels on namespace are preserved": {
"existing pod-security.kubernetes.io labels on the namespace are overwritten": {
nsLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"pod-security.kubernetes.io/enforce": "baseline",
"pod-security.kubernetes.io/audit": "baseline",
},
optLabels: map[string]string{
"pod-security.kubernetes.io/enforce": "privileged",
"app-label": "value",
},
expectedLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"pod-security.kubernetes.io/enforce": "baseline",
"pod-security.kubernetes.io/audit": "baseline",
"pod-security.kubernetes.io/enforce": "privileged",
"app-label": "value",
},
},
"pod-security.kubernetes.io labels not in optLabels are removed like any other label": {
nsLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"pod-security.kubernetes.io/audit": "baseline",
},
optLabels: map[string]string{
"app-label": "value",
},
expectedLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"app-label": "value",
},
},
"non-security labels work normally": {
nsLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
Expand All @@ -397,23 +409,25 @@ func TestAddLabelsFromOptions_PodSecurityLabelsFiltered(t *testing.T) {
"new-label": "new-value",
},
},
"pod-security.kubernetes.io labels with custom suffixes are also filtered": {
"pod-security.kubernetes.io labels with version suffixes are applied as well": {
nsLabels: map[string]string{"kubernetes.io/metadata.name": "ns"},
optLabels: map[string]string{
"pod-security.kubernetes.io/enforce-version": "v1.25",
"pod-security.kubernetes.io/audit-version": "v1.25",
"safe-label": "value",
},
expectedLabels: map[string]string{
"kubernetes.io/metadata.name": "ns",
"safe-label": "value",
"kubernetes.io/metadata.name": "ns",
"pod-security.kubernetes.io/enforce-version": "v1.25",
"pod-security.kubernetes.io/audit-version": "v1.25",
"safe-label": "value",
},
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
addLabelsFromOptions(logr.Discard(), test.nsLabels, test.optLabels)
addLabelsFromOptions(test.nsLabels, test.optLabels)

if len(test.nsLabels) != len(test.expectedLabels) {
t.Errorf("expected %d labels, got %d: %v", len(test.expectedLabels), len(test.nsLabels), test.nsLabels)
Expand All @@ -427,7 +441,12 @@ func TestAddLabelsFromOptions_PodSecurityLabelsFiltered(t *testing.T) {
}
}

func TestSetNamespaceLabelsAndAnnotations_PodSecurityLabelsPreserved(t *testing.T) {
// TestSetNamespaceLabelsAndAnnotations_PodSecurityLabelsApplied verifies that
// pod-security labels declared in namespaceLabels reach the namespace, which is
// what SURE-5906 (#1484) added the option for. Setting them is gated by the
// downstream RBAC of the deployment's service account, not by a filter in the
// agent.
func TestSetNamespaceLabelsAndAnnotations_PodSecurityLabelsApplied(t *testing.T) {
bd := &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
Options: fleet.BundleDeploymentOptions{
NamespaceLabels: map[string]string{
Expand Down Expand Up @@ -466,14 +485,16 @@ func TestSetNamespaceLabelsAndAnnotations_PodSecurityLabelsPreserved(t *testing.
t.Fatalf("unexpected error: %v", err)
}

if result.Labels["pod-security.kubernetes.io/enforce"] != "restricted" {
t.Errorf("pod-security.kubernetes.io/enforce: got %s, want restricted", result.Labels["pod-security.kubernetes.io/enforce"])
}
if result.Labels["pod-security.kubernetes.io/audit"] != "restricted" {
t.Errorf("pod-security.kubernetes.io/audit: got %s, want restricted", result.Labels["pod-security.kubernetes.io/audit"])
expected := map[string]string{
"pod-security.kubernetes.io/enforce": "privileged",
"pod-security.kubernetes.io/audit": "privileged",
"pod-security.kubernetes.io/warn": "privileged",
"app-label": "value",
}
if result.Labels["app-label"] != "value" {
t.Errorf("app-label: got %s, want value", result.Labels["app-label"])
for k, v := range expected {
if result.Labels[k] != v {
t.Errorf("%s: got %s, want %s", k, result.Labels[k], v)
}
}
}

Expand Down