Skip to content

Commit d92608e

Browse files
spikecurtisspikecurtis
committed
code review fixes & e2e fixes
Signed-off-by: Spike Curtis <spike@coder.com>
1 parent e39b8a2 commit d92608e

15 files changed

+288
-250
lines changed

‎cli/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,7 @@ func newProvisionerDaemon(
13061306
err := echo.Serve(ctx, &provisionersdk.ServeOptions{
13071307
Listener: echoServer,
13081308
WorkDirectory: workDir,
1309+
Logger: logger.Named("echo"),
13091310
})
13101311
if err != nil {
13111312
select {

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer {
472472
err := echo.Serve(ctx, &provisionersdk.ServeOptions{
473473
Listener: echoServer,
474474
WorkDirectory: t.TempDir(),
475+
Logger: coderAPI.Logger.Named("echo").Leveled(slog.LevelDebug),
475476
})
476477
assert.NoError(t, err)
477478
}()

‎provisionersdk/proto/provisioner.pb.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎provisionersdk/proto/provisioner.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ service Provisioner {
272272
// request an apply. If the daemon closes the Session without an apply, the plan data may be safely discarded.
273273
//
274274
// The daemon may send a CancelRequest, asynchronously to ask the provisioner to cancel the previous ParseRequest,
275-
// PlanRequest, or ApplyRequest. The provisioner MUST reply with a complete message cooresponding to the request
275+
// PlanRequest, or ApplyRequest. The provisioner MUST reply with a complete message corresponding to the request
276276
// that was canceled. If the provisioner has already completed the request, it may ignore the CancelRequest.
277277
rpc Session(stream Request) returns (stream Response);
278278
}

‎provisionersdk/proto/provisioner_drpc.pb.go

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎provisionersdk/serve_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ func TestProvisionerSDK(t *testing.T) {
3535
}()
3636

3737
api := proto.NewDRPCProvisionerClient(client)
38-
ctx, cancel := context.WithCancel(context.Background())
39-
defer cancel()
40-
s, err := api.Session(context.Background())
38+
s, err := api.Session(ctx)
4139
require.NoError(t, err)
4240
err = s.Send(&proto.Request{Type: &proto.Request_Config{Config: &proto.Config{}}})
4341
require.NoError(t, err)

‎provisionersdk/session.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ func (s *Session) ExtractArchive() error {
214214
if err != nil {
215215
return xerrors.Errorf("mkdir %q: %w", headerPath, err)
216216
}
217-
s.Logger.Debug(context.Background(), "extracted directory", slog.F("path", headerPath))
217+
s.Logger.Debug(context.Background(), "extracted directory",
218+
slog.F("path", headerPath),
219+
slog.F("mode", fmt.Sprintf("%O", mode)))
218220
case tar.TypeReg:
219221
file, err := os.OpenFile(headerPath, os.O_CREATE|os.O_RDWR, mode)
220222
if err != nil {

‎site/e2e/helpers.ts

Lines changed: 106 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ import {
88
Agent,
99
App,
1010
AppSharingLevel,
11-
Parse_Complete,
12-
Parse_Response,
13-
Provision_Complete,
14-
Provision_Response,
11+
Response,
12+
ParseComplete,
13+
PlanComplete,
14+
ApplyComplete,
1515
Resource,
1616
RichParameter,
1717
} from "./provisionerGenerated"
@@ -337,11 +337,11 @@ type RecursivePartial<T> = {
337337

338338
interface EchoProvisionerResponses {
339339
// parse is for observing any Terraform variables
340-
parse?: RecursivePartial<Parse_Response>[]
340+
parse?: RecursivePartial<Response>[]
341341
// plan occurs when the template is imported
342-
plan?: RecursivePartial<Provision_Response>[]
342+
plan?: RecursivePartial<Response>[]
343343
// apply occurs when the workspace is built
344-
apply?: RecursivePartial<Provision_Response>[]
344+
apply?: RecursivePartial<Response>[]
345345
}
346346

347347
// createTemplateVersionTar consumes a series of echo provisioner protobufs and
@@ -353,109 +353,128 @@ const createTemplateVersionTar = async (
353353
responses = {}
354354
}
355355
if (!responses.parse) {
356-
responses.parse = [{}]
356+
responses.parse = [{
357+
parse: {}
358+
}]
357359
}
358360
if (!responses.apply) {
359-
responses.apply = [{}]
361+
responses.apply = [{
362+
apply: {}
363+
}]
360364
}
361365
if (!responses.plan) {
362-
responses.plan = responses.apply
366+
responses.plan = responses.apply.map(
367+
response => {
368+
if (response.log) {
369+
return response
370+
}
371+
return {
372+
plan: {
373+
error: response.apply?.error ?? "",
374+
resources: response.apply?.resources ?? [],
375+
parameters: response.apply?.parameters ?? [],
376+
gitAuthProviders: response.apply?.gitAuthProviders ?? [],
377+
}
378+
}
379+
})
363380
}
364381

365382
const tar = new TarWriter()
366383
responses.parse.forEach((response, index) => {
367-
response.complete = {
384+
response.parse = {
368385
templateVariables: [],
369-
...response.complete,
370-
} as Parse_Complete
386+
error: "",
387+
readme: new Uint8Array(),
388+
...response.parse,
389+
} as ParseComplete
371390
tar.addFile(
372391
`${index}.parse.protobuf`,
373-
Parse_Response.encode(response as Parse_Response).finish(),
392+
Response.encode(response as Response).finish(),
374393
)
375394
})
376395

377-
const fillProvisionResponse = (
378-
response: RecursivePartial<Provision_Response>,
379-
) => {
380-
response.complete = {
381-
error: "",
382-
state: new Uint8Array(),
383-
resources: [],
384-
parameters: [],
385-
gitAuthProviders: [],
386-
plan: new Uint8Array(),
387-
...response.complete,
388-
} as Provision_Complete
389-
response.complete.resources = response.complete.resources?.map(
390-
(resource) => {
391-
if (resource.agents) {
392-
resource.agents = resource.agents?.map((agent) => {
393-
if (agent.apps) {
394-
agent.apps = agent.apps?.map((app) => {
395-
return {
396-
command: "",
397-
displayName: "example",
398-
external: false,
399-
icon: "",
400-
sharingLevel: AppSharingLevel.PUBLIC,
401-
slug: "example",
402-
subdomain: false,
403-
url: "",
404-
...app,
405-
} as App
406-
})
407-
}
396+
const fillResource = (resource: RecursivePartial<Resource>) => {
397+
if (resource.agents) {
398+
resource.agents = resource.agents?.map((agent: RecursivePartial<Agent>) => {
399+
if (agent.apps) {
400+
agent.apps = agent.apps?.map((app: RecursivePartial<App>) => {
408401
return {
409-
apps: [],
410-
architecture: "amd64",
411-
connectionTimeoutSeconds: 300,
412-
directory: "",
413-
env: {},
414-
id: randomUUID(),
415-
metadata: [],
416-
motdFile: "",
417-
name: "dev",
418-
operatingSystem: "linux",
419-
shutdownScript: "",
420-
shutdownScriptTimeoutSeconds: 0,
421-
startupScript: "",
422-
startupScriptBehavior: "",
423-
startupScriptTimeoutSeconds: 300,
424-
troubleshootingUrl: "",
425-
token: randomUUID(),
426-
...agent,
427-
} as Agent
402+
command: "",
403+
displayName: "example",
404+
external: false,
405+
icon: "",
406+
sharingLevel: AppSharingLevel.PUBLIC,
407+
slug: "example",
408+
subdomain: false,
409+
url: "",
410+
...app,
411+
} as App
428412
})
429413
}
430414
return {
431-
agents: [],
432-
dailyCost: 0,
433-
hide: false,
434-
icon: "",
435-
instanceType: "",
415+
apps: [],
416+
architecture: "amd64",
417+
connectionTimeoutSeconds: 300,
418+
directory: "",
419+
env: {},
420+
id: randomUUID(),
436421
metadata: [],
422+
motdFile: "",
437423
name: "dev",
438-
type: "echo",
439-
...resource,
440-
} as Resource
441-
},
442-
)
424+
operatingSystem: "linux",
425+
shutdownScript: "",
426+
shutdownScriptTimeoutSeconds: 0,
427+
startupScript: "",
428+
startupScriptBehavior: "",
429+
startupScriptTimeoutSeconds: 300,
430+
troubleshootingUrl: "",
431+
token: randomUUID(),
432+
...agent,
433+
} as Agent
434+
})
435+
}
436+
return {
437+
agents: [],
438+
dailyCost: 0,
439+
hide: false,
440+
icon: "",
441+
instanceType: "",
442+
metadata: [],
443+
name: "dev",
444+
type: "echo",
445+
...resource,
446+
} as Resource
443447
}
444448

445449
responses.apply.forEach((response, index) => {
446-
fillProvisionResponse(response)
450+
response.apply = {
451+
error: "",
452+
state: new Uint8Array(),
453+
resources: [],
454+
parameters: [],
455+
gitAuthProviders: [],
456+
...response.apply,
457+
} as ApplyComplete
458+
response.apply.resources = response.apply.resources?.map(fillResource)
447459

448460
tar.addFile(
449-
`${index}.provision.apply.protobuf`,
450-
Provision_Response.encode(response as Provision_Response).finish(),
461+
`${index}.apply.protobuf`,
462+
Response.encode(response as Response).finish(),
451463
)
452464
})
453465
responses.plan.forEach((response, index) => {
454-
fillProvisionResponse(response)
466+
response.plan = {
467+
error: "",
468+
resources: [],
469+
parameters: [],
470+
gitAuthProviders: [],
471+
...response.plan,
472+
} as PlanComplete
473+
response.plan.resources = response.plan.resources?.map(fillResource)
455474

456475
tar.addFile(
457-
`${index}.provision.plan.protobuf`,
458-
Provision_Response.encode(response as Provision_Response).finish(),
476+
`${index}.plan.protobuf`,
477+
Response.encode(response as Response).finish(),
459478
)
460479
})
461480
const tarFile = await tar.write()
@@ -512,16 +531,21 @@ export const echoResponsesWithParameters = (
512531
richParameters: RichParameter[],
513532
): EchoProvisionerResponses => {
514533
return {
534+
parse: [
535+
{
536+
parse: {}
537+
},
538+
],
515539
plan: [
516540
{
517-
complete: {
541+
plan: {
518542
parameters: richParameters,
519543
},
520544
},
521545
],
522546
apply: [
523547
{
524-
complete: {
548+
apply: {
525549
resources: [
526550
{
527551
name: "example",

0 commit comments

Comments
 (0)