Skip to content

Commit a69aac4

Browse files
authored
feat(internal/librarian): rework generate container contract (#807)
Fixes #770
1 parent 87bdb2c commit a69aac4

File tree

9 files changed

+262
-42
lines changed

9 files changed

+262
-42
lines changed

internal/config/config.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,15 @@ import (
2424
)
2525

2626
const (
27+
DefaultPushConfig string = "[email protected],Google Cloud SDK"
2728
// GeneratorInputDir is the default directory to store files that generator
2829
// needs to regenerate libraries from an empty directory.
29-
GeneratorInputDir string = "generator-input"
30-
DefaultPushConfig string = "[email protected],Google Cloud SDK"
30+
GeneratorInputDir string = ".librarian/generator-input"
31+
// GenerateRequest is a JSON file that describes which library to generate.
32+
GenerateRequest string = "generate-request.json"
33+
// LibrarianDir is the default directory to store librarian state/config files,
34+
// along with any additional configuration.
35+
LibrarianDir string = ".librarian"
3136
)
3237

3338
// Config holds all configuration values parsed from flags or environment

internal/docker/docker.go

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ package docker
2020

2121
import (
2222
"context"
23+
"encoding/json"
2324
"fmt"
2425
"log/slog"
2526
"os"
2627
"os/exec"
28+
"path/filepath"
2729
"strings"
2830

2931
"github.com/googleapis/librarian/internal/config"
@@ -62,6 +64,26 @@ type Docker struct {
6264
run func(args ...string) error
6365
}
6466

67+
// GenerateRequest contains all the information required for a language
68+
// container to run the generate command.
69+
type GenerateRequest struct {
70+
// cfg is a pointer to the [config.Config] struct, holding general configuration
71+
// values parsed from flags or environment variables.
72+
Cfg *config.Config
73+
// state is a pointer to the [config.LibrarianState] struct, representing
74+
// the overall state of the generation and release pipeline.
75+
State *config.LibrarianState
76+
// apiRoot specifies the root directory of the API specification repo.
77+
ApiRoot string
78+
// libraryID specifies the ID of the library to generate
79+
LibraryID string
80+
// output specifies the empty output directory into which the command should
81+
// generate code
82+
Output string
83+
// RepoDir is the local root directory of the language repository.
84+
RepoDir string
85+
}
86+
6587
// New constructs a Docker instance which will invoke the specified
6688
// Docker image as required to implement language-specific commands,
6789
// providing the container with required environment variables.
@@ -79,25 +101,30 @@ func New(workRoot, image, secretsProject, uid, gid string, pipelineConfig *confi
79101
return docker, nil
80102
}
81103

82-
// Generate performs generation for an API which is configured as part of a library.
83-
// apiRoot specifies the root directory of the API specification repo,
84-
// output specifies the empty output directory into which the command should
85-
// generate code, and libraryID specifies the ID of the library to generate,
86-
// as configured in the Librarian state file for the repository.
87-
func (c *Docker) Generate(ctx context.Context, cfg *config.Config, apiRoot, output, generatorInput, libraryID string) error {
104+
// Generate performs generation for an API which is configured as part of a
105+
// library.
106+
func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
107+
jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.GenerateRequest)
108+
if err := writeGenerateRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
109+
return err
110+
}
88111
commandArgs := []string{
89-
"--source=/apis",
112+
"--librarian=/librarian",
113+
"--input=/input",
90114
"--output=/output",
91-
fmt.Sprintf("--%s=/%s", config.GeneratorInputDir, config.GeneratorInputDir),
92-
fmt.Sprintf("--library-id=%s", libraryID),
115+
"--source=/source",
116+
fmt.Sprintf("--library-id=%s", request.LibraryID),
93117
}
118+
119+
generatorInput := filepath.Join(request.RepoDir, config.GeneratorInputDir)
94120
mounts := []string{
95-
fmt.Sprintf("%s:/apis", apiRoot),
96-
fmt.Sprintf("%s:/output", output),
97-
fmt.Sprintf("%s:/%s", generatorInput, config.GeneratorInputDir),
121+
fmt.Sprintf("%s:/librarian:ro", config.LibrarianDir), // readonly volume.
122+
fmt.Sprintf("%s:/input", generatorInput),
123+
fmt.Sprintf("%s:/output", request.Output),
124+
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume.
98125
}
99126

100-
return c.runDocker(ctx, cfg, CommandGenerate, mounts, commandArgs)
127+
return c.runDocker(ctx, request.Cfg, CommandGenerate, mounts, commandArgs)
101128
}
102129

