diff --git a/pkg/cmd/apply/prune.go b/pkg/cmd/apply/prune.go index 04ce54e5..8c07d32b 100644 --- a/pkg/cmd/apply/prune.go +++ b/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) diff --git a/pkg/cmd/diff/diff.go b/pkg/cmd/diff/diff.go index 82cf5133..0ba5b13f 100644 --- a/pkg/cmd/diff/diff.go +++ b/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) } diff --git a/pkg/cmd/diff/prune.go b/pkg/cmd/diff/prune.go index 64a6d4fe..ffb37a2e 100644 --- a/pkg/cmd/diff/prune.go +++ b/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) } diff --git a/pkg/util/prune/prune.go b/pkg/util/prune/prune.go index 0d49153f..5b195632 100644 --- a/pkg/util/prune/prune.go +++ b/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 = []Resource{ - {"", "v1", "ConfigMap", true}, - {"", "v1", "Endpoints", true}, - {"", "v1", "Namespace", false}, - {"", "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}, + pruneResources = defaultNamespacedPruneResources + // TODO in kubectl v1.29, add back non-namespaced resource only if namespace is not specified + pruneResources = append(pruneResources, defaultNonNamespacedPruneResources...) + if namespaceSpecified { + 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.") } } diff --git a/pkg/util/prune/prune_test.go b/pkg/util/prune/prune_test.go index cbc389b4..fba46ca4 100644 --- a/pkg/util/prune/prune_test.go +++ b/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 - pr []Resource - expectedns int - expectednns int - expectederr error + mapper *testRESTMapper + pr []Resource + namespaceSpecified bool + expectedns int + expectednns int + expectederr error }{ { - mapper: &testRESTMapper{}, - pr: []Resource{}, + mapper: &testRESTMapper{}, + 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") }