Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ cli: write diagnostics in kubectl-workspace to stderr #3133

Merged
merged 3 commits into from
May 17, 2024
Merged

✨ cli: write diagnostics in kubectl-workspace to stderr #3133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b6c5c0
kubectl-workspace: write notes and warnings to stderr
embik May 16, 2024
94aa1ea
remove dedicated OWNERS file for cli
embik May 16, 2024
2c4396c
Adjust kubectl-workspace tests for diagnostic information output on s…
embik May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions cli/pkg/OWNERS
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions cli/pkg/workspace/plugin/use.go
View file
Open in desktop
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
Open in desktop
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
Loading