Merge pull request #11720 from mattfarina/fix-11712

Fix improper use of Table request/response to k8s API
pull/10584/merge
Matt Farina 2 years ago committed by GitHub
commit 76157c6d06
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -65,6 +65,13 @@ func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
return compListReleases(toComplete, args, cfg) return compListReleases(toComplete, args, cfg)
}, },
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
// When the output format is a table the resources should be fetched
// and displayed as a table. When YAML or JSON the resources will be
// returned. This mirrors the handling in kubectl.
if outfmt == output.Table {
client.ShowResourcesTable = true
}
rel, err := client.Run(args[0]) rel, err := client.Run(args[0])
if err != nil { if err != nil {
return err return err

@ -18,6 +18,7 @@ package action
import ( import (
"bytes" "bytes"
"errors"
"helm.sh/helm/v3/pkg/kube" "helm.sh/helm/v3/pkg/kube"
"helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/release"
@ -36,9 +37,13 @@ type Status struct {
// TODO Helm 4: Remove this flag and output the description by default. // TODO Helm 4: Remove this flag and output the description by default.
ShowDescription bool ShowDescription bool
// If true, display resources of release to output format // ShowResources sets if the resources should be retrieved with the status.
// TODO Helm 4: Remove this flag and output the resources by default. // TODO Helm 4: Remove this flag and output the resources by default.
ShowResources bool ShowResources bool
// ShowResourcesTable is used with ShowResources. When true this will cause
// the resulting objects to be retrieved as a kind=table.
ShowResourcesTable bool
} }
// NewStatus creates a new Status object with the given configuration. // NewStatus creates a new Status object with the given configuration.
@ -63,10 +68,21 @@ func (s *Status) Run(name string) (*release.Release, error) {
return nil, err return nil, err
} }
resources, _ := s.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), false)
if kubeClient, ok := s.cfg.KubeClient.(kube.InterfaceResources); ok { if kubeClient, ok := s.cfg.KubeClient.(kube.InterfaceResources); ok {
resp, err := kubeClient.Get(resources, bytes.NewBufferString(rel.Manifest)) var resources kube.ResourceList
if s.ShowResourcesTable {
resources, err = kubeClient.BuildTable(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
} else {
resources, err = s.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
}
resp, err := kubeClient.Get(resources, true)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -75,5 +91,5 @@ func (s *Status) Run(name string) (*release.Release, error) {
return rel, nil return rel, nil
} }
return nil, err return nil, errors.New("unable to get kubeClient with interface InterfaceResources")
} }

