Skip to content

Commit 365019e

Browse files
JoeWang1127JoeWang1127
authored
feat: mount output dir in configure docker command (#2439)
Fixes #1922
1 parent ea54ba6 commit 365019e

File tree

10 files changed

+351
-246
lines changed

10 files changed

+351
-246
lines changed

‎doc/language-onboarding.md‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ The container is expected to produce up to two artifacts:
7070

7171
**Contract:**
7272

73-
| Context | Type | Description |
74-
| :----------- | :------------------ | :----------------------------------------------------------------------------- |
75-
| `/librarian` | Mount (Read/Write) | Contains `configure-request.json`. The container must process this and write back `configure-response.json`. |
76-
| `/input` | Mount (Read/Write) | The contents of the `.librarian/generator-input` directory. The container can add new language-specific configuration here. |
77-
| `/repo` | Mount (Read) | Contains only the files specified in the `global_files_allowlist` from `config.yaml`. |
78-
| `/source` | Mount (Read). | Contains the complete contents of the API definition repository (e.g., [googleapis/googleapis](https://github.com/googleapis/googleapis)). |
79-
| `/output` | Mount (Read/Write) | An output directory for writing any global file edits allowed by `global_files_allowlist`. |
80-
| `command` | Positional Argument | The value will always be `configure`. |
81-
| flags | Flags | Flags indicating the locations of the mounts: `--librarian`, `--input`, `--source`, `--repo`, `--output` |
73+
| Context | Type | Description |
74+
| :----------- | :------------------ |:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
75+
| `/librarian` | Mount (Read/Write) | Contains `configure-request.json`. The container must process this and write back `configure-response.json`. |
76+
| `/input` | Mount (Read/Write) | The contents of the `.librarian/generator-input` directory. The container can add new language-specific configuration here. |
77+
| `/repo` | Mount (Read) | Contains all of the files are specified in the libraries source_roots , if any already exist, as well as the files specified in the global_files_allowlist from `config.yaml`. |
78+
| `/source` | Mount (Read). | Contains the complete contents of the API definition repository (e.g., [googleapis/googleapis](https://github.com/googleapis/googleapis)). |
79+
| `/output` | Mount (Read/Write) | An output directory for writing any global file edits allowed by `global_files_allowlist`.<br/>Additionally, the container can write arbitrary files as long as they are contained within the library’s source_roots specified in the container's response message.|
80+
| `command` | Positional Argument | The value will always be `configure`. |
81+
| flags | Flags | Flags indicating the locations of the mounts: `--librarian`, `--input`, `--source`, `--repo`, `--output` |
8282

8383
**Example `configure-request.json`:**
8484

‎internal/docker/docker.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ type ConfigureRequest struct {
9494
// libraryID specifies the ID of the library to configure.
9595
LibraryID string
9696

97+
// Output specifies the empty output directory into which the command should
98+
// generate code
99+
Output string
100+
97101
// RepoDir is the local root directory of the language repository.
98102
RepoDir string
99103

@@ -277,6 +281,7 @@ func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (stri
277281
commandArgs := []string{
278282
"--librarian=/librarian",
279283
"--input=/input",
284+
"--output=/output",
280285
"--repo=/repo",
281286
"--source=/source",
282287
}
@@ -285,6 +290,7 @@ func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (stri
285290
mounts := []string{
286291
fmt.Sprintf("%s:/librarian", librarianDir),
287292
fmt.Sprintf("%s:/input", generatorInput),
293+
fmt.Sprintf("%s:/output", request.Output),
288294
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume
289295
}
290296
// Mount existing source roots as a readonly volume.

‎internal/docker/docker_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ func TestDockerRun(t *testing.T) {
243243
LibraryID: testLibraryID,
244244
RepoDir: repoDir,
245245
ApiRoot: testAPIRoot,
246+
Output: testOutput,
246247
GlobalFiles: []string{
247248
"a/b/go.mod",
248249
"go.mod",
@@ -257,13 +258,15 @@ func TestDockerRun(t *testing.T) {
257258
"run", "--rm",
258259
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
259260
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
261+
"-v", fmt.Sprintf("%s:/output", testOutput),
260262
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
261263
"-v", fmt.Sprintf("%s/a/b/go.mod:/repo/a/b/go.mod:ro", repoDir),
262264
"-v", fmt.Sprintf("%s/go.mod:/repo/go.mod:ro", repoDir),
263265
testImage,
264266
string(CommandConfigure),
265267
"--librarian=/librarian",
266268
"--input=/input",
269+
"--output=/output",
267270
"--repo=/repo",
268271
"--source=/source",
269272
},
@@ -279,6 +282,7 @@ func TestDockerRun(t *testing.T) {
279282
LibraryID: testLibraryID,
280283
RepoDir: repoDir,
281284
ApiRoot: testAPIRoot,
285+
Output: testOutput,
282286
GlobalFiles: nil,
283287
}
284288

@@ -290,11 +294,13 @@ func TestDockerRun(t *testing.T) {
290294
"run", "--rm",
291295
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
292296
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
297+
"-v", fmt.Sprintf("%s:/output", testOutput),
293298
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
294299
testImage,
295300
string(CommandConfigure),
296301
"--librarian=/librarian",
297302
"--input=/input",
303+
"--output=/output",
298304
"--repo=/repo",
299305
"--source=/source",
300306
},
@@ -310,6 +316,7 @@ func TestDockerRun(t *testing.T) {
310316
LibraryID: testLibraryID,
311317
RepoDir: repoDir,
312318
ApiRoot: testAPIRoot,
319+
Output: testOutput,
313320
ExistingSourceRoots: []string{
314321
"a/path",
315322
"b/path",
@@ -324,13 +331,15 @@ func TestDockerRun(t *testing.T) {
324331
"run", "--rm",
325332
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
326333
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
334+
"-v", fmt.Sprintf("%s:/output", testOutput),
327335
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
328336
"-v", fmt.Sprintf("%s/a/path:/repo/a/path:ro", repoDir),
329337
"-v", fmt.Sprintf("%s/b/path:/repo/b/path:ro", repoDir),
330338
testImage,
331339
string(CommandConfigure),
332340
"--librarian=/librarian",
333341
"--input=/input",
342+
"--output=/output",
334343
"--repo=/repo",
335344
"--source=/source",
336345
},
@@ -346,6 +355,7 @@ func TestDockerRun(t *testing.T) {
346355
LibraryID: testLibraryID,
347356
RepoDir: repoDir,
348357
ApiRoot: testAPIRoot,
358+
Output: testOutput,
349359
ExistingSourceRoots: nil,
350360
}
351361

@@ -357,11 +367,13 @@ func TestDockerRun(t *testing.T) {
357367
"run", "--rm",
358368
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
359369
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
370+
"-v", fmt.Sprintf("%s:/output", testOutput),
360371
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
361372
testImage,
362373
string(CommandConfigure),
363374
"--librarian=/librarian",
364375
"--input=/input",
376+
"--output=/output",
365377
"--repo=/repo",
366378
"--source=/source",
367379
},
@@ -402,6 +414,7 @@ func TestDockerRun(t *testing.T) {
402414
LibraryID: testLibraryID,
403415
RepoDir: repoDir,
404416
ApiRoot: testAPIRoot,
417+
Output: testOutput,
405418
GlobalFiles: []string{
406419
"a/b/go.mod",
407420
"go.mod",
@@ -419,13 +432,15 @@ func TestDockerRun(t *testing.T) {
419432
"run", "--rm",
420433
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
421434
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
435+
"-v", fmt.Sprintf("%s:/output", testOutput),
422436
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
423437
"-v", fmt.Sprintf("%s/a/b/go.mod:/repo/a/b/go.mod:ro", repoDir),
424438
"-v", fmt.Sprintf("%s/go.mod:/repo/go.mod:ro", repoDir),
425439
testImage,
426440
string(CommandConfigure),
427441
"--librarian=/librarian",
428442
"--input=/input",
443+
"--output=/output",
429444
"--repo=/repo",
430445
"--source=/source",
431446
},

‎internal/librarian/command.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ func writePRBody(info *commitInfo) {
406406
slog.Warn("Unable to create PR body", "error", err)
407407
return
408408
}
409-
// Note: we can't accurately predict whether or not a PR would have been created,
409+
// Note: we can't accurately predict whether a PR would have been created,
410410
// as we're not checking whether the repo is clean or not. The intention is to be
411411
// as light-touch as possible.
412412
fullPath := filepath.Join(info.workRoot, prBodyFile)
@@ -448,6 +448,27 @@ func createPRBody(info *commitInfo, gitHubRepo *github.Repository) (string, erro
448448
}
449449
}
450450

451+
// copyGlobalAllowlist copies files in the global file allowlist from src to dst.
452+
func copyGlobalAllowlist(cfg *config.LibrarianConfig, dst, src string, copyReadOnly bool) error {
453+
if cfg == nil {
454+
slog.Info("librarian config is not setup, skip copying global allowlist")
455+
return nil
456+
}
457+
slog.Info("Copying global allowlist files", "destination", dst, "source", src)
458+
for _, globalFile := range cfg.GlobalFilesAllowlist {
459+
if globalFile.Permissions == config.PermissionReadOnly && !copyReadOnly {
460+
slog.Debug("skipping read-only file", "path", globalFile.Path)
461+
continue
462+
}
463+
srcPath := filepath.Join(src, globalFile.Path)
464+
dstPath := filepath.Join(dst, globalFile.Path)
465+
if err := copyFile(dstPath, srcPath); err != nil {
466+
return fmt.Errorf("failed to copy global file %s from %s: %w", dstPath, srcPath, err)
467+
}
468+
}
469+
return nil
470+
}
471+
451472
func copyFile(dst, src string) (err error) {
452473
lstat, err := os.Lstat(src)
453474
if err != nil {

0 commit comments

Comments
 (0)