From 6c449dd272c95f2aeb3bb77e67d312d8df21bd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 8 Nov 2021 11:22:10 +0300 Subject: [PATCH] Refactor diff/prune This PR does some refactors for diff/prune; - GetRESTMappings takes value array instead reference - Move getObjectName into diff instead prune - License, etc. changes --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 4 +-- .../src/k8s.io/kubectl/pkg/cmd/apply/prune.go | 5 ++- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 31 +++++++++++++--- .../src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 35 +++---------------- .../k8s.io/kubectl/pkg/util/prune/prune.go | 8 ++--- .../kubectl/pkg/util/prune/prune_test.go | 4 +-- 6 files changed, 41 insertions(+), 46 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index 56c689b68f1..ecdcf87c7b8 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -21,9 +21,8 @@ import ( "io" "net/http" - "k8s.io/kubectl/pkg/util/prune" - "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -44,6 +43,7 @@ import ( "k8s.io/kubectl/pkg/util" "k8s.io/kubectl/pkg/util/i18n" "k8s.io/kubectl/pkg/util/openapi" + "k8s.io/kubectl/pkg/util/prune" "k8s.io/kubectl/pkg/util/templates" "k8s.io/kubectl/pkg/validation" ) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go index 609f45e5a76..04ce54e5685 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go @@ -21,8 +21,6 @@ import ( "fmt" "io" - "k8s.io/kubectl/pkg/util/prune" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -30,6 +28,7 @@ import ( "k8s.io/cli-runtime/pkg/printers" "k8s.io/client-go/dynamic" cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/util/prune" ) type pruner struct { @@ -71,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) if err != nil { return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 0315afceebf..f2a1d439e52 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -25,8 +25,6 @@ import ( "regexp" "strings" - "k8s.io/kubectl/pkg/util/prune" - "github.com/jonboulle/clockwork" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/errors" @@ -46,6 +44,7 @@ import ( "k8s.io/kubectl/pkg/util" "k8s.io/kubectl/pkg/util/i18n" "k8s.io/kubectl/pkg/util/openapi" + "k8s.io/kubectl/pkg/util/prune" "k8s.io/kubectl/pkg/util/templates" "k8s.io/utils/exec" "sigs.k8s.io/yaml" @@ -173,7 +172,7 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") - cmd.Flags().StringArray("prunewhitelist", []string{}, "Overwrite the default whitelist with for --prune") + cmd.Flags().StringArray("prune-allowlist", []string{}, "Overwrite the default whitelist with for --prune") cmd.Flags().Bool("prune", false, "Include resources that would be deleted by pruning. Can be used with -l and default shows all resources would be pruned") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) @@ -653,7 +652,7 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } - resources, err := prune.ParseResources(mapper, cmdutil.GetFlagStringArray(cmd, "prunewhitelist")) + resources, err := prune.ParseResources(mapper, cmdutil.GetFlagStringArray(cmd, "prune-allowlist")) if err != nil { return err } @@ -749,7 +748,7 @@ func (o *DiffOptions) Run() error { // Print pruned objects into old file and thus, diff // command will show them as pruned. for _, p := range prunedObjs { - name, err := o.pruner.GetObjectName(p) + name, err := getObjectName(p) if err != nil { klog.Warningf("pruning failed and object name could not be retrieved: %v", err) continue @@ -766,3 +765,25 @@ func (o *DiffOptions) Run() error { return differ.Run(o.Diff) } + +func getObjectName(obj runtime.Object) (string, error) { + gvk := obj.GetObjectKind().GroupVersionKind() + metadata, err := meta.Accessor(obj) + if err != nil { + return "", err + } + name := metadata.GetName() + ns := metadata.GetNamespace() + + group := "" + if gvk.Group != "" { + group = fmt.Sprintf("%v.", gvk.Group) + } + return group + fmt.Sprintf( + "%v.%v.%v.%v", + gvk.Version, + gvk.Kind, + ns, + name, + ), nil +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go index d7d5d288db3..64a6d4fee36 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2021 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,17 +20,14 @@ import ( "context" "fmt" - "k8s.io/cli-runtime/pkg/resource" - - "k8s.io/apimachinery/pkg/runtime" - - "k8s.io/kubectl/pkg/util/prune" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/dynamic" + "k8s.io/kubectl/pkg/util/prune" ) type pruner struct { @@ -55,7 +52,7 @@ func newPruner(dc dynamic.Interface, m meta.RESTMapper, r []prune.Resource) *pru func (p *pruner) pruneAll() ([]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) if err != nil { return allPruned, fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } @@ -115,28 +112,6 @@ func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) ([]runtime.O return pobjs, nil } -func (p *pruner) GetObjectName(obj runtime.Object) (string, error) { - gvk := obj.GetObjectKind().GroupVersionKind() - metadata, err := meta.Accessor(obj) - if err != nil { - return "", err - } - name := metadata.GetName() - ns := metadata.GetNamespace() - - group := "" - if gvk.Group != "" { - group = fmt.Sprintf("%v.", gvk.Group) - } - return group + fmt.Sprintf( - "%v.%v.%v.%v", - gvk.Version, - gvk.Kind, - ns, - name, - ), nil -} - // MarkVisited marks visited namespaces and uids func (p *pruner) MarkVisited(info *resource.Info) { if info.Namespaced() { diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go index f44b2c639f2..0d49153fe4c 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go @@ -35,10 +35,10 @@ 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 len(*pruneResources) == 0 { +func GetRESTMappings(mapper meta.RESTMapper, pruneResources []Resource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { + if len(pruneResources) == 0 { // default allowlist - *pruneResources = []Resource{ + pruneResources = []Resource{ {"", "v1", "ConfigMap", true}, {"", "v1", "Endpoints", true}, {"", "v1", "Namespace", false}, @@ -58,7 +58,7 @@ func GetRESTMappings(mapper meta.RESTMapper, pruneResources *[]Resource) (namesp } } - for _, resource := range *pruneResources { + for _, resource := range pruneResources { addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) if err != nil { return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go index 54f5d4c76aa..cbc389b4af0 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go @@ -49,14 +49,14 @@ func (m *testRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (* func TestGetRESTMappings(t *testing.T) { tests := []struct { mapper *testRESTMapper - pr *[]Resource + pr []Resource expectedns int expectednns int expectederr error }{ { mapper: &testRESTMapper{}, - pr: &[]Resource{}, + pr: []Resource{}, expectedns: 14, expectednns: 2, expectederr: nil,