@ -149,7 +149,10 @@ func transformRequests(req *rest.Request) {
req.Param("includeObject", "Object") req.Param("includeObject", "Object")
} }
func (c *Client) Get(resources ResourceList, reader io.Reader) (map[string][]runtime.Object, error) { // Get retrieves the resource objects supplied. If related is set to true the
// related pods are fetched as well. If the passed in resources are a table kind
// the related resources will also be fetched as kind=table.
func (c *Client) Get(resources ResourceList, related bool) (map[string][]runtime.Object, error) {
buf := new(bytes.Buffer) buf := new(bytes.Buffer)
objs := make(map[string][]runtime.Object) objs := make(map[string][]runtime.Object)
@ -167,11 +170,22 @@ func (c *Client) Get(resources ResourceList, reader io.Reader) (map[string][]run
} else { } else {
objs[vk] = append(objs[vk], obj) objs[vk] = append(objs[vk], obj)
objs, err = c.getSelectRelationPod(info, objs, &podSelectors) // Only fetch related pods if they are requested
if related {
// Discover if the existing object is a table. If it is, request
// the pods as Tables. Otherwise request them normally.
objGVK := obj.GetObjectKind().GroupVersionKind()
var isTable bool
if objGVK.Kind == "Table" {
isTable = true
}
objs, err = c.getSelectRelationPod(info, objs, isTable, &podSelectors)
if err != nil { if err != nil {
c.Log("Warning: get the relation pod is failed, err:%s", err.Error()) c.Log("Warning: get the relation pod is failed, err:%s", err.Error())
} }
} }
}
return nil return nil
}) })
@ -182,7 +196,7 @@ func (c *Client) Get(resources ResourceList, reader io.Reader) (map[string][]run
return objs, nil return objs, nil
} }
func (c *Client) getSelectRelationPod(info *resource.Info, objs map[string][]runtime.Object, podSelectors *[]map[string]string) (map[string][]runtime.Object, error) { func (c *Client) getSelectRelationPod(info *resource.Info, objs map[string][]runtime.Object, table bool, podSelectors *[]map[string]string) (map[string][]runtime.Object, error) {
if info == nil { if info == nil {
return objs, nil return objs, nil
} }
@ -201,7 +215,10 @@ func (c *Client) getSelectRelationPod(info *resource.Info, objs map[string][]run
*podSelectors = append(*podSelectors, selector) *podSelectors = append(*podSelectors, selector)
infos, err := c.Factory.NewBuilder(). var infos []*resource.Info
var err error
if table {
infos, err = c.Factory.NewBuilder().
Unstructured(). Unstructured().
ContinueOnError(). ContinueOnError().
NamespaceParam(info.Namespace). NamespaceParam(info.Namespace).
@ -213,6 +230,19 @@ func (c *Client) getSelectRelationPod(info *resource.Info, objs map[string][]run
if err != nil { if err != nil {
return objs, err return objs, err
} }
} else {
infos, err = c.Factory.NewBuilder().
Unstructured().
ContinueOnError().
NamespaceParam(info.Namespace).
DefaultNamespace().
ResourceTypes("pods").
LabelSelector(labels.Set(selector).AsSelector().String()).
Do().Infos()
if err != nil {
return objs, err
}
}
vk := "v1/Pod(related)" vk := "v1/Pod(related)"
for _, info := range infos { for _, info := range infos {
@ -317,21 +347,38 @@ func (c *Client) Build(reader io.Reader, validate bool) (ResourceList, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
var result ResourceList result, err := c.newBuilder().
if validate {
result, err = c.newBuilder().
Unstructured(). Unstructured().
Schema(schema). Schema(schema).
Stream(reader, ""). Stream(reader, "").
Do().Infos() Do().Infos()
} else { return result, scrubValidationError(err)
result, err = c.newBuilder(). }
// BuildTable validates for Kubernetes objects and returns unstructured infos.
// The returned kind is a Table.
func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, error) {
validationDirective := metav1.FieldValidationIgnore
if validate {
validationDirective = metav1.FieldValidationStrict
}
dynamicClient, err := c.Factory.DynamicClient()
if err != nil {
return nil, err
}
verifier := resource.NewQueryParamVerifier(dynamicClient, c.Factory.OpenAPIGetter(), resource.QueryParamFieldValidation)
schema, err := c.Factory.Validator(validationDirective, verifier)
if err != nil {
return nil, err
}
result, err := c.newBuilder().
Unstructured(). Unstructured().
Schema(schema). Schema(schema).
Stream(reader, ""). Stream(reader, "").
TransformRequests(transformRequests). TransformRequests(transformRequests).
Do().Infos() Do().Infos()
}
return result, scrubValidationError(err) return result, scrubValidationError(err)
} }

@ -253,6 +253,45 @@ func TestBuild(t *testing.T) {
} }
} }
func TestBuildTable(t *testing.T) {
tests := []struct {
name string
namespace string
reader io.Reader
count int
err bool
}{
{
name: "Valid input",
namespace: "test",
reader: strings.NewReader(guestbookManifest),
count: 6,
}, {
name: "Valid input, deploying resources into different namespaces",
namespace: "test",
reader: strings.NewReader(namespacedGuestbookManifest),
count: 1,
},
}
c := newTestClient(t)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test for an invalid manifest
infos, err := c.BuildTable(tt.reader, false)
if err != nil && !tt.err {
t.Errorf("Got error message when no error should have occurred: %v", err)
} else if err != nil && strings.Contains(err.Error(), "--validate=false") {
t.Error("error message was not scrubbed")
}
if len(infos) != tt.count {
t.Errorf("expected %d result objects, got %d", tt.count, len(infos))
}
})
}
}
func TestPerform(t *testing.T) { func TestPerform(t *testing.T) {
tests := []struct { tests := []struct {
name string name string

@ -22,6 +22,7 @@ import (
"time" "time"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/cli-runtime/pkg/resource" "k8s.io/cli-runtime/pkg/resource"
"helm.sh/helm/v3/pkg/kube" "helm.sh/helm/v3/pkg/kube"
@ -33,11 +34,13 @@ import (
type FailingKubeClient struct { type FailingKubeClient struct {
PrintingKubeClient PrintingKubeClient
CreateError error CreateError error
GetError error
WaitError error WaitError error
DeleteError error DeleteError error
WatchUntilReadyError error WatchUntilReadyError error
UpdateError error UpdateError error
BuildError error BuildError error
BuildTableError error
BuildUnstructuredError error BuildUnstructuredError error
WaitAndGetCompletedPodPhaseError error WaitAndGetCompletedPodPhaseError error
WaitDuration time.Duration WaitDuration time.Duration
@ -51,6 +54,14 @@ func (f *FailingKubeClient) Create(resources kube.ResourceList) (*kube.Result, e
return f.PrintingKubeClient.Create(resources) return f.PrintingKubeClient.Create(resources)
} }
// Get returns the configured error if set or prints
func (f *FailingKubeClient) Get(resources kube.ResourceList, related bool) (map[string][]runtime.Object, error) {
if f.GetError != nil {
return nil, f.GetError
}
return f.PrintingKubeClient.Get(resources, related)
}
// Waits the amount of time defined on f.WaitDuration, then returns the configured error if set or prints. // Waits the amount of time defined on f.WaitDuration, then returns the configured error if set or prints.
func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration) error { func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration) error {
time.Sleep(f.WaitDuration) time.Sleep(f.WaitDuration)
@ -108,6 +119,14 @@ func (f *FailingKubeClient) Build(r io.Reader, _ bool) (kube.ResourceList, error
return f.PrintingKubeClient.Build(r, false) return f.PrintingKubeClient.Build(r, false)
} }
// BuildTable returns the configured error if set or prints
func (f *FailingKubeClient) BuildTable(r io.Reader, _ bool) (kube.ResourceList, error) {
if f.BuildTableError != nil {
return []*resource.Info{}, f.BuildTableError
}
return f.PrintingKubeClient.BuildTable(r, false)
}
// WaitAndGetCompletedPodPhase returns the configured error if set or prints // WaitAndGetCompletedPodPhase returns the configured error if set or prints
func (f *FailingKubeClient) WaitAndGetCompletedPodPhase(s string, d time.Duration) (v1.PodPhase, error) { func (f *FailingKubeClient) WaitAndGetCompletedPodPhase(s string, d time.Duration) (v1.PodPhase, error) {
if f.WaitAndGetCompletedPodPhaseError != nil { if f.WaitAndGetCompletedPodPhaseError != nil {

@ -48,7 +48,7 @@ func (p *PrintingKubeClient) Create(resources kube.ResourceList) (*kube.Result,
return &kube.Result{Created: resources}, nil return &kube.Result{Created: resources}, nil
} }
func (p *PrintingKubeClient) Get(resources kube.ResourceList, reader io.Reader) (map[string][]runtime.Object, error) { func (p *PrintingKubeClient) Get(resources kube.ResourceList, related bool) (map[string][]runtime.Object, error) {
_, err := io.Copy(p.Out, bufferize(resources)) _, err := io.Copy(p.Out, bufferize(resources))
if err != nil { if err != nil {
return nil, err return nil, err
@ -105,6 +105,11 @@ func (p *PrintingKubeClient) Build(_ io.Reader, _ bool) (kube.ResourceList, erro
return []*resource.Info{}, nil return []*resource.Info{}, nil
} }
// BuildTable implements KubeClient BuildTable.
func (p *PrintingKubeClient) BuildTable(_ io.Reader, _ bool) (kube.ResourceList, error) {
return []*resource.Info{}, nil
}
// WaitAndGetCompletedPodPhase implements KubeClient WaitAndGetCompletedPodPhase. // WaitAndGetCompletedPodPhase implements KubeClient WaitAndGetCompletedPodPhase.
func (p *PrintingKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Duration) (v1.PodPhase, error) { func (p *PrintingKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Duration) (v1.PodPhase, error) {
return v1.PodSucceeded, nil return v1.PodSucceeded, nil

@ -83,8 +83,22 @@ type InterfaceExt interface {
// //
// TODO Helm 4: Remove InterfaceResources and integrate its method(s) into the Interface. // TODO Helm 4: Remove InterfaceResources and integrate its method(s) into the Interface.
type InterfaceResources interface { type InterfaceResources interface {
// Get details of deployed resources in ResourceList to be printed. // Get details of deployed resources.
Get(resources ResourceList, reader io.Reader) (map[string][]runtime.Object, error) // The first argument is a list of resources to get. The second argument
// specifies if related pods should be fetched. For example, the pods being
// managed by a deployment.
Get(resources ResourceList, related bool) (map[string][]runtime.Object, error)
// BuildTable creates a resource list from a Reader. This differs from
// Interface.Build() in that a table kind is returned. A table is useful
// if you want to use a printer to display the information.
//
// Reader must contain a YAML stream (one or more YAML documents separated
// by "\n---\n")
//
// Validates against OpenAPI schema if validate is true.
// TODO Helm 4: Integrate into Build with an argument
BuildTable(reader io.Reader, validate bool) (ResourceList, error)
} }
var _ Interface = (*Client)(nil) var _ Interface = (*Client)(nil)

Loading…
Cancel
Save