tiltfile: make probe types play nicer with None (#4163)

Add `Unpack()` implementations that check for `starlark.None` first
and then attempt to cast to the correct type.

integration/local_resource/Tiltfile

@@ -7,5 +7,7 @@ p = probe(initial_delay_secs=0,
 
 local_resource('foo', serve_cmd='./hello.sh foo', readiness_probe=p)
 
+# readiness probe explicitly set to None
 local_resource('bar', serve_cmd='./hello.sh bar',
+               readiness_probe=None,
                resource_deps=['foo'])

internal/tiltfile/probe/probe.go

@@ -14,6 +14,13 @@ import (
 	"github.com/tilt-dev/tilt/internal/tiltfile/value"
 )
 
+const (
+	typeProbe           = "Probe"
+	typeExecAction      = "ExecAction"
+	typeHTTPGetAction   = "HTTPGetAction"
+	typeTCPSocketAction = "TCPSocketAction"
+)
+
 var errInvalidProbeAction = errors.New("exactly one of exec, http_get, or tcp_socket must be specified")
 
 func NewExtension() Extension {
@@ -47,6 +54,25 @@ type Probe struct {
 
 var _ starlark.Value = Probe{}
 
+// Unpack handles the possibility of receiving starlark.None but otherwise just casts to Probe
+func (p *Probe) Unpack(v starlark.Value) error {
+	if v == nil || v == starlark.None {
+		return nil
+	}
+
+	if probe, ok := v.(Probe); ok {
+		*p = probe
+	} else {
+		return fmt.Errorf("got %T, want %s", v, p.Type())
+	}
+
+	return nil
+}
+
+func (p Probe) Type() string {
+	return typeProbe
+}
+
 // Spec returns the probe specification in the canonical format. It must not be modified.
 func (p Probe) Spec() *v1.Probe {
 	return p.spec
@@ -89,7 +115,7 @@ func (e Extension) probe(thread *starlark.Thread, fn *starlark.Builtin, args sta
 	}
 
 	return Probe{
-		Struct: starlarkstruct.FromKeywords(starlark.String("probe"), []starlark.Tuple{
+		Struct: starlarkstruct.FromKeywords(starlark.String(typeProbe), []starlark.Tuple{
 			{starlark.String("initial_delay_secs"), initialDelayVal},
 			{starlark.String("timeout_secs"), timeoutVal},
 			{starlark.String("period_secs"), periodVal},
@@ -136,6 +162,25 @@ func (e ExecAction) ValueOrNone() starlark.Value {
 	return starlark.None
 }
 
+// Unpack handles the possibility of receiving starlark.None but otherwise just casts to ExecAction
+func (e *ExecAction) Unpack(v starlark.Value) error {
+	if v == nil || v == starlark.None {
+		return nil
+	}
+
+	if exec, ok := v.(ExecAction); ok {
+		*e = exec
+	} else {
+		return fmt.Errorf("got %T, want %s", v, e.Type())
+	}
+
+	return nil
+}
+
+func (e ExecAction) Type() string {
+	return typeExecAction
+}
+
 func (e Extension) execAction(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 	var command value.StringSequence
 	err := starkit.UnpackArgs(thread, fn.Name(), args, kwargs, "command", &command)
@@ -144,7 +189,7 @@ func (e Extension) execAction(thread *starlark.Thread, fn *starlark.Builtin, arg
 	}
 	spec := &v1.ExecAction{Command: []string(command)}
 	return ExecAction{
-		Struct: starlarkstruct.FromKeywords(starlark.String("exec_action"), []starlark.Tuple{
+		Struct: starlarkstruct.FromKeywords(starlark.String(typeExecAction), []starlark.Tuple{
 			{starlark.String("command"), command.Sequence()},
 		}),
 		action: spec,
@@ -158,6 +203,21 @@ type HTTPGetAction struct {
 
 var _ starlark.Value = HTTPGetAction{}
 
+// Unpack handles the possibility of receiving starlark.None but otherwise just casts to HTTPGetAction
+func (h *HTTPGetAction) Unpack(v starlark.Value) error {
+	if v == nil || v == starlark.None {
+		return nil
+	}
+
+	if httpGet, ok := v.(HTTPGetAction); ok {
+		*h = httpGet
+	} else {
+		return fmt.Errorf("got %T, want %s", v, h.Type())
+	}
+
+	return nil
+}
+
 func (h HTTPGetAction) ValueOrNone() starlark.Value {
 	// starlarkstruct does not handle being nil well, so need to explicitly return a NoneType
 	// instead of it when embedding in another value (i.e. within the probe)
@@ -167,6 +227,10 @@ func (h HTTPGetAction) ValueOrNone() starlark.Value {
 	return starlark.None
 }
 
+func (h HTTPGetAction) Type() string {
+	return typeHTTPGetAction
+}
+
 func (e Extension) httpGetAction(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 	var host, scheme, path starlark.String
 	var port int
@@ -189,7 +253,7 @@ func (e Extension) httpGetAction(thread *starlark.Thread, fn *starlark.Builtin,
 	}
 
 	return HTTPGetAction{
-		Struct: starlarkstruct.FromKeywords(starlark.String("http_get_action"), []starlark.Tuple{
+		Struct: starlarkstruct.FromKeywords(starlark.String(typeHTTPGetAction), []starlark.Tuple{
 			{starlark.String("host"), host},
 			{starlark.String("port"), starlark.MakeInt(port)},
 			{starlark.String("scheme"), scheme},
@@ -206,6 +270,21 @@ type TCPSocketAction struct {
 
 var _ starlark.Value = TCPSocketAction{}
 
+// Unpack handles the possibility of receiving starlark.None but otherwise just casts to TCPSocketAction
+func (t *TCPSocketAction) Unpack(v starlark.Value) error {
+	if v == nil || v == starlark.None {
+		return nil
+	}
+
+	if tcpSocket, ok := v.(TCPSocketAction); ok {
+		*t = tcpSocket
+	} else {
+		return fmt.Errorf("got %T, want %s", v, t.Type())
+	}
+
+	return nil
+}
+
 func (t TCPSocketAction) ValueOrNone() starlark.Value {
 	// starlarkstruct does not handle being nil well, so need to explicitly return a NoneType
 	// instead of it when embedding in another value (i.e. within the probe)
@@ -215,6 +294,10 @@ func (t TCPSocketAction) ValueOrNone() starlark.Value {
 	return starlark.None
 }
 
+func (t TCPSocketAction) Type() string {
+	return typeTCPSocketAction
+}
+
 func (e Extension) tcpSocketAction(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 	var host starlark.String
 	var port int
@@ -227,7 +310,7 @@ func (e Extension) tcpSocketAction(thread *starlark.Thread, fn *starlark.Builtin
 	}
 	spec := &v1.TCPSocketAction{Host: host.GoString(), Port: intstr.FromInt(port)}
 	return TCPSocketAction{
-		Struct: starlarkstruct.FromKeywords(starlark.String("tcp_socket_action"), []starlark.Tuple{
+		Struct: starlarkstruct.FromKeywords(starlark.String(typeTCPSocketAction), []starlark.Tuple{
 			{starlark.String("host"), host},
 			{starlark.String("port"), starlark.MakeInt(port)},
 		}),

internal/tiltfile/probe/probe_test.go

@@ -19,7 +19,8 @@ p = probe(initial_delay_secs=123,
           period_secs=789,
           success_threshold=987,
           failure_threshold=654,
-          exec=exec_action([]))
+          exec=exec_action([]),
+          http_get=None)
 
 print(p.initial_delay_secs)
 print(p.timeout_secs)
@@ -40,7 +41,7 @@ print("tcp_socket:", p.tcp_socket)
 789
 987
 654
-exec: "exec_action"(command = [])
+exec: "ExecAction"(command = [])
 http_get: None
 tcp_socket: None
 `)
@@ -62,7 +63,9 @@ func TestProbeActions_Multiple(t *testing.T) {
 	f := starkit.NewFixture(t, NewExtension())
 	defer f.TearDown()
 
-	f.File("Tiltfile", `p = probe(exec=exec_action([]), http_get=http_get_action(8000))`)
+	f.File("Tiltfile", `
+p = probe(exec=exec_action([]), http_get=http_get_action(8000), tcp_socket=None)
+`)
 
 	_, err := f.ExecFile("Tiltfile")
 	require.EqualError(t, err, `exactly one of exec, http_get, or tcp_socket must be specified`)