From 10a6885da153e6df814f5fb41693bec2df7839b6 Mon Sep 17 00:00:00 2001 From: Ivo Gosemann Date: Wed, 3 Jan 2024 11:53:18 +0100 Subject: [PATCH 1/4] feat(cli-runtime): wrap meta.NoKindMatchErrors --- staging/src/k8s.io/cli-runtime/pkg/resource/builder.go | 2 +- staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go | 3 ++- staging/src/k8s.io/cli-runtime/pkg/resource/mapper.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go index 47ec83bbbad..36a9b3f2f26 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go @@ -803,7 +803,7 @@ func (b *Builder) mappingFor(resourceOrKindArg string) (*meta.RESTMapping, error // if the error is _not_ a *meta.NoKindMatchError, then we had trouble doing discovery, // so we should return the original error since it may help a user diagnose what is actually wrong if meta.IsNoMatchError(err) { - return nil, fmt.Errorf("the server doesn't have a resource type %q", groupResource.Resource) + return nil, fmt.Errorf("the server doesn't have a resource type %q: %w", groupResource.Resource, err) } return nil, err } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go index 3ec23461ece..6f5232cd3ae 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go @@ -46,6 +46,7 @@ import ( "k8s.io/client-go/rest/fake" restclientwatch "k8s.io/client-go/rest/watch" "k8s.io/client-go/restmapper" + // TODO we need to remove this linkage and create our own scheme v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" @@ -1050,7 +1051,7 @@ func TestRestMappingErrors(t *testing.T) { // ensure that requesting a resource we _know_ not to exist results in an expected *meta.NoKindMatchError err := b.Do().IntoSingleItemImplied(&singleItemImplied).Visit(test.Handle) if err != nil { - if !strings.Contains(err.Error(), "server doesn't have a resource type \"foo\"") { + if !errors.Is(err, &meta.NoKindMatchError{}) { t.Fatalf("unexpected error: %v", err) } } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/mapper.go b/staging/src/k8s.io/cli-runtime/pkg/resource/mapper.go index 5180610e206..03b66684205 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/mapper.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/mapper.go @@ -66,7 +66,7 @@ func (m *mapper) infoForData(data []byte, source string) (*Info, error) { mapping, err := restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { if _, ok := err.(*meta.NoKindMatchError); ok { - return nil, fmt.Errorf("resource mapping not found for name: %q namespace: %q from %q: %v\nensure CRDs are installed first", + return nil, fmt.Errorf("resource mapping not found for name: %q namespace: %q from %q: %w\nensure CRDs are installed first", name, namespace, source, err) } return nil, fmt.Errorf("unable to recognize %q: %v", source, err) From fd5353eb721ed9c904097d2fb4c5ffcb8ea5e2a9 Mon Sep 17 00:00:00 2001 From: Ivo Gosemann Date: Fri, 5 Jan 2024 15:04:33 +0100 Subject: [PATCH 2/4] check for error message and type --- staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go index 6f5232cd3ae..0246d6a43ed 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go @@ -1051,9 +1051,12 @@ func TestRestMappingErrors(t *testing.T) { // ensure that requesting a resource we _know_ not to exist results in an expected *meta.NoKindMatchError err := b.Do().IntoSingleItemImplied(&singleItemImplied).Visit(test.Handle) if err != nil { - if !errors.Is(err, &meta.NoKindMatchError{}) { + if !strings.Contains(err.Error(), "server doesn't have a resource type \"foo\"") { t.Fatalf("unexpected error: %v", err) } + if !errors.Is(err, &meta.NoKindMatchError{}) { + t.Fatalf("unexpected error type: %v", err) + } } expectedErr := fmt.Errorf("expected error") From 365cbe1b41f2e8ed2de9b4fe3f8fe62dbd1314ed Mon Sep 17 00:00:00 2001 From: Ivo Gosemann Date: Thu, 18 Jan 2024 14:14:43 +0100 Subject: [PATCH 3/4] remove formatted newline --- staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go index 0246d6a43ed..460bfee4551 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go @@ -46,7 +46,6 @@ import ( "k8s.io/client-go/rest/fake" restclientwatch "k8s.io/client-go/rest/watch" "k8s.io/client-go/restmapper" - // TODO we need to remove this linkage and create our own scheme v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" From e821e0de15ea45be5885088d4a08ea9aaba2b184 Mon Sep 17 00:00:00 2001 From: Ivo Gosemann Date: Thu, 8 Feb 2024 09:51:15 +0100 Subject: [PATCH 4/4] remove error wrapping from builder --- staging/src/k8s.io/cli-runtime/pkg/resource/builder.go | 2 +- staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go index 36a9b3f2f26..47ec83bbbad 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go @@ -803,7 +803,7 @@ func (b *Builder) mappingFor(resourceOrKindArg string) (*meta.RESTMapping, error // if the error is _not_ a *meta.NoKindMatchError, then we had trouble doing discovery, // so we should return the original error since it may help a user diagnose what is actually wrong if meta.IsNoMatchError(err) { - return nil, fmt.Errorf("the server doesn't have a resource type %q: %w", groupResource.Resource, err) + return nil, fmt.Errorf("the server doesn't have a resource type %q", groupResource.Resource) } return nil, err } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go index 460bfee4551..3ec23461ece 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go @@ -1053,9 +1053,6 @@ func TestRestMappingErrors(t *testing.T) { if !strings.Contains(err.Error(), "server doesn't have a resource type \"foo\"") { t.Fatalf("unexpected error: %v", err) } - if !errors.Is(err, &meta.NoKindMatchError{}) { - t.Fatalf("unexpected error type: %v", err) - } } expectedErr := fmt.Errorf("expected error")