Skip to content

Commit 2382b7c

Browse files
JoeWang1127JoeWang1127
authored
feat(interanl/config): add host-mount flag (#788)
Fixes #768
1 parent bd4dea5 commit 2382b7c

File tree

7 files changed

+181
-107
lines changed

7 files changed

+181
-107
lines changed

‎internal/config/config.go‎

Lines changed: 88 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const (
2727
// GeneratorInputDir is the default directory to store files that generator
2828
// needs to regenerate libraries from an empty directory.
2929
GeneratorInputDir string = "generator-input"
30+
DefaultPushConfig string = "[email protected],Google Cloud SDK"
3031
)
3132

3233
// Config holds all configuration values parsed from flags or environment
@@ -44,16 +45,6 @@ type Config struct {
4445
// API Path is specified with the -api flag.
4546
API string
4647

47-
// Source is the path to the root of the googleapis repository.
48-
// When this is not specified, the googleapis repository is cloned
49-
// automatically.
50-
//
51-
// Source is used by generate, update-apis, update-image-tag and configure
52-
// commands.
53-
//
54-
// Source is specified with the -source flag.
55-
Source string
56-
5748
// ArtifactRoot is the path to previously-created release artifacts to be published.
5849
// It is only used by the publish-release-artifacts command, and is expected
5950
// to be the output directory from a previous create-release-artifacts command.
@@ -78,68 +69,24 @@ type Config struct {
7869
// the tool is executing.
7970
CI string
8071

81-
// DockerHostRootDir specifies the host view of a mount point that is
82-
// mounted as DockerMountRootDir from the Librarian view, when Librarian
83-
// is running in Docker. For example, if Librarian has been run via a command
84-
// such as "docker run -v /home/user/librarian:/app" then DockerHostRootDir
85-
// would be "/home/user/librarian" and DockerMountRootDir would be "/app".
86-
//
87-
// This information is required to enable Docker-in-Docker scenarios. When
88-
// creating a Docker container for language-specific operations, the methods
89-
// accepting directory names are all expected to be from the perspective of
90-
// the Librarian code - but the mount point specified when running Docker
91-
// from within Librarian needs to be specified from the host perspective,
92-
// as the new Docker container is created as a sibling of the one running
93-
// Librarian.
94-
//
95-
// For example, if we're in the scenario above, with
96-
// DockerHostRootDir=/home/user/librarian and DockerMountRootDir=/app,
97-
// executing a command which tries to mount the /app/work/googleapis directory
98-
// as /apis in the container, the eventual Docker command would need to include
99-
// "-v /home/user/librarian/work/googleapis:/apis"
100-
//
101-
// DockerHostRootDir and DockerMountDir are currently populated from
102-
// LIBRARIAN_HOST_ROOT_DIR and LIBRARIAN_ROOT_DIR environment variables respectively.
103-
// These are automatically supplied by Kokoro. Other Docker-in-Docker scenarios
104-
// are not currently supported, but could be implemented by populating these
105-
// configuration values in a similar way.
106-
DockerHostRootDir string
107-
108-
// DockerMountRootDir specifies the Librarian view of a mount point that is
109-
// mounted as DockerHostRootDir from the host view, when Librarian is running in
110-
// Docker. See the documentation for DockerHostRootDir for more information.
111-
DockerMountRootDir string
112-
11372
// GitHubToken is the access token to use for all operations involving
11473
// GitHub.
11574
//
116-
// GitHubToken is used by the configure, update-apis and update-image-tag commands,
75+
// GitHubToken is used by to configure, update-apis and update-image-tag commands,
11776
// when Push is true. It is always used by publish-release-artifacts commands.
11877
//
11978
// GitHubToken is not specified by a flag, as flags are logged and the
12079
// access token is sensitive information. Instead, it is fetched from the
12180
// LIBRARIAN_GITHUB_TOKEN environment variable.
12281
GitHubToken string
12382

124-
// PushConfig specifies the email address and display name used in Git commits,
125-
// in the format "email,name".
126-
//
127-
// PushConfig is used in all commands that create commits in a language repository:
128-
// configure, update-apis and update-image-tag.
129-
//
130-
// PushConfig is optional. If unspecified, commits will use a default name of
131-
// "Google Cloud SDK" and a default email of [email protected].
132-
//
133-
// PushConfig is specified with the -push-config flag.
134-
PushConfig string
135-
136-
// UserGID is the group ID of the current user. It is used to run Docker
137-
// containers with the same user, so that created files have the correct
138-
// ownership.
83+
// HostMount is used to remap Docker mount paths when running in environments
84+
// where Docker containers are siblings (e.g., Kokoro).
85+
// It specifies a mount point from the Docker host into the Docker container.
86+
// The format is "{host-dir}:{local-dir}".
13987
//
140-
// This is populated automatically after flag parsing. No user setup is
141-
// expected.
142-
UserGID string
88+
// HostMount is specified with the -host-mount flag.
89+
HostMount string
14390

14491
// Image is the language-specific container image to use for language-specific
14592
// operations. It is primarily used for testing Librarian and/or new images.
@@ -159,6 +106,19 @@ type Config struct {
159106
// variable.
160107
LibrarianRepository string
161108

109+
// Project is the Google Cloud project containing Secret Manager secrets to
110+
// provide to the language-specific container commands via environment variables.
111+
//
112+
// Project is used by all commands which perform language-specific operations.
113+
// (This covers all commands other than merge-release-pr.)
114+
// If no value is set, any language-specific operations which include an
115+
// environment variable based on a secret will act as if the secret name
116+
// wasn't set (so will just use a host environment variable or default value,
117+
// if any).
118+
//
119+
// Project is specified with the -project flag.
120+
Project string
121+
162122
// Push determines whether to push changes to GitHub. It is used in
163123
// all commands that create commits in a language repository:
164124
// configure, update-apis and update-image-tag.
@@ -175,6 +135,18 @@ type Config struct {
175135
// Push is specified with the -push flag. No value is required.
176136
Push bool
177137

138+
// PushConfig specifies the email address and display name used in Git commits,
139+
// in the format "email,name".
140+
//
141+
// PushConfig is used in all commands that create commits in a language repository:
142+
// create-release-pr, configure, update-apis and update-image-tag.
143+
//
144+
// PushConfig is optional. If unspecified, commits will use a default name of
145+
// "Google Cloud SDK" and a default email of [email protected].
146+
//
147+
// PushConfig is specified with the -push-config flag.
148+
PushConfig string
149+
178150
// ReleaseID is the identifier of a release PR. Each release PR created by
179151
// Librarian has a release ID, which is included in both the PR description and
180152
// the commit message of every commit within the release PR. This is effectively
@@ -207,17 +179,8 @@ type Config struct {
207179
// Repo is specified with the -repo flag.
208180
Repo string
209181

210-
// Project is the Google Cloud project containing Secret Manager secrets to
211-
// provide to the language-specific container commands via environment variables.
212-
//
213-
// Project is used by all commands which perform language-specific operations.
214-
// If no value is set, any language-specific operations which include an
215-
// environment variable based on a secret will act as if the secret name
216-
// wasn't set (so will just use a host environment variable or default value,
217-
// if any).
218-
//
219-
// Project is specified with the -project flag.
220-
Project string
182+
// SkipIntegrationTests is used by the create-release-pr and create-release-artifacts
183+
// commands, and disables integration tests if it is set to a non-empty value.
221184

222185
// SkipIntegrationTests is used by the create-release-artifacts
223186
// command, and disables integration tests if it is set to a non-empty value.
@@ -226,6 +189,16 @@ type Config struct {
226189
// SkipIntegrationTests is specified with the -skip-integration-tests flag.
227190
SkipIntegrationTests string
228191

192+
// Source is the path to the root of the googleapis repository.
193+
// When this is not specified, the googleapis repository is cloned
194+
// automatically.
195+
//
196+
// Source is used by generate, update-apis, update-image-tag and configure
197+
// commands.
198+
//
199+
// Source is specified with the -source flag.
200+
Source string
201+
229202
// Tag is the new tag for the language-specific Docker image, used only by the
230203
// update-image-tag command. All operations within update-image-tag are performed
231204
// using the new tag.
@@ -242,6 +215,14 @@ type Config struct {
242215
// TagRepoURL is specified with the -tag-repo-url flag.
243216
TagRepoURL string
244217

218+
// UserGID is the group ID of the current user. It is used to run Docker
219+
// containers with the same user, so that created files have the correct
220+
// ownership.
221+
//
222+
// This is populated automatically after flag parsing. No user setup is
223+
// expected.
224+
UserGID string
225+
245226
// UserUID is the user ID of the current user. It is used to run Docker
246227
// containers with the same user, so that created files have the correct
247228
// ownership.
@@ -262,9 +243,8 @@ type Config struct {
262243
// New returns a new Config populated with environment variables.
263244
func New() *Config {
264245
return &Config{
265-
DockerHostRootDir: os.Getenv("LIBRARIAN_HOST_ROOT_DIR"),
266-
DockerMountRootDir: os.Getenv("LIBRARIAN_ROOT_DIR"),
267-
GitHubToken: os.Getenv("LIBRARIAN_GITHUB_TOKEN"),
246+
GitHubToken: os.Getenv("LIBRARIAN_GITHUB_TOKEN"),
247+
PushConfig: DefaultPushConfig,
268248
}
269249
}
270250

@@ -289,9 +269,40 @@ func (c *Config) IsValid() (bool, error) {
289269
if c.Push && c.GitHubToken == "" {
290270
return false, errors.New("no GitHub token supplied for push")
291271
}
292-
parts := strings.Split(c.PushConfig, ",")
293-
if len(parts) != 2 {
272+
273+
if _, err := validatePushConfig(c.PushConfig, DefaultPushConfig); err != nil {
274+
return false, err
275+
}
276+
277+
if _, err := validateHostMount(c.HostMount, ""); err != nil {
278+
return false, err
279+
}
280+
281+
return true, nil
282+
}
283+
284+
func validateHostMount(hostMount, defaultValue string) (bool, error) {
285+
if hostMount == defaultValue {
286+
return true, nil
287+
}
288+
289+
parts := strings.Split(hostMount, ":")
290+
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
291+
return false, errors.New("unable to parse host mount")
292+
}
293+
294+
return true, nil
295+
}
296+
297+
func validatePushConfig(pushConfig, defaultValue string) (bool, error) {
298+
if pushConfig == defaultValue {
299+
return true, nil
300+
}
301+
302+
parts := strings.Split(pushConfig, ",")
303+
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
294304
return false, errors.New("unable to parse push config")
295305
}
306+
296307
return true, nil
297308
}

‎internal/config/config_test.go‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,36 +31,30 @@ func TestNew(t *testing.T) {
3131
{
3232
name: "All environment variables set",
3333
envVars: map[string]string{
34-
"LIBRARIAN_HOST_ROOT_DIR": "/host/root",
35-
"LIBRARIAN_ROOT_DIR": "/mount/root",
3634
"LIBRARIAN_GITHUB_TOKEN": "gh_token",
3735
"LIBRARIAN_SYNC_AUTH_TOKEN": "sync_token",
3836
},
3937
want: Config{
40-
DockerHostRootDir: "/host/root",
41-
DockerMountRootDir: "/mount/root",
42-
GitHubToken: "gh_token",
38+
GitHubToken: "gh_token",
39+
PushConfig: DefaultPushConfig,
4340
},
4441
},
4542
{
4643
name: "No environment variables set",
4744
envVars: map[string]string{},
4845
want: Config{
49-
DockerHostRootDir: "",
50-
DockerMountRootDir: "",
51-
GitHubToken: "",
46+
GitHubToken: "",
47+
PushConfig: DefaultPushConfig,
5248
},
5349
},
5450
{
5551
name: "Some environment variables set",
5652
envVars: map[string]string{
57-
"LIBRARIAN_HOST_ROOT_DIR": "/host/root",
58-
"LIBRARIAN_GITHUB_TOKEN": "gh_token",
53+
"LIBRARIAN_GITHUB_TOKEN": "gh_token",
5954
},
6055
want: Config{
61-
DockerHostRootDir: "/host/root",
62-
DockerMountRootDir: "",
63-
GitHubToken: "gh_token",
56+
GitHubToken: "gh_token",
57+
PushConfig: DefaultPushConfig,
6458
},
6559
},
6660
} {
@@ -182,6 +176,36 @@ func TestIsValid(t *testing.T) {
182176
wantValid: false,
183177
wantErr: true,
184178
},
179+
{
180+
name: "Invalid config - host mount invalid, missing local-dir",
181+
cfg: Config{
182+
Push: false,
183+
PushConfig: "[email protected],abc",
184+
HostMount: "host-dir:",
185+
},
186+
wantValid: false,
187+
wantErr: true,
188+
},
189+
{
190+
name: "Invalid config - host mount invalid, missing host-dir",
191+
cfg: Config{
192+
Push: false,
193+
PushConfig: "[email protected],abc",
194+
HostMount: ":local-dir",
195+
},
196+
wantValid: false,
197+
wantErr: true,
198+
},
199+
{
200+
name: "Invalid config - host mount invalid, missing separator",
201+
cfg: Config{
202+
Push: false,
203+
PushConfig: "[email protected],abc",
204+
HostMount: "host-dir/local-dir",
205+
},
206+
wantValid: false,
207+
wantErr: true,
208+
},
185209
} {
186210
t.Run(test.name, func(t *testing.T) {
187211
gotValid, err := test.cfg.IsValid()
@@ -193,7 +217,11 @@ func TestIsValid(t *testing.T) {
193217
if (err != nil) != test.wantErr {
194218
t.Errorf("IsValid() got error = %v, want error = %t", err, test.wantErr)
195219
}
196-
if test.wantErr && err != nil && err.Error() != "no GitHub token supplied for push" && err.Error() != "unable to parse push config" {
220+
if test.wantErr &&
221+
err != nil &&
222+
err.Error() != "no GitHub token supplied for push" &&
223+
err.Error() != "unable to parse push config" &&
224+
err.Error() != "unable to parse host mount" {
197225
t.Errorf("IsValid() got unexpected error message: %q", err.Error())
198226
}
199227
})

‎internal/docker/docker.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,13 +312,15 @@ func (c *Docker) runDocker(ctx context.Context, cfg *config.Config, command Comm
312312
func maybeRelocateMounts(cfg *config.Config, mounts []string) []string {
313313
// When running in Kokoro, we'll be running sibling containers.
314314
// Make sure we specify the "from" part of the mount as the host directory.
315-
if cfg.DockerMountRootDir == "" || cfg.DockerHostRootDir == "" {
315+
if cfg.HostMount == "" {
316316
return mounts
317317
}
318+
318319
relocatedMounts := []string{}
320+
hostMount := strings.Split(cfg.HostMount, ":")
319321
for _, mount := range mounts {
320-
if strings.HasPrefix(mount, cfg.DockerMountRootDir) {
321-
mount = strings.Replace(mount, cfg.DockerMountRootDir, cfg.DockerHostRootDir, 1)
322+
if strings.HasPrefix(mount, hostMount[0]) {
323+
mount = strings.Replace(mount, hostMount[0], hostMount[1], 1)
322324
}
323325
relocatedMounts = append(relocatedMounts, mount)
324326
}

‎internal/docker/docker_test.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ func TestDockerRun(t *testing.T) {
7373
)
7474

7575
cfg := &config.Config{}
76-
76+
cfgInDocker := &config.Config{
77+
HostMount: "hostDir:localDir",
78+
}
7779
for _, test := range []struct {
7880
name string
7981
docker *Docker
@@ -145,6 +147,28 @@ func TestDockerRun(t *testing.T) {
145147
fmt.Sprintf("--library-id=%s", testLibraryID),
146148
},
147149
},
150+
{
151+
name: "GenerateLibrary runs in docker",
152+
docker: &Docker{
153+
Image: testImage,
154+
},
155+
runCommand: func(ctx context.Context, d *Docker) error {
156+
return d.GenerateLibrary(ctx, cfgInDocker, testAPIRoot, "hostDir", testGeneratorInput, testLibraryID)
157+
},
158+
want: []string{
159+
"run", "--rm",
160+
"-v", fmt.Sprintf("%s:/apis", testAPIRoot),
161+
"-v", "localDir:/output",
162+
"-v", fmt.Sprintf("%s:/generator-input", testGeneratorInput),
163+
"--network=none",
164+
testImage,
165+
string(CommandGenerateLibrary),
166+
"--source=/apis",
167+
"--output=/output",
168+
"--generator-input=/generator-input",
169+
fmt.Sprintf("--library-id=%s", testLibraryID),
170+
},
171+
},
148172
{
149173
name: "Clean",
150174
docker: &Docker{

0 commit comments

Comments
 (0)