From 182b6fdeba3cb04d57775ef21c6b63573de1a391 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Wed, 27 Mar 2024 11:14:06 -0400 Subject: [PATCH 1/3] check for single args object' --- .../enclaveBuilder/nodes/KurtosisPackageNode.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx b/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx index 6d8ad5106c..84a4f7e4b2 100644 --- a/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx +++ b/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx @@ -26,7 +26,7 @@ export const KurtosisPackageNode = memo( useEffect(() => { const packageId = nodeData?.packageId; - const args = nodeData?.args; + let args = nodeData?.args; if (isDefined(packageId) && isDefined(args) && packageId !== "") { let cancelled = false; (async () => { @@ -40,6 +40,14 @@ export const KurtosisPackageNode = memo( setMode({ type: "error", error: "APIC info missing from temporary enclave" }); return; } + + // If args only has one record, and its value is args, ASSUME user is passing args via a JSON or YAML into the args object of def run(plan, args) + // via Json or Yaml editor + // If only an `args` object is provided, kurtosis will not interpret the value in the args object as passing args via the args dictionary is (technically) deprecated even though it's still allowed + if (Object.keys(args).length === 1 && args.hasOwnProperty('args')) { + args = args['args'] as Record // TODO(tedi): ideally we'd validate and handle this in transform args utils + } + const plan = await kurtosisClient.getStarlarkPackagePlanYaml( enclave.value.enclaveInfo.apiContainerInfo, packageId, From f8f3dfec42d96bcfeadbd77d2a8984bc09bab5ac Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Wed, 27 Mar 2024 11:18:13 -0400 Subject: [PATCH 2/3] add a test highlighting args behavior --- .../startosis_engine/startosis_interpreter_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 108cb3f7b9..f35a42bed7 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 @@ -160,6 +160,20 @@ def my_func(my_arg1, my_arg2, args): require.Equal(suite.T(), expectedResult, result) } +func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_RandomMainFunctionAndParamsErrDueToDeprecatedArgsObject() { + script := ` +def run(plan, args): + all_arg_values = args["arg1"] + ":" + args["arg2"] + return all_arg_values +` + mainFunctionName := "run" + inputArgs := `{"args": {"arg1": "arg1-value", "arg2": "arg2-value"}}` + + _, _, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, mainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, inputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) + require.NotNil(suite.T(), interpretationError) + require.Equal(suite.T(), "Evaluation error: key \"arg1\" not in dict\n\tat [3:23]: run", interpretationError.GetErrorMessage()) +} + func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_Test() { script := ` def run(plan): From 374f4875a6a10c4b6b3e6c1e3f229135366e5d51 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Wed, 27 Mar 2024 11:39:20 -0400 Subject: [PATCH 3/3] elaborate test --- .../startosis_engine/startosis_interpreter_test.go | 13 ++++++++++++- .../enclaveBuilder/nodes/KurtosisPackageNode.tsx | 4 ++-- 2 files changed, 14 insertions(+), 3 deletions(-) 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 f35a42bed7..7e2d1de37e 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 @@ -160,18 +160,29 @@ def my_func(my_arg1, my_arg2, args): require.Equal(suite.T(), expectedResult, result) } -func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_RandomMainFunctionAndParamsErrDueToDeprecatedArgsObject() { +func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_MainFunctionAndParamsErrWhenOnlyDeprecatedArgsObjectProvided() { script := ` def run(plan, args): all_arg_values = args["arg1"] + ":" + args["arg2"] return all_arg_values ` mainFunctionName := "run" + // passing in only "args" dictionary is deprecated inputArgs := `{"args": {"arg1": "arg1-value", "arg2": "arg2-value"}}` _, _, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, mainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, inputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) require.NotNil(suite.T(), interpretationError) require.Equal(suite.T(), "Evaluation error: key \"arg1\" not in dict\n\tat [3:23]: run", interpretationError.GetErrorMessage()) + + scriptWithKwarg := ` +def run(plan, arg0, args): + all_arg_values = arg0 + args["arg1"] + ":" + args["arg2"] + return all_arg_values +` + // however passing in kwargs, with args at the end is still fine + inputArgsWithKwarg := `{"arg0": "foo", "args": {"arg1": "arg1-value", "arg2": "arg2-value"}}` + _, _, interpretationError = suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, mainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, scriptWithKwarg, inputArgsWithKwarg, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) + require.Nil(suite.T(), interpretationError) } func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_Test() { diff --git a/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx b/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx index 84a4f7e4b2..0e6d39ca74 100644 --- a/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx +++ b/enclave-manager/web/packages/app/src/emui/enclaves/components/enclaveBuilder/nodes/KurtosisPackageNode.tsx @@ -44,8 +44,8 @@ export const KurtosisPackageNode = memo( // If args only has one record, and its value is args, ASSUME user is passing args via a JSON or YAML into the args object of def run(plan, args) // via Json or Yaml editor // If only an `args` object is provided, kurtosis will not interpret the value in the args object as passing args via the args dictionary is (technically) deprecated even though it's still allowed - if (Object.keys(args).length === 1 && args.hasOwnProperty('args')) { - args = args['args'] as Record // TODO(tedi): ideally we'd validate and handle this in transform args utils + if (Object.keys(args).length === 1 && args.hasOwnProperty("args")) { + args = args["args"] as Record; // TODO(tedi): ideally we'd validate and handle this in transform args utils } const plan = await kurtosisClient.getStarlarkPackagePlanYaml(