Repository: kurtosis-tech/kurtosis
Pull request #484 (2023-04-07 09:23:59)

refactor: Migrate ExecRecipe to Starlark framework
## Description:
Migrate ExecRecipe to Starlark command framework

## Is this change user facing?
NO
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful information. -->



Patch:
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go
index 0666c8f76..a26e0fa6e 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go
@@ -81,7 +81,7 @@ func KurtosisHelpers(recursiveInterpret func(moduleId string, scriptContent stri
 // Example: ServiceConfig, PortSpec, etc.
 func KurtosisTypeConstructors() []*starlark.Builtin {
 	return []*starlark.Builtin{
-		starlark.NewBuiltin(recipe.ExecRecipeName, recipe.MakeExecRequestRecipe),
+		starlark.NewBuiltin(recipe.ExecRecipeTypeName, recipe.NewExecRecipeType().CreateBuiltin()),
 		starlark.NewBuiltin(recipe.GetHttpRecipeTypeName, recipe.MakeGetHttpRequestRecipe),
 		starlark.NewBuiltin(recipe.PostHttpRecipeTypeName, recipe.MakePostHttpRequestRecipe),
 		starlark.NewBuiltin(connection_config.ConnectionConfigTypeName, connection_config.NewConnectionConfigType().CreateBuiltin()),
diff --git a/core/server/api_container/server/startosis_engine/recipe/exec_recipe.go b/core/server/api_container/server/startosis_engine/recipe/exec_recipe.go
index b6f65b7b7..1cc227a3b 100644
--- a/core/server/api_container/server/startosis_engine/recipe/exec_recipe.go
+++ b/core/server/api_container/server/startosis_engine/recipe/exec_recipe.go
@@ -6,7 +6,9 @@ import (
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/magic_string_helper"
-	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/builtin_argument"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_type_constructor"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors"
 	"github.com/kurtosis-tech/stacktrace"
@@ -17,75 +19,58 @@ import (
 )
 
 const (
+	CommandAttr        = "command"
+	ExecRecipeTypeName = "ExecRecipe"
+
 	execOutputKey   = "output"
 	execExitCodeKey = "code"
 	newlineChar     = "\n"
-
-	commandKey     = "command"
-	serviceNameKey = "service_name"
-	ExecRecipeName = "ExecRecipe"
 )
 
-// TODO: maybe change command to startlark.List once remove backward compatability support
-type ExecRecipe struct {
-	command []string
-}
-
-func NewExecRecipe(command []string) *ExecRecipe {
-	return &ExecRecipe{
-		command: command,
+func NewExecRecipeType() *kurtosis_type_constructor.KurtosisTypeConstructor {
+	return &kurtosis_type_constructor.KurtosisTypeConstructor{
+		KurtosisBaseBuiltin: &kurtosis_starlark_framework.KurtosisBaseBuiltin{
+			Name: ExecRecipeTypeName,
+			Arguments: []*builtin_argument.BuiltinArgument{
+				{
+					Name:              CommandAttr,
+					IsOptional:        false,
+					ZeroValueProvider: builtin_argument.ZeroValueProvider[*starlark.List],
+					Validator: func(value starlark.Value) *startosis_errors.InterpretationError {
+						if _, interpretationErr := convertStarlarkListToStringList(value); interpretationErr != nil {
+							return interpretationErr
+						}
+						return nil
+					},
+				},
+			},
+		},
+		Instantiate: instantiateExecRecipe,
 	}
 }
 
-// String the starlark.Value interface
-func (recipe *ExecRecipe) String() string {
-	buffer := new(strings.Builder)
-	buffer.WriteString(ExecRecipeName + "(")
-	buffer.WriteString(commandKey + "=")
-
-	command := convertListToStarlarkList(recipe.command)
-	if command.Len() > 0 {
-		buffer.WriteString(fmt.Sprintf("%v)", command))
-	} else {
-		buffer.WriteString(fmt.Sprintf("%q)", ""))
+func instantiateExecRecipe(arguments *builtin_argument.ArgumentValuesSet) (builtin_argument.KurtosisValueType, *startosis_errors.InterpretationError) {
+	kurtosisValueType, interpretationErr := kurtosis_type_constructor.CreateKurtosisStarlarkTypeDefault(ExecRecipeTypeName, arguments)
+	if interpretationErr != nil {
+		return nil, interpretationErr
 	}
-	return buffer.String()
-}
-
-// Type implements the starlark.Value interface
-func (recipe *ExecRecipe) Type() string {
-	return ExecRecipeName
-}
-
-// Freeze implements the starlark.Value interface
-func (recipe *ExecRecipe) Freeze() {
-	// this is a no-op its already immutable
-}
-
-// Truth implements the starlark.Value interface
-func (recipe *ExecRecipe) Truth() starlark.Bool {
-	return len(recipe.command) != 0
+	return &ExecRecipe{
+		KurtosisValueTypeDefault: kurtosisValueType,
+	}, nil
 }
 
-// Hash implements the starlark.Value interface
-// This shouldn't be hashed, users should use a portId instead
-func (recipe *ExecRecipe) Hash() (uint32, error) {
-	return 0, startosis_errors.NewInterpretationError("unhashable type: '%v'", ExecRecipeName)
+type ExecRecipe struct {
+	*kurtosis_type_constructor.KurtosisValueTypeDefault
 }
 
-// Attr implements the starlark.HasAttrs interface.
-func (recipe *ExecRecipe) Attr(name string) (starlark.Value, error) {
-	switch name {
-	case commandKey:
-		return convertListToStarlarkList(recipe.command), nil
-	default:
-		return nil, startosis_errors.NewInterpretationError("'%v' has no attribute '%v;", ExecRecipeName, name)
+func (recipe *ExecRecipe) Copy() (builtin_argument.KurtosisValueType, error) {
+	copiedValueType, err := recipe.KurtosisValueTypeDefault.Copy()
+	if err != nil {
+		return nil, err
 	}
-}
-
-// AttrNames implements the starlark.HasAttrs interface.
-func (recipe *ExecRecipe) AttrNames() []string {
-	return []string{serviceNameKey, commandKey}
+	return &ExecRecipe{
+		KurtosisValueTypeDefault: copiedValueType,
+	}, nil
 }
 
 func (recipe *ExecRecipe) Execute(
@@ -94,8 +79,25 @@ func (recipe *ExecRecipe) Execute(
 	runtimeValueStore *runtime_value_store.RuntimeValueStore,
 	serviceName service.ServiceName,
 ) (map[string]starlark.Comparable, error) {
+	// parse argument
+	commandStarlarkList, found, interpretationErr := kurtosis_type_constructor.ExtractAttrValue[*starlark.List](
+		recipe.KurtosisValueTypeDefault, CommandAttr)
+	if interpretationErr != nil {
+		return nil, interpretationErr
+	}
+	if !found {
+		return nil, startosis_errors.NewInterpretationError("Mandatory argument '%s' not found", CommandAttr)
+	}
+	command, interpretationErr := convertStarlarkListToStringList(commandStarlarkList)
+	if interpretationErr != nil {
+		// that should never happen as it's being validated at interpretation time
+		return nil, stacktrace.NewError("Unexpected '%s' attribute for '%s'. Error was: \n%s",
+			CommandAttr, ExecRecipeTypeName, interpretationErr.Error())
+	}
+
+	// execute recipe
 	var commandWithRuntimeValue []string
-	for _, subCommand := range recipe.command {
+	for _, subCommand := range command {
 		maybeSubCommandWithRuntimeValues, err := magic_string_helper.ReplaceRuntimeValueInString(subCommand, runtimeValueStore)
 		if err != nil {
 			return nil, stacktrace.Propagate(err, "An error occurred while replacing runtime values in the command of the exec recipe")
@@ -110,7 +112,7 @@ func (recipe *ExecRecipe) Execute(
 
 	exitCode, commandOutput, err := serviceNetwork.ExecCommand(ctx, serviceNameStr, commandWithRuntimeValue)
 	if err != nil {
-		return nil, stacktrace.Propagate(err, "Failed to execute command '%v' on service '%v'", recipe.command, serviceName)
+		return nil, stacktrace.Propagate(err, "Failed to execute command '%v' on service '%s'", command, serviceName)
 	}
 	return map[string]starlark.Comparable{
 		execOutputKey:   starlark.String(commandOutput),
@@ -153,28 +155,20 @@ func (recipe *ExecRecipe) CreateStarlarkReturnValue(resultUuid string) (*starlar
 	return dict, nil
 }
 
-func MakeExecRequestRecipe(_ *starlark.Thread, builtin *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
-	var unpackedCommandList *starlark.List
-
-	if err := starlark.UnpackArgs(builtin.Name(), args, kwargs,
-		commandKey, &unpackedCommandList,
-	); err != nil {
-		return nil, startosis_errors.NewInterpretationError("%v", err.Error())
-	}
-
-	commands, err := kurtosis_types.SafeCastToStringSlice(unpackedCommandList, commandKey)
-	if err != nil {
-		return nil, err
+func convertStarlarkListToStringList(starlarkValue starlark.Value) ([]string, *startosis_errors.InterpretationError) {
+	starlarkList, isList := starlarkValue.(*starlark.List)
+	if !isList {
+		return nil, startosis_errors.NewInterpretationError("Attribute '%s' is expected to be a list of strings, got '%s'", CommandAttr, reflect.TypeOf(starlarkValue))
 	}
 
-	return NewExecRecipe(commands), nil
-}
-
-func convertListToStarlarkList(inputList []string) *starlark.List {
-	sizeOfCommandArr := len(inputList)
-	var elems []starlark.Value
-	for i := 0; i < sizeOfCommandArr; i++ {
-		elems = append(elems, starlark.String(inputList[i]))
+	var stringListResult []string
+	for idx := 0; idx < starlarkList.Len(); idx++ {
+		item := starlarkList.Index(idx)
+		itemStr, isString := item.(starlark.String)
+		if !isString {
+			return nil, startosis_errors.NewInterpretationError("Item number %d in '%s' list was not a string. Expecting '%s' to be a string list", idx, CommandAttr, CommandAttr)
+		}
+		stringListResult = append(stringListResult, itemStr.GoString())
 	}
-	return starlark.NewList(elems)
+	return stringListResult, nil
 }


Test patch:
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/exec_recipe_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/exec_recipe_framework_test.go
new file mode 100644
index 000000000..526bfdb0e
--- /dev/null
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/exec_recipe_framework_test.go
@@ -0,0 +1,65 @@
+package test_engine
+
+import (
+	"context"
+	"fmt"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/builtin_argument"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_type_constructor"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/recipe"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+type execRecipeTestCase struct {
+	*testing.T
+	serviceNetwork    *service_network.MockServiceNetwork
+	runtimeValueStore *runtime_value_store.RuntimeValueStore
+}
+
+func newExecRecipeTestCase(t *testing.T) *execRecipeTestCase {
+	runtimeValueStore := runtime_value_store.NewRuntimeValueStore()
+	serviceNetwork := service_network.NewMockServiceNetwork(t)
+	serviceNetwork.EXPECT().ExecCommand(
+		mock.Anything,
+		string(TestServiceName),
+		[]string{"echo", "run"},
+	).Times(1).Return(
+		int32(0),
+		"run",
+		nil,
+	)
+	return &execRecipeTestCase{
+		T:                 t,
+		serviceNetwork:    serviceNetwork,
+		runtimeValueStore: runtimeValueStore,
+	}
+}
+
+func (t *execRecipeTestCase) GetId() string {
+	return recipe.ExecRecipeTypeName
+}
+
+func (t *execRecipeTestCase) GetTypeConstructor() *kurtosis_type_constructor.KurtosisTypeConstructor {
+	return recipe.NewExecRecipeType()
+}
+
+func (t *execRecipeTestCase) GetStarlarkCode() string {
+	command := fmt.Sprintf("[%q, %q]", "echo", "run")
+	return fmt.Sprintf("%s(%s=%s)", recipe.ExecRecipeTypeName, recipe.CommandAttr, command)
+}
+
+func (t *execRecipeTestCase) Assert(typeValue builtin_argument.KurtosisValueType) {
+	execRecipe, ok := typeValue.(*recipe.ExecRecipe)
+	require.True(t, ok)
+
+	_, err := execRecipe.Execute(context.Background(), t.serviceNetwork, t.runtimeValueStore, TestServiceName)
+	require.NoError(t, err)
+
+	returnValue, err := execRecipe.CreateStarlarkReturnValue("result-fake-uuid")
+	require.Nil(t, err)
+	expectedInterpretationResult := `{"code": "{{kurtosis:result-fake-uuid:code.runtime_value}}", "output": "{{kurtosis:result-fake-uuid:output.runtime_value}}"}`
+	require.Equal(t, expectedInterpretationResult, returnValue.String())
+}
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/starlark_framework_engine_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/starlark_framework_engine_test.go
index b0fbf0e81..4022a2ec4 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/starlark_framework_engine_test.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/starlark_framework_engine_test.go
@@ -51,6 +51,7 @@ func TestAllRegisteredBuiltins(t *testing.T) {
 	testKurtosisTypeConstructor(t, newConnectionConfigFullTestCase(t))
 	testKurtosisTypeConstructor(t, newConnectionConfigWithPacketDelayTestCase(t))
 	testKurtosisTypeConstructor(t, newConnectionConfigWithPacketLossTestCase(t))
+	testKurtosisTypeConstructor(t, newExecRecipeTestCase(t))
 	testKurtosisTypeConstructor(t, newNormalPacketDelayDistributionFullTestCase(t))
 	testKurtosisTypeConstructor(t, newNormalPacketDelayDistributionMinimalTestCase(t))
 	testKurtosisTypeConstructor(t, newPortSpecFullTestCase(t))
diff --git a/core/server/api_container/server/startosis_engine/recipe/exec_recipe_test.go b/core/server/api_container/server/startosis_engine/recipe/exec_recipe_test.go
deleted file mode 100644
index b59de44d1..000000000
--- a/core/server/api_container/server/startosis_engine/recipe/exec_recipe_test.go
+++ /dev/null
@@ -1,15 +0,0 @@
-package recipe
-
-import (
-	"github.com/stretchr/testify/require"
-	"testing"
-)
-
-func TestExecRecipe_String(t *testing.T) {
-	commands := []string{"echo", "run"}
-
-	expectedRecipeString := `ExecRecipe(command=["echo", "run"])`
-	execRecipe := NewExecRecipe(commands)
-	execRecipeString := execRecipe.String()
-	require.Equal(t, expectedRecipeString, execRecipeString)
-}
diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go
index c96a4aa79..299150065 100644
--- a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go
+++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go
@@ -772,51 +772,6 @@ Adding service example-datastore-server
 	validateScriptOutputFromPrintInstructions(t, instructions, expectedOutputFromScriptB)
 }
 
-func TestStartosisInterpreter_ValidExecScript(t *testing.T) {
-	packageContentProvider := mock_package_content_provider.NewMockPackageContentProvider()
-	defer packageContentProvider.RemoveAll()
-	testRuntimeValueStore := runtime_value_store.NewRuntimeValueStore()
-	interpreter := NewStartosisInterpreter(testServiceNetwork, packageContentProvider, testRuntimeValueStore)
-	script := `
-def run(plan):
-	plan.print("Executing mkdir!")
-	recipe = ExecRecipe(
-		command = ["mkdir", "/tmp/foo"]
-	)
-	plan.exec(recipe = recipe, service_name = "web-server")
-`
-
-	_, instructions, interpretationError := interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, script, startosis_constants.EmptyInputArgs)
-	require.Nil(t, interpretationError)
-	require.Len(t, instructions, 2)
-}
-
-func TestStartosisInterpreter_InvalidExecRecipeMissingRequiredCommand(t *testing.T) {
-	packageContentProvider := mock_package_content_provider.NewMockPackageContentProvider()
-	defer packageContentProvider.RemoveAll()
-	testRuntimeValueStore := runtime_value_store.NewRuntimeValueStore()
-	interpreter := NewStartosisInterpreter(testServiceNetwork, packageContentProvider, testRuntimeValueStore)
-	script := `
-def run(plan):
-	plan.print("Executing mkdir!")
-	recipe = ExecRecipe()
-	plan.exec(recipe = recipe)
-`
-
-	_, _, interpretationError := interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, script, startosis_constants.EmptyInputArgs)
-
-	expectedError := startosis_errors.NewInterpretationErrorWithCustomMsg(
-		[]startosis_errors.CallFrame{
-			*startosis_errors.NewCallFrame("run", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 4, 21)),
-			*startosis_errors.NewCallFrame("ExecRecipe", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 0, 0)),
-		},
-		"Evaluation error: ExecRecipe: missing argument for command",
-	).ToAPIType()
-
-	require.NotNil(t, interpretationError)
-	require.Equal(t, expectedError, interpretationError)
-}
-
 func TestStartosisInterpreter_ReadFileFromGithub(t *testing.T) {
 	src := "github.com/foo/bar/static_files/main.txt"
 	seed := map[string]string{