Handle unknown inputs and buildOnPreview

provider/cmd/pulumi-resource-docker/schema.json

@@ -2105,6 +2105,10 @@
                     },
                     "description": "\nAn optional map of named build-time argument variables to set during\nthe Docker build. This flag allows you to pass build-time variables that\ncan be accessed like environment variables inside the RUN\ninstruction."
                 },
+                "buildOnPreview": {
+                    "type": "boolean",
+                    "description": "\nWhen true, attempt to build the image during previews. Outputs are not\npushed to registries, however caches are still populated.\n"
+                },
                 "builder": {
                     "type": "string",
                     "description": "\nBuild with a specific builder instance"
@@ -2186,6 +2190,10 @@
                     },
                     "description": "\nAn optional map of named build-time argument variables to set during\nthe Docker build. This flag allows you to pass build-time variables that\ncan be accessed like environment variables inside the RUN\ninstruction."
                 },
+                "buildOnPreview": {
+                    "type": "boolean",
+                    "description": "\nWhen true, attempt to build the image during previews. Outputs are not\npushed to registries, however caches are still populated.\n"
+                },
                 "builder": {
                     "type": "string",
                     "description": "\nBuild with a specific builder instance"

provider/internal/image.go

@@ -51,17 +51,18 @@ func (i *Image) Annotate(a infer.Annotator) {
 
 // ImageArgs instantiates a new Image.
 type ImageArgs struct {
-	BuildArgs  map[string]string         `pulumi:"buildArgs,optional"`
-	Builder    string                    `pulumi:"builder,optional"`
-	CacheFrom  []string                  `pulumi:"cacheFrom,optional"`
-	CacheTo    []string                  `pulumi:"cacheTo,optional"`
-	Context    string                    `pulumi:"context,optional"`
-	Exports    []string                  `pulumi:"exports,optional"`
-	File       string                    `pulumi:"file,optional"`
-	Platforms  []string                  `pulumi:"platforms,optional"`
-	Pull       bool                      `pulumi:"pull,optional"`
-	Registries []properties.RegistryAuth `pulumi:"registries,optional"`
-	Tags       []string                  `pulumi:"tags"`
+	BuildArgs      map[string]string         `pulumi:"buildArgs,optional"`
+	Builder        string                    `pulumi:"builder,optional"`
+	BuildOnPreview bool                      `pulumi:"buildOnPreview,optional"`
+	CacheFrom      []string                  `pulumi:"cacheFrom,optional"`
+	CacheTo        []string                  `pulumi:"cacheTo,optional"`
+	Context        string                    `pulumi:"context,optional"`
+	Exports        []string                  `pulumi:"exports,optional"`
+	File           string                    `pulumi:"file,optional"`
+	Platforms      []string                  `pulumi:"platforms,optional"`
+	Pull           bool                      `pulumi:"pull,optional"`
+	Registries     []properties.RegistryAuth `pulumi:"registries,optional"`
+	Tags           []string                  `pulumi:"tags"`
 }
 
 // Annotate describes inputs to the Image resource.
@@ -75,6 +76,10 @@ func (ia *ImageArgs) Annotate(a infer.Annotator) {
 	a.Describe(&ia.Builder, dedent.String(`
 		Build with a specific builder instance`,
 	))
+	a.Describe(&ia.BuildOnPreview, dedent.String(`
+		When true, attempt to build the image during previews. Outputs are not
+		pushed to registries, however caches are still populated.
+	`))
 	a.Describe(&ia.CacheFrom, dedent.String(`
 		External cache sources (e.g., "user/app:cache", "type=local,src=path/to/dir")`,
 	))
@@ -135,7 +140,11 @@ func (*Image) Check(
 	if err != nil || len(failures) != 0 {
 		return args, failures, err
 	}
-	if _, berr := args.toBuildOptions(); berr != nil {
+
+	// :(
+	preview := news.ContainsUnknowns()
+
+	if _, berr := args.toBuildOptions(preview); berr != nil {
 		errs := berr.(interface{ Unwrap() []error }).Unwrap()
 		for _, e := range errs {
 			if cf, ok := e.(checkFailure); ok {
@@ -173,62 +182,92 @@ func newCheckFailure(property string, err error) checkFailure {
 	return checkFailure{provider.CheckFailure{Property: property, Reason: err.Error()}}
 }
 
-func (ia *ImageArgs) toBuildOptions() (controllerapi.BuildOptions, error) {
+func (ia *ImageArgs) withoutUnknowns(preview bool) ImageArgs {
+	sk := stringKeeper{preview}
+	filtered := ImageArgs{
+		BuildArgs:      make(map[string]string),
+		Builder:        ia.Builder,
+		BuildOnPreview: ia.BuildOnPreview,
+		CacheFrom:      filter(sk, ia.CacheFrom...),
+		CacheTo:        filter(sk, ia.CacheTo...),
+		Context:        ia.Context,
+		Exports:        filter(exportKeeper{preview}, ia.Exports...),
+		File:           ia.File, //
+		Platforms:      filter(sk, ia.Platforms...),
+		Pull:           ia.Pull,
+		Registries:     filter(registryKeeper{preview}, ia.Registries...),
+		Tags:           filter(sk, ia.Tags...),
+	}
+	if m, ok := (argKeeper{preview}).keep(ia.BuildArgs); ok {
+		filtered.BuildArgs = m
+	}
+
+	return filtered
+}
+
+func (ia *ImageArgs) buildable() bool {
+	filtered := ia.withoutUnknowns(true)
+	return reflect.DeepEqual(ia, &filtered)
+}
+
+func (ia *ImageArgs) toBuildOptions(preview bool) (controllerapi.BuildOptions, error) {
 	var multierr error
-	exports, err := buildflags.ParseExports(ia.Exports)
+
+	if len(ia.Tags) == 0 {
+		multierr = errors.Join(multierr, newCheckFailure("tags", errors.New("at least one tag is required")))
+	}
+
+	// TODO(https://github.com/pulumi/pulumi-docker/issues/860): Empty build context
+	if ia.Context != "" && !preview {
+		if ia.File == "" {
+			ia.File = filepath.Join(ia.Context, "Dockerfile")
+		}
+		if _, err := os.Stat(ia.File); err != nil {
+			multierr = errors.Join(multierr, newCheckFailure("context", fmt.Errorf("%q: %w", ia.File, err)))
+		}
+	}
+
+	// Discard any unknown inputs if this is a preview -- we don't want them to
+	// cause validation errors.
+	filtered := ia.withoutUnknowns(preview)
+
+	exports, err := buildflags.ParseExports(filtered.Exports)
 	if err != nil {
 		multierr = errors.Join(multierr, newCheckFailure("exports", err))
 	}
 
-	_, err = platformutil.Parse(ia.Platforms)
+	_, err = platformutil.Parse(filtered.Platforms)
 	if err != nil {
 		multierr = errors.Join(multierr, newCheckFailure("platforms", err))
 	}
 
-	cacheFrom, err := buildflags.ParseCacheEntry(ia.CacheFrom)
+	cacheFrom, err := buildflags.ParseCacheEntry(filtered.CacheFrom)
 	if err != nil {
 		multierr = errors.Join(multierr, newCheckFailure("cacheFrom", err))
 	}
 
-	cacheTo, err := buildflags.ParseCacheEntry(ia.CacheTo)
+	cacheTo, err := buildflags.ParseCacheEntry(filtered.CacheTo)
 	if err != nil {
 		multierr = errors.Join(multierr, newCheckFailure("cacheTo", err))
 	}
 
-	// TODO(https://github.com/pulumi/pulumi-docker/issues/860): Empty build context
-	if ia.Context != "" {
-		if ia.File == "" {
-			ia.File = filepath.Join(ia.Context, "Dockerfile")
-		}
-		if _, err := os.Stat(ia.File); err != nil {
-			multierr = errors.Join(multierr, newCheckFailure("context", err))
-		}
-	}
-
-	if len(ia.Tags) == 0 {
-		multierr = errors.Join(multierr, newCheckFailure("tags", errors.New("at least one tag is required")))
-	}
-	for _, t := range ia.Tags {
-		if t == "" {
-			// TODO(https://github.com/pulumi/pulumi-go-provider/pull/155): This is likely unresolved.
-			continue
-		}
+	for _, t := range filtered.Tags {
 		if _, err := reference.Parse(t); err != nil {
 			multierr = errors.Join(multierr, newCheckFailure("tags", err))
 		}
 	}
 
 	opts := controllerapi.BuildOptions{
-		BuildArgs:      ia.BuildArgs,
-		Builder:        ia.Builder,
+		BuildArgs:      filtered.BuildArgs,
+		Builder:        filtered.Builder,
 		CacheFrom:      cacheFrom,
 		CacheTo:        cacheTo,
-		ContextPath:    ia.Context,
-		DockerfileName: ia.File,
+		ContextPath:    filtered.Context,
+		DockerfileName: filtered.File,
 		Exports:        exports,
-		Platforms:      ia.Platforms,
-		Pull:           ia.Pull,
-		Tags:           ia.Tags,
+		Platforms:      filtered.Platforms,
+		Pull:           filtered.Pull,
+		Tags:           filtered.Tags,
 	}
 
 	return opts, multierr
@@ -254,7 +293,7 @@ func (i *Image) Update(
 		return state, fmt.Errorf("buildkit is not supported on this host")
 	}
 
-	opts, err := input.toBuildOptions()
+	opts, err := input.toBuildOptions(preview)
 	if err != nil {
 		return state, fmt.Errorf("validating input: %w", err)
 	}
@@ -265,7 +304,11 @@ func (i *Image) Update(
 	}
 	state.ContextHash = hash
 
-	if preview {
+	if preview && !input.BuildOnPreview {
+		return state, nil
+	}
+	if preview && !input.buildable() {
+		ctx.Log(diag.Warning, "Skipping preview build because some inputs are unknown.")
 		return state, nil
 	}
 
@@ -318,7 +361,7 @@ func (*Image) Read(
 	ImageState, // normalized state
 	error,
 ) {
-	opts, err := input.toBuildOptions()
+	opts, err := input.toBuildOptions(false)
 	if err != nil {
 		return id, input, state, err
 	}
@@ -331,6 +374,7 @@ func (*Image) Read(
 		}
 	}
 
+	expectManifest := false
 	manifests := []properties.Manifest{}
 	for _, export := range opts.Exports {
 		switch export.GetType() {
@@ -340,6 +384,7 @@ func (*Image) Read(
 				continue
 			}
 			for _, tag := range input.Tags {
+				expectManifest = true
 				infos, err := cfg.client.Inspect(ctx, tag)
 				if err != nil {
 					continue
@@ -372,6 +417,12 @@ func (*Image) Read(
 		}
 	}
 
+	// If we couldn't find the tags we expected then return an empty ID to
+	// delete the resource.
+	if expectManifest && len(manifests) == 0 {
+		return "", input, state, nil
+	}
+
 	state.id = id
 	state.Manifests = manifests
 

provider/internal/image_test.go

@@ -36,6 +36,9 @@ func TestLifecycle(t *testing.T) {
 		return mock.NewMockClient(ctrl)
 	}
 
+	ref, err := reference.ParseNamed("docker.io/pulumibot/myapp")
+	require.NoError(t, err)
+
 	tests := []struct {
 		name string
 
@@ -58,7 +61,12 @@ func TestLifecycle(t *testing.T) {
 						},
 					),
 					c.EXPECT().Inspect(gomock.Any(), "docker.io/blampe/buildkit-e2e").Return(
-						[]manifesttypes.ImageManifest{}, nil,
+						[]manifesttypes.ImageManifest{
+							{
+								Ref:        &manifesttypes.SerializableNamed{Named: ref},
+								Descriptor: v1.Descriptor{Platform: &v1.Platform{}},
+							},
+						}, nil,
 					),
 					c.EXPECT().Inspect(gomock.Any(), "docker.io/blampe/buildkit-e2e:main"),
 					c.EXPECT().Delete(gomock.Any(), "SHA256:digest").Return(
@@ -501,21 +509,76 @@ func TestDiff(t *testing.T) {
 	}
 }
 
-func TestBuildOptionParsing(t *testing.T) {
-	args := ImageArgs{
-		Tags:      []string{"a/bad:tag:format"},
-		Exports:   []string{"badexport,-"},
-		Context:   "does/not/exist",
-		Platforms: []string{","},
-		CacheFrom: []string{"=badcachefrom"},
-		CacheTo:   []string{"=badcacheto"},
-	}
+func TestBuildOptions(t *testing.T) {
+	t.Run("invalid inputs", func(t *testing.T) {
+		args := ImageArgs{
+			Tags:      []string{"a/bad:tag:format"},
+			Exports:   []string{"badexport,-"},
+			Context:   "does/not/exist",
+			Platforms: []string{","},
+			CacheFrom: []string{"=badcachefrom"},
+			CacheTo:   []string{"=badcacheto"},
+		}
 
-	_, err := args.toBuildOptions()
-	assert.ErrorContains(t, err, "invalid value badexport")
-	assert.ErrorContains(t, err, "platform specifier component must match")
-	assert.ErrorContains(t, err, "badcachefrom")
-	assert.ErrorContains(t, err, "badcacheto")
-	assert.ErrorContains(t, err, "invalid reference format")
-	assert.ErrorContains(t, err, "does/not/exist/Dockerfile: no such file or directory")
+		_, err := args.toBuildOptions(false)
+		assert.ErrorContains(t, err, "invalid value badexport")
+		assert.ErrorContains(t, err, "platform specifier component must match")
+		assert.ErrorContains(t, err, "badcachefrom")
+		assert.ErrorContains(t, err, "badcacheto")
+		assert.ErrorContains(t, err, "invalid reference format")
+		assert.ErrorContains(t, err, "does/not/exist/Dockerfile: no such file or directory")
+	})
+
+	t.Run("buildOnPreview", func(t *testing.T) {
+		args := ImageArgs{
+			Tags:    []string{"my-tag"},
+			Exports: []string{"type=registry", "type=local", "type=docker"},
+		}
+		actual, err := args.toBuildOptions(true)
+		assert.NoError(t, err)
+		assert.Equal(t, "image", actual.Exports[0].Type)
+		assert.Equal(t, "false", actual.Exports[0].Attrs["push"])
+
+		actual, err = args.toBuildOptions(false)
+		assert.NoError(t, err)
+		assert.Equal(t, "image", actual.Exports[0].Type)
+		assert.Equal(t, "true", actual.Exports[0].Attrs["push"])
+	})
+
+	t.Run("unknowns", func(t *testing.T) {
+		// pulumi-go-provider gives us zero-values when a property is unknown.
+		// We can't distinguish this from user-provided zero-values, but we
+		// should:
+		// - not fail previews due to these zero values,
+		// - not attempt builds with invalid zero values,
+		// - not allow invalid zero values in non-preview operations.
+		unknowns := ImageArgs{
+			BuildArgs: map[string]string{
+				"known": "value",
+				"":      "",
+			},
+			Builder:   "",
+			CacheFrom: []string{"type=gha", ""},
+			CacheTo:   []string{"type=gha", ""},
+			Context:   "",
+			Exports:   []string{"type=gha", ""},
+			File:      "",
+			Platforms: []string{"linux/amd64", ""},
+			Registries: []properties.RegistryAuth{
+				{
+					Address:  "",
+					Password: "",
+					Username: "",
+				},
+			},
+			Tags: []string{"known", ""},
+		}
+
+		_, err := unknowns.toBuildOptions(true)
+		assert.NoError(t, err)
+		assert.False(t, unknowns.buildable())
+
+		_, err = unknowns.toBuildOptions(false)
+		assert.Error(t, err)
+	})
 }