103130
// Build builds the library with an ID of libraryID, as configured in
@@ -200,3 +227,31 @@ func (c *Docker) runCommand(cmdName string, args ...string) error {
200227
slog.Info(fmt.Sprintf("=== Docker end %s", strings.Repeat("=", 65)))
201228
return err
202229
}
230+
231+
func writeGenerateRequest(state *config.LibrarianState, libraryID, jsonFilePath string) error {
232+
if err := os.MkdirAll(filepath.Dir(jsonFilePath), 0755); err != nil {
233+
return fmt.Errorf("failed to make directory: %w", err)
234+
}
235+
jsonFile, err := os.Create(jsonFilePath)
236+
if err != nil {
237+
return fmt.Errorf("failed to create generate request JSON file: %w", err)
238+
}
239+
defer jsonFile.Close()
240+
241+
for _, library := range state.Libraries {
242+
if library.ID != libraryID {
243+
continue
244+
}
245+
246+
data, err := json.MarshalIndent(library, "", " ")
247+
if err != nil {
248+
return fmt.Errorf("failed to marshal state to JSON: %w", err)
249+
}
250+
_, err = jsonFile.Write(data)
251+
if err != nil {
252+
return fmt.Errorf("failed to write generate request JSON file: %w", err)
253+
}
254+
}
255+
256+
return nil
257+
}

internal/docker/docker_test.go

