Add default namespace labels to all namespaces for selectors (#96968)

* namespace by name default labelling

Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
Co-authored-by: Abhishek Raut <rauta@vmware.com>

* Make some logic improvement into default namespace label

* Fix unit tests

* minor change to trigger the CI

* Correct some tests and validation behaviors

* Add Canonicalize normalization and improve validation

* Remove label validation that should be dealt by strategy

* Update defaults_test.go
add fuzzer
ns spec

* remove the finalizer thingy

* Fix integration test

* Add namespace canonicalize unit test

* Improve validation code and code comments

* move validation of labels to validateupdate

* spacex will save us all

* add comment to testget

* readablility of canonicalize

* Added namespace finalize and status update validation

* comment about ungenerated names

* correcting a missing line on storage_test

* Update the namespace validation unit test

* Add more missing unit test changes

* Let's just blast the value. Also documenting the workflow here

* Remove unnecessary validations

Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
Co-authored-by: Abhishek Raut <rauta@vmware.com>
Co-authored-by: Ricardo Pchevuzinske Katz <ricardo.katz@gmail.com>

pkg/apis/core/fuzzer/fuzzer.go

@@ -472,6 +472,16 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 		func(s *core.NamespaceSpec, c fuzz.Continue) {
 			s.Finalizers = []core.FinalizerName{core.FinalizerKubernetes}
 		},
+		func(s *core.Namespace, c fuzz.Continue) {
+			c.FuzzNoCustom(s) // fuzz self without calling this function again
+			// Match name --> label defaulting
+			if len(s.Name) > 0 {
+				if s.Labels == nil {
+					s.Labels = map[string]string{}
+				}
+				s.Labels["kubernetes.io/metadata.name"] = s.Name
+			}
+		},
 		func(s *core.NamespaceStatus, c fuzz.Continue) {
 			s.Phase = core.NamespaceActive
 		},

pkg/apis/core/v1/defaults.go

@@ -323,6 +323,26 @@ func SetDefaults_HTTPGetAction(obj *v1.HTTPGetAction) {
 		obj.Scheme = v1.URISchemeHTTP
 	}
 }
