model: better support for no-build no-push custom_builds (#3747)

internal/tiltfile/tiltfile_state.go

@@ -1406,7 +1406,9 @@ func (s *tiltfileState) imgTargetsForDependencyIDsHelper(ids []model.TargetID, c
 				SkipsLocalDocker: image.skipsLocalDocker,
 				LiveUpdate:       lu,
 			}
-			iTarget = iTarget.WithBuildDetails(r)
+			iTarget = iTarget.WithBuildDetails(r).
+				MaybeIgnoreRegistry()
+
 			// TODO(dbentley): validate that syncs is a subset of deps
 		case UnknownBuild:
 			return nil, fmt.Errorf("no build info for image %s", image.configurationRef)

internal/tiltfile/tiltfile_test.go

@@ -4029,6 +4029,24 @@ k8s_yaml('foo.yaml')
 		f.assertNextManifest("foo").ImageTargets[0].OverrideArgs)
 }
 
+// See comments on ImageTarget#MaybeIgnoreRegistry()
+func TestCustomBuildSkipsLocalDockerAndTagPassedIgnoresLocalRegistry(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.dockerfile("Dockerfile")
+	f.yaml("foo.yaml", deployment("foo", image("gcr.io/foo")))
+	f.file("Tiltfile", `
+default_registry('localhost:5000')
+custom_build('gcr.io/foo', ':', ["."], tag='gcr.io/foo:latest', skips_local_docker=True)
+k8s_yaml('foo.yaml')
+`)
+
+	f.load()
+	refs := f.assertNextManifest("foo").ImageTargets[0].Refs
+	assert.Equal(t, "gcr.io/foo", refs.ClusterRef().String())
+}
+
 func TestDuplicateYAMLEntityWithinSingleResource(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()

pkg/model/image_target.go

@@ -143,6 +143,35 @@ func (i ImageTarget) WithBuildDetails(details BuildDetails) ImageTarget {
 	return i
 }
 
+// I (Nick) am deeply unhappy with the parameters of CustomBuild.  They're not
+// well-specified, and often interact in weird and unpredictable ways.  This
+// function is a good example.
+//
+// custom_build(tag) means "My custom_build script already has a tag that it
+// wants to use". In practice, it becomes the "You can't tell me what to do"
+// flag.
+//
+// custom_build(skips_local_docker) means "My custom_build script doesn't use
+// Docker for storage, so you shouldn't expect to find the image there." In
+// practice, it becomes the "You can't touch my outputs" flag.
+//
+// When used together, you have a script that takes no inputs and doesn't let Tilt
+// fix the outputs. So people use custom_build(tag=x, skips_local_docker=True) to
+// enable all sorts of off-road experimental image-building flows that need better
+// primitives.
+//
+// For now, when we detect this case, we strip off registry information, since
+// the script isn't going to use it anyway.  This is tightly coupled with
+// CustomBuilder, which already has similar logic for handling these two cases
+// together.
+func (i ImageTarget) MaybeIgnoreRegistry() ImageTarget {
+	customBuild, ok := i.BuildDetails.(CustomBuild)
+	if ok && customBuild.SkipsLocalDocker && customBuild.Tag != "" {
+		i.Refs = i.Refs.WithoutRegistry()
+	}
+	return i
+}
+
 func (i ImageTarget) WithCachePaths(paths []string) ImageTarget {
 	i.cachePaths = append(append([]string{}, i.cachePaths...), paths...)
 	sort.Strings(i.cachePaths)