diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b822392..0b023e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,12 @@ Both client libraries are pre-1.0, and they have separate versioning. ## Unreleased -No unreleased changes. +- Added support for the `MODAL_FORCE_BUILD` environment variable and `force_build` config option for `modal.toml`. When set, this applies to all image builds. +- Added an optional `forceBuild` parameter to `images.fromRegistry`, `images.fromAwsEcr`, `images.fromGcpArtifactRegistry`, and `Image.build` to the JS SDK. +- Added `secret` field to `ImageFromRegistryParams` in the JS SDK to match the Go SDK pattern. The separate `secret` parameter in `images.fromRegistry()` is now deprecated. + +**Breaking changes:** +- Added `...Params` struct arguments, and an optional `ForceBuild` parameter, to `Images.FromRegistry`, `Images.FromAwsEcr`, `Images.FromGcpArtifactRegistry`, and `Image.Build` to the Go SDK. ## modal-js/v0.6.0, modal-go/v0.6.0 diff --git a/modal-go/config.go b/modal-go/config.go index 08b62b99..6ed21e4a 100644 --- a/modal-go/config.go +++ b/modal-go/config.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/pelletier/go-toml/v2" ) @@ -20,6 +21,7 @@ type Profile struct { Environment string ImageBuilderVersion string LogLevel string + ForceBuild bool } // rawProfile mirrors the TOML structure on disk. @@ -30,6 +32,7 @@ type rawProfile struct { Environment string `toml:"environment"` ImageBuilderVersion string `toml:"image_builder_version"` LogLevel string `toml:"loglevel"` + ForceBuild bool `toml:"force_build"` Active bool `toml:"active"` } @@ -95,6 +98,7 @@ func getProfile(name string, cfg config) Profile { environment := firstNonEmpty(os.Getenv("MODAL_ENVIRONMENT"), raw.Environment) imageBuilderVersion := firstNonEmpty(os.Getenv("MODAL_IMAGE_BUILDER_VERSION"), raw.ImageBuilderVersion) logLevel := firstNonEmpty(os.Getenv("MODAL_LOGLEVEL"), raw.LogLevel) + forceBuild := envBool("MODAL_FORCE_BUILD") || raw.ForceBuild return Profile{ ServerURL: serverURL, @@ -103,6 +107,7 @@ func getProfile(name string, cfg config) Profile { Environment: environment, ImageBuilderVersion: imageBuilderVersion, LogLevel: logLevel, + ForceBuild: forceBuild, } } @@ -115,6 +120,11 @@ func firstNonEmpty(values ...string) string { return "" } +func envBool(key string) bool { + v := strings.ToLower(os.Getenv(key)) + return v != "" && v != "0" && v != "false" +} + func environmentName(environment string, profile Profile) string { return firstNonEmpty(environment, profile.Environment) } diff --git a/modal-go/examples/sandbox-prewarm/main.go b/modal-go/examples/sandbox-prewarm/main.go index b45359ba..1ca9a065 100644 --- a/modal-go/examples/sandbox-prewarm/main.go +++ b/modal-go/examples/sandbox-prewarm/main.go @@ -25,7 +25,7 @@ func main() { // With `.Build(app)`, we create an Image object on Modal that eagerly pulls // from the registry. - image, err := mc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app) + image, err := mc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app, nil) if err != nil { log.Fatalf("Unable to build Image: %v", err) } diff --git a/modal-go/examples/sandbox-private-image/main.go b/modal-go/examples/sandbox-private-image/main.go index 7b4cc63e..bd846440 100644 --- a/modal-go/examples/sandbox-private-image/main.go +++ b/modal-go/examples/sandbox-private-image/main.go @@ -28,7 +28,7 @@ func main() { log.Fatalf("Failed to get Secret: %v", err) } - image := mc.Images.FromAwsEcr("459781239556.dkr.ecr.us-east-1.amazonaws.com/ecr-private-registry-test-7522615:python", secret) + image := mc.Images.FromAwsEcr("459781239556.dkr.ecr.us-east-1.amazonaws.com/ecr-private-registry-test-7522615:python", secret, nil) sb, err := mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{ Command: []string{"python", "-c", `import sys; sys.stdout.write(sys.stdin.read())`}, diff --git a/modal-go/image.go b/modal-go/image.go index c2e87b0a..767a488d 100644 --- a/modal-go/image.go +++ b/modal-go/image.go @@ -14,8 +14,8 @@ import ( // ImageService provides Image related operations. type ImageService interface { FromRegistry(tag string, params *ImageFromRegistryParams) *Image - FromAwsEcr(tag string, secret *Secret) *Image - FromGcpArtifactRegistry(tag string, secret *Secret) *Image + FromAwsEcr(tag string, secret *Secret, params *ImageFromAwsEcrParams) *Image + FromGcpArtifactRegistry(tag string, secret *Secret, params *ImageFromGcpArtifactRegistryParams) *Image FromID(ctx context.Context, imageID string) (*Image, error) Delete(ctx context.Context, imageID string, params *ImageDeleteParams) error } @@ -53,13 +53,30 @@ type Image struct { imageRegistryConfig *pb.ImageRegistryConfig tag string layers []layer + forceBuild bool client *Client } // ImageFromRegistryParams are options for creating an Image from a registry. type ImageFromRegistryParams struct { - Secret *Secret // Secret for private registry authentication. + Secret *Secret // Secret containing credentials for private registry authentication. + ForceBuild bool // Ignore cached builds, similar to 'docker build --no-cache'. +} + +// ImageFromAwsEcrParams are options for creating an Image from AWS ECR. +type ImageFromAwsEcrParams struct { + ForceBuild bool // Ignore cached builds, similar to 'docker build --no-cache'. +} + +// ImageFromGcpArtifactRegistryParams are options for creating an Image from GCP Artifact Registry. +type ImageFromGcpArtifactRegistryParams struct { + ForceBuild bool // Ignore cached builds, similar to 'docker build --no-cache'. +} + +// ImageBuildParams are options for Image.Build(). +type ImageBuildParams struct { + ForceBuild bool // Ignore cached builds, similar to 'docker build --no-cache'. } // FromRegistry builds a Modal Image from a public or private image registry without any changes. @@ -80,12 +97,16 @@ func (s *imageServiceImpl) FromRegistry(tag string, params *ImageFromRegistryPar imageRegistryConfig: imageRegistryConfig, tag: tag, layers: []layer{{}}, + forceBuild: params.ForceBuild, client: s.client, } } // FromAwsEcr creates an Image from an AWS ECR tag -func (s *imageServiceImpl) FromAwsEcr(tag string, secret *Secret) *Image { +func (s *imageServiceImpl) FromAwsEcr(tag string, secret *Secret, params *ImageFromAwsEcrParams) *Image { + if params == nil { + params = &ImageFromAwsEcrParams{} + } imageRegistryConfig := pb.ImageRegistryConfig_builder{ RegistryAuthType: pb.RegistryAuthType_REGISTRY_AUTH_TYPE_AWS, SecretId: secret.SecretID, @@ -96,12 +117,16 @@ func (s *imageServiceImpl) FromAwsEcr(tag string, secret *Secret) *Image { imageRegistryConfig: imageRegistryConfig, tag: tag, layers: []layer{{}}, + forceBuild: params.ForceBuild, client: s.client, } } // FromGcpArtifactRegistry creates an Image from a GCP Artifact Registry tag. -func (s *imageServiceImpl) FromGcpArtifactRegistry(tag string, secret *Secret) *Image { +func (s *imageServiceImpl) FromGcpArtifactRegistry(tag string, secret *Secret, params *ImageFromGcpArtifactRegistryParams) *Image { + if params == nil { + params = &ImageFromGcpArtifactRegistryParams{} + } imageRegistryConfig := pb.ImageRegistryConfig_builder{ RegistryAuthType: pb.RegistryAuthType_REGISTRY_AUTH_TYPE_GCP, SecretId: secret.SecretID, @@ -111,6 +136,7 @@ func (s *imageServiceImpl) FromGcpArtifactRegistry(tag string, secret *Secret) * imageRegistryConfig: imageRegistryConfig, tag: tag, layers: []layer{{}}, + forceBuild: params.ForceBuild, client: s.client, } } @@ -166,6 +192,7 @@ func (image *Image) DockerfileCommands(commands []string, params *ImageDockerfil tag: image.tag, imageRegistryConfig: image.imageRegistryConfig, layers: newLayers, + forceBuild: image.forceBuild, client: image.client, } } @@ -219,7 +246,7 @@ func (image *Image) waitForBuildIteration(ctx context.Context, imageID string, l } // Build eagerly builds an Image on Modal. -func (image *Image) Build(ctx context.Context, app *App) (*Image, error) { +func (image *Image) Build(ctx context.Context, app *App, params *ImageBuildParams) (*Image, error) { // Image is already hyrdated if image.ImageID != "" { return image, nil @@ -233,8 +260,12 @@ func (image *Image) Build(ctx context.Context, app *App) (*Image, error) { } } - var currentImageID string + forceBuild := image.client.profile.ForceBuild || image.forceBuild + if params != nil { + forceBuild = forceBuild || params.ForceBuild + } + var currentImageID string for i, currentLayer := range image.layers { if err := ctx.Err(); err != nil { return nil, err @@ -273,6 +304,8 @@ func (image *Image) Build(ctx context.Context, app *App) (*Image, error) { }.Build()} } + forceBuild = forceBuild || currentLayer.forceBuild + resp, err := image.client.cpClient.ImageGetOrCreate( ctx, pb.ImageGetOrCreateRequest_builder{ @@ -286,7 +319,7 @@ func (image *Image) Build(ctx context.Context, app *App) (*Image, error) { BaseImages: baseImages, }.Build(), BuilderVersion: imageBuilderVersion("", image.client.profile), - ForceBuild: currentLayer.forceBuild, + ForceBuild: forceBuild, }.Build(), ) if err != nil { diff --git a/modal-go/sandbox.go b/modal-go/sandbox.go index ab5bc7ae..d8455c04 100644 --- a/modal-go/sandbox.go +++ b/modal-go/sandbox.go @@ -288,7 +288,7 @@ func (s *sandboxServiceImpl) Create(ctx context.Context, app *App, image *Image, params = &SandboxCreateParams{} } - image, err := image.Build(ctx, app) + image, err := image.Build(ctx, app, nil) if err != nil { return nil, err } diff --git a/modal-go/test/image_test.go b/modal-go/test/image_test.go index 61272a7b..c77c201e 100644 --- a/modal-go/test/image_test.go +++ b/modal-go/test/image_test.go @@ -20,7 +20,7 @@ func TestImageFromId(t *testing.T) { app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true}) g.Expect(err).ShouldNot(gomega.HaveOccurred()) - image, err := tc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app) + image, err := tc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) imageFromID, err := tc.Images.FromID(ctx, image.ImageID) @@ -40,7 +40,7 @@ func TestImageFromRegistry(t *testing.T) { app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true}) g.Expect(err).ShouldNot(gomega.HaveOccurred()) - image, err := tc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app) + image, err := tc.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(image.ImageID).Should(gomega.HavePrefix("im-")) } @@ -66,7 +66,7 @@ func TestImageFromRegistryWithSecret(t *testing.T) { image, err := tc.Images.FromRegistry("us-east1-docker.pkg.dev/modal-prod-367916/private-repo-test/my-image", &modal.ImageFromRegistryParams{ Secret: secret, - }).Build(ctx, app) + }).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(image.ImageID).Should(gomega.HavePrefix("im-")) } @@ -85,7 +85,7 @@ func TestImageFromAwsEcr(t *testing.T) { }) g.Expect(err).ShouldNot(gomega.HaveOccurred()) - image, err := tc.Images.FromAwsEcr("459781239556.dkr.ecr.us-east-1.amazonaws.com/ecr-private-registry-test-7522615:python", secret).Build(ctx, app) + image, err := tc.Images.FromAwsEcr("459781239556.dkr.ecr.us-east-1.amazonaws.com/ecr-private-registry-test-7522615:python", secret, nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(image.ImageID).Should(gomega.HavePrefix("im-")) } @@ -104,7 +104,7 @@ func TestImageFromGcpArtifactRegistry(t *testing.T) { }) g.Expect(err).ShouldNot(gomega.HaveOccurred()) - image, err := tc.Images.FromGcpArtifactRegistry("us-east1-docker.pkg.dev/modal-prod-367916/private-repo-test/my-image", secret).Build(ctx, app) + image, err := tc.Images.FromGcpArtifactRegistry("us-east1-docker.pkg.dev/modal-prod-367916/private-repo-test/my-image", secret, nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(image.ImageID).Should(gomega.HavePrefix("im-")) } @@ -137,7 +137,7 @@ func TestImageDelete(t *testing.T) { app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true}) g.Expect(err).ShouldNot(gomega.HaveOccurred()) - image, err := tc.Images.FromRegistry("alpine:3.13", nil).Build(ctx, app) + image, err := tc.Images.FromRegistry("alpine:3.13", nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(image.ImageID).Should(gomega.HavePrefix("im-")) @@ -151,7 +151,7 @@ func TestImageDelete(t *testing.T) { _, err = tc.Images.FromID(ctx, image.ImageID) g.Expect(err).Should(gomega.MatchError(gomega.MatchRegexp("Image .+ not found"))) - newImage, err := tc.Images.FromRegistry("alpine:3.13", nil).Build(ctx, app) + newImage, err := tc.Images.FromRegistry("alpine:3.13", nil).Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(newImage.ImageID).ShouldNot(gomega.Equal(image.ImageID)) @@ -248,14 +248,14 @@ func TestDockerfileCommandsCopyCommandValidation(t *testing.T) { []string{"COPY --from=alpine:latest /etc/os-release /tmp/os-release"}, nil, ) - _, err = validImage.Build(ctx, app) + _, err = validImage.Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) invalidImage := image.DockerfileCommands( []string{"COPY ./file.txt /root/"}, nil, ) - _, err = invalidImage.Build(ctx, app) + _, err = invalidImage.Build(ctx, app, nil) g.Expect(err).Should(gomega.HaveOccurred()) g.Expect(err.Error()).Should(gomega.ContainSubstring("COPY commands that copy from local context are not yet supported")) @@ -263,7 +263,7 @@ func TestDockerfileCommandsCopyCommandValidation(t *testing.T) { []string{"RUN echo 'COPY ./file.txt /root/'"}, nil, ) - _, err = runImage.Build(ctx, app) + _, err = runImage.Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) multiInvalidImage := image.DockerfileCommands( @@ -274,7 +274,7 @@ func TestDockerfileCommandsCopyCommandValidation(t *testing.T) { }, nil, ) - _, err = multiInvalidImage.Build(ctx, app) + _, err = multiInvalidImage.Build(ctx, app, nil) g.Expect(err).Should(gomega.HaveOccurred()) g.Expect(err.Error()).Should(gomega.ContainSubstring("COPY commands that copy from local context are not yet supported")) } @@ -376,10 +376,238 @@ func TestDockerfileCommandsWithOptions(t *testing.T) { DockerfileCommands([]string{"RUN echo layer3"}, &modal.ImageDockerfileCommandsParams{ ForceBuild: true, }). - Build(ctx, app) + Build(ctx, app, nil) g.Expect(err).ShouldNot(gomega.HaveOccurred()) g.Expect(builtImage.ImageID).To(gomega.Equal("im-layer3")) g.Expect(mock.AssertExhausted()).ShouldNot(gomega.HaveOccurred()) } + +func TestForceBuildOnImageCreation(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + buildImage func(ctx context.Context, mock *grpcmock.MockClient, app *modal.App) (*modal.Image, error) + }{ + { + name: "FromRegistry", + buildImage: func(ctx context.Context, mock *grpcmock.MockClient, app *modal.App) (*modal.Image, error) { + return mock.Images.FromRegistry("alpine:3.21", &modal.ImageFromRegistryParams{ + ForceBuild: true, + }).Build(ctx, app, nil) + }, + }, + { + name: "FromAwsEcr", + buildImage: func(ctx context.Context, mock *grpcmock.MockClient, app *modal.App) (*modal.Image, error) { + secret := &modal.Secret{SecretID: "sc-test"} + return mock.Images.FromAwsEcr("alpine:3.21", secret, &modal.ImageFromAwsEcrParams{ + ForceBuild: true, + }).Build(ctx, app, nil) + }, + }, + { + name: "FromGcpArtifactRegistry", + buildImage: func(ctx context.Context, mock *grpcmock.MockClient, app *modal.App) (*modal.Image, error) { + secret := &modal.Secret{SecretID: "sc-test"} + return mock.Images.FromGcpArtifactRegistry("alpine:3.21", secret, &modal.ImageFromGcpArtifactRegistryParams{ + ForceBuild: true, + }).Build(ctx, app, nil) + }, + }, + { + name: "BuildParams", + buildImage: func(ctx context.Context, mock *grpcmock.MockClient, app *modal.App) (*modal.Image, error) { + return mock.Images.FromRegistry("alpine:3.21", nil).Build(ctx, app, &modal.ImageBuildParams{ + ForceBuild: true, + }) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := gomega.NewWithT(t) + ctx := context.Background() + + mock := newGRPCMockClient(t) + + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + g.Expect(req.GetAppId()).To(gomega.Equal("ap-test")) + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-test", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + app := &modal.App{AppID: "ap-test"} + image, err := tc.buildImage(ctx, mock, app) + + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + g.Expect(image.ImageID).To(gomega.Equal("im-test")) + g.Expect(mock.AssertExhausted()).ShouldNot(gomega.HaveOccurred()) + }) + } +} + +func TestForceBuildPropagation(t *testing.T) { + t.Parallel() + g := gomega.NewWithT(t) + ctx := context.Background() + + mock := newGRPCMockClient(t) + + // from FromRegistry, without ForceBuild + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + image := req.GetImage() + g.Expect(image.GetDockerfileCommands()).To(gomega.Equal([]string{"FROM alpine:3.21"})) + g.Expect(req.GetForceBuild()).To(gomega.BeFalse()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-base", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + // from DockerfileCommands, with ForceBuild: true + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + image := req.GetImage() + g.Expect(image.GetDockerfileCommands()).To(gomega.Equal([]string{"FROM base", "RUN echo test"})) + g.Expect(image.GetBaseImages()).To(gomega.HaveLen(1)) + g.Expect(image.GetBaseImages()[0].GetImageId()).To(gomega.Equal("im-base")) + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-layer1", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + // from second DockerfileCommands, without ForceBuild, should still have ForceBuild: true, propagated from previous layer + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + image := req.GetImage() + g.Expect(image.GetDockerfileCommands()).To(gomega.Equal([]string{"FROM base", "RUN echo test2"})) + g.Expect(image.GetBaseImages()).To(gomega.HaveLen(1)) + g.Expect(image.GetBaseImages()[0].GetImageId()).To(gomega.Equal("im-layer1")) + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-layer2", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + app := &modal.App{AppID: "ap-test"} + image, err := mock.Images.FromRegistry("alpine:3.21", nil). + DockerfileCommands([]string{"RUN echo test"}, &modal.ImageDockerfileCommandsParams{ + ForceBuild: true, + }). + DockerfileCommands([]string{"RUN echo test2"}, nil). + Build(ctx, app, nil) + + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + g.Expect(image.ImageID).To(gomega.Equal("im-layer2")) + g.Expect(mock.AssertExhausted()).ShouldNot(gomega.HaveOccurred()) +} + +func TestForceBuildBackPropagation(t *testing.T) { + t.Parallel() + g := gomega.NewWithT(t) + ctx := context.Background() + + mock := newGRPCMockClient(t) + + // from FromRegistry, without ForceBuild, should still have ForceBuild: true, overridden by .Build + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + image := req.GetImage() + g.Expect(image.GetDockerfileCommands()).To(gomega.Equal([]string{"FROM alpine:3.21"})) + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-base", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + // from DockerfileCommands, without ForceBuild, should still have ForceBuild: true, overridden by .Build + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + image := req.GetImage() + g.Expect(image.GetDockerfileCommands()).To(gomega.Equal([]string{"FROM base", "RUN echo test"})) + g.Expect(image.GetBaseImages()).To(gomega.HaveLen(1)) + g.Expect(image.GetBaseImages()[0].GetImageId()).To(gomega.Equal("im-base")) + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-layer1", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + app := &modal.App{AppID: "ap-test"} + image, err := mock.Images.FromRegistry("alpine:3.21", nil). + DockerfileCommands([]string{"RUN echo test"}, nil). + Build(ctx, app, &modal.ImageBuildParams{ + ForceBuild: true, + }) + + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + g.Expect(image.ImageID).To(gomega.Equal("im-layer1")) + g.Expect(mock.AssertExhausted()).ShouldNot(gomega.HaveOccurred()) +} + +func TestForceBuildFromEnvironmentVariable(t *testing.T) { + g := gomega.NewWithT(t) + ctx := context.Background() + + t.Setenv("MODAL_FORCE_BUILD", "true") + + mock := newGRPCMockClient(t) + + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-base", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + grpcmock.HandleUnary( + mock, "ImageGetOrCreate", + func(req *pb.ImageGetOrCreateRequest) (*pb.ImageGetOrCreateResponse, error) { + g.Expect(req.GetForceBuild()).To(gomega.BeTrue()) + return pb.ImageGetOrCreateResponse_builder{ + ImageId: "im-layer1", + Result: pb.GenericResult_builder{Status: pb.GenericResult_GENERIC_STATUS_SUCCESS}.Build(), + }.Build(), nil + }, + ) + + app := &modal.App{AppID: "ap-test"} + image, err := mock.Images.FromRegistry("alpine:3.21", nil). + DockerfileCommands([]string{"RUN echo test"}, nil). + Build(ctx, app, nil) + + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + g.Expect(image.ImageID).To(gomega.Equal("im-layer1")) + g.Expect(mock.AssertExhausted()).ShouldNot(gomega.HaveOccurred()) +} diff --git a/modal-js/src/app.ts b/modal-js/src/app.ts index 39291165..98b12716 100644 --- a/modal-js/src/app.ts +++ b/modal-js/src/app.ts @@ -135,7 +135,7 @@ export class App { * @deprecated Use {@link ImageService#fromRegistry client.images.fromRegistry()} instead. */ async imageFromRegistry(tag: string, secret?: Secret): Promise { - return getDefaultClient().images.fromRegistry(tag, secret).build(this); + return getDefaultClient().images.fromRegistry(tag, { secret }).build(this); } /** diff --git a/modal-js/src/config.ts b/modal-js/src/config.ts index a7bcd555..e9d944c5 100644 --- a/modal-js/src/config.ts +++ b/modal-js/src/config.ts @@ -13,6 +13,7 @@ interface Config { environment?: string; imageBuilderVersion?: string; loglevel?: string; + force_build?: boolean; active?: boolean; }; } @@ -25,6 +26,7 @@ export interface Profile { environment?: string; imageBuilderVersion?: string; logLevel?: string; + forceBuild?: boolean; } export function configFilePath(): string { @@ -59,6 +61,12 @@ function readConfigFile(): Config { // synchronously, for instance. const config: Config = readConfigFile(); +function toBoolean(value: string | undefined): boolean { + if (!value) return false; + const lower = value.toLowerCase(); + return lower !== "" && lower !== "0" && lower !== "false"; +} + export function getProfile(profileName?: string): Profile { if (!profileName) { for (const [name, profileData] of Object.entries(config)) { @@ -85,6 +93,8 @@ export function getProfile(profileName?: string): Profile { process.env["MODAL_IMAGE_BUILDER_VERSION"] || profileData.imageBuilderVersion, logLevel: process.env["MODAL_LOGLEVEL"] || profileData.loglevel, + forceBuild: + toBoolean(process.env["MODAL_FORCE_BUILD"]) || profileData.force_build, }; return profile as Profile; // safe to null-cast because of check above } diff --git a/modal-js/src/image.ts b/modal-js/src/image.ts index 622bc8cd..e59a4bf7 100644 --- a/modal-js/src/image.ts +++ b/modal-js/src/image.ts @@ -50,26 +50,50 @@ export class ImageService { } } + /** + * Creates an {@link Image} from a raw registry tag, optionally using a {@link Secret} for authentication. + * + * @param tag - The registry tag for the Image. + * @param params - Optional configuration parameters including Secret for authentication. + */ + fromRegistry(tag: string, params?: ImageFromRegistryParams): Image; /** * Creates an {@link Image} from a raw registry tag, optionally using a {@link Secret} for authentication. * * @param tag - The registry tag for the Image. * @param secret - Optional. A Secret containing credentials for registry authentication. + * @deprecated Pass secret via params instead: `fromRegistry(tag, { secret })` */ - fromRegistry(tag: string, secret?: Secret): Image { + fromRegistry(tag: string, secret?: Secret): Image; + fromRegistry( + tag: string, + secretOrParams?: Secret | ImageFromRegistryParams, + ): Image { + let secret: Secret | undefined; + let params: ImageFromRegistryParams | undefined; + + if (secretOrParams instanceof Secret) { + secret = secretOrParams; + } else { + params = secretOrParams; + secret = params?.secret; + } + let imageRegistryConfig; if (secret) { - if (!(secret instanceof Secret)) { - throw new TypeError( - "secret must be a reference to an existing Secret, e.g. `await Secret.fromName('my_secret')`", - ); - } imageRegistryConfig = { registryAuthType: RegistryAuthType.REGISTRY_AUTH_TYPE_STATIC_CREDS, secretId: secret.secretId, }; } - return new Image(this.#client, "", tag, imageRegistryConfig); + return new Image( + this.#client, + "", + tag, + imageRegistryConfig, + undefined, + params?.forceBuild, + ); } /** @@ -77,8 +101,13 @@ export class ImageService { * * @param tag - The registry tag for the Image. * @param secret - A Secret containing credentials for registry authentication. + * @param params - Optional configuration parameters. */ - fromAwsEcr(tag: string, secret: Secret): Image { + fromAwsEcr( + tag: string, + secret: Secret, + params?: ImageFromAwsEcrParams, + ): Image { let imageRegistryConfig; if (secret) { if (!(secret instanceof Secret)) { @@ -91,7 +120,14 @@ export class ImageService { secretId: secret.secretId, }; } - return new Image(this.#client, "", tag, imageRegistryConfig); + return new Image( + this.#client, + "", + tag, + imageRegistryConfig, + undefined, + params?.forceBuild, + ); } /** @@ -99,8 +135,13 @@ export class ImageService { * * @param tag - The registry tag for the Image. * @param secret - A Secret containing credentials for registry authentication. + * @param params - Optional configuration parameters. */ - fromGcpArtifactRegistry(tag: string, secret: Secret): Image { + fromGcpArtifactRegistry( + tag: string, + secret: Secret, + params?: ImageFromGcpArtifactRegistryParams, + ): Image { let imageRegistryConfig; if (secret) { if (!(secret instanceof Secret)) { @@ -113,7 +154,14 @@ export class ImageService { secretId: secret.secretId, }; } - return new Image(this.#client, "", tag, imageRegistryConfig); + return new Image( + this.#client, + "", + tag, + imageRegistryConfig, + undefined, + params?.forceBuild, + ); } /** @@ -146,6 +194,32 @@ export class ImageService { /** Optional parameters for {@link ImageService#delete client.images.delete()}. */ export type ImageDeleteParams = Record; +/** Optional parameters for {@link ImageService#fromRegistry client.images.fromRegistry()}. */ +export type ImageFromRegistryParams = { + /** Secret containing credentials for private registry authentication. */ + secret?: Secret; + /** Ignore cached builds, similar to 'docker build --no-cache'. */ + forceBuild?: boolean; +}; + +/** Optional parameters for {@link ImageService#fromAwsEcr client.images.fromAwsEcr()}. */ +export type ImageFromAwsEcrParams = { + /** Ignore cached builds, similar to 'docker build --no-cache'. */ + forceBuild?: boolean; +}; + +/** Optional parameters for {@link ImageService#fromGcpArtifactRegistry client.images.fromGcpArtifactRegistry()}. */ +export type ImageFromGcpArtifactRegistryParams = { + /** Ignore cached builds, similar to 'docker build --no-cache'. */ + forceBuild?: boolean; +}; + +/** Optional parameters for {@link Image#build Image.build()}. */ +export type ImageBuildParams = { + /** Ignore cached builds, similar to 'docker build --no-cache'. */ + forceBuild?: boolean; +}; + /** Optional parameters for {@link Image#dockerfileCommands Image.dockerfileCommands()}. */ export type ImageDockerfileCommandsParams = { /** Environment variables to set in the build environment. */ @@ -177,6 +251,7 @@ export class Image { #tag: string; #imageRegistryConfig?: ImageRegistryConfig; #layers: Layer[]; + #forceBuild: boolean; /** @ignore */ constructor( @@ -185,6 +260,7 @@ export class Image { tag: string, imageRegistryConfig?: ImageRegistryConfig, layers?: Layer[], + forceBuild?: boolean, ) { this.#client = client; this.#imageId = imageId; @@ -199,6 +275,7 @@ export class Image { forceBuild: false, }, ]; + this.#forceBuild = forceBuild || false; } get imageId(): string { return this.#imageId; @@ -215,7 +292,7 @@ export class Image { * @deprecated Use {@link ImageService#fromRegistry client.images.fromRegistry()} instead. */ static fromRegistry(tag: string, secret?: Secret): Image { - return getDefaultClient().images.fromRegistry(tag, secret); + return getDefaultClient().images.fromRegistry(tag, { secret }); } /** @@ -271,18 +348,23 @@ export class Image { forceBuild: params?.forceBuild, }; - return new Image(this.#client, "", this.#tag, this.#imageRegistryConfig, [ - ...this.#layers, - newLayer, - ]); + return new Image( + this.#client, + "", + this.#tag, + this.#imageRegistryConfig, + [...this.#layers, newLayer], + this.#forceBuild, + ); } /** * Eagerly builds an Image on Modal. * * @param app - App to use to build the Image. + * @param params - Optional configuration parameters. */ - async build(app: App): Promise { + async build(app: App, params?: ImageBuildParams): Promise { if (this.imageId !== "") { // Image is already built with an Image ID return this; @@ -291,6 +373,11 @@ export class Image { this.#client.logger.debug("Building image", "app_id", app.appId); let baseImageId: string | undefined; + let forceBuild = + this.#client.profile.forceBuild || + this.#forceBuild || + params?.forceBuild || + false; for (let i = 0; i < this.#layers.length; i++) { const layer = this.#layers[i]; @@ -315,6 +402,8 @@ export class Image { baseImages = [{ dockerTag: "base", imageId: baseImageId! }]; } + forceBuild = forceBuild || layer.forceBuild || false; + const resp = await this.#client.cpClient.imageGetOrCreate({ appId: app.appId, image: ImageProto.create({ @@ -326,7 +415,7 @@ export class Image { baseImages, }), builderVersion: this.#client.imageBuilderVersion(), - forceBuild: layer.forceBuild || false, + forceBuild, }); let result: GenericResult; diff --git a/modal-js/src/index.ts b/modal-js/src/index.ts index 068fc200..c8eb31e3 100644 --- a/modal-js/src/index.ts +++ b/modal-js/src/index.ts @@ -55,8 +55,12 @@ export { export { Image, ImageService, + type ImageBuildParams, type ImageDeleteParams, type ImageDockerfileCommandsParams, + type ImageFromAwsEcrParams, + type ImageFromGcpArtifactRegistryParams, + type ImageFromRegistryParams, } from "./image"; export { Retries } from "./retries"; export type { diff --git a/modal-js/test/image.test.ts b/modal-js/test/image.test.ts index 1d34d02e..1d1729f1 100644 --- a/modal-js/test/image.test.ts +++ b/modal-js/test/image.test.ts @@ -1,5 +1,5 @@ import { tc } from "../test-support/test-client"; -import { expect, onTestFinished, test } from "vitest"; +import { expect, onTestFinished, test, vi } from "vitest"; import { createMockModalClients } from "../test-support/grpc_mock"; import { Secret } from "../src/secret"; import { App } from "../src/app"; @@ -41,9 +41,14 @@ test("ImageFromRegistryWithSecret", async () => { const image = await tc.images .fromRegistry( "us-east1-docker.pkg.dev/modal-prod-367916/private-repo-test/my-image", - await tc.secrets.fromName("libmodal-gcp-artifact-registry-test", { - requiredKeys: ["REGISTRY_USERNAME", "REGISTRY_PASSWORD"], - }), + { + secret: await tc.secrets.fromName( + "libmodal-gcp-artifact-registry-test", + { + requiredKeys: ["REGISTRY_USERNAME", "REGISTRY_PASSWORD"], + }, + ), + }, ) .build(app); expect(image.imageId).toBeTruthy(); @@ -278,3 +283,187 @@ test("DockerfileCommandsWithOptions", async () => { mock.assertExhausted(); }); + +test.each([ + { + name: "FromRegistryWithForceBuild", + createImage: ( + mc: ReturnType["mockClient"], + ) => + mc.images + .fromRegistry("alpine:3.21", { forceBuild: true }) + .build(new App("ap-test", "libmodal-test")), + }, + { + name: "FromAwsEcrWithForceBuild", + createImage: ( + mc: ReturnType["mockClient"], + ) => + mc.images + .fromAwsEcr("alpine:3.21", new Secret("sc-test", "test_secret"), { + forceBuild: true, + }) + .build(new App("ap-test", "libmodal-test")), + }, + { + name: "FromGcpArtifactRegistryWithForceBuild", + createImage: ( + mc: ReturnType["mockClient"], + ) => + mc.images + .fromGcpArtifactRegistry( + "alpine:3.21", + new Secret("sc-test", "test_secret"), + { forceBuild: true }, + ) + .build(new App("ap-test", "libmodal-test")), + }, + { + name: "BuildWithForceBuildParam", + createImage: ( + mc: ReturnType["mockClient"], + ) => + mc.images + .fromRegistry("alpine:3.21") + .build(new App("ap-test", "libmodal-test"), { forceBuild: true }), + }, +])("$name", async ({ createImage }) => { + const { mockClient: mc, mockCpClient: mock } = createMockModalClients(); + + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + forceBuild: true, + }); + return { imageId: "im-test", result: { status: 1 } }; + }); + + const image = await createImage(mc); + + expect(image.imageId).toBe("im-test"); + mock.assertExhausted(); +}); + +test("ForceBuildPropagation", async () => { + const { mockClient: mc, mockCpClient: mock } = createMockModalClients(); + + // from fromRegistry, without forceBuild + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM alpine:3.21"], + }, + forceBuild: false, + }); + return { imageId: "im-base", result: { status: 1 } }; + }); + + // from dockerfileCommands, with forceBuild: true + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM base", "RUN echo test"], + baseImages: [{ dockerTag: "base", imageId: "im-base" }], + }, + forceBuild: true, + }); + return { imageId: "im-layer1", result: { status: 1 } }; + }); + + // from second dockerfileCommands, without forceBuild, should still have forceBuild: true, propagated from previous layer + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM base", "RUN echo test2"], + baseImages: [{ dockerTag: "base", imageId: "im-layer1" }], + }, + forceBuild: true, + }); + return { imageId: "im-layer2", result: { status: 1 } }; + }); + + const image = await mc.images + .fromRegistry("alpine:3.21") + .dockerfileCommands(["RUN echo test"], { forceBuild: true }) + .dockerfileCommands(["RUN echo test2"]) + .build(new App("ap-test", "libmodal-test")); + + expect(image.imageId).toBe("im-layer2"); + mock.assertExhausted(); +}); + +test("ForceBuildBackPropagation", async () => { + const { mockClient: mc, mockCpClient: mock } = createMockModalClients(); + + // from fromRegistry, without forceBuild, should still have forceBuild: true, overridden by .build + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM alpine:3.21"], + }, + forceBuild: true, + }); + return { imageId: "im-base", result: { status: 1 } }; + }); + + // from dockerfileCommands, without forceBuild, should still have forceBuild: true, overridden by .build + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM base", "RUN echo test"], + baseImages: [{ dockerTag: "base", imageId: "im-base" }], + }, + forceBuild: true, + }); + return { imageId: "im-layer1", result: { status: 1 } }; + }); + + const image = await mc.images + .fromRegistry("alpine:3.21", undefined) + .dockerfileCommands(["RUN echo test"]) + .build(new App("ap-test", "libmodal-test"), { forceBuild: true }); + + expect(image.imageId).toBe("im-layer1"); + mock.assertExhausted(); +}); + +test("ForceBuildFromEnvironmentVariable", async () => { + vi.stubEnv("MODAL_FORCE_BUILD", "true"); + + const { mockClient: mc, mockCpClient: mock } = createMockModalClients(); + + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + forceBuild: true, + }); + return { imageId: "im-base", result: { status: 1 } }; + }); + + mock.handleUnary("/ImageGetOrCreate", (req: any) => { + expect(req).toMatchObject({ + appId: "ap-test", + image: { + dockerfileCommands: ["FROM base", "RUN echo test"], + baseImages: [{ dockerTag: "base", imageId: "im-base" }], + }, + forceBuild: true, + }); + return { imageId: "im-layer1", result: { status: 1 } }; + }); + + const image = await mc.images + .fromRegistry("alpine:3.21") + .dockerfileCommands(["RUN echo test"]) + .build(new App("ap-test", "libmodal-test")); + + expect(image.imageId).toBe("im-layer1"); + mock.assertExhausted(); + + vi.unstubAllEnvs(); +});