Skip to content

Commit

Permalink
Merge pull request #3133 from embik/kubectl-workspace-stderr
Browse files Browse the repository at this point in the history 
✨ cli: write diagnostics in `kubectl-workspace` to stderr
  • Loading branch information
@kcp-ci-bot
kcp-ci-bot committed May 17, 2024
2 parents b8fbcff + 2c4396c commit d8daae8
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 28 deletions.
6 changes: 0 additions & 6 deletions cli/pkg/OWNERS
View file

This file was deleted.

8 changes: 4 additions & 4 deletions cli/pkg/workspace/plugin/use.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "~"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
56 changes: 38 additions & 18 deletions cli/pkg/workspace/plugin/use_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func TestUse(t *testing.T) {

expected *clientcmdapi.Config
wantStdout []string
wantStderr []string
wantErrors []string
wantErr bool
noWarn bool
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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())},
},
{
Expand All @@ -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'"},
},
{
Expand Down Expand Up @@ -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'"},
},
{
Expand All @@ -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'"},
},
{
Expand All @@ -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'"},
},
{
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit d8daae8

Please sign in to comment.