Repository: kurtosis-tech/kurtosis (go)

Issue:
bug: relative locators fails for quickstart package
### What's your CLI version?

0.83.7

### Description & steps to reproduce

There is another bug related to relative imports this time happens with the  quickstart packagein theawesome repository and it seems it happens when a package uses relative reference to a sibling package, data-packgein this case.
[If you check this change](https://github.com/kurtosis-tech/awesome-kurtosis/pull/187/files#diff-abf5ef9d44740e5825e0f95de966f699d74df8f7948ef4183ff1a821a57947c7R7) you will see  the data-package/main.star file is uploading a file inside the same repo using a relative path, and then [if you check here](https://github.com/kurtosis-tech/awesome-kurtosis/blob/main/quickstart/main.star#L1)  you will see that quickstart package is importing this module from its sibling, so when you run `kurtosis run [github.com/kurtosis-tech/awesome-kurtosis/quickstart](http://github.com/kurtosis-tech/awesome-kurtosis/quickstart) it fails (I reverted the change so it won’t fails now) you can also [see the error in the mono-repo CI test](https://app.circleci.com/pipelines/github/kurtosis-tech/kurtosis/7072/workflows/ba012562-5cb2-480e-b49b-806592107a01/jobs/95120) because it’s using it.

It seems the relative path works when quickstart is executed in awesome CI (because I think it’s doing kurtosis run .) but the relative breaks when it’s executed from the mono repo, because, It does `kurtosis run [github.com/kurtosis-tech/awesome-kurtosis/quickstart](http://github.com/kurtosis-tech/awesome-kurtosis/quickstart)

We created an example to reproduce the issue:

`kurtosis run github.com/h4ck3rk3y/package-panic/primary`   will break meanwhile 

`kurtosis run [github.com/h4ck3rk3y/package-panic/data`  will works.


### Desired behavior

Quickstar package can use relative locators

### What is the severity of this bug?

Painful; this is causing significant friction in my workflow.

Pull request:
fix: relative locators for read_file and upload_files instructions
## Description:
relative locators for `read_file` and `upload_files` instructions

## Is this change user-facing?
NO
Because I think users are not using relative locators yet

## References (if applicable):
Fix #1412 


Patch:
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_builtin_position.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_builtin_position.go
index 3233c442d..21bdc1865 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_builtin_position.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_builtin_position.go
@@ -28,3 +28,7 @@ func (position *KurtosisBuiltinPosition) String() string {
 func (position *KurtosisBuiltinPosition) ToAPIType() *kurtosis_core_rpc_api_bindings.StarlarkInstructionPosition {
 	return binding_constructors.NewStarlarkInstructionPosition(position.filename, position.line, position.column)
 }
+
+func (position *KurtosisBuiltinPosition) GetFilename() string {
+	return position.filename
+}
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper.go
index a522827d4..36eea35d6 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper.go
@@ -22,8 +22,7 @@ func (builtin *KurtosisHelper) CreateBuiltin() func(thread *starlark.Thread, b *
 			return nil, interpretationErr
 		}
 		helperInternal := newKurtosisHelperInternal(wrappedBuiltin, builtin.Capabilities)
-		locatorOfModuleInWhichThisBuiltInIsBeingCalled := thread.Name
-		result, interpretationErr := helperInternal.interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled)
+		result, interpretationErr := helperInternal.interpret()
 		if interpretationErr != nil {
 			return nil, interpretationErr
 		}
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper_internal.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper_internal.go
index 0371fd6ed..06839ab6d 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper_internal.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper/kurtosis_helper_internal.go
@@ -20,6 +20,6 @@ func newKurtosisHelperInternal(wrappedBuiltin *kurtosis_starlark_framework.Kurto
 	}
 }
 
-func (builtin *kurtosisHelperInternal) interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled string) (starlark.Value, *startosis_errors.InterpretationError) {
-	return builtin.capabilities.Interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled, builtin.GetArguments())
+func (builtin *kurtosisHelperInternal) interpret() (starlark.Value, *startosis_errors.InterpretationError) {
+	return builtin.capabilities.Interpret(builtin.GetPosition().GetFilename(), builtin.GetArguments())
 }
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go
index 843658930..3a11db48c 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go
@@ -53,8 +53,8 @@ func (builtin *KurtosisPlanInstructionWrapper) CreateBuiltin() func(thread *star
 		}
 
 		instructionWrapper := newKurtosisPlanInstructionInternal(wrappedBuiltin, builtin.Capabilities(), builtin.DefaultDisplayArguments)
-		locatorOfModuleInWhichInstructionIsBeingInterpreted := thread.Name
-		returnedFutureValue, interpretationErr := instructionWrapper.interpret(locatorOfModuleInWhichInstructionIsBeingInterpreted)
+
+		returnedFutureValue, interpretationErr := instructionWrapper.interpret()
 		if interpretationErr != nil {
 			return nil, interpretationErr
 		}
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction_internal.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction_internal.go
index adcc43f94..0db17f395 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction_internal.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction_internal.go
@@ -97,8 +97,8 @@ func (builtin *kurtosisPlanInstructionInternal) GetPersistableAttributes() *encl
 	return enclavePlaneInstructionBuilder.SetStarlarkCode(builtin.String())
 }
 
-func (builtin *kurtosisPlanInstructionInternal) interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled string) (starlark.Value, *startosis_errors.InterpretationError) {
-	result, interpretationErr := builtin.capabilities.Interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled, builtin.GetArguments())
+func (builtin *kurtosisPlanInstructionInternal) interpret() (starlark.Value, *startosis_errors.InterpretationError) {
+	result, interpretationErr := builtin.capabilities.Interpret(builtin.GetPosition().GetFilename(), builtin.GetArguments())
 	if interpretationErr != nil {
 		return nil, interpretationErr
 	}
diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter.go b/core/server/api_container/server/startosis_engine/startosis_interpreter.go
index fc2d2e64e..9821cda6c 100644
--- a/core/server/api_container/server/startosis_engine/startosis_interpreter.go
+++ b/core/server/api_container/server/startosis_engine/startosis_interpreter.go
@@ -316,7 +316,6 @@ func (interpreter *StartosisInterpreter) interpretInternal(packageId string, mod
 	// We spin up a new thread for every call to interpreterInternal such that the stacktrace provided by the Starlark
 	// Go interpreter is relative to each individual thread, and we don't keep accumulating stacktrace entries from the
 	// previous calls inside the same thread
-	// The thread name is set to the locator of the module so that we can use it to resolve relative paths
 	thread := newStarlarkThread(moduleLocator)
 	predeclared, interpretationErr := interpreter.buildBindings(packageId, thread, instructionPlan, moduleGlobalCache)
 	if interpretationErr != nil {
diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/parsed_git_url.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/parsed_git_url.go
index 8f3e7e756..847005fda 100644
--- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/parsed_git_url.go
+++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/parsed_git_url.go
@@ -18,6 +18,8 @@ const (
 
 	tagBranchOrCommitDelimiter = "@"
 	emptyTagBranchOrCommit     = ""
+
+	packageRootPrefixIndicatorInRelativeLocators = "/"
 )
 
 // ParsedGitURL an object representing a parsed moduleURL
@@ -104,8 +106,10 @@ func parseGitURL(packageURL string) (*ParsedGitURL, *startosis_errors.Interpreta
 }
 
 func (parsedUrl *ParsedGitURL) getAbsoluteLocatorRelativeToThisURL(relativeUrl string) string {
-	absoluteUrl := path.Join(startosis_constants.GithubDomainPrefix, path.Dir(parsedUrl.relativeFilePath), relativeUrl)
-	return absoluteUrl
+	if strings.HasPrefix(relativeUrl, packageRootPrefixIndicatorInRelativeLocators) {
+		return path.Join(startosis_constants.GithubDomainPrefix, parsedUrl.relativeRepoPath, relativeUrl)
+	}
+	return path.Join(startosis_constants.GithubDomainPrefix, path.Dir(parsedUrl.relativeFilePath), relativeUrl)
 }
 
 // cleanPath removes empty "" from the string slice


Test patch:
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/import_module_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/import_module_framework_test.go
index 88edfd680..4cfac143d 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/import_module_framework_test.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/import_module_framework_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/builtins/import_module"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_constants"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages"
 	"github.com/stretchr/testify/require"
@@ -38,7 +39,7 @@ func (suite *KurtosisHelperTestSuite) TestImportFile() {
 	moduleGlobalCache := map[string]*startosis_packages.ModuleCacheEntry{}
 
 	suite.packageContentProvider.EXPECT().GetModuleContents(TestModuleFileName).Return("Hello World!", nil)
-	suite.packageContentProvider.EXPECT().GetAbsoluteLocatorForRelativeModuleLocator(kurtosisHelperThreadName, TestModuleFileName).Return(TestModuleFileName, nil)
+	suite.packageContentProvider.EXPECT().GetAbsoluteLocatorForRelativeModuleLocator(startosis_constants.PackageIdPlaceholderForStandaloneScript, TestModuleFileName).Return(TestModuleFileName, nil)
 
 	suite.run(&importModuleTestCase{
 		T:                      suite.T(),
diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/read_file_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/read_file_framework_test.go
index 50940502f..c7c729f9b 100644
--- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/read_file_framework_test.go
+++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/read_file_framework_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/builtins/read_file"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_helper"
+	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_constants"
 	"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages"
 	"github.com/stretchr/testify/require"
 	"go.starlark.net/starlark"
@@ -17,7 +18,7 @@ type readFileTestCase struct {
 }
 
 func (suite *KurtosisHelperTestSuite) TestReadFile() {
-	suite.packageContentProvider.EXPECT().GetAbsoluteLocatorForRelativeModuleLocator(kurtosisHelperThreadName, TestModuleFileName).Return(TestModuleFileName, nil)
+	suite.packageContentProvider.EXPECT().GetAbsoluteLocatorForRelativeModuleLocator(startosis_constants.PackageIdPlaceholderForStandaloneScript, TestModuleFileName).Return(TestModuleFileName, nil)
 	suite.packageContentProvider.EXPECT().GetModuleContents(TestModuleFileName).Return("Hello World!", nil)
 
 	suite.run(&readFileTestCase{
@@ -39,7 +40,7 @@ func (t *readFileTestCase) GetStarlarkCodeForAssertion() string {
 }
 
 func (t *readFileTestCase) Assert(result starlark.Value) {
-	t.packageContentProvider.AssertCalled(t, "GetAbsoluteLocatorForRelativeModuleLocator", kurtosisHelperThreadName, TestModuleFileName)
+	t.packageContentProvider.AssertCalled(t, "GetAbsoluteLocatorForRelativeModuleLocator", startosis_constants.PackageIdPlaceholderForStandaloneScript, TestModuleFileName)
 	t.packageContentProvider.AssertCalled(t, "GetModuleContents", TestModuleFileName)
 	require.Equal(t, result, starlark.String("Hello World!"))
 }
diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider_test.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider_test.go
index c902d9b46..30680e3a8 100644
--- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider_test.go
+++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider_test.go
@@ -176,6 +176,33 @@ func TestGetAbsolutePathOnDisk_WorksForPureDirectories(t *testing.T) {
 	require.Equal(t, path.Join(packageDir, "kurtosis-tech", "datastore-army-package", "src/helpers.star"), pathOnDisk)
 }
 
+func TestGetAbsoluteLocatorForRelativeModuleLocator_SucceedsForRelativeFile(t *testing.T) {
+	packageDir, err := os.MkdirTemp("", packagesDirRelPath)
+	require.Nil(t, err)
+	defer os.RemoveAll(packageDir)
+	packageTmpDir, err := os.MkdirTemp("", packagesTmpDirRelPath)
+	require.Nil(t, err)
+	defer os.RemoveAll(packageTmpDir)
+
+	provider := NewGitPackageContentProvider(packageDir, packageTmpDir)
+
+	parentModuleId := "github.com/kurtosis-tech/avalanche-package/src/builder.star"
+	maybeRelativeLocator := "../static_files/config.json.tmpl"
+	absoluteLocator, err := provider.GetAbsoluteLocatorForRelativeModuleLocator(parentModuleId, maybeRelativeLocator)
+
+	expectedAbsoluteLocator := "github.com/kurtosis-tech/avalanche-package/static_files/config.json.tmpl"
+	require.Nil(t, err)
+	require.Equal(t, expectedAbsoluteLocator, absoluteLocator)
+
+	parentModuleId2 := "github.com/kurtosis-tech/avalanche-package/src/builder.star"
+	maybeRelativeLocator2 := "/static_files/genesis.json"
+	absoluteLocator2, err2 := provider.GetAbsoluteLocatorForRelativeModuleLocator(parentModuleId2, maybeRelativeLocator2)
+
+	expectedAbsoluteLocator2 := "github.com/kurtosis-tech/avalanche-package/static_files/genesis.json"
+	require.Nil(t, err2)
+	require.Equal(t, expectedAbsoluteLocator2, absoluteLocator2)
+}
+
 func Test_getPathToPackageRoot(t *testing.T) {
 	githubUrlWithKurtosisPackageInSubfolder := "github.com/sample/sample-package/folder/subpackage"
 	parsedGitUrl, err := parseGitURL(githubUrlWithKurtosisPackageInSubfolder)
diff --git a/internal_testsuites/golang/testsuite/startosis_subpackage_test/startosis_remote_subpackage_test.go b/internal_testsuites/golang/testsuite/startosis_subpackage_test/startosis_remote_subpackage_test.go
index da4a4eb5e..07f09ddd2 100644
--- a/internal_testsuites/golang/testsuite/startosis_subpackage_test/startosis_remote_subpackage_test.go
+++ b/internal_testsuites/golang/testsuite/startosis_subpackage_test/startosis_remote_subpackage_test.go
@@ -10,6 +10,8 @@ const (
 	remotePackage        = "github.com/kurtosis-tech/awesome-kurtosis/quickstart"
 	expectedOutputLength = 4
 	expectedServiceName  = "postgres"
+
+	packageWithSiblingImport = "github.com/kurtosis-tech/sample-startosis-load/primary-package"
 )
 
 func (suite *StartosisSubpackageTestSuite) TestStarlarkRemotePackage() {
@@ -30,3 +32,17 @@ func (suite *StartosisSubpackageTestSuite) TestStarlarkRemotePackage() {
 	require.Equal(t, expectedOutputLength, len(runOutputList))
 	require.Contains(t, runOutputTrimmedString, expectedServiceName)
 }
+
+func (suite *StartosisSubpackageTestSuite) TestStartosisSiblingRemotePackages_RelativeImports() {
+	ctx := context.Background()
+	isRemotePackage := true
+	runResult, _ := suite.RunPackage(ctx, packageWithSiblingImport, isRemotePackage)
+
+	t := suite.T()
+	require.Nil(t, runResult.InterpretationError)
+	require.Empty(t, runResult.ValidationErrors)
+	require.Nil(t, runResult.ExecutionError)
+
+	expectedResult := "Primary package loaded.\n\"Hello world!\"\n"
+	require.Regexp(t, expectedResult, string(runResult.RunOutput))
+}