kubectl apply: warning that in future release, prune will ignores no-namespaced resource if -n is not empty

Signed-off-by: Paco Xu <paco.xu@daocloud.io>

Kubernetes-commit: f3b49adadf2182ba0a7fb930e6340da725053520

pkg/cmd/apply/prune.go

@@ -70,7 +70,7 @@ func newPruner(o *ApplyOptions) pruner {
 
 func (p *pruner) pruneAll(o *ApplyOptions) error {
 
-	namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(o.Mapper, o.PruneResources)
+	namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(o.Mapper, o.PruneResources, o.Namespace != "")
 	if err != nil {
 		return fmt.Errorf("error retrieving RESTMappings to prune: %v", err)
 	}
@@ -82,6 +82,7 @@ func (p *pruner) pruneAll(o *ApplyOptions) error {
 			}
 		}
 	}
+
 	for _, m := range nonNamespacedRESTMappings {
 		if err := p.prune(metav1.NamespaceNone, m); err != nil {
 			return fmt.Errorf("error pruning nonNamespaced object %v: %v", m.GroupVersionKind, err)

pkg/cmd/diff/diff.go

@@ -747,7 +747,7 @@ func (o *DiffOptions) Run() error {
 	})
 
 	if o.pruner != nil {
-		prunedObjs, err := o.pruner.pruneAll()
+		prunedObjs, err := o.pruner.pruneAll(o.CmdNamespace != "")
 		if err != nil {
 			klog.Warningf("pruning failed and could not be evaluated err: %v", err)
 		}

pkg/cmd/diff/prune.go

@@ -50,9 +50,9 @@ func newPruner(dc dynamic.Interface, m meta.RESTMapper, r []prune.Resource) *pru
 	}
 }
 
-func (p *pruner) pruneAll() ([]runtime.Object, error) {
+func (p *pruner) pruneAll(namespaceSpecified bool) ([]runtime.Object, error) {
 	var allPruned []runtime.Object
-	namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, p.resources)
+	namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, p.resources, namespaceSpecified)
 	if err != nil {
 		return allPruned, fmt.Errorf("error retrieving RESTMappings to prune: %v", err)
 	}

pkg/util/prune/prune.go

@@ -22,8 +22,33 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/klog/v2"
 )
 
+// default allowlist of namespaced resources
+var defaultNamespacedPruneResources = []Resource{
+	{"", "v1", "ConfigMap", true},
+	{"", "v1", "Endpoints", true},
+	{"", "v1", "PersistentVolumeClaim", true},
+	{"", "v1", "Pod", true},
+	{"", "v1", "ReplicationController", true},
+	{"", "v1", "Secret", true},
+	{"", "v1", "Service", true},
+	{"batch", "v1", "Job", true},
+	{"batch", "v1", "CronJob", true},
+	{"networking.k8s.io", "v1", "Ingress", true},
+	{"apps", "v1", "DaemonSet", true},
+	{"apps", "v1", "Deployment", true},
+	{"apps", "v1", "ReplicaSet", true},
+	{"apps", "v1", "StatefulSet", true},
+}
+
+// default allowlist of non-namespaced resources
+var defaultNonNamespacedPruneResources = []Resource{
+	{"", "v1", "Namespace", false},
+	{"", "v1", "PersistentVolume", false},
+}
+
 type Resource struct {
 	group      string
 	version    string
@@ -35,26 +60,15 @@ func (pr Resource) String() string {
 	return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced)
 }
 
-func GetRESTMappings(mapper meta.RESTMapper, pruneResources []Resource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) {
+// if namespace is explicitly specified, the default allow list should not include non-namespaced resources.
+// if pruneResources is specified by user, respect the user setting.
+func GetRESTMappings(mapper meta.RESTMapper, pruneResources []Resource, namespaceSpecified bool) (namespaced, nonNamespaced []*meta.RESTMapping, err error) {
 	if len(pruneResources) == 0 {
-		// default allowlist
+		pruneResources = defaultNamespacedPruneResources
-		pruneResources = []Resource{
+		// TODO in kubectl v1.29, add back non-namespaced resource only if namespace is not specified
-			{"", "v1", "ConfigMap", true},
+		pruneResources = append(pruneResources, defaultNonNamespacedPruneResources...)
-			{"", "v1", "Endpoints", true},
+		if namespaceSpecified {
-			{"", "v1", "Namespace", false},
+			klog.Warning("Deprecated: kubectl apply will no longer prune non-namespaced resources by default when used with the --namespace flag in a future release. To preserve the current behaviour, list the resources you want to target explicitly in the --prune-allowlist flag.")
-			{"", "v1", "PersistentVolumeClaim", true},
-			{"", "v1", "PersistentVolume", false},
-			{"", "v1", "Pod", true},
-			{"", "v1", "ReplicationController", true},
-			{"", "v1", "Secret", true},
-			{"", "v1", "Service", true},
-			{"batch", "v1", "Job", true},
-			{"batch", "v1", "CronJob", true},
-			{"networking.k8s.io", "v1", "Ingress", true},
-			{"apps", "v1", "DaemonSet", true},
-			{"apps", "v1", "Deployment", true},
-			{"apps", "v1", "ReplicaSet", true},
-			{"apps", "v1", "StatefulSet", true},
 		}
 	}
 

pkg/util/prune/prune_test.go

@@ -48,23 +48,48 @@ func (m *testRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*
 
 func TestGetRESTMappings(t *testing.T) {
 	tests := []struct {
-		mapper      *testRESTMapper
+		mapper             *testRESTMapper
-		pr          []Resource
+		pr                 []Resource
-		expectedns  int
+		namespaceSpecified bool
-		expectednns int
+		expectedns         int
-		expectederr error
+		expectednns        int
+		expectederr        error
 	}{
 		{
-			mapper:      &testRESTMapper{},
+			mapper:             &testRESTMapper{},
-			pr:          []Resource{},
+			pr:                 []Resource{},
+			namespaceSpecified: false,
+			expectedns:         14,
+			expectednns:        2,
+			expectederr:        nil,
+		},
+		{
+			mapper:             &testRESTMapper{},
+			pr:                 []Resource{},
+			namespaceSpecified: true,
 			expectedns:  14,
+			// it will be 0 non-namespaced resources after the deprecation period has passed.
+			// for details, refer to https://github.com/kubernetes/kubernetes/pull/110907/.
 			expectednns: 2,
 			expectederr: nil,
 		},
+		{
+			mapper: &testRESTMapper{},
+			pr: []Resource{
+				{"apps", "v1", "DaemonSet", true},
+				{"core", "v1", "Pod", true},
+				{"", "v1", "Foo2", false},
+				{"foo", "v1", "Foo3", false},
+			},
+			namespaceSpecified: false,
+			expectedns:         2,
+			expectednns:        2,
+			expectederr:        nil,
+		},
 	}
 
 	for _, tc := range tests {
-		actualns, actualnns, actualerr := GetRESTMappings(tc.mapper, tc.pr)
+		actualns, actualnns, actualerr := GetRESTMappings(tc.mapper, tc.pr, tc.namespaceSpecified)
 		if tc.expectederr != nil {
 			assert.NotEmptyf(t, actualerr, "getRESTMappings error expected but not fired")
 		}