Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Relative import breaks for 'non-main branchs' #1364

Merged
merged 8 commits into from
Sep 26, 2023
22 changes: 13 additions & 9 deletions core/server/api_container/server/api_container_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,13 @@ func (apicService ApiContainerService) RunStarlarkPackage(args *kurtosis_core_rp
// right now the TS SDK still uses the old deprecated behavior
var scriptWithRunFunction string
var interpretationError *startosis_errors.InterpretationError
var packageName string
if args.ClonePackage != nil {
scriptWithRunFunction, interpretationError = apicService.runStarlarkPackageSetup(packageId, args.GetClonePackage(), nil, relativePathToMainFile)
scriptWithRunFunction, packageName, interpretationError = apicService.runStarlarkPackageSetup(packageId, args.GetClonePackage(), nil, relativePathToMainFile)
} else {
// old deprecated syntax in use
moduleContentIfLocal := args.GetLocal()
scriptWithRunFunction, interpretationError = apicService.runStarlarkPackageSetup(packageId, args.GetRemote(), moduleContentIfLocal, relativePathToMainFile)
scriptWithRunFunction, packageName, interpretationError = apicService.runStarlarkPackageSetup(packageId, args.GetRemote(), moduleContentIfLocal, relativePathToMainFile)
}
if interpretationError != nil {
if err := stream.SendMsg(binding_constructors.NewStarlarkRunResponseLineFromInterpretationError(interpretationError.ToAPIType())); err != nil {
Expand All @@ -201,7 +202,7 @@ func (apicService ApiContainerService) RunStarlarkPackage(args *kurtosis_core_rp
return nil
}

apicService.runStarlark(parallelism, dryRun, packageId, mainFuncName, relativePathToMainFile, scriptWithRunFunction, serializedParams, args.ExperimentalFeatures, stream)
apicService.runStarlark(parallelism, dryRun, packageName, mainFuncName, relativePathToMainFile, scriptWithRunFunction, serializedParams, args.ExperimentalFeatures, stream)
return nil
}

Expand Down Expand Up @@ -645,36 +646,39 @@ func (apicService ApiContainerService) runStarlarkPackageSetup(
clonePackage bool,
moduleContentIfLocal []byte,
relativePathToMainFile string,
) (string, *startosis_errors.InterpretationError) {
) (string, string, *startosis_errors.InterpretationError) {
var packageRootPathOnDisk string
var interpretationError *startosis_errors.InterpretationError
var packageName string
if clonePackage {
packageRootPathOnDisk, interpretationError = apicService.startosisModuleContentProvider.ClonePackage(packageId)
packageRootPathOnDisk, packageName, interpretationError = apicService.startosisModuleContentProvider.ClonePackage(packageId)
} else if moduleContentIfLocal != nil {
// TODO: remove this once UploadStarlarkPackage is called prior to calling RunStarlarkPackage by all consumers
// of this API
packageRootPathOnDisk, interpretationError = apicService.startosisModuleContentProvider.StorePackageContents(packageId, bytes.NewReader(moduleContentIfLocal), doOverwriteExistingModule)
packageName = packageId
} else {
// We just need to retrieve the absolute path, the content is will not be stored here since it has been uploaded
// prior to this call
packageRootPathOnDisk, interpretationError = apicService.startosisModuleContentProvider.GetOnDiskAbsolutePackagePath(packageId)
packageName = packageId
}
if interpretationError != nil {
return "", interpretationError
return "", "", interpretationError
}

pathToMainFile := path.Join(packageRootPathOnDisk, relativePathToMainFile)

if _, err := os.Stat(pathToMainFile); err != nil {
return "", startosis_errors.WrapWithInterpretationError(err, "An error occurred while verifying that '%v' exists in the package '%v' at '%v'", startosis_constants.MainFileName, packageId, pathToMainFile)
return "", "", startosis_errors.WrapWithInterpretationError(err, "An error occurred while verifying that '%v' exists in the package '%v' at '%v'", startosis_constants.MainFileName, packageId, pathToMainFile)
}

mainScriptToExecute, err := os.ReadFile(pathToMainFile)
if err != nil {
return "", startosis_errors.WrapWithInterpretationError(err, "An error occurred while reading '%v' in the package '%v' at '%v'", startosis_constants.MainFileName, packageId, pathToMainFile)
return "", "", startosis_errors.WrapWithInterpretationError(err, "An error occurred while reading '%v' in the package '%v' at '%v'", startosis_constants.MainFileName, packageId, pathToMainFile)
}

return string(mainScriptToExecute), nil
return string(mainScriptToExecute), packageName, nil
}

func (apicService ApiContainerService) runStarlark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,30 +50,31 @@ func NewGitPackageContentProvider(moduleDir string, tmpDir string) *GitPackageCo
}
}

func (provider *GitPackageContentProvider) ClonePackage(packageId string) (string, *startosis_errors.InterpretationError) {
func (provider *GitPackageContentProvider) ClonePackage(packageId string) (string, string, *startosis_errors.InterpretationError) {
parsedURL, interpretationError := parseGitURL(packageId)
if interpretationError != nil {
return "", interpretationError
return "", "", interpretationError
}

interpretationError = provider.atomicClone(parsedURL)
if interpretationError != nil {
return "", interpretationError
return "", "", interpretationError
}

relPackagePathToPackagesDir := getPathToPackageRoot(parsedURL)
packageAbsolutePathOnDisk := path.Join(provider.packagesDir, relPackagePathToPackagesDir)

pathToKurtosisYaml := path.Join(packageAbsolutePathOnDisk, startosis_constants.KurtosisYamlName)
if _, err := os.Stat(pathToKurtosisYaml); err != nil {
return "", startosis_errors.WrapWithInterpretationError(err, "Couldn't find a '%v' in the root of the package: '%v'. Packages are expected to have a '%v' at root; for more information have a look at %v",
return "", "", startosis_errors.WrapWithInterpretationError(err, "Couldn't find a '%v' in the root of the package: '%v'. Packages are expected to have a '%v' at root; for more information have a look at %v",
startosis_constants.KurtosisYamlName, packageId, startosis_constants.KurtosisYamlName, packageDocLink)
}

if interpretationError = validateKurtosisYaml(pathToKurtosisYaml, provider.packagesDir); interpretationError != nil {
return "", interpretationError
kurtosisYaml, interpretationError := validateAndGetKurtosisYaml(pathToKurtosisYaml, provider.packagesDir)
if interpretationError != nil {
return "", "", interpretationError
}
return packageAbsolutePathOnDisk, nil
return packageAbsolutePathOnDisk, kurtosisYaml.PackageName, nil
}

func (provider *GitPackageContentProvider) GetOnDiskAbsoluteFilePath(fileInsidePackageUrl string) (string, *startosis_errors.InterpretationError) {
Expand Down Expand Up @@ -115,7 +116,7 @@ func (provider *GitPackageContentProvider) GetOnDiskAbsoluteFilePath(fileInsideP
return "", startosis_errors.NewInterpretationError("%v is not found in the path of '%v'; files can only be accessed from Kurtosis packages. For more information, go to: %v", startosis_constants.KurtosisYamlName, fileInsidePackageUrl, howImportWorksLink)
}

if interpretationError = validateKurtosisYaml(maybeKurtosisYamlPath, provider.packagesDir); interpretationError != nil {
if _, interpretationError = validateAndGetKurtosisYaml(maybeKurtosisYamlPath, provider.packagesDir); interpretationError != nil {
return "", interpretationError
}

Expand Down Expand Up @@ -352,18 +353,18 @@ func getReferenceName(repo *git.Repository, parsedURL *ParsedGitURL) (plumbing.R
}

// this method validates the contents of the kurtosis.yml found at path identified by the absPathToKurtosisYmlInThePackage
func validateKurtosisYaml(absPathToKurtosisYmlInThePackage string, packageDir string) *startosis_errors.InterpretationError {
func validateAndGetKurtosisYaml(absPathToKurtosisYmlInThePackage string, packageDir string) (*yaml_parser.KurtosisYaml, *startosis_errors.InterpretationError) {
kurtosisYaml, errWhileParsing := yaml_parser.ParseKurtosisYaml(absPathToKurtosisYmlInThePackage)
if errWhileParsing != nil {
return startosis_errors.WrapWithInterpretationError(errWhileParsing, "Error occurred while parsing %v", absPathToKurtosisYmlInThePackage)
return nil, startosis_errors.WrapWithInterpretationError(errWhileParsing, "Error occurred while parsing %v", absPathToKurtosisYmlInThePackage)
}

// this method validates whether the package name is also the locator - it should the location where kurtosis.yml exists
if err := validatePackageNameMatchesKurtosisYamlLocation(kurtosisYaml, absPathToKurtosisYmlInThePackage, packageDir); err != nil {
return startosis_errors.WrapWithInterpretationError(err, "Error occurred while validating %v", absPathToKurtosisYmlInThePackage)
return nil, startosis_errors.WrapWithInterpretationError(err, "Error occurred while validating %v", absPathToKurtosisYmlInThePackage)
}

return nil
return kurtosisYaml, nil
}

// this method validates whether the package name found in kurtosis yml is same as the location where kurtosis.yml is found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,20 @@ func TestParsedGitUrl_ResolvesRelativeUrl(t *testing.T) {
expected = "github.com/kurtosis-tech/sample-startosis-load/src/lib.star"
require.Equal(t, expected, absoluteUrl)
}

func TestParsedGitUrl_ResolvesRelativeUrlForUrlWithTag(t *testing.T) {
parsedUrl, err := parseGitURL(githubSampleUrlWithTag)
require.Nil(t, err)

relativeUrl := "./lib.star"
absoluteUrl := parsedUrl.getAbsoluteLocatorRelativeToThisURL(relativeUrl)
require.Nil(t, err)
expected := "github.com/kurtosis-tech/sample-startosis-load/lib.star"
require.Equal(t, expected, absoluteUrl)

relativeUrl = "./src/lib.star"
absoluteUrl = parsedUrl.getAbsoluteLocatorRelativeToThisURL(relativeUrl)
require.Nil(t, err)
expected = "github.com/kurtosis-tech/sample-startosis-load/src/lib.star"
require.Equal(t, expected, absoluteUrl)
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (provider *MockPackageContentProvider) GetOnDiskAbsoluteFilePath(packageId
return absFilePath, nil
}

func (provider *MockPackageContentProvider) ClonePackage(_ string) (string, *startosis_errors.InterpretationError) {
func (provider *MockPackageContentProvider) ClonePackage(_ string) (string, string, *startosis_errors.InterpretationError) {
panic(unimplementedMessage)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ type PackageContentProvider interface {
// StorePackageContents writes on disk the content of the package passed as params
StorePackageContents(packageId string, packageContent io.Reader, overwriteExisting bool) (string, *startosis_errors.InterpretationError)

// ClonePackage clones the package with the given id and returns the absolute path on disk
ClonePackage(packageId string) (string, *startosis_errors.InterpretationError)
// ClonePackage clones the package with the given id and returns the absolute path on disk and the package name from Kurtosis yaml file
ClonePackage(packageId string) (string, string, *startosis_errors.InterpretationError)

// GetAbsoluteLocatorForRelativeModuleLocator returns the absolute package path for a relative module path
GetAbsoluteLocatorForRelativeModuleLocator(packageId string, relativeOrAbsoluteModulePath string) (string, *startosis_errors.InterpretationError)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ func (suite *StartosisPackageTestSuite) RunPackage(ctx context.Context, packageR
return suite.RunPackageWithParams(ctx, packageRelativeDirpath, emptyRunParams)
}

func (suite *StartosisPackageTestSuite) RunRemotePackage(ctx context.Context, remotePackage string) (*enclaves.StarlarkRunResult, error) {
return suite.enclaveCtx.RunStarlarkRemotePackageBlocking(ctx, remotePackage, useDefaultMainFile, useDefaultFunctionName, emptyRunParams, defaultDryRun, defaultParallelism, noExperimentalFeature)
}

func (suite *StartosisPackageTestSuite) RunPackageWithParams(ctx context.Context, packageRelativeDirpath string, params string) (*enclaves.StarlarkRunResult, error) {
logrus.Infof("Executing Startosis package...")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package startosis_package_test

import (
"context"
"github.com/stretchr/testify/require"
)

const (
noMainBranchWithRelativeImportRemotePackage = "github.com/kurtosis-tech/sample-startosis-load@test-branch"
)

func (suite *StartosisPackageTestSuite) TestStartosisNoMainBranchRemotePackage_RelativeImports() {
ctx := context.Background()
runResult, _ := suite.RunRemotePackage(ctx, noMainBranchWithRelativeImportRemotePackage)

t := suite.T()
require.Nil(t, runResult.InterpretationError)
require.Empty(t, runResult.ValidationErrors)
require.Nil(t, runResult.ExecutionError)

expectedResult := "Package loaded.\n"
require.Regexp(t, expectedResult, string(runResult.RunOutput))
}