Skip to content

Commit

Permalink
fix: Relative import breaks for 'non-main branchs' (#1364)
Browse files Browse the repository at this point in the history
## Description:
Relative import breaks for 'non-main branchs'

## Is this change user-facing?
NO

## References (if applicable):
fix #1361
  • Loading branch information
leoporoli committed Sep 26, 2023
1 parent dd0ebde commit 5496082
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 39 deletions.
22 changes: 13 additions & 9 deletions core/server/api_container/server/api_container_service.go
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
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
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.

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
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
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
@@ -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))
}

0 comments on commit 5496082

Please sign in to comment.