Skip to content
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.

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

Add default namespace labels to all namespaces for selectors #96968

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0f930bc
namespace by name default labelling
jayunit100 Dec 1, 2020
aa51c76
Make some logic improvement into default namespace label
rikatz Mar 1, 2021
4696c3a
Fix unit tests
rikatz Mar 1, 2021
9fbd555
minor change to trigger the CI
rikatz Mar 1, 2021
636ab68
Correct some tests and validation behaviors
rikatz Mar 2, 2021
f2d7560
Add Canonicalize normalization and improve validation
rikatz Mar 2, 2021
1a8cd99
Remove label validation that should be dealt by strategy
rikatz Mar 2, 2021
a3391e0
Update defaults_test.go
jayunit100 Mar 2, 2021
d46ae98
remove the finalizer thingy
jayunit100 Mar 3, 2021
69bd09a
Fix integration test
rikatz Mar 3, 2021
b39a87d
Add namespace canonicalize unit test
rikatz Mar 4, 2021
26db9fc
Improve validation code and code comments
rikatz Mar 4, 2021
49e6cc9
move validation of labels to validateupdate
rikatz Mar 5, 2021
22200ee
spacex will save us all
jayunit100 Mar 6, 2021
e673cab
add comment to testget
jayunit100 Mar 6, 2021
d9a8645
readablility of canonicalize
jayunit100 Mar 6, 2021
e2c7632
Added namespace finalize and status update validation
jayunit100 Mar 6, 2021
90d5605
comment about ungenerated names
jayunit100 Mar 6, 2021
afe021e
correcting a missing line on storage_test
rikatz Mar 6, 2021
8f2f100
Update the namespace validation unit test
rikatz Mar 7, 2021
2bdfe9d
Add more missing unit test changes
rikatz Mar 7, 2021
920096b
Let's just blast the value. Also documenting the workflow here
rikatz Mar 8, 2021
4654742
Remove unnecessary validations
rikatz Mar 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/apis/core/fuzzer/fuzzer.go
Expand Up @@ -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
}
liggitt marked this conversation as resolved.
Show resolved Hide resolved
},
func(s *core.NamespaceStatus, c fuzz.Continue) {
s.Phase = core.NamespaceActive
},
Expand Down
20 changes: 20 additions & 0 deletions pkg/apis/core/v1/defaults.go
Expand Up @@ -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) {
jayunit100 marked this conversation as resolved.
Show resolved Hide resolved
// 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)

jayunit100 marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
34 changes: 34 additions & 0 deletions pkg/apis/core/v1/defaults_test.go
Expand Up @@ -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)
}
}
liggitt marked this conversation as resolved.
Show resolved Hide resolved

func TestSetDefaultPodSpecHostNetwork(t *testing.T) {
portNum := int32(8080)
s := v1.PodSpec{}
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/core/v1/zz_generated.defaults.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/features/kube_features.go
Expand Up @@ -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() {
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions pkg/registry/core/namespace/storage/storage_test.go
Expand Up @@ -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())
}

Expand Down
27 changes: 27 additions & 0 deletions pkg/registry/core/namespace/strategy.go
Expand Up @@ -20,16 +20,19 @@ import (
"context"
"fmt"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"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
Expand Down Expand Up @@ -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)
liggitt marked this conversation as resolved.
Show resolved Hide resolved
// 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.
Expand Down
45 changes: 33 additions & 12 deletions pkg/registry/core/namespace/strategy_test.go
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -78,13 +95,15 @@ func TestNamespaceStatusStrategy(t *testing.T) {
}
now := metav1.Now()
oldNamespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10", DeletionTimestamp: &now},
Spec: api.NamespaceSpec{Finalizers: []api.FinalizerName{"kubernetes"}},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
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},
Status: api.NamespaceStatus{Phase: api.NamespaceTerminating},
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)
if namespace.Status.Phase != api.NamespaceTerminating {
Expand All @@ -111,14 +130,16 @@ func TestNamespaceFinalizeStrategy(t *testing.T) {
t.Errorf("Namespaces should not allow create on update")
}
oldNamespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "10"},
Spec: api.NamespaceSpec{Finalizers: []api.FinalizerName{"kubernetes", "example.com/org"}},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
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"},
Spec: api.NamespaceSpec{Finalizers: []api.FinalizerName{"example.com/foo"}},
Status: api.NamespaceStatus{Phase: api.NamespaceTerminating},
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},
}
FinalizeStrategy.PrepareForUpdate(ctx, namespace, oldNamespace)
if namespace.Status.Phase != api.NamespaceActive {
Expand Down
2 changes: 2 additions & 0 deletions staging/src/k8s.io/api/core/v1/well_known_labels.go
Expand Up @@ -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"
)
34 changes: 34 additions & 0 deletions test/integration/namespace/ns_conditions_test.go
Expand Up @@ -19,6 +19,7 @@ package namespace
import (
"context"
"encoding/json"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -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)
}
liggitt marked this conversation as resolved.
Show resolved Hide resolved

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) {
Expand Down