+
+// SetDefaults_Namespace adds a default label for all namespaces
+func SetDefaults_Namespace(obj *v1.Namespace) {
+	// TODO, remove the feature gate in 1.22
+	// we can't SetDefaults for nameless namespaces (generateName).
+	// This code needs to be kept in sync with the implementation that exists
+	// in Namespace Canonicalize strategy (pkg/registry/core/namespace)
+
+	// note that this can result in many calls to feature enablement in some cases, but
+	// we assume that there's no real cost there.
+	if utilfeature.DefaultFeatureGate.Enabled(features.NamespaceDefaultLabelName) {
+		if len(obj.Name) > 0 {
+			if obj.Labels == nil {
+				obj.Labels = map[string]string{}
+			}
+			obj.Labels[v1.LabelMetadataName] = obj.Name
+		}
+	}
+}
+
 func SetDefaults_NamespaceStatus(obj *v1.NamespaceStatus) {
 	if obj.Phase == "" {
 		obj.Phase = v1.NamespaceActive

pkg/apis/core/v1/defaults_test.go

@@ -1416,6 +1416,40 @@ func TestSetDefaultNamespace(t *testing.T) {
 	}
 }
 
+func TestSetDefaultNamespaceLabels(t *testing.T) {
+	// Although this is defaulted to true, it's still worth to enable the feature gate during the test
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NamespaceDefaultLabelName, true)()
+
+	theNs := "default-ns-labels-are-great"
+	s := &v1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: theNs,
+		},
+	}
+	obj2 := roundTrip(t, runtime.Object(s))
+	s2 := obj2.(*v1.Namespace)
+
+	if s2.ObjectMeta.Labels[v1.LabelMetadataName] != theNs {
+		t.Errorf("Expected default namespace label value of %v, but got %v", theNs, s2.ObjectMeta.Labels[v1.LabelMetadataName])
+	}
+
+	// And let's disable the FG and check if it still defaults creating the labels
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NamespaceDefaultLabelName, false)()
+
+	theNs = "default-ns-labels-are-not-that-great"
+	s = &v1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: theNs,
+		},
+	}
+	obj2 = roundTrip(t, runtime.Object(s))
+	s2 = obj2.(*v1.Namespace)
+
+	if _, ok := s2.ObjectMeta.Labels[v1.LabelMetadataName]; ok {
+		t.Errorf("Default namespace shouldn't exist here, as the feature gate is disabled %v", s)
+	}
+}
+
 func TestSetDefaultPodSpecHostNetwork(t *testing.T) {
 	portNum := int32(8080)
 	s := v1.PodSpec{}

pkg/apis/core/v1/zz_generated.defaults.go

@@ -158,6 +158,7 @@ func SetObjectDefaults_LimitRangeList(in *v1.LimitRangeList) {
 }
 
 func SetObjectDefaults_Namespace(in *v1.Namespace) {
+	SetDefaults_Namespace(in)
 	SetDefaults_NamespaceStatus(&in.Status)
 }
 

pkg/features/kube_features.go

@@ -723,6 +723,12 @@ const (
 	//
 	// Allows jobs to be created in the suspended state.
 	SuspendJob featuregate.Feature = "SuspendJob"
+
+	// owner: @jayunit100 @abhiraut @rikatz
+	// beta: v1.21
+	//
+	// Labels all namespaces with a default label "kubernetes.io/metadata.name: <namespaceName>"
+	NamespaceDefaultLabelName featuregate.Feature = "NamespaceDefaultLabelName"
 )
 
 func init() {
@@ -832,6 +838,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	IngressClassNamespacedParams:                   {Default: false, PreRelease: featuregate.Alpha},
 	ServiceInternalTrafficPolicy:                   {Default: false, PreRelease: featuregate.Alpha},
 	SuspendJob:                                     {Default: false, PreRelease: featuregate.Alpha},
+	NamespaceDefaultLabelName:                      {Default: true, PreRelease: featuregate.Beta}, // graduate to GA and lock to default in 1.22, remove in 1.24
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:

pkg/registry/core/namespace/storage/storage_test.go

@@ -109,6 +109,8 @@ func TestGet(t *testing.T) {
 	defer server.Terminate(t)
 	defer storage.store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.store).ClusterScope()
+
+	// note that this ultimately may call validation
 	test.TestGet(validNewNamespace())
 }
 

pkg/registry/core/namespace/strategy.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -27,9 +28,11 @@ import (
 	"k8s.io/apiserver/pkg/registry/generic"
 	apistorage "k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/validation"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 // namespaceStrategy implements behavior for Namespaces
@@ -88,6 +91,30 @@ func (namespaceStrategy) Validate(ctx context.Context, obj runtime.Object) field
 
 // Canonicalize normalizes the object after validation.
 func (namespaceStrategy) Canonicalize(obj runtime.Object) {
+	// Ensure the label matches the name for namespaces just created using GenerateName,
+	// where the final name wasn't available for defaulting to make this change.
+	// This code needs to be kept in sync with the implementation that exists
+	// in Namespace defaulting (pkg/apis/core/v1)
+	// Why this hook *and* defaults.go?
+	//
+	// CREATE:
+	// Defaulting and PrepareForCreate happen before generateName is completed
+	// (i.e. the name is not yet known). Validation happens after generateName
+	// but should not modify objects. Canonicalize happens later, which makes
+	// it the best hook for setting the label.
+	//
+	// UPDATE:
+	// Defaulting and Canonicalize will both trigger with the full name.
+	//
+	// GET:
+	// Only defaulting will be applied.
+	ns := obj.(*api.Namespace)
+	if utilfeature.DefaultFeatureGate.Enabled(features.NamespaceDefaultLabelName) {
+		if ns.Labels == nil {
+			ns.Labels = map[string]string{}
+		}
+		ns.Labels[v1.LabelMetadataName] = ns.Name
+	}
 }
 
 // AllowCreateOnUpdate is false for namespaces.

pkg/registry/core/namespace/strategy_test.go

@@ -19,10 +19,14 @@ package namespace
 import (
 	"testing"
 
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	apitesting "k8s.io/kubernetes/pkg/api/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/features"
 
 	// ensure types are installed
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
@@ -37,7 +41,7 @@ func TestNamespaceStrategy(t *testing.T) {
 		t.Errorf("Namespaces should not allow create on update")
 	}
 	namespace := &api.Namespace{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10", Labels: map[string]string{v1.LabelMetadataName: "foo"}},
 		Status:     api.NamespaceStatus{Phase: api.NamespaceTerminating},
 	}
 	Strategy.PrepareForCreate(ctx, namespace)
@@ -68,6 +72,19 @@ func TestNamespaceStrategy(t *testing.T) {
 	}
 }
 
+func TestNamespaceDefaultLabelCanonicalize(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NamespaceDefaultLabelName, true)()
+
+	namespace := &api.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+	}
+
+	Strategy.Canonicalize(namespace)
+	if namespace.Labels[v1.LabelMetadataName] != namespace.Name {
+		t.Errorf("Invalid namespace, default label was not added")
+	}
+}
+
 func TestNamespaceStatusStrategy(t *testing.T) {
 	ctx := genericapirequest.NewDefaultContext()
 	if StatusStrategy.NamespaceScoped() {
@@ -78,12 +95,14 @@ func TestNamespaceStatusStrategy(t *testing.T) {
 	}
 	now := metav1.Now()
 	oldNamespace := &api.Namespace{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10", DeletionTimestamp: &now},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10", DeletionTimestamp: &now,
+			Labels: map[string]string{v1.LabelMetadataName: "foo"}},
 		Spec:   api.NamespaceSpec{Finalizers: []api.FinalizerName{"kubernetes"}},
 		Status: api.NamespaceStatus{Phase: api.NamespaceActive},
 	}
 	namespace := &api.Namespace{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "9", DeletionTimestamp: &now},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "9", DeletionTimestamp: &now,
+			Labels: map[string]string{v1.LabelMetadataName: "foo"}},
 		Status: api.NamespaceStatus{Phase: api.NamespaceTerminating},
 	}
 	StatusStrategy.PrepareForUpdate(ctx, namespace, oldNamespace)
@@ -111,12 +130,14 @@ func TestNamespaceFinalizeStrategy(t *testing.T) {
 		t.Errorf("Namespaces should not allow create on update")
 	}
 	oldNamespace := &api.Namespace{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10",
+			Labels: map[string]string{v1.LabelMetadataName: "foo"}},
 		Spec:   api.NamespaceSpec{Finalizers: []api.FinalizerName{"kubernetes", "example.com/org"}},
 		Status: api.NamespaceStatus{Phase: api.NamespaceActive},
 	}
 	namespace := &api.Namespace{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "9"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "9",
+			Labels: map[string]string{v1.LabelMetadataName: "foo"}},
 		Spec:   api.NamespaceSpec{Finalizers: []api.FinalizerName{"example.com/foo"}},
 		Status: api.NamespaceStatus{Phase: api.NamespaceTerminating},
 	}

staging/src/k8s.io/api/core/v1/well_known_labels.go

@@ -65,4 +65,6 @@ const (
 	// any backends on excluded nodes are not reachable by those external load-balancers.
 	// Implementations of this exclusion may vary based on provider.
 	LabelNodeExcludeBalancers = "node.kubernetes.io/exclude-from-external-load-balancers"
+	// LabelMetadataName is the label name which, in-tree, is used to automatically label namespaces, so they can be selected easily by tools which require definitive labels
+	LabelMetadataName = "kubernetes.io/metadata.name"
 )

test/integration/namespace/ns_conditions_test.go

@@ -19,6 +19,7 @@ package namespace
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
@@ -115,6 +116,39 @@ func TestNamespaceCondition(t *testing.T) {
 	}
 }
 
+// TestNamespaceLabels tests for default labels added in https://github.com/kubernetes/kubernetes/pull/96968
+func TestNamespaceLabels(t *testing.T) {
+	closeFn, _, _, kubeClient, _ := namespaceLifecycleSetup(t)
+	defer closeFn()
+	nsName := "test-namespace-labels-generated"
+	// Create a new namespace w/ no name
+	ns, err := kubeClient.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: nsName,
+		},
+	}, metav1.CreateOptions{})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if ns.Name != ns.Labels[corev1.LabelMetadataName] {
+		t.Fatal(fmt.Errorf("expected %q, got %q", ns.Name, ns.Labels[corev1.LabelMetadataName]))
+	}
+
+	nsList, err := kubeClient.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, ns := range nsList.Items {
+		if ns.Name != ns.Labels[corev1.LabelMetadataName] {
+			t.Fatal(fmt.Errorf("expected %q, got %q", ns.Name, ns.Labels[corev1.LabelMetadataName]))
+		}
+	}
+}
+
 // JSONToUnstructured converts a JSON stub to unstructured.Unstructured and
 // returns a dynamic resource client that can be used to interact with it
 func jsonToUnstructured(stub, version, kind string) (*unstructured.Unstructured, error) {