Skip to content

Commit 2bd6d29

Browse files
ammarioammario
authored
feat: convert entire CLI to clibase (#6491)
I'm sorry.
1 parent b71b8da commit 2bd6d29

File tree

345 files changed

+9977
-9094
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

345 files changed

+9977
-9094
lines changed

‎.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ jobs:
302302
echo "cover=false" >> $GITHUB_OUTPUT
303303
fi
304304
305-
gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=5m -short -failfast $COVERAGE_FLAGS
305+
gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=7m -short -failfast $COVERAGE_FLAGS
306306
307307
- uses: actions/upload-artifact@v3
308308
if: success() || failure()

‎Makefile

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,8 +501,6 @@ docs/admin/prometheus.md: scripts/metricsdocgen/main.go scripts/metricsdocgen/me
501501
yarn run format:write:only ../docs/admin/prometheus.md
502502

503503
docs/cli.md: scripts/clidocgen/main.go $(GO_SRC_FILES) docs/manifest.json
504-
# TODO(@ammario): re-enable server.md once we finish clibase migration.
505-
ls ./docs/cli/*.md | grep -vP "\/coder_server" | xargs rm
506504
BASE_PATH="." go run ./scripts/clidocgen
507505
cd site
508506
yarn run format:write:only ../docs/cli.md ../docs/cli/*.md ../docs/manifest.json
@@ -519,7 +517,7 @@ coderd/apidoc/swagger.json: $(shell find ./scripts/apidocgen $(FIND_EXCLUSIONS)
519517
update-golden-files: cli/testdata/.gen-golden helm/tests/testdata/.gen-golden
520518
.PHONY: update-golden-files
521519

522-
cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(GO_SRC_FILES)
520+
cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES)
523521
go test ./cli -run=TestCommandHelp -update
524522
touch "$@"
525523

‎cli/agent.go

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"time"
1717

1818
"cloud.google.com/go/compute/metadata"
19-
"github.com/spf13/cobra"
2019
"golang.org/x/xerrors"
2120
"gopkg.in/natefinch/lumberjack.v2"
2221

@@ -25,34 +24,27 @@ import (
2524
"github.com/coder/coder/agent"
2625
"github.com/coder/coder/agent/reaper"
2726
"github.com/coder/coder/buildinfo"
28-
"github.com/coder/coder/cli/cliflag"
27+
"github.com/coder/coder/cli/clibase"
2928
"github.com/coder/coder/codersdk/agentsdk"
3029
)
3130

32-
func workspaceAgent() *cobra.Command {
31+
func (r *RootCmd) workspaceAgent() *clibase.Cmd {
3332
var (
3433
auth string
3534
logDir string
3635
pprofAddress string
3736
noReap bool
3837
sshMaxTimeout time.Duration
3938
)
40-
cmd := &cobra.Command{
41-
Use: "agent",
39+
cmd := &clibase.Cmd{
40+
Use: "agent",
41+
Short: `Starts the Coder workspace agent.`,
4242
// This command isn't useful to manually execute.
4343
Hidden: true,
44-
RunE: func(cmd *cobra.Command, _ []string) error {
45-
ctx, cancel := context.WithCancel(cmd.Context())
44+
Handler: func(inv *clibase.Invocation) error {
45+
ctx, cancel := context.WithCancel(inv.Context())
4646
defer cancel()
4747

48-
rawURL, err := cmd.Flags().GetString(varAgentURL)
49-
if err != nil {
50-
return xerrors.Errorf("CODER_AGENT_URL must be set: %w", err)
51-
}
52-
coderURL, err := url.Parse(rawURL)
53-
if err != nil {
54-
return xerrors.Errorf("parse %q: %w", rawURL, err)
55-
}
5648
agentPorts := map[int]string{}
5749

5850
isLinux := runtime.GOOS == "linux"
@@ -65,7 +57,7 @@ func workspaceAgent() *cobra.Command {
6557
MaxSize: 5, // MB
6658
}
6759
defer logWriter.Close()
68-
logger := slog.Make(sloghuman.Sink(cmd.ErrOrStderr()), sloghuman.Sink(logWriter)).Leveled(slog.LevelDebug)
60+
logger := slog.Make(sloghuman.Sink(inv.Stderr), sloghuman.Sink(logWriter)).Leveled(slog.LevelDebug)
6961

7062
logger.Info(ctx, "spawning reaper process")
7163
// Do not start a reaper on the child process. It's important
@@ -107,15 +99,15 @@ func workspaceAgent() *cobra.Command {
10799
logWriter := &closeWriter{w: ljLogger}
108100
defer logWriter.Close()
109101

110-
logger := slog.Make(sloghuman.Sink(cmd.ErrOrStderr()), sloghuman.Sink(logWriter)).Leveled(slog.LevelDebug)
102+
logger := slog.Make(sloghuman.Sink(inv.Stderr), sloghuman.Sink(logWriter)).Leveled(slog.LevelDebug)
111103

112104
version := buildinfo.Version()
113105
logger.Info(ctx, "starting agent",
114-
slog.F("url", coderURL),
106+
slog.F("url", r.agentURL),
115107
slog.F("auth", auth),
116108
slog.F("version", version),
117109
)
118-
client := agentsdk.New(coderURL)
110+
client := agentsdk.New(r.agentURL)
119111
client.SDK.Logger = logger
120112
// Set a reasonable timeout so requests can't hang forever!
121113
// The timeout needs to be reasonably long, because requests
@@ -139,7 +131,7 @@ func workspaceAgent() *cobra.Command {
139131
var exchangeToken func(context.Context) (agentsdk.AuthenticateResponse, error)
140132
switch auth {
141133
case "token":
142-
token, err := cmd.Flags().GetString(varAgentToken)
134+
token, err := inv.ParsedFlags().GetString(varAgentToken)
143135
if err != nil {
144136
return xerrors.Errorf("CODER_AGENT_TOKEN must be set for token auth: %w", err)
145137
}
@@ -220,11 +212,44 @@ func workspaceAgent() *cobra.Command {
220212
},
221213
}
222214

223-
cliflag.StringVarP(cmd.Flags(), &auth, "auth", "", "CODER_AGENT_AUTH", "token", "Specify the authentication type to use for the agent")
224-
cliflag.StringVarP(cmd.Flags(), &logDir, "log-dir", "", "CODER_AGENT_LOG_DIR", os.TempDir(), "Specify the location for the agent log files")
225-
cliflag.StringVarP(cmd.Flags(), &pprofAddress, "pprof-address", "", "CODER_AGENT_PPROF_ADDRESS", "127.0.0.1:6060", "The address to serve pprof.")
226-
cliflag.BoolVarP(cmd.Flags(), &noReap, "no-reap", "", "", false, "Do not start a process reaper.")
227-
cliflag.DurationVarP(cmd.Flags(), &sshMaxTimeout, "ssh-max-timeout", "", "CODER_AGENT_SSH_MAX_TIMEOUT", time.Duration(0), "Specify the max timeout for a SSH connection")
215+
cmd.Options = clibase.OptionSet{
216+
{
217+
Flag: "auth",
218+
Default: "token",
219+
Description: "Specify the authentication type to use for the agent.",
220+
Env: "CODER_AGENT_AUTH",
221+
Value: clibase.StringOf(&auth),
222+
},
223+
{
224+
Flag: "log-dir",
225+
Default: os.TempDir(),
226+
Description: "Specify the location for the agent log files.",
227+
Env: "CODER_AGENT_LOG_DIR",
228+
Value: clibase.StringOf(&logDir),
229+
},
230+
{
231+
Flag: "pprof-address",
232+
Default: "127.0.0.1:6060",
233+
Env: "CODER_AGENT_PPROF_ADDRESS",
234+
Value: clibase.StringOf(&pprofAddress),
235+
Description: "The address to serve pprof.",
236+
},
237+
{
238+
Flag: "no-reap",
239+
240+
Env: "",
241+
Description: "Do not start a process reaper.",
242+
Value: clibase.BoolOf(&noReap),
243+
},
244+
{
245+
Flag: "ssh-max-timeout",
246+
Default: "0",
247+
Env: "CODER_AGENT_SSH_MAX_TIMEOUT",
248+
Description: "Specify the max timeout for a SSH connection.",
249+
Value: clibase.DurationOf(&sshMaxTimeout),
250+
},
251+
}
252+
228253
return cmd
229254
}
230255

‎cli/agent_test.go

Lines changed: 35 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"github.com/coder/coder/coderd/coderdtest"
1717
"github.com/coder/coder/provisioner/echo"
1818
"github.com/coder/coder/provisionersdk/proto"
19-
"github.com/coder/coder/testutil"
19+
"github.com/coder/coder/pty/ptytest"
2020
)
2121

2222
func TestWorkspaceAgent(t *testing.T) {
@@ -40,24 +40,20 @@ func TestWorkspaceAgent(t *testing.T) {
4040
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
4141

4242
logDir := t.TempDir()
43-
cmd, _ := clitest.New(t,
43+
inv, _ := clitest.New(t,
4444
"agent",
4545
"--auth", "token",
4646
"--agent-token", authToken,
4747
"--agent-url", client.URL.String(),
4848
"--log-dir", logDir,
4949
)
50-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
51-
defer cancel()
52-
errC := make(chan error, 1)
53-
go func() {
54-
errC <- cmd.ExecuteContext(ctx)
55-
}()
56-
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
5750

58-
cancel()
59-
err := <-errC
60-
require.NoError(t, err)
51+
pty := ptytest.New(t).Attach(inv)
52+
53+
clitest.Start(t, inv)
54+
pty.ExpectMatch("starting agent")
55+
56+
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
6157

6258
info, err := os.Stat(filepath.Join(logDir, "coder-agent.log"))
6359
require.NoError(t, err)
@@ -96,16 +92,14 @@ func TestWorkspaceAgent(t *testing.T) {
9692
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
9793
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
9894

99-
cmd, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String())
95+
inv, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String())
96+
inv = inv.WithContext(
97+
//nolint:revive,staticcheck
98+
context.WithValue(inv.Context(), "azure-client", metadataClient),
99+
)
100100
ctx, cancelFunc := context.WithCancel(context.Background())
101101
defer cancelFunc()
102-
errC := make(chan error)
103-
go func() {
104-
// A linting error occurs for weakly typing the context value here.
105-
//nolint // The above seems reasonable for a one-off test.
106-
ctx := context.WithValue(ctx, "azure-client", metadataClient)
107-
errC <- cmd.ExecuteContext(ctx)
108-
}()
102+
clitest.Start(t, inv)
109103
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
110104
workspace, err := client.Workspace(ctx, workspace.ID)
111105
require.NoError(t, err)
@@ -117,9 +111,6 @@ func TestWorkspaceAgent(t *testing.T) {
117111
require.NoError(t, err)
118112
defer dialer.Close()
119113
require.True(t, dialer.AwaitReachable(context.Background()))
120-
cancelFunc()
121-
err = <-errC
122-
require.NoError(t, err)
123114
})
124115

125116
t.Run("AWS", func(t *testing.T) {
@@ -154,36 +145,29 @@ func TestWorkspaceAgent(t *testing.T) {
154145
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
155146
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
156147

157-
cmd, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String())
158-
ctx, cancelFunc := context.WithCancel(context.Background())
159-
defer cancelFunc()
160-
errC := make(chan error)
161-
go func() {
162-
// A linting error occurs for weakly typing the context value here.
163-
//nolint // The above seems reasonable for a one-off test.
164-
ctx := context.WithValue(ctx, "aws-client", metadataClient)
165-
errC <- cmd.ExecuteContext(ctx)
166-
}()
148+
inv, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String())
149+
inv = inv.WithContext(
150+
//nolint:revive,staticcheck
151+
context.WithValue(inv.Context(), "aws-client", metadataClient),
152+
)
153+
clitest.Start(t, inv)
167154
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
168-
workspace, err := client.Workspace(ctx, workspace.ID)
155+
workspace, err := client.Workspace(inv.Context(), workspace.ID)
169156
require.NoError(t, err)
170157
resources := workspace.LatestBuild.Resources
171158
if assert.NotEmpty(t, resources) && assert.NotEmpty(t, resources[0].Agents) {
172159
assert.NotEmpty(t, resources[0].Agents[0].Version)
173160
}
174-
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
161+
dialer, err := client.DialWorkspaceAgent(inv.Context(), resources[0].Agents[0].ID, nil)
175162
require.NoError(t, err)
176163
defer dialer.Close()
177164
require.True(t, dialer.AwaitReachable(context.Background()))
178-
cancelFunc()
179-
err = <-errC
180-
require.NoError(t, err)
181165
})
182166

183167
t.Run("GoogleCloud", func(t *testing.T) {
184168
t.Parallel()
185169
instanceID := "instanceidentifier"
186-
validator, metadata := coderdtest.NewGoogleInstanceIdentity(t, instanceID, false)
170+
validator, metadataClient := coderdtest.NewGoogleInstanceIdentity(t, instanceID, false)
187171
client := coderdtest.New(t, &coderdtest.Options{
188172
GoogleTokenValidator: validator,
189173
IncludeProvisionerDaemon: true,
@@ -212,16 +196,18 @@ func TestWorkspaceAgent(t *testing.T) {
212196
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
213197
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
214198

215-
cmd, _ := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String())
216-
ctx, cancelFunc := context.WithCancel(context.Background())
217-
defer cancelFunc()
218-
errC := make(chan error)
219-
go func() {
220-
// A linting error occurs for weakly typing the context value here.
221-
//nolint // The above seems reasonable for a one-off test.
222-
ctx := context.WithValue(ctx, "gcp-client", metadata)
223-
errC <- cmd.ExecuteContext(ctx)
224-
}()
199+
inv, cfg := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String())
200+
ptytest.New(t).Attach(inv)
201+
clitest.SetupConfig(t, client, cfg)
202+
clitest.Start(t,
203+
inv.WithContext(
204+
//nolint:revive,staticcheck
205+
context.WithValue(context.Background(), "gcp-client", metadataClient),
206+
),
207+
)
208+
209+
ctx := inv.Context()
210+
225211
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
226212
workspace, err := client.Workspace(ctx, workspace.ID)
227213
require.NoError(t, err)
@@ -248,9 +234,5 @@ func TestWorkspaceAgent(t *testing.T) {
248234
require.NoError(t, err)
249235
_, err = uuid.Parse(strings.TrimSpace(string(token)))
250236
require.NoError(t, err)
251-
252-
cancelFunc()
253-
err = <-errC
254-
require.NoError(t, err)
255237
})
256238
}

‎cli/clibase/clibase.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
// Package clibase offers an all-in-one solution for a highly configurable CLI
2-
// application. Within Coder, we use it for our `server` subcommand, which
3-
// demands more functionality than cobra/viper can offer.
4-
//
5-
// We will extend its usage to the rest of our application, completely replacing
6-
// cobra/viper. It's also a candidate to be broken out into its own open-source
7-
// library, so we avoid deep coupling with Coder concepts.
2+
// application. Within Coder, we use it for all of our subcommands, which
3+
// demands more functionality than cobra/viber offers.
84
//
95
// The Command interface is loosely based on the chi middleware pattern and
106
// http.Handler/HandlerFunc.

0 commit comments

Comments
 (0)