Skip to content
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
37 changes: 11 additions & 26 deletions internal/cmd/agent/deployer/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,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 @@ -311,36 +311,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)

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"errors"
"testing"

"github.com/go-logr/logr"

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

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -305,8 +303,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 @@ -347,13 +345,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 @@ -362,27 +360,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 @@ -396,23 +408,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 @@ -426,7 +440,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 @@ -465,14 +484,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)
}
}
}
func TestIsStateAccepted(t *testing.T) {
Expand Down