Skip to content

Commit a36167d

Browse files
kylecarbskylecarbs
committed
feat: Add GIT_COMMITTER information to agent env vars
This makes setting up git a bit simpler, and users can always override these values! We'll probably add a way to disable our Git integration anyways, so these could be part of that.
1 parent 947e8f9 commit a36167d

File tree

6 files changed

+89
-49
lines changed

6 files changed

+89
-49
lines changed

‎agent/agent.go

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ import (
3333
"golang.org/x/xerrors"
3434
)
3535

36-
type Options struct {
37-
EnvironmentVariables map[string]string
38-
StartupScript string
36+
type Metadata struct {
37+
OwnerEmail string `json:"owner_email"`
38+
OwnerUsername string `json:"owner_username"`
39+
EnvironmentVariables map[string]string `json:"environment_variables"`
40+
StartupScript string `json:"startup_script"`
3941
}
4042

41-
type Dialer func(ctx context.Context, logger slog.Logger) (*Options, *peerbroker.Listener, error)
43+
type Dialer func(ctx context.Context, logger slog.Logger) (Metadata, *peerbroker.Listener, error)
4244

4345
func New(dialer Dialer, logger slog.Logger) io.Closer {
4446
ctx, cancelFunc := context.WithCancel(context.Background())
@@ -62,14 +64,16 @@ type agent struct {
6264
closed chan struct{}
6365

6466
// Environment variables sent by Coder to inject for shell sessions.
65-
// This is atomic because values can change after reconnect.
67+
// These are atomic because values can change after reconnect.
6668
envVars atomic.Value
69+
ownerEmail atomic.String
70+
ownerUsername atomic.String
6771
startupScript atomic.Bool
6872
sshServer *ssh.Server
6973
}
7074

7175
func (a *agent) run(ctx context.Context) {
72-
var options *Options
76+
var options Metadata
7377
var peerListener *peerbroker.Listener
7478
var err error
7579
// An exponential back-off occurs when the connection is failing to dial.
@@ -95,6 +99,8 @@ func (a *agent) run(ctx context.Context) {
9599
default:
96100
}
97101
a.envVars.Store(options.EnvironmentVariables)
102+
a.ownerEmail.Store(options.OwnerEmail)
103+
a.ownerUsername.Store(options.OwnerUsername)
98104

99105
if a.startupScript.CAS(false, true) {
100106
// The startup script has not ran yet!
@@ -303,8 +309,20 @@ func (a *agent) handleSSHSession(session ssh.Session) error {
303309
}
304310
cmd := exec.CommandContext(session.Context(), shell, caller, command)
305311
cmd.Env = append(os.Environ(), session.Environ()...)
312+
executablePath, err := os.Executable()
313+
if err != nil {
314+
return xerrors.Errorf("getting os executable: %w", err)
315+
}
316+
// Git on Windows resolves with UNIX-style paths.
317+
// If using backslashes, it's unable to find the executable.
318+
executablePath = strings.ReplaceAll(executablePath, "\\", "/")
319+
cmd.Env = append(cmd.Env, fmt.Sprintf(`GIT_SSH_COMMAND=%s gitssh --`, executablePath))
320+
// These prevent the user from having to specify _anything_ to successfully commit.
321+
cmd.Env = append(cmd.Env, fmt.Sprintf(`GIT_COMMITTER_EMAIL=%s`, a.ownerEmail.Load()))
322+
cmd.Env = append(cmd.Env, fmt.Sprintf(`GIT_COMMITTER_NAME=%s`, a.ownerUsername.Load()))
306323

307324
// Load environment variables passed via the agent.
325+
// These should override all variables we manually specify.
308326
envVars := a.envVars.Load()
309327
if envVars != nil {
310328
envVarMap, ok := envVars.(map[string]string)
@@ -315,15 +333,6 @@ func (a *agent) handleSSHSession(session ssh.Session) error {
315333
}
316334
}
317335

318-
executablePath, err := os.Executable()
319-
if err != nil {
320-
return xerrors.Errorf("getting os executable: %w", err)
321-
}
322-
// Git on Windows resolves with UNIX-style paths.
323-
// If using backslashes, it's unable to find the executable.
324-
executablePath = strings.ReplaceAll(executablePath, "\\", "/")
325-
cmd.Env = append(cmd.Env, fmt.Sprintf(`GIT_SSH_COMMAND=%s gitssh --`, executablePath))
326-
327336
sshPty, windowSize, isPty := session.Pty()
328337
if isPty {
329338
cmd.Env = append(cmd.Env, fmt.Sprintf("TERM=%s", sshPty.Term))

‎agent/agent_test.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestAgent(t *testing.T) {
4040
t.Parallel()
4141
t.Run("SessionExec", func(t *testing.T) {
4242
t.Parallel()
43-
session := setupSSHSession(t, nil)
43+
session := setupSSHSession(t, agent.Metadata{})
4444

4545
command := "echo test"
4646
if runtime.GOOS == "windows" {
@@ -53,7 +53,7 @@ func TestAgent(t *testing.T) {
5353

5454
t.Run("GitSSH", func(t *testing.T) {
5555
t.Parallel()
56-
session := setupSSHSession(t, nil)
56+
session := setupSSHSession(t, agent.Metadata{})
5757
command := "sh -c 'echo $GIT_SSH_COMMAND'"
5858
if runtime.GOOS == "windows" {
5959
command = "cmd.exe /c echo %GIT_SSH_COMMAND%"
@@ -71,7 +71,7 @@ func TestAgent(t *testing.T) {
7171
// it seems like it could be either.
7272
t.Skip("ConPTY appears to be inconsistent on Windows.")
7373
}
74-
session := setupSSHSession(t, nil)
74+
session := setupSSHSession(t, agent.Metadata{})
7575
command := "bash"
7676
if runtime.GOOS == "windows" {
7777
command = "cmd.exe"
@@ -131,7 +131,7 @@ func TestAgent(t *testing.T) {
131131

132132
t.Run("SFTP", func(t *testing.T) {
133133
t.Parallel()
134-
sshClient, err := setupAgent(t, nil).SSHClient()
134+
sshClient, err := setupAgent(t, agent.Metadata{}).SSHClient()
135135
require.NoError(t, err)
136136
client, err := sftp.NewClient(sshClient)
137137
require.NoError(t, err)
@@ -148,7 +148,7 @@ func TestAgent(t *testing.T) {
148148
t.Parallel()
149149
key := "EXAMPLE"
150150
value := "value"
151-
session := setupSSHSession(t, &agent.Options{
151+
session := setupSSHSession(t, agent.Metadata{
152152
EnvironmentVariables: map[string]string{
153153
key: value,
154154
},
@@ -166,7 +166,7 @@ func TestAgent(t *testing.T) {
166166
t.Parallel()
167167
tempPath := filepath.Join(os.TempDir(), "content.txt")
168168
content := "somethingnice"
169-
setupAgent(t, &agent.Options{
169+
setupAgent(t, agent.Metadata{
170170
StartupScript: "echo " + content + " > " + tempPath,
171171
})
172172
var gotContent string
@@ -191,7 +191,7 @@ func TestAgent(t *testing.T) {
191191
}
192192

193193
func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exec.Cmd {
194-
agentConn := setupAgent(t, nil)
194+
agentConn := setupAgent(t, agent.Metadata{})
195195
listener, err := net.Listen("tcp", "127.0.0.1:0")
196196
require.NoError(t, err)
197197
go func() {
@@ -219,20 +219,17 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exe
219219
return exec.Command("ssh", args...)
220220
}
221221

222-
func setupSSHSession(t *testing.T, options *agent.Options) *ssh.Session {
222+
func setupSSHSession(t *testing.T, options agent.Metadata) *ssh.Session {
223223
sshClient, err := setupAgent(t, options).SSHClient()
224224
require.NoError(t, err)
225225
session, err := sshClient.NewSession()
226226
require.NoError(t, err)
227227
return session
228228
}
229229

230-
func setupAgent(t *testing.T, options *agent.Options) *agent.Conn {
231-
if options == nil {
232-
options = &agent.Options{}
233-
}
230+
func setupAgent(t *testing.T, options agent.Metadata) *agent.Conn {
234231
client, server := provisionersdk.TransportPipe()
235-
closer := agent.New(func(ctx context.Context, logger slog.Logger) (*agent.Options, *peerbroker.Listener, error) {
232+
closer := agent.New(func(ctx context.Context, logger slog.Logger) (agent.Metadata, *peerbroker.Listener, error) {
236233
listener, err := peerbroker.Listen(server, nil)
237234
return options, listener, err
238235
}, slogtest.Make(t, nil).Leveled(slog.LevelDebug))

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func New(options *Options) (http.Handler, func()) {
201201
r.Post("/google-instance-identity", api.postWorkspaceAuthGoogleInstanceIdentity)
202202
r.Route("/me", func(r chi.Router) {
203203
r.Use(httpmw.ExtractWorkspaceAgent(options.Database))
204-
r.Get("/", api.workspaceAgentMe)
204+
r.Get("/metadata", api.workspaceAgentMetadata)
205205
r.Get("/listen", api.workspaceAgentListen)
206206
r.Get("/gitsshkey", api.agentGitSSHKey)
207207
r.Get("/turn", api.workspaceAgentTurn)

‎coderd/workspaceagents.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"nhooyr.io/websocket"
1616

1717
"cdr.dev/slog"
18+
"github.com/coder/coder/agent"
1819
"github.com/coder/coder/coderd/database"
1920
"github.com/coder/coder/coderd/httpapi"
2021
"github.com/coder/coder/coderd/httpmw"
@@ -88,16 +89,49 @@ func (api *api) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) {
8889
}
8990
}
9091

91-
func (api *api) workspaceAgentMe(rw http.ResponseWriter, r *http.Request) {
92-
agent := httpmw.WorkspaceAgent(r)
93-
apiAgent, err := convertWorkspaceAgent(agent, api.AgentConnectionUpdateFrequency)
92+
func (api *api) workspaceAgentMetadata(rw http.ResponseWriter, r *http.Request) {
93+
workspaceAgent := httpmw.WorkspaceAgent(r)
94+
apiAgent, err := convertWorkspaceAgent(workspaceAgent, api.AgentConnectionUpdateFrequency)
9495
if err != nil {
9596
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
9697
Message: fmt.Sprintf("convert workspace agent: %s", err),
9798
})
9899
return
99100
}
100-
httpapi.Write(rw, http.StatusOK, apiAgent)
101+
resource, err := api.Database.GetWorkspaceResourceByID(r.Context(), workspaceAgent.ResourceID)
102+
if err != nil {
103+
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
104+
Message: fmt.Sprintf("get workspace resource: %s", err),
105+
})
106+
return
107+
}
108+
build, err := api.Database.GetWorkspaceBuildByJobID(r.Context(), resource.JobID)
109+
if err != nil {
110+
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
111+
Message: fmt.Sprintf("get workspace build: %s", err),
112+
})
113+
return
114+
}
115+
workspace, err := api.Database.GetWorkspaceByID(r.Context(), build.WorkspaceID)
116+
if err != nil {
117+
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
118+
Message: fmt.Sprintf("get workspace build: %s", err),
119+
})
120+
return
121+
}
122+
owner, err := api.Database.GetUserByID(r.Context(), workspace.OwnerID)
123+
if err != nil {
124+
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
125+
Message: fmt.Sprintf("get workspace build: %s", err),
126+
})
127+
return
128+
}
129+
httpapi.Write(rw, http.StatusOK, agent.Metadata{
130+
OwnerEmail: owner.Email,
131+
OwnerUsername: owner.Username,
132+
EnvironmentVariables: apiAgent.EnvironmentVariables,
133+
StartupScript: apiAgent.StartupScript,
134+
})
101135
}
102136

103137
func (api *api) workspaceAgentListen(rw http.ResponseWriter, r *http.Request) {

‎coderd/workspaceagents_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,6 @@ func TestWorkspaceAgentListen(t *testing.T) {
102102
})
103103
_, err = conn.Ping()
104104
require.NoError(t, err)
105-
_, err = agentClient.WorkspaceAgent(context.Background(), codersdk.Me)
106-
require.NoError(t, err)
107105
}
108106

109107
func TestWorkspaceAgentTURN(t *testing.T) {

‎codersdk/workspaceagents.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,14 @@ func (c *Client) AuthWorkspaceAzureInstanceIdentity(ctx context.Context) (Worksp
178178

179179
// ListenWorkspaceAgent connects as a workspace agent identifying with the session token.
180180
// On each inbound connection request, connection info is fetched.
181-
func (c *Client) ListenWorkspaceAgent(ctx context.Context, logger slog.Logger) (*agent.Options, *peerbroker.Listener, error) {
181+
func (c *Client) ListenWorkspaceAgent(ctx context.Context, logger slog.Logger) (agent.Metadata, *peerbroker.Listener, error) {
182182
serverURL, err := c.URL.Parse("/api/v2/workspaceagents/me/listen")
183183
if err != nil {
184-
return nil, nil, xerrors.Errorf("parse url: %w", err)
184+
return agent.Metadata{}, nil, xerrors.Errorf("parse url: %w", err)
185185
}
186186
jar, err := cookiejar.New(nil)
187187
if err != nil {
188-
return nil, nil, xerrors.Errorf("create cookie jar: %w", err)
188+
return agent.Metadata{}, nil, xerrors.Errorf("create cookie jar: %w", err)
189189
}
190190
jar.SetCookies(serverURL, []*http.Cookie{{
191191
Name: httpmw.AuthCookie,
@@ -201,15 +201,15 @@ func (c *Client) ListenWorkspaceAgent(ctx context.Context, logger slog.Logger) (
201201
})
202202
if err != nil {
203203
if res == nil {
204-
return nil, nil, err
204+
return agent.Metadata{}, nil, err
205205
}
206-
return nil, nil, readBodyAsError(res)
206+
return agent.Metadata{}, nil, readBodyAsError(res)
207207
}
208208
config := yamux.DefaultConfig()
209209
config.LogOutput = io.Discard
210210
session, err := yamux.Client(websocket.NetConn(ctx, conn, websocket.MessageBinary), config)
211211
if err != nil {
212-
return nil, nil, xerrors.Errorf("multiplex client: %w", err)
212+
return agent.Metadata{}, nil, xerrors.Errorf("multiplex client: %w", err)
213213
}
214214
listener, err := peerbroker.Listen(session, func(ctx context.Context) ([]webrtc.ICEServer, *peer.ConnOptions, error) {
215215
// This can be cached if it adds to latency too much.
@@ -238,16 +238,18 @@ func (c *Client) ListenWorkspaceAgent(ctx context.Context, logger slog.Logger) (
238238
}, nil
239239
})
240240
if err != nil {
241-
return nil, nil, xerrors.Errorf("listen peerbroker: %w", err)
241+
return agent.Metadata{}, nil, xerrors.Errorf("listen peerbroker: %w", err)
242242
}
243-
workspaceAgent, err := c.WorkspaceAgent(ctx, Me)
243+
res, err = c.request(ctx, http.MethodGet, "/api/v2/workspaceagents/me/metadata", nil)
244244
if err != nil {
245-
return nil, nil, xerrors.Errorf("get workspace agent: %w", err)
245+
return agent.Metadata{}, nil, err
246+
}
247+
defer res.Body.Close()
248+
if res.StatusCode != http.StatusOK {
249+
return agent.Metadata{}, nil, readBodyAsError(res)
246250
}
247-
return &agent.Options{
248-
EnvironmentVariables: workspaceAgent.EnvironmentVariables,
249-
StartupScript: workspaceAgent.StartupScript,
250-
}, listener, err
251+
var metadata agent.Metadata
252+
return metadata, listener, json.NewDecoder(res.Body).Decode(&metadata)
251253
}
252254

253255
// DialWorkspaceAgent creates a connection to the specified resource.
@@ -324,7 +326,7 @@ func (c *Client) DialWorkspaceAgent(ctx context.Context, agentID uuid.UUID, opti
324326

325327
// WorkspaceAgent returns an agent by ID.
326328
func (c *Client) WorkspaceAgent(ctx context.Context, id uuid.UUID) (WorkspaceAgent, error) {
327-
res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s", uuidOrMe(id)), nil)
329+
res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s", id), nil)
328330
if err != nil {
329331
return WorkspaceAgent{}, err
330332
}

0 commit comments

Comments
 (0)