From 6b6c5c075e11401566137871bf5bee3b3bd0ce4b Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Thu, 16 May 2024 15:50:46 +0200 Subject: [PATCH 1/3] kubectl-workspace: write notes and warnings to stderr Signed-off-by: Marvin Beckers --- cli/pkg/workspace/plugin/use.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/pkg/workspace/plugin/use.go b/cli/pkg/workspace/plugin/use.go index 34d48876fa8..797ef32de50 100644 --- a/cli/pkg/workspace/plugin/use.go +++ b/cli/pkg/workspace/plugin/use.go @@ -128,12 +128,12 @@ func (o *UseWorkspaceOptions) Run(ctx context.Context) (err error) { if name == core.RootCluster.String() || strings.HasPrefix(name, core.RootCluster.String()+":") { // LEGACY(mjudeikis): Remove once everybody gets used to this name = ":" + name - fmt.Fprintf(o.Out, "Note: Using 'root:' to define an absolute path is no longer supported. Instead, use ':root' to specify an absolute path.\n") + fmt.Fprintf(o.ErrOut, "Note: Using 'root:' to define an absolute path is no longer supported. Instead, use ':root' to specify an absolute path.\n") } if name == "" { defer func() { if err == nil { - _, err = fmt.Fprintf(o.Out, "Note: 'kubectl ws' now matches 'cd' semantics: go to home workspace. 'kubectl ws -' to go back. 'kubectl ws .' to print current workspace.\n") + _, err = fmt.Fprintf(o.ErrOut, "Note: 'kubectl ws' now matches 'cd' semantics: go to home workspace. 'kubectl ws -' to go back. 'kubectl ws .' to print current workspace.\n") } }() name = "~" @@ -305,7 +305,7 @@ func (o *UseWorkspaceOptions) swapContexts(ctx context.Context, currentContext * // display the error, but don't stop the current workspace from being reported. fmt.Fprintf(o.ErrOut, "error checking APIBindings: %v\n", err) } - if err = findUnresolvedPermissionClaims(o.Out, bindings); err != nil { + if err = findUnresolvedPermissionClaims(o.ErrOut, bindings); err != nil { // display the error, but don't stop the current workspace from being reported. fmt.Fprintf(o.ErrOut, "error checking APIBindings: %v\n", err) } @@ -347,7 +347,7 @@ func (o *UseWorkspaceOptions) commitConfig(ctx context.Context, currentContext * // display the error, but don't stop the current workspace from being reported. fmt.Fprintf(o.ErrOut, "error checking APIBindings: %v\n", err) } - if err := findUnresolvedPermissionClaims(o.Out, bindings); err != nil { + if err := findUnresolvedPermissionClaims(o.ErrOut, bindings); err != nil { // display the error, but don't stop the current workspace from being reported. fmt.Fprintf(o.ErrOut, "error checking APIBindings: %v\n", err) } From 94aa1ea11e42dce27f88af084d78408026b6894d Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Thu, 16 May 2024 15:50:59 +0200 Subject: [PATCH 2/3] remove dedicated OWNERS file for cli Signed-off-by: Marvin Beckers --- cli/pkg/OWNERS | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 cli/pkg/OWNERS diff --git a/cli/pkg/OWNERS b/cli/pkg/OWNERS deleted file mode 100644 index 04070c226cb..00000000000 --- a/cli/pkg/OWNERS +++ /dev/null @@ -1,6 +0,0 @@ -approvers: -- ncdc -- sttts -reviewers: -- stevekuznetsov -- davidfestal From 2c4396c468b3bdb8c395eee3706e9339be4a4da1 Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Thu, 16 May 2024 16:15:45 +0200 Subject: [PATCH 3/3] Adjust kubectl-workspace tests for diagnostic information output on stderr Signed-off-by: Marvin Beckers --- cli/pkg/workspace/plugin/use_test.go | 56 +++++++++++++++++++--------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/cli/pkg/workspace/plugin/use_test.go b/cli/pkg/workspace/plugin/use_test.go index 012a8f514e2..ef27495f0f1 100644 --- a/cli/pkg/workspace/plugin/use_test.go +++ b/cli/pkg/workspace/plugin/use_test.go @@ -75,6 +75,7 @@ func TestUse(t *testing.T) { expected *clientcmdapi.Config wantStdout []string + wantStderr []string wantErrors []string wantErr bool noWarn bool @@ -451,7 +452,10 @@ func TestUse(t *testing.T) { param: "", expected: NewKubeconfig().WithKcpCurrent(homeWorkspace.String()).WithKcpPrevious("root:foo").Build(), destination: homeWorkspace.String(), - wantStdout: []string{fmt.Sprintf("Current workspace is '%s'.\nNote: 'kubectl ws' now matches 'cd' semantics: go to home workspace. 'kubectl ws -' to go back. 'kubectl ws .' to print current workspace.", homeWorkspace.String())}, + wantStderr: []string{ + "Note: 'kubectl ws' now matches 'cd' semantics: go to home workspace. 'kubectl ws -' to go back. 'kubectl ws .' to print current workspace.", + }, + wantStdout: []string{fmt.Sprintf("Current workspace is '%s'.", homeWorkspace.String())}, }, { name: "workspace name, apibindings have matching permission and export claims", @@ -488,9 +492,10 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, - wantStdout: []string{ + wantStderr: []string{ "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", - "Current workspace is 'root:foo:bar'"}, + }, + wantStdout: []string{"Current workspace is 'root:foo:bar'"}, }, { name: "~, apibinding claims/exports don't match", @@ -509,8 +514,10 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, + wantStderr: []string{ + "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + }, wantStdout: []string{ - "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", fmt.Sprintf("Current workspace is '%s'", homeWorkspace.String())}, }, { @@ -529,8 +536,10 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, + wantStderr: []string{ + "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + }, wantStdout: []string{ - "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", "Current workspace is 'root:foo:bar'"}, }, { @@ -590,8 +599,10 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, + wantStderr: []string{ + "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + }, wantStdout: []string{ - "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", "Current workspace is 'root:foo:bar'"}, }, { @@ -610,8 +621,10 @@ func TestUse(t *testing.T) { WithExportClaim("test.kcp.io", "test", "abcdef"). Build(), }, + wantStderr: []string{ + "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.", + }, wantStdout: []string{ - "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.\n", "Current workspace is 'root:foo:bar'"}, }, { @@ -631,9 +644,11 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, + wantStderr: []string{ + "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.", + }, wantStdout: []string{ - "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", - "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.\n", "Current workspace is 'root:foo:bar'"}, }, { @@ -654,10 +669,11 @@ func TestUse(t *testing.T) { WithExportClaim("test2.kcp.io", "test2", "abcdef"). Build(), }, - wantStdout: []string{ - "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.\n", - "Warning: claim for test2.test2.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.\n", - "Current workspace is 'root:foo:bar'"}, + wantStderr: []string{ + "Warning: claim for test.test.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.", + "Warning: claim for test2.test2.kcp.io:abcdef specified on APIBinding a but not accepted or rejected.", + }, + wantStdout: []string{"Current workspace is 'root:foo:bar'"}, }, { name: "workspace name, multiple APIBindings unspecified", @@ -675,10 +691,11 @@ func TestUse(t *testing.T) { WithExportClaim("", "configmaps", ""). Build(), }, - wantStdout: []string{ - "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", - "Warning: claim for test.test.kcp.io:abcdef exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.\n", - "Current workspace is 'root:foo:bar'"}, + wantStderr: []string{ + "Warning: claim for configmaps exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + "Warning: claim for test.test.kcp.io:abcdef exported but not specified on APIBinding a\nAdd this claim to the APIBinding's Spec.", + }, + wantStdout: []string{"Current workspace is 'root:foo:bar'"}, }, { name: "relative change multiple jumps from non root", @@ -836,7 +853,10 @@ func TestUse(t *testing.T) { require.Contains(t, stdout.String(), s) } if tt.noWarn { - require.NotContains(t, stdout.String(), "Warning") + require.NotContains(t, stderr.String(), "Warning") + } + for _, s := range tt.wantStderr { + require.Contains(t, stderr.String(), s) } if err != nil { for _, s := range tt.wantErrors {