Lines changed: 145 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ package docker
1717
import (
1818
"context"
1919
"fmt"
20+
"os"
21+
"path/filepath"
22+
"strings"
2023
"testing"
2124

2225
"github.com/googleapis/librarian/internal/config"
@@ -56,22 +59,17 @@ func TestNew(t *testing.T) {
5659

5760
func TestDockerRun(t *testing.T) {
5861
const (
59-
testUID = "1000"
60-
testGID = "1001"
6162
testAPIPath = "testAPIPath"
6263
testAPIRoot = "testAPIRoot"
64+
testGenerateRequest = "testGenerateRequest"
6365
testGeneratorInput = "testGeneratorInput"
64-
testGeneratorOutput = "testGeneratorOutput"
6566
testImage = "testImage"
66-
testInputsDirectory = "testInputsDirectory"
67-
testLanguageRepo = "testLanguageRepo"
6867
testLibraryID = "testLibraryID"
69-
testLibraryVersion = "testLibraryVersion"
7068
testOutput = "testOutput"
71-
testReleaseVersion = "testReleaseVersion"
7269
testRepoRoot = "testRepoRoot"
7370
)
7471

72+
state := &config.LibrarianState{}
7573
cfg := &config.Config{}
7674
cfgInDocker := &config.Config{
7775
HostMount: "hostDir:localDir",
@@ -88,18 +86,28 @@ func TestDockerRun(t *testing.T) {
8886
Image: testImage,
8987
},
9088
runCommand: func(ctx context.Context, d *Docker) error {
91-
return d.Generate(ctx, cfg, testAPIRoot, testOutput, testGeneratorInput, testLibraryID)
89+
generateRequest := &GenerateRequest{
90+
Cfg: cfg,
91+
State: state,
92+
RepoDir: ".",
93+
ApiRoot: testAPIRoot,
94+
Output: testOutput,
95+
LibraryID: testLibraryID,
96+
}
97+
return d.Generate(ctx, generateRequest)
9298
},
9399
want: []string{
94100
"run", "--rm",
95-
"-v", fmt.Sprintf("%s:/apis", testAPIRoot),
101+
"-v", ".librarian:/librarian:ro",
102+
"-v", ".librarian/generator-input:/input",
96103
"-v", fmt.Sprintf("%s:/output", testOutput),
97-
"-v", fmt.Sprintf("%s:/generator-input", testGeneratorInput),
104+
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
98105
testImage,
99106
string(CommandGenerate),
100-
"--source=/apis",
107+
"--librarian=/librarian",
108+
"--input=/input",
101109
"--output=/output",
102-
"--generator-input=/generator-input",
110+
"--source=/source",
103111
fmt.Sprintf("--library-id=%s", testLibraryID),
104112
},
105113
},
@@ -109,18 +117,28 @@ func TestDockerRun(t *testing.T) {
109117
Image: testImage,
110118
},
111119
runCommand: func(ctx context.Context, d *Docker) error {
112-
return d.Generate(ctx, cfgInDocker, testAPIRoot, "hostDir", testGeneratorInput, testLibraryID)
120+
generateRequest := &GenerateRequest{
121+
Cfg: cfgInDocker,
122+
State: state,
123+
RepoDir: ".",
124+
ApiRoot: testAPIRoot,
125+
Output: "hostDir",
126+
LibraryID: testLibraryID,
127+
}
128+
return d.Generate(ctx, generateRequest)
113129
},
114130
want: []string{
115131
"run", "--rm",
116-
"-v", fmt.Sprintf("%s:/apis", testAPIRoot),
132+
"-v", ".librarian:/librarian:ro",
133+
"-v", ".librarian/generator-input:/input",
117134
"-v", "localDir:/output",
118-
"-v", fmt.Sprintf("%s:/generator-input", testGeneratorInput),
135+
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
119136
testImage,
120137
string(CommandGenerate),
121-
"--source=/apis",
138+
"--librarian=/librarian",
139+
"--input=/input",
122140
"--output=/output",
123-
"--generator-input=/generator-input",
141+
"--source=/source",
124142
fmt.Sprintf("--library-id=%s", testLibraryID),
125143
},
126144
},
@@ -153,11 +171,11 @@ func TestDockerRun(t *testing.T) {
153171
want: []string{
154172
"run", "--rm",
155173
"-v", fmt.Sprintf("%s:/apis", testAPIRoot),
156-
"-v", fmt.Sprintf("%s:/generator-input", testGeneratorInput),
174+
"-v", fmt.Sprintf("%s:/.librarian/generator-input", testGeneratorInput),
157175
testImage,
158176
string(CommandConfigure),
159177
"--source=/apis",
160-
"--generator-input=/generator-input",
178+
"--.librarian/generator-input=/.librarian/generator-input",
161179
fmt.Sprintf("--api=%s", testAPIPath),
162180
},
163181
},
@@ -173,6 +191,114 @@ func TestDockerRun(t *testing.T) {
173191
if err := test.runCommand(ctx, test.docker); err != nil {
174192
t.Fatal(err)
175193
}
194+
os.RemoveAll(".librarian")
195+
os.Remove(testGenerateRequest)
196+
})
197+
}
198+
}
199+
200+
func TestToGenerateRequestJSON(t *testing.T) {
201+
t.Parallel()
202+
for _, test := range []struct {
203+
name string
204+
state *config.LibrarianState
205+
expectErr bool
206+
}{
207+
{
208+
name: "successful-marshaling-and-writing",
209+
state: &config.LibrarianState{
210+
Image: "v1.0.0",
211+
Libraries: []*config.LibraryState{
212+
{
213+
ID: "google-cloud-go",
214+
Version: "1.0.0",
215+
LastGeneratedCommit: "abcd123",
216+
APIs: []config.API{
217+
{
218+
Path: "google/cloud/compute/v1",
219+
ServiceConfig: "example_service_config.yaml",
220+
},
221+
},
222+
SourcePaths: []string{
223+
"src/example/path",
224+
},
225+
PreserveRegex: []string{
226+
"example-preserve-regex",
227+
},
228+
RemoveRegex: []string{
229+
"example-remove-regex",
230+
},
231+
},
232+
{
233+
ID: "google-cloud-storage",
234+
Version: "1.2.3",
235+
APIs: []config.API{
236+
{
237+
Path: "google/storage/v1",
238+
ServiceConfig: "storage_service_config.yaml",
239+
},
240+
},
241+
},
242+
},
243+
},
244+
expectErr: false,
245+
},
246+
{
247+
name: "empty-pipelineState",
248+
state: &config.LibrarianState{},
249+
expectErr: false,
250+
},
251+
{
252+
name: "nonexistent_dir_for_test",
253+
state: &config.LibrarianState{},
254+
expectErr: true,
255+
},
256+
{
257+
name: "invalid_file_name",
258+
state: &config.LibrarianState{},
259+
expectErr: true,
260+
},
261+
} {
262+
t.Run(test.name, func(t *testing.T) {
263+
tempDir := t.TempDir()
264+
if test.name == "invalid_file_name" {
265+
filePath := filepath.Join(tempDir, "my\x00file.json")
266+
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
267+
if err == nil {
268+
t.Errorf("writeGenerateRequest() expected an error but got nil")
269+
}
270+
return
271+
} else if test.expectErr {
272+
filePath := filepath.Join("/non-exist-dir", "generate-request.json")
273+
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
274+
if err == nil {
275+
t.Errorf("writeGenerateRequest() expected an error but got nil")
276+
}
277+
return
278+
}
279+
280+
filePath := filepath.Join(tempDir, "generate-request.json")
281+
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
282+
283+
if err != nil {
284+
t.Fatalf("writeGenerateRequest() unexpected error: %v", err)
285+
}
286+
287+
// Verify the file content
288+
gotBytes, err := os.ReadFile(filePath)
289+
if err != nil {
290+
t.Fatalf("Failed to read generated file: %v", err)
291+
}
292+
293+
fileName := fmt.Sprintf("%s.json", test.name)
294+
wantBytes, readErr := os.ReadFile(filepath.Join("..", "..", "testdata", fileName))
295+
if readErr != nil {
296+
t.Fatalf("Failed to read expected state for comparison: %v", readErr)
297+
}
298+
299+
if diff := cmp.Diff(strings.TrimSpace(string(wantBytes)), string(gotBytes)); diff != "" {
300+
t.Errorf("Generated JSON mismatch (-want +got):\n%s", diff)
301+
}
176302
})
177303
}
178304
}

internal/librarian/generate.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,17 @@ func (r *generateRunner) runGenerateCommand(ctx context.Context, outputDir strin
196196
if libraryID == "" {
197197
return "", errors.New("bug in Librarian: Library not found during generation, despite being found in earlier steps")
198198
}
199-
generatorInput := filepath.Join(r.repo.Dir, config.GeneratorInputDir)
199+
200+
generateRequest := &docker.GenerateRequest{
201+
Cfg: r.cfg,
202+
State: r.state,
203+
ApiRoot: apiRoot,
204+
LibraryID: libraryID,
205+
Output: outputDir,
206+
RepoDir: r.repo.Dir,
207+
}
200208
slog.Info("Performing refined generation for library", "id", libraryID)
201-
return libraryID, r.containerClient.Generate(ctx, r.cfg, apiRoot, outputDir, generatorInput, libraryID)
209+
return libraryID, r.containerClient.Generate(ctx, generateRequest)
202210
}
203211
slog.Info("No matching library found (or no repo specified)", "path", r.cfg.API)
204212
return "", fmt.Errorf("library not found")

0 commit comments

Comments
 (0)