From 187499614c24edfdab5d71f00e1719eae7a57193 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Mon, 9 Oct 2023 15:24:20 -0300 Subject: [PATCH 01/14] local replace implemented in server and golang SDK --- .../core/lib/enclaves/enclave_context.go | 34 ++++++++++++++++--- api/golang/core/lib/enclaves/kurtosis_yaml.go | 6 ++-- .../git_package_content_provider.go | 13 +++++++ .../git_package_content_provider_test.go | 15 ++++++++ .../startosis_replace_with_local_test.go | 23 +++++++++++++ .../kurtosis.yml | 1 + .../local-sample-dependency-package/main.star | 11 ++++++ .../regular-replace/main.star | 1 - .../replace-with-local/kurtosis.yml | 4 +++ .../replace-with-local/main.star | 28 +++++++++++++++ .../replace-with-no-main-branch/main.star | 1 - 11 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_test.go create mode 100644 internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/kurtosis.yml create mode 100644 internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/main.star create mode 100644 internal_testsuites/starlark/packages-with-replace/replace-with-local/kurtosis.yml create mode 100644 internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star diff --git a/api/golang/core/lib/enclaves/enclave_context.go b/api/golang/core/lib/enclaves/enclave_context.go index b9b89cac7c..d689dbdb07 100644 --- a/api/golang/core/lib/enclaves/enclave_context.go +++ b/api/golang/core/lib/enclaves/enclave_context.go @@ -136,7 +136,7 @@ func (enclaveCtx *EnclaveContext) RunStarlarkPackage( }() starlarkResponseLineChan := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) - executeStartosisPackageArgs, err := enclaveCtx.assembleRunStartosisPackageArg(packageRootPath, runConfig.RelativePathToMainFile, runConfig.MainFunctionName, serializedParams, runConfig.DryRun, runConfig.Parallelism, runConfig.ExperimentalFeatureFlags, runConfig.CloudInstanceId, runConfig.CloudUserId) + executeStartosisPackageArgs, kurtosisYml, err := enclaveCtx.assembleRunStartosisPackageArg(packageRootPath, runConfig.RelativePathToMainFile, runConfig.MainFunctionName, serializedParams, runConfig.DryRun, runConfig.Parallelism, runConfig.ExperimentalFeatureFlags, runConfig.CloudInstanceId, runConfig.CloudUserId) if err != nil { return nil, nil, stacktrace.Propagate(err, "Error preparing package '%s' for execution", packageRootPath) } @@ -146,6 +146,12 @@ func (enclaveCtx *EnclaveContext) RunStarlarkPackage( return nil, nil, stacktrace.Propagate(err, "Error uploading package '%s' prior to executing it", packageRootPath) } + if len(kurtosisYml.PackageReplaceOptions) > 0 { + if err = enclaveCtx.uploadLocalStarlarkPackageDependencies(packageRootPath, kurtosisYml.PackageReplaceOptions); err != nil { + return nil, nil, stacktrace.Propagate(err, "An error occurred while uploading the local starlark package dependencies from the replace options '%+v'", kurtosisYml.PackageReplaceOptions) + } + } + stream, err := enclaveCtx.client.RunStarlarkPackage(ctxWithCancel, executeStartosisPackageArgs) if err != nil { return nil, nil, stacktrace.Propagate(err, "Unexpected error happened executing Starlark package '%v'", packageRootPath) @@ -156,6 +162,26 @@ func (enclaveCtx *EnclaveContext) RunStarlarkPackage( return starlarkResponseLineChan, cancelCtxFunc, nil } +func (enclaveCtx *EnclaveContext) uploadLocalStarlarkPackageDependencies(packageRootPath string, packageReplaceOptions map[string]string) error { + for dependencyPackageId, replaceOption := range packageReplaceOptions { + if isLocalDependencyReplace(replaceOption) { + localPackagePath := path.Join(packageRootPath, replaceOption) + if err := enclaveCtx.uploadStarlarkPackage(dependencyPackageId, localPackagePath); err != nil { + return stacktrace.Propagate(err, "Error uploading package '%s' prior to executing it", replaceOption) + } + } + } + return nil +} + +func isLocalDependencyReplace(replace string) bool { + //TODO replace harcoded values + if strings.HasPrefix(replace, "/") || strings.HasPrefix(replace, ".") { + return true + } + return false +} + // Docs available at https://docs.kurtosis.com/sdk/#runstarlarkpackageblockingstring-packagerootpath-string-serializedparams-boolean-dryrun---starlarkrunresult-runresult-error-error func (enclaveCtx *EnclaveContext) RunStarlarkPackageBlocking( ctx context.Context, @@ -502,14 +528,14 @@ func (enclaveCtx *EnclaveContext) assembleRunStartosisPackageArg( experimentalFeatures []kurtosis_core_rpc_api_bindings.KurtosisFeatureFlag, cloudInstanceId string, cloudUserId string, -) (*kurtosis_core_rpc_api_bindings.RunStarlarkPackageArgs, error) { +) (*kurtosis_core_rpc_api_bindings.RunStarlarkPackageArgs, *KurtosisYaml, error) { kurtosisYamlFilepath := path.Join(packageRootPath, kurtosisYamlFilename) kurtosisYaml, err := ParseKurtosisYaml(kurtosisYamlFilepath) if err != nil { - return nil, stacktrace.Propagate(err, "There was an error parsing the '%v' at '%v'", kurtosisYamlFilename, packageRootPath) + return nil, nil, stacktrace.Propagate(err, "There was an error parsing the '%v' at '%v'", kurtosisYamlFilename, packageRootPath) } - return binding_constructors.NewRunStarlarkPackageArgs(kurtosisYaml.PackageName, relativePathToMainFile, mainFunctionName, serializedParams, dryRun, parallelism, experimentalFeatures, cloudInstanceId, cloudUserId), nil + return binding_constructors.NewRunStarlarkPackageArgs(kurtosisYaml.PackageName, relativePathToMainFile, mainFunctionName, serializedParams, dryRun, parallelism, experimentalFeatures, cloudInstanceId, cloudUserId), kurtosisYaml, nil } func (enclaveCtx *EnclaveContext) uploadStarlarkPackage(packageId string, packageRootPath string) error { diff --git a/api/golang/core/lib/enclaves/kurtosis_yaml.go b/api/golang/core/lib/enclaves/kurtosis_yaml.go index 5f984cef15..0473b18238 100644 --- a/api/golang/core/lib/enclaves/kurtosis_yaml.go +++ b/api/golang/core/lib/enclaves/kurtosis_yaml.go @@ -12,7 +12,9 @@ const ( // fields are public because it's needed for YAML decoding type KurtosisYaml struct { - PackageName string `yaml:"name"` + PackageName string `yaml:"name"` + PackageDescription string `yaml:"description"` + PackageReplaceOptions map[string]string `yaml:"replace"` } func ParseKurtosisYaml(kurtosisYamlFilepath string) (*KurtosisYaml, error) { @@ -25,7 +27,7 @@ func ParseKurtosisYaml(kurtosisYamlFilepath string) (*KurtosisYaml, error) { } var kurtosisYaml KurtosisYaml - err = yaml.Unmarshal(kurtosisYamlContents, &kurtosisYaml) + err = yaml.UnmarshalStrict(kurtosisYamlContents, &kurtosisYaml) if err != nil { return nil, stacktrace.Propagate(err, "An error occurred while parsing the '%v' file at '%v'", kurtosisYamlFilename, kurtosisYamlFilepath) } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index a0c4dccbfe..85ccb72102 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -232,6 +232,11 @@ func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[st found, packageToBeReplaced, replaceWithPackage := findPackageReplace(absoluteLocator, packageReplaceOptions) if found { + // we skip if it's a local replace because we will use the same absolute locator + // due the file was already uploaded in the enclave's package cache + if isLocalDependencyReplace(replaceWithPackage) { + return absoluteLocator + } replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) return replacedAbsoluteLocator @@ -491,3 +496,11 @@ func getKurtosisYamlPathForFileUrlInternal(absPathToFile string, packagesDir str } return filePathToKurtosisYamlNotFound, nil } + +func isLocalDependencyReplace(replace string) bool { + //TODO replace harcoded values + if strings.HasPrefix(replace, "/") || strings.HasPrefix(replace, ".") { + return true + } + return false +} 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 98692710f5..d46350ca48 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 @@ -296,6 +296,21 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_NoMainBranchReplaceSucceeds( require.Equal(t, expectedAbsoluteLocator, absoluteLocator) } +func TestGetAbsoluteLocatorForRelativeModuleLocator_LocalPackagehReplaceSucceeds(t *testing.T) { + provider := NewGitPackageContentProvider("", "") + + parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" + maybeRelativeLocator := "github.com/kurtosis-tech/sample-dependency-package/main.star" + packageReplaceOptions := map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "../local-sample-dependency-package", + } + absoluteLocator, err := provider.GetAbsoluteLocatorForRelativeLocator(parentModuleId, maybeRelativeLocator, packageReplaceOptions) + + expectedAbsoluteLocator := "github.com/kurtosis-tech/sample-dependency-package/main.star" + require.Nil(t, err) + require.Equal(t, expectedAbsoluteLocator, absoluteLocator) +} + 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_replace_test/startosis_replace_with_local_test.go b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_test.go new file mode 100644 index 0000000000..942b827d45 --- /dev/null +++ b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_test.go @@ -0,0 +1,23 @@ +package startosis_replace_test + +import ( + "context" + "github.com/stretchr/testify/require" +) + +const ( + packageWithLocalReplaceRelPath = "../../../starlark/packages-with-replace/replace-with-local" + packageWithLocalReplaceParams = `{ "message_origin" : "local" }` +) + +func (suite *StartosisReplaceTestSuite) TestStartosisReplaceWithLocal() { + ctx := context.Background() + runResult, _ := suite.RunPackageWithParams(ctx, packageWithLocalReplaceRelPath, packageWithLocalReplaceParams) + + t := suite.T() + require.Nil(t, runResult.InterpretationError) + require.Empty(t, runResult.ValidationErrors) + require.Nil(t, runResult.ExecutionError) + expectedResult := "Replace with local package loaded.\nVerification succeeded. Value is '\"msg-loaded-from-local-dependency\"'.\n" + require.Equal(t, expectedResult, string(runResult.RunOutput)) +} diff --git a/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/kurtosis.yml b/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/kurtosis.yml new file mode 100644 index 0000000000..15274738a6 --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/kurtosis.yml @@ -0,0 +1 @@ +name: github.com/kurtosis-tech/sample-dependency-package \ No newline at end of file diff --git a/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/main.star b/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/main.star new file mode 100644 index 0000000000..76590dbc1f --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/local-sample-dependency-package/main.star @@ -0,0 +1,11 @@ +MSG = "msg-loaded-from-local-dependency" + +def run(plan, args): + plan.print("Local sample dependency package loaded.") + + msg_to_return = get_msg() + + return msg_to_return + +def get_msg(): + return MSG diff --git a/internal_testsuites/starlark/packages-with-replace/regular-replace/main.star b/internal_testsuites/starlark/packages-with-replace/regular-replace/main.star index 92729bd3cd..a8766934b9 100644 --- a/internal_testsuites/starlark/packages-with-replace/regular-replace/main.star +++ b/internal_testsuites/starlark/packages-with-replace/regular-replace/main.star @@ -6,7 +6,6 @@ EXPECTED_MSG_FROM_ANOTHER_SAMPLE_MAIN = "another-dependency-loaded-from-main" MSG_ORIGIN_MAIN = "main" MSG_ORIGIN_ANOTHER_SAMPLE_MAIN = "another-main" -# TODO remove https://github.com/kurtosis-tech/sample-startosis-load/tree/main/sample-package if it's not used def run(plan, message_origin=MSG_ORIGIN_MAIN): plan.print("Regular replace package loaded.") diff --git a/internal_testsuites/starlark/packages-with-replace/replace-with-local/kurtosis.yml b/internal_testsuites/starlark/packages-with-replace/replace-with-local/kurtosis.yml new file mode 100644 index 0000000000..0ab0ff8b69 --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/replace-with-local/kurtosis.yml @@ -0,0 +1,4 @@ +name: github.com/kurtosis-tech/sample-startosis-load/regular-replace +replace: + # Replacing the package with the 'local version' of the package + github.com/kurtosis-tech/sample-dependency-package: ../local-sample-dependency-package diff --git a/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star b/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star new file mode 100644 index 0000000000..f9c88c2406 --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star @@ -0,0 +1,28 @@ +dependency = import_module("github.com/kurtosis-tech/sample-dependency-package/main.star") + +EXPECTED_MSG_FROM_MAIN = "dependency-loaded-from-main" +EXPECTED_MSG_FROM_LOCAL_PACKAGE_MAIN = "msg-loaded-from-local-dependency" + +MSG_ORIGIN_MAIN = "main" +MSG_ORIGIN_LOCAL_DEPENDENCY = "local" + +# TODO remove https://github.com/kurtosis-tech/sample-startosis-load/tree/main/sample-package if it's not used +def run(plan, message_origin=MSG_ORIGIN_MAIN): + plan.print("Replace with local package loaded.") + + msg_from_dependency = dependency.get_msg() + + if message_origin == MSG_ORIGIN_MAIN: + expected_msg = EXPECTED_MSG_FROM_MAIN + elif message_origin == MSG_ORIGIN_LOCAL_DEPENDENCY: + expected_msg = EXPECTED_MSG_FROM_LOCAL_PACKAGE_MAIN + else: + expected_msg = "" + + plan.verify( + value = expected_msg, + assertion = "==", + target_value = msg_from_dependency, + ) + + return diff --git a/internal_testsuites/starlark/packages-with-replace/replace-with-no-main-branch/main.star b/internal_testsuites/starlark/packages-with-replace/replace-with-no-main-branch/main.star index e2cfa745df..11d6f76042 100644 --- a/internal_testsuites/starlark/packages-with-replace/replace-with-no-main-branch/main.star +++ b/internal_testsuites/starlark/packages-with-replace/replace-with-no-main-branch/main.star @@ -6,7 +6,6 @@ EXPECTED_MSG_FROM_BRANCH = "dependency-loaded-from-test-branch" MSG_ORIGIN_MAIN = "main" MSG_ORIGIN_BRANCH = "branch" -# TODO remove https://github.com/kurtosis-tech/sample-startosis-load/tree/main/sample-package if it's not used def run(plan, message_origin=MSG_ORIGIN_MAIN): plan.print("Replace with no main branch package loaded.") From b24410c985dd3125e25fcabc021e6870d1a7cfd4 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Mon, 9 Oct 2023 17:58:51 -0300 Subject: [PATCH 02/14] starting with ReplaceWithLocalAndThenWithRemote --- .../startosis_engine/startosis_interpreter.go | 4 + .../dependency_handler/dependency_handler.go | 15 +++ .../git_package_content_provider.go | 119 ++++++++++++------ .../mock_package_content_provider.go | 44 +++++++ .../mock_package_content_provider.go | 4 + .../package_content_provider.go | 4 + .../package_replace_options_repository.go | 12 ++ .../startosis_replace_testsuite_test.go | 4 +- ...ce_with_local_and_then_with_remote_test.go | 31 +++++ .../without-replace/kurtosis.yml | 1 + .../without-replace/main.star | 23 ++++ 11 files changed, 223 insertions(+), 38 deletions(-) create mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go create mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go create mode 100644 internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_and_then_with_remote_test.go create mode 100644 internal_testsuites/starlark/packages-with-replace/without-replace/kurtosis.yml create mode 100644 internal_testsuites/starlark/packages-with-replace/without-replace/main.star 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 3d19be23db..2f90199724 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter.go @@ -93,6 +93,10 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan( currentEnclavePlan *enclave_plan_persistence.EnclavePlan, ) (string, *instructions_plan.InstructionsPlan, *kurtosis_core_rpc_api_bindings.StarlarkInterpretationError) { + if interpretationErr := interpreter.moduleContentProvider.RefreshCache(packageReplaceOptions); interpretationErr != nil { + return "", nil, interpretationErr.ToAPIType() + } + // run interpretation with no mask at all to generate the list of instructions as if the enclave was empty enclaveComponents := enclave_structure.NewEnclaveComponents() emptyPlanInstructionsMask := resolver.NewInstructionsPlanMask(0) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go b/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go new file mode 100644 index 0000000000..d71794b768 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go @@ -0,0 +1,15 @@ +package dependency_handler + +import ( + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository" +) + +type DependencyHandler struct { + packageReplaceOptionsRepository *package_replace_options_repository.PackageReplaceOptionsRepository + gitPackageContentProvider *git_package_content_provider.GitPackageContentProvider +} + +func NewDependencyHandler(packageReplaceOptionsRepository *package_replace_options_repository.PackageReplaceOptionsRepository, gitPackageContentProvider *git_package_content_provider.GitPackageContentProvider) *DependencyHandler { + return &DependencyHandler{packageReplaceOptionsRepository: packageReplaceOptionsRepository, gitPackageContentProvider: gitPackageContentProvider} +} diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 85ccb72102..793fb38803 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -84,13 +84,13 @@ func (provider *GitPackageContentProvider) GetKurtosisYaml(packageAbsolutePathOn return kurtosisYaml, nil } -func (provider *GitPackageContentProvider) GetOnDiskAbsoluteFilePath(fileInsidePackageUrl string) (string, *startosis_errors.InterpretationError) { - parsedURL, interpretationError := parseGitURL(fileInsidePackageUrl) +func (provider *GitPackageContentProvider) GetOnDiskAbsoluteFilePath(absoluteFileLocator string) (string, *startosis_errors.InterpretationError) { + parsedURL, interpretationError := parseGitURL(absoluteFileLocator) if interpretationError != nil { return "", interpretationError } if parsedURL.relativeFilePath == "" { - return "", startosis_errors.NewInterpretationError("The path '%v' needs to point to a specific file but it didn't. Users can only read or import specific files and not entire packages.", fileInsidePackageUrl) + return "", startosis_errors.NewInterpretationError("The path '%v' needs to point to a specific file but it didn't. Users can only read or import specific files and not entire packages.", absoluteFileLocator) } pathToFileOnDisk := path.Join(provider.packagesDir, parsedURL.relativeFilePath) packagePath := path.Join(provider.packagesDir, parsedURL.relativeRepoPath) @@ -116,11 +116,11 @@ func (provider *GitPackageContentProvider) GetOnDiskAbsoluteFilePath(fileInsideP // check whether kurtosis yaml exists in the path maybeKurtosisYamlPath, err := getKurtosisYamlPathForFileUrl(pathToFileOnDisk, provider.packagesDir) if err != nil { - return "", startosis_errors.WrapWithInterpretationError(err, "Error occurred while verifying whether '%v' belongs to a Kurtosis package.", fileInsidePackageUrl) + return "", startosis_errors.WrapWithInterpretationError(err, "Error occurred while verifying whether '%v' belongs to a Kurtosis package.", absoluteFileLocator) } if maybeKurtosisYamlPath == filePathToKurtosisYamlNotFound { - 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) + 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, absoluteFileLocator, howImportWorksLink) } if _, interpretationError = validateAndGetKurtosisYaml(maybeKurtosisYamlPath, provider.packagesDir); interpretationError != nil { @@ -225,48 +225,51 @@ func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( return replacedAbsoluteLocator, nil } -func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) string { - if absoluteLocator == "" { - return absoluteLocator - } +func (provider *GitPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { - found, packageToBeReplaced, replaceWithPackage := findPackageReplace(absoluteLocator, packageReplaceOptions) - if found { - // we skip if it's a local replace because we will use the same absolute locator - // due the file was already uploaded in the enclave's package cache - if isLocalDependencyReplace(replaceWithPackage) { - return absoluteLocator - } - replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) - logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) - return replacedAbsoluteLocator + historicalPackageReplaceOptions := map[string]string{} //TODO get this from the repository + + //TODO remove this line it's just for test purpose + historicalPackageReplaceOptions = map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "../local-sample-dependency-package", } - return absoluteLocator -} + for packageId, historicalReplace := range historicalPackageReplaceOptions { -func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string) { - pathToAnalyze := absoluteLocator - for { - numberSlashes := strings.Count(pathToAnalyze, urlPathSeparator) + shouldClonePackage := false - // check for the minimal path e.g.: github.com/org/package - if numberSlashes < minimumSubPathsForValidGitURL { - break + isHistoricalLocalReplace := isLocalDependencyReplace(historicalReplace) + logrus.Debugf("historicalReplace '%v' isHistoricalLocalReplace? '%v', ", historicalReplace, isHistoricalLocalReplace) + + currentReplace, isCurrentReplace := currentPackageReplaceOptions[packageId] + if isCurrentReplace { + // the package will be cloned if the current replace is remote and the historical is local + isCurrentRemoteReplace := !isLocalDependencyReplace(currentReplace) + logrus.Debugf("currentReplace '%v' isCurrentRemoteReplace? '%v', ", isCurrentRemoteReplace, currentReplace) + if isCurrentRemoteReplace && isHistoricalLocalReplace { + shouldClonePackage = true + } } - lastIndex := strings.LastIndex(pathToAnalyze, urlPathSeparator) - packageToBeReplaced := pathToAnalyze[:lastIndex] - replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] - if ok { - logrus.Debugf("dependency replace found for '%s', package '%s' will be replaced with '%s'", absoluteLocator, packageToBeReplaced, replaceWithPackage) - return true, packageToBeReplaced, replaceWithPackage + // there is no current replace for this dependency but the version in the cache is local + if !isCurrentReplace && isHistoricalLocalReplace { + shouldClonePackage = true } - pathToAnalyze = packageToBeReplaced + if shouldClonePackage { + if _, err := provider.ClonePackage(packageId); err != nil { + return startosis_errors.WrapWithInterpretationError(err, "An error occurred cloning package '%v'", packageId) + } + } + + historicalPackageReplaceOptions[packageId] = currentReplace } - return false, "", "" + //TODO remove this debug + logrus.Debugf("historicalPackageReplaceOptions '%v' ", historicalPackageReplaceOptions) + + //TODO store historical in the repository + return nil } // atomicClone This first clones to a temporary directory and then moves it @@ -367,6 +370,50 @@ func (provider *GitPackageContentProvider) atomicClone(parsedURL *ParsedGitURL) return nil } +func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) string { + if absoluteLocator == "" { + return absoluteLocator + } + + found, packageToBeReplaced, replaceWithPackage := findPackageReplace(absoluteLocator, packageReplaceOptions) + if found { + // we skip if it's a local replace because we will use the same absolute locator + // due the file was already uploaded in the enclave's package cache + if isLocalDependencyReplace(replaceWithPackage) { + return absoluteLocator + } + replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) + logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) + return replacedAbsoluteLocator + } + + return absoluteLocator +} + +func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string) { + pathToAnalyze := absoluteLocator + for { + numberSlashes := strings.Count(pathToAnalyze, urlPathSeparator) + + // check for the minimal path e.g.: github.com/org/package + if numberSlashes < minimumSubPathsForValidGitURL { + break + } + lastIndex := strings.LastIndex(pathToAnalyze, urlPathSeparator) + + packageToBeReplaced := pathToAnalyze[:lastIndex] + replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] + if ok { + logrus.Debugf("dependency replace found for '%s', package '%s' will be replaced with '%s'", absoluteLocator, packageToBeReplaced, replaceWithPackage) + return true, packageToBeReplaced, replaceWithPackage + } + + pathToAnalyze = packageToBeReplaced + } + + return false, "", "" +} + // methods checks whether the root of the package is same as repository root // or it is a sub-folder under it func getPathToPackageRoot(parsedPackagePath *ParsedGitURL) string { diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go index 94cad82006..53a7c531b7 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go @@ -351,6 +351,50 @@ func (_c *MockPackageContentProvider_GetOnDiskAbsolutePackagePath_Call) RunAndRe return _c } +// RefreshCache provides a mock function with given fields: currentPackageReplaceOptions +func (_m *MockPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { + ret := _m.Called(currentPackageReplaceOptions) + + var r0 *startosis_errors.InterpretationError + if rf, ok := ret.Get(0).(func(map[string]string) *startosis_errors.InterpretationError); ok { + r0 = rf(currentPackageReplaceOptions) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*startosis_errors.InterpretationError) + } + } + + return r0 +} + +// MockPackageContentProvider_RefreshCache_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RefreshCache' +type MockPackageContentProvider_RefreshCache_Call struct { + *mock.Call +} + +// RefreshCache is a helper method to define mock.On call +// - currentPackageReplaceOptions map[string]string +func (_e *MockPackageContentProvider_Expecter) RefreshCache(currentPackageReplaceOptions interface{}) *MockPackageContentProvider_RefreshCache_Call { + return &MockPackageContentProvider_RefreshCache_Call{Call: _e.mock.On("RefreshCache", currentPackageReplaceOptions)} +} + +func (_c *MockPackageContentProvider_RefreshCache_Call) Run(run func(currentPackageReplaceOptions map[string]string)) *MockPackageContentProvider_RefreshCache_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(map[string]string)) + }) + return _c +} + +func (_c *MockPackageContentProvider_RefreshCache_Call) Return(_a0 *startosis_errors.InterpretationError) *MockPackageContentProvider_RefreshCache_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockPackageContentProvider_RefreshCache_Call) RunAndReturn(run func(map[string]string) *startosis_errors.InterpretationError) *MockPackageContentProvider_RefreshCache_Call { + _c.Call.Return(run) + return _c +} + // StorePackageContents provides a mock function with given fields: packageId, packageContent, overwriteExisting func (_m *MockPackageContentProvider) StorePackageContents(packageId string, packageContent io.Reader, overwriteExisting bool) (string, *startosis_errors.InterpretationError) { ret := _m.Called(packageId, packageContent, overwriteExisting) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go index 433408bc03..6d854da420 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go @@ -60,6 +60,10 @@ func (provider *MockPackageContentProvider) GetKurtosisYaml(packageAbsolutePathO panic(unimplementedMessage) } +func (provider *MockPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { + return nil +} + func (provider *MockPackageContentProvider) GetModuleContents(fileInsidePackageUrl string) (string, *startosis_errors.InterpretationError) { absFilePath, found := provider.starlarkPackages[fileInsidePackageUrl] if !found { diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go index 9780003cf5..4e02dea9ac 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go @@ -35,4 +35,8 @@ type PackageContentProvider interface { // GetKurtosisYaml returns the package kurtosis.yml file content GetKurtosisYaml(packageAbsolutePathOnDisk string) (*yaml_parser.KurtosisYaml, *startosis_errors.InterpretationError) + + //TODO add comments + // RefreshCache it compare the + RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go b/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go new file mode 100644 index 0000000000..99a49623d1 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go @@ -0,0 +1,12 @@ +package package_replace_options_repository + +type PackageReplaceOptionsRepository struct { +} + +func NewPackageReplaceOptionsRepository() *PackageReplaceOptionsRepository { + return &PackageReplaceOptionsRepository{} +} + +func (repository *PackageReplaceOptionsRepository) Get(key string) map[string]string { + panic("Implement me") +} diff --git a/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go index 9636c6194d..b09c474b35 100644 --- a/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go +++ b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go @@ -39,8 +39,8 @@ func (suite *StartosisReplaceTestSuite) SetupTest() { } func (suite *StartosisReplaceTestSuite) TearDownTest() { - err := suite.destroyEnclaveFunc() - require.NoError(suite.T(), err, "Destroying the test suite's enclave process has failed, you will have to remove it manually") + //err := suite.destroyEnclaveFunc() + //require.NoError(suite.T(), err, "Destroying the test suite's enclave process has failed, you will have to remove it manually") } func (suite *StartosisReplaceTestSuite) RunPackage(ctx context.Context, packageRelativeDirpath string) (*enclaves.StarlarkRunResult, error) { diff --git a/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_and_then_with_remote_test.go b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_and_then_with_remote_test.go new file mode 100644 index 0000000000..ac1c7a68bb --- /dev/null +++ b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_with_local_and_then_with_remote_test.go @@ -0,0 +1,31 @@ +package startosis_replace_test + +import ( + "context" + "github.com/stretchr/testify/require" +) + +const ( + packageWithoutReplaceRelPath = "../../../starlark/packages-with-replace/without-replace" + packageWithoutReplaceParams = `{ "message_origin" : "main" }` +) + +func (suite *StartosisReplaceTestSuite) TestStartosisReplaceWithLocalAndThenWithRemote() { + ctx := context.Background() + runResult, _ := suite.RunPackageWithParams(ctx, packageWithLocalReplaceRelPath, packageWithLocalReplaceParams) + + t := suite.T() + require.Nil(t, runResult.InterpretationError) + require.Empty(t, runResult.ValidationErrors) + require.Nil(t, runResult.ExecutionError) + expectedResult := "Replace with local package loaded.\nVerification succeeded. Value is '\"msg-loaded-from-local-dependency\"'.\n" + require.Equal(t, expectedResult, string(runResult.RunOutput)) + + runResult2, _ := suite.RunPackageWithParams(ctx, packageWithoutReplaceRelPath, packageWithoutReplaceParams) + + require.Nil(t, runResult.InterpretationError) + require.Empty(t, runResult.ValidationErrors) + require.Nil(t, runResult.ExecutionError) + expectedResult2 := "Without replace package loaded.\nVerification succeeded. Value is '\"dependency-loaded-from-main\"'.\n" + require.Equal(t, expectedResult2, string(runResult2.RunOutput)) +} diff --git a/internal_testsuites/starlark/packages-with-replace/without-replace/kurtosis.yml b/internal_testsuites/starlark/packages-with-replace/without-replace/kurtosis.yml new file mode 100644 index 0000000000..48c9ec94d2 --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/without-replace/kurtosis.yml @@ -0,0 +1 @@ +name: github.com/kurtosis-tech/sample-startosis-load/without-replace \ No newline at end of file diff --git a/internal_testsuites/starlark/packages-with-replace/without-replace/main.star b/internal_testsuites/starlark/packages-with-replace/without-replace/main.star new file mode 100644 index 0000000000..e9a1fd8bde --- /dev/null +++ b/internal_testsuites/starlark/packages-with-replace/without-replace/main.star @@ -0,0 +1,23 @@ +dependency = import_module("github.com/kurtosis-tech/sample-dependency-package/main.star") + +EXPECTED_MSG_FROM_MAIN = "dependency-loaded-from-main" + +MSG_ORIGIN_MAIN = "main" + +def run(plan, message_origin=MSG_ORIGIN_MAIN): + plan.print("Without replace package loaded.") + + msg_from_dependency = dependency.get_msg() + + if message_origin == MSG_ORIGIN_MAIN: + expected_msg = EXPECTED_MSG_FROM_MAIN + else: + expected_msg = "" + + plan.verify( + value = expected_msg, + assertion = "==", + target_value = msg_from_dependency, + ) + + return From 2575b4b86f91d7e1eb2e7d8e3b3ca8b36f680fda Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Tue, 10 Oct 2023 14:12:29 -0300 Subject: [PATCH 03/14] packageReplaceOptionsRepository added --- core/server/api_container/main.go | 8 +- .../startosis_engine/startosis_interpreter.go | 2 +- .../git_package_content_provider.go | 33 +++---- .../git_package_content_provider_test.go | 84 +++++++++++++---- .../package_replace_options_repository.go | 90 +++++++++++++++++++ ...package_replace_options_repository_test.go | 69 ++++++++++++++ .../mock_package_content_provider.go | 6 +- .../mock_package_content_provider.go | 2 +- .../package_content_provider.go | 8 +- .../enclave_data_directory.go | 5 +- 10 files changed, 259 insertions(+), 48 deletions(-) create mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go create mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go diff --git a/core/server/api_container/main.go b/core/server/api_container/main.go index 480ffaa8c6..9b18253cf7 100644 --- a/core/server/api_container/main.go +++ b/core/server/api_container/main.go @@ -125,14 +125,14 @@ func runMain() error { return stacktrace.NewError("Kurtosis backend type is '%v' but cluster configuration parameters are null.", args.KurtosisBackendType_Kubernetes.String()) } - gitPackageContentProvider, err := enclaveDataDir.GetGitPackageContentProvider() + enclaveDb, err := enclave_db.GetOrCreateEnclaveDatabase() if err != nil { - return stacktrace.Propagate(err, "An error occurred while creating the Git module content provider") + return stacktrace.Propagate(err, "An error occurred while getting the enclave db") } - enclaveDb, err := enclave_db.GetOrCreateEnclaveDatabase() + gitPackageContentProvider, err := enclaveDataDir.GetGitPackageContentProvider(enclaveDb) if err != nil { - return stacktrace.Propagate(err, "An error occurred while getting the enclave db") + return stacktrace.Propagate(err, "An error occurred while creating the Git module content provider") } // TODO Extract into own function 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 2f90199724..37a71283e5 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter.go @@ -93,7 +93,7 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan( currentEnclavePlan *enclave_plan_persistence.EnclavePlan, ) (string, *instructions_plan.InstructionsPlan, *kurtosis_core_rpc_api_bindings.StarlarkInterpretationError) { - if interpretationErr := interpreter.moduleContentProvider.RefreshCache(packageReplaceOptions); interpretationErr != nil { + if interpretationErr := interpreter.moduleContentProvider.CloneReplacedPackagesIfNeeded(packageReplaceOptions); interpretationErr != nil { return "", nil, interpretationErr.ToAPIType() } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 793fb38803..76671db699 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" "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/commons/yaml_parser" @@ -41,14 +42,16 @@ const ( ) type GitPackageContentProvider struct { - packagesTmpDir string - packagesDir string + packagesTmpDir string + packagesDir string + packageReplaceOptionsRepository *packageReplaceOptionsRepository } -func NewGitPackageContentProvider(moduleDir string, tmpDir string) *GitPackageContentProvider { +func NewGitPackageContentProvider(moduleDir string, tmpDir string, enclaveDb *enclave_db.EnclaveDB) *GitPackageContentProvider { return &GitPackageContentProvider{ - packagesDir: moduleDir, - packagesTmpDir: tmpDir, + packagesDir: moduleDir, + packagesTmpDir: tmpDir, + packageReplaceOptionsRepository: NewPackageReplaceOptionsRepository(enclaveDb), } } @@ -225,13 +228,11 @@ func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( return replacedAbsoluteLocator, nil } -func (provider *GitPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { +func (provider *GitPackageContentProvider) CloneReplacedPackagesIfNeeded(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { - historicalPackageReplaceOptions := map[string]string{} //TODO get this from the repository - - //TODO remove this line it's just for test purpose - historicalPackageReplaceOptions = map[string]string{ - "github.com/kurtosis-tech/sample-dependency-package": "../local-sample-dependency-package", + historicalPackageReplaceOptions, err := provider.packageReplaceOptionsRepository.Get() + if err != nil { + return startosis_errors.WrapWithInterpretationError(err, "An error occurred getting the historical package replace options from the repository") } for packageId, historicalReplace := range historicalPackageReplaceOptions { @@ -261,14 +262,16 @@ func (provider *GitPackageContentProvider) RefreshCache(currentPackageReplaceOpt return startosis_errors.WrapWithInterpretationError(err, "An error occurred cloning package '%v'", packageId) } } + } + // upgrade the historical-replace list with the new values + for packageId, currentReplace := range currentPackageReplaceOptions { historicalPackageReplaceOptions[packageId] = currentReplace } - //TODO remove this debug - logrus.Debugf("historicalPackageReplaceOptions '%v' ", historicalPackageReplaceOptions) - - //TODO store historical in the repository + if err = provider.packageReplaceOptionsRepository.Save(historicalPackageReplaceOptions); err != nil { + return startosis_errors.WrapWithInterpretationError(err, "An error occurred saving the historical package replace options from the repository") + } return nil } 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 d46350ca48..964464c5c8 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 @@ -2,11 +2,13 @@ package git_package_content_provider import ( "fmt" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" "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/commons/yaml_parser" "github.com/kurtosis-tech/stacktrace" "github.com/stretchr/testify/require" + bolt "go.etcd.io/bbolt" "os" "path" "testing" @@ -28,7 +30,7 @@ func TestGitPackageProvider_SucceedsForValidPackage(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -44,7 +46,7 @@ func TestGitPackageProvider_SucceedsForValidPackageWithExplicitMasterSet(t *test require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@main" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -60,7 +62,7 @@ func TestGitPackageProvider_SucceedsForValidPackageWithBranch(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@test-branch" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -76,7 +78,7 @@ func TestGitPackageProvider_FailsForInvalidBranch(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@non-existent-branch" _, err = provider.GetModuleContents(sampleStartosisModule) @@ -91,7 +93,7 @@ func TestGitPackageProvider_SucceedsForValidPackageWithTag(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@0.1.1" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -107,7 +109,7 @@ func TestGitPackageProvider_SucceedsForValidPackageWithCommit(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@ec9062828e1a687a5db7dfa750f754f88119e4c0" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -123,7 +125,7 @@ func TestGitPackageProvider_SucceedsForValidPackageWithCommitOnABranch(t *testin require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStartosisModule := "github.com/kurtosis-tech/sample-startosis-load/sample.star@df88baf51caffbe7e8f66c0e54715f680f4482b2" contents, err := provider.GetModuleContents(sampleStartosisModule) @@ -139,7 +141,7 @@ func TestGitPackageProvider_SucceedsForNonStarlarkFile(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) sampleStarlarkPackage := "github.com/kurtosis-tech/ethereum-package/static_files/prometheus-config/prometheus.yml.tmpl" contents, err := provider.GetModuleContents(sampleStarlarkPackage) @@ -155,7 +157,7 @@ func TestGitPackageProvider_FailsForNonExistentPackage(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(oackageDir, packageTmpDir) + provider := NewGitPackageContentProvider(oackageDir, packageTmpDir, nil) nonExistentModulePath := "github.com/kurtosis-tech/non-existent-startosis-load/sample.star" _, err = provider.GetModuleContents(nonExistentModulePath) @@ -170,7 +172,7 @@ func TestGetAbsolutePathOnDisk_WorksForPureDirectories(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) packagePath := "github.com/kurtosis-tech/datastore-army-package/src/helpers.star" pathOnDisk, err := provider.GetOnDiskAbsoluteFilePath(packagePath) @@ -187,7 +189,7 @@ func TestGetAbsolutePathOnDisk_WorksForNonInMainBranchLocators(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(packageTmpDir) - provider := NewGitPackageContentProvider(packageDir, packageTmpDir) + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, nil) absoluteFileLocator := "github.com/kurtosis-tech/sample-dependency-package@test-branch/main.star" pathOnDisk, err := provider.GetOnDiskAbsoluteFilePath(absoluteFileLocator) @@ -197,7 +199,7 @@ func TestGetAbsolutePathOnDisk_WorksForNonInMainBranchLocators(t *testing.T) { } func TestGetAbsoluteLocatorForRelativeModuleLocator_SucceedsForRelativeFile(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/avalanche-package/src/builder.star" maybeRelativeLocator := "../static_files/config.json.tmpl" @@ -217,7 +219,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_SucceedsForRelativeFile(t *t } func TestGetAbsoluteLocatorForRelativeModuleLocator_RegularReplaceSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/sample-dependency-package/main.star" @@ -233,7 +235,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_RegularReplaceSucceeds(t *te } func TestGetAbsoluteLocatorForRelativeModuleLocator_RootPackageReplaceSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/another-sample-dependency-package/main.star" @@ -250,7 +252,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_RootPackageReplaceSucceeds(t } func TestGetAbsoluteLocatorForRelativeModuleLocator_SubPackageReplaceSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/another-sample-dependency-package/subpackage/main.star" @@ -267,7 +269,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_SubPackageReplaceSucceeds(t } func TestGetAbsoluteLocatorForRelativeModuleLocator_ReplacePackageInternalModuleSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/another-sample-dependency-package/folder/module.star" @@ -282,7 +284,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_ReplacePackageInternalModule } func TestGetAbsoluteLocatorForRelativeModuleLocator_NoMainBranchReplaceSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/sample-dependency-package/main.star" @@ -297,7 +299,7 @@ func TestGetAbsoluteLocatorForRelativeModuleLocator_NoMainBranchReplaceSucceeds( } func TestGetAbsoluteLocatorForRelativeModuleLocator_LocalPackagehReplaceSucceeds(t *testing.T) { - provider := NewGitPackageContentProvider("", "") + provider := NewGitPackageContentProvider("", "", nil) parentModuleId := "github.com/kurtosis-tech/sample-startosis-load/sample-package/main.star" maybeRelativeLocator := "github.com/kurtosis-tech/sample-dependency-package/main.star" @@ -492,6 +494,37 @@ func Test_validatePackageNameMatchesKurtosisYamlLocation(t *testing.T) { } } +func TestCloneReplacedPackagesIfNeeded_Succeeds(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) + + enclaveDb := getEnclaveDbForTest(t) + + provider := NewGitPackageContentProvider(packageDir, packageTmpDir, enclaveDb) + + firstRunReplacePackageOptions := map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "../from-local-folder", + } + + err = provider.CloneReplacedPackagesIfNeeded(firstRunReplacePackageOptions) + require.Nil(t, err) + + secondRunReplacePackageOptions := allPackageReplaceOptionsForTest + + err = provider.CloneReplacedPackagesIfNeeded(secondRunReplacePackageOptions) + require.Nil(t, err) + + expectedSamplePackageDirpathOnCache := packageDir + "/kurtosis-tech/sample-dependency-package" + + fileInfo, err := os.Stat(expectedSamplePackageDirpathOnCache) + require.NoError(t, err) + require.True(t, fileInfo.IsDir()) +} + func createKurtosisYml(packageName string) *yaml_parser.KurtosisYaml { return &yaml_parser.KurtosisYaml{ PackageName: packageName, @@ -499,3 +532,18 @@ func createKurtosisYml(packageName string) *yaml_parser.KurtosisYaml { PackageReplaceOptions: noPackageReplaceOptions, } } + +func getEnclaveDbForTest(t *testing.T) *enclave_db.EnclaveDB { + file, err := os.CreateTemp("/tmp", "*.db") + defer func() { + err = os.Remove(file.Name()) + require.NoError(t, err) + }() + + require.NoError(t, err) + db, err := bolt.Open(file.Name(), 0666, nil) + require.NoError(t, err) + return &enclave_db.EnclaveDB{ + DB: db, + } +} diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go new file mode 100644 index 0000000000..d055733d7a --- /dev/null +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go @@ -0,0 +1,90 @@ +package git_package_content_provider + +import ( + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" + "github.com/kurtosis-tech/stacktrace" + "github.com/sirupsen/logrus" + bolt "go.etcd.io/bbolt" +) + +var ( + packageReplaceOptionsBucketName = []byte("package-replace-options-repository") + emptyValue = []byte{} +) + +type packageReplaceOptionsRepository struct { + enclaveDb *enclave_db.EnclaveDB +} + +func NewPackageReplaceOptionsRepository( + enclaveDb *enclave_db.EnclaveDB, +) *packageReplaceOptionsRepository { + return &packageReplaceOptionsRepository{ + enclaveDb: enclaveDb, + } +} + +func (repository *packageReplaceOptionsRepository) Save( + allReplacePackageOptions map[string]string, +) error { + + if allReplacePackageOptions == nil { + return stacktrace.NewError("Expected to receive a replace package options map but a nil value was received instead, this is a bug in Kurtosis") + } + + logrus.Debugf("Saving package replace options '%v' in the repository...", allReplacePackageOptions) + if err := repository.enclaveDb.Update(func(tx *bolt.Tx) error { + + bucket, err := tx.CreateBucketIfNotExists(packageReplaceOptionsBucketName) + if err != nil { + return stacktrace.Propagate(err, "An error occurred while creating the package replace options bucket") + } + + for packageId, replaceOption := range allReplacePackageOptions { + key := getKey(packageId) + + replaceOptionBytes := []byte(replaceOption) + + if err := bucket.Put(key, replaceOptionBytes); err != nil { + return stacktrace.Propagate(err, "An error occurred while saving replace option '%s' for package ID '%s' into the enclave db bucket", replaceOption, packageId) + } + } + + return nil + }); err != nil { + return stacktrace.Propagate(err, "An error occurred while saving package replace options '%v' into the enclave db", allReplacePackageOptions) + } + logrus.Debugf("... replace options successfully saved.") + return nil +} + +func (repository *packageReplaceOptionsRepository) Get() (map[string]string, error) { + allReplacePackageOptions := map[string]string{} + + if err := repository.enclaveDb.View(func(tx *bolt.Tx) error { + bucket := tx.Bucket(packageReplaceOptionsBucketName) + if bucket == nil { + return nil + } + + if err := bucket.ForEach(func(packageIdKey, replaceOptionBytes []byte) error { + packageId := string(packageIdKey) + replaceOption := string(replaceOptionBytes) + allReplacePackageOptions[packageId] = replaceOption + + return nil + }); err != nil { + return stacktrace.Propagate(err, "An error occurred while iterating the replace package options repository to get all the replace options") + } + + return nil + }); err != nil { + return nil, stacktrace.Propagate(err, "An error occurred while iterating the replace package options from the package replace options repository") + } + + return allReplacePackageOptions, nil +} + +func getKey(keyString string) []byte { + return []byte(keyString) +} diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go new file mode 100644 index 0000000000..b10aaef371 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go @@ -0,0 +1,69 @@ +package git_package_content_provider + +import ( + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" + "github.com/stretchr/testify/require" + bolt "go.etcd.io/bbolt" + "os" + "testing" +) + +var allPackageReplaceOptionsForTest = map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "github.com/kurtosis-tech/another-sample-dependency-package", + "github.com/kurtosis-tech/ethereum-package": "github.com/another-org/ethereum-package", +} + +func TestSaveAnGet_Success(t *testing.T) { + repository := getPackageReplaceOptionsRepositoryForTest(t) + + err := repository.Save(allPackageReplaceOptionsForTest) + require.NoError(t, err) + + historicalReplacePackageOptions, err := repository.Get() + require.NoError(t, err) + require.Equal(t, allPackageReplaceOptionsForTest, historicalReplacePackageOptions) +} + +func TestSaveAnGet_SuccessForNoReplacePackageOptions(t *testing.T) { + repository := getPackageReplaceOptionsRepositoryForTest(t) + + err := repository.Save(noPackageReplaceOptions) + require.NoError(t, err) + + historicalReplacePackageOptions, err := repository.Get() + require.NoError(t, err) + require.Equal(t, noPackageReplaceOptions, historicalReplacePackageOptions) +} + +func TestSave_ErrorWhenSavingNil(t *testing.T) { + repository := getPackageReplaceOptionsRepositoryForTest(t) + + err := repository.Save(nil) + require.Error(t, err) +} + +func TestGet_SuccessEmptyRepository(t *testing.T) { + repository := getPackageReplaceOptionsRepositoryForTest(t) + + historicalReplacePackageOptions, err := repository.Get() + require.NoError(t, err) + require.Equal(t, noPackageReplaceOptions, historicalReplacePackageOptions) +} + +func getPackageReplaceOptionsRepositoryForTest(t *testing.T) *packageReplaceOptionsRepository { + file, err := os.CreateTemp("/tmp", "*.db") + defer func() { + err = os.Remove(file.Name()) + require.NoError(t, err) + }() + + require.NoError(t, err) + db, err := bolt.Open(file.Name(), 0666, nil) + require.NoError(t, err) + enclaveDb := &enclave_db.EnclaveDB{ + DB: db, + } + repository := NewPackageReplaceOptionsRepository(enclaveDb) + + return repository +} diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go index 53a7c531b7..83e9472cf1 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider.go @@ -352,7 +352,7 @@ func (_c *MockPackageContentProvider_GetOnDiskAbsolutePackagePath_Call) RunAndRe } // RefreshCache provides a mock function with given fields: currentPackageReplaceOptions -func (_m *MockPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { +func (_m *MockPackageContentProvider) CloneReplacedPackagesIfNeeded(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { ret := _m.Called(currentPackageReplaceOptions) var r0 *startosis_errors.InterpretationError @@ -367,7 +367,7 @@ func (_m *MockPackageContentProvider) RefreshCache(currentPackageReplaceOptions return r0 } -// MockPackageContentProvider_RefreshCache_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RefreshCache' +// MockPackageContentProvider_RefreshCache_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CloneReplacedPackagesIfNeeded' type MockPackageContentProvider_RefreshCache_Call struct { *mock.Call } @@ -375,7 +375,7 @@ type MockPackageContentProvider_RefreshCache_Call struct { // RefreshCache is a helper method to define mock.On call // - currentPackageReplaceOptions map[string]string func (_e *MockPackageContentProvider_Expecter) RefreshCache(currentPackageReplaceOptions interface{}) *MockPackageContentProvider_RefreshCache_Call { - return &MockPackageContentProvider_RefreshCache_Call{Call: _e.mock.On("RefreshCache", currentPackageReplaceOptions)} + return &MockPackageContentProvider_RefreshCache_Call{Call: _e.mock.On("CloneReplacedPackagesIfNeeded", currentPackageReplaceOptions)} } func (_c *MockPackageContentProvider_RefreshCache_Call) Run(run func(currentPackageReplaceOptions map[string]string)) *MockPackageContentProvider_RefreshCache_Call { diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go index 6d854da420..7d540ddb15 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider/mock_package_content_provider.go @@ -60,7 +60,7 @@ func (provider *MockPackageContentProvider) GetKurtosisYaml(packageAbsolutePathO panic(unimplementedMessage) } -func (provider *MockPackageContentProvider) RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { +func (provider *MockPackageContentProvider) CloneReplacedPackagesIfNeeded(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { return nil } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go index 4e02dea9ac..48812e2461 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go @@ -29,14 +29,14 @@ type PackageContentProvider interface { // ClonePackage clones the package with the given id and returns the absolute path on disk ClonePackage(packageId string) (string, *startosis_errors.InterpretationError) - // GetAbsoluteLocatorForRelativeModuleLocator returns the absolute package path for a relative module path and replace the package path if + // GetAbsoluteLocatorForRelativeLocator returns the absolute package path for a relative module path and replace the package path if // there is a valid option in the noPackageReplaceOptions map GetAbsoluteLocatorForRelativeLocator(packageId string, relativeOrAbsoluteLocator string, packageReplaceOptions map[string]string) (string, *startosis_errors.InterpretationError) // GetKurtosisYaml returns the package kurtosis.yml file content GetKurtosisYaml(packageAbsolutePathOnDisk string) (*yaml_parser.KurtosisYaml, *startosis_errors.InterpretationError) - //TODO add comments - // RefreshCache it compare the - RefreshCache(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError + // CloneReplacedPackagesIfIsNeeded will compare the received currentPackageReplaceOptions with the historical replace options (from previous run) + // and will clone the packages depending on the comparison result + CloneReplacedPackagesIfNeeded(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError } diff --git a/core/server/commons/enclave_data_directory/enclave_data_directory.go b/core/server/commons/enclave_data_directory/enclave_data_directory.go index c48783f958..e3134ede22 100644 --- a/core/server/commons/enclave_data_directory/enclave_data_directory.go +++ b/core/server/commons/enclave_data_directory/enclave_data_directory.go @@ -6,6 +6,7 @@ package enclave_data_directory import ( + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db/file_artifacts_db" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider" "github.com/kurtosis-tech/stacktrace" @@ -65,7 +66,7 @@ func (dir EnclaveDataDirectory) GetFilesArtifactStore() (*FilesArtifactStore, er return currentFilesArtifactStore, dbError } -func (dir EnclaveDataDirectory) GetGitPackageContentProvider() (*git_package_content_provider.GitPackageContentProvider, error) { +func (dir EnclaveDataDirectory) GetGitPackageContentProvider(enclaveDb *enclave_db.EnclaveDB) (*git_package_content_provider.GitPackageContentProvider, error) { packageStoreDirpath := path.Join(dir.absMountDirpath, startosisPackageStoreDirname) if err := ensureDirpathExists(packageStoreDirpath); err != nil { return nil, stacktrace.Propagate(err, "An error occurred ensuring the Starlark package store dirpath '%v' exists.", packageStoreDirpath) @@ -76,5 +77,5 @@ func (dir EnclaveDataDirectory) GetGitPackageContentProvider() (*git_package_con return nil, stacktrace.Propagate(err, "An error occurred ensuring the Starlark temporary package store dirpath '%v' exists.", tempPackageStoreDirpath) } - return git_package_content_provider.NewGitPackageContentProvider(packageStoreDirpath, tempPackageStoreDirpath), nil + return git_package_content_provider.NewGitPackageContentProvider(packageStoreDirpath, tempPackageStoreDirpath, enclaveDb), nil } From 90435b7b0e990683a93d7bc16332bcad79c2b88c Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Tue, 10 Oct 2023 17:12:00 -0300 Subject: [PATCH 04/14] Added TS library --- .../core/lib/enclaves/enclave_context.go | 49 ++++++---- .../src/core/lib/enclaves/enclave_context.ts | 93 +++++++++++++++---- .../enclaves/generic_api_container_client.ts | 1 + .../grpc_node_api_container_client.ts | 44 +++++++++ .../src/core/lib/enclaves/kurtosis_yaml.ts | 4 +- .../git_package_content_provider.go | 5 +- 6 files changed, 160 insertions(+), 36 deletions(-) diff --git a/api/golang/core/lib/enclaves/enclave_context.go b/api/golang/core/lib/enclaves/enclave_context.go index d689dbdb07..418add5ce0 100644 --- a/api/golang/core/lib/enclaves/enclave_context.go +++ b/api/golang/core/lib/enclaves/enclave_context.go @@ -32,6 +32,7 @@ import ( "google.golang.org/grpc" "google.golang.org/protobuf/types/known/emptypb" "io" + "os" "path" "strings" ) @@ -41,6 +42,10 @@ type EnclaveUUID string const ( kurtosisYamlFilename = "kurtosis.yml" enforceMaxFileSizeLimit = true + + osPathSeparatorString = string(os.PathSeparator) + + dotRelativePathIndicatorString = "." ) // Docs available at https://docs.kurtosis.com/sdk/#enclavecontext @@ -136,7 +141,13 @@ func (enclaveCtx *EnclaveContext) RunStarlarkPackage( }() starlarkResponseLineChan := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) - executeStartosisPackageArgs, kurtosisYml, err := enclaveCtx.assembleRunStartosisPackageArg(packageRootPath, runConfig.RelativePathToMainFile, runConfig.MainFunctionName, serializedParams, runConfig.DryRun, runConfig.Parallelism, runConfig.ExperimentalFeatureFlags, runConfig.CloudInstanceId, runConfig.CloudUserId) + + kurtosisYml, err := getKurtosisYaml(packageRootPath) + if err != nil { + return nil, nil, stacktrace.Propagate(err, "An error occurred getting Kurtosis yaml file from path '%s'", packageRootPath) + } + + executeStartosisPackageArgs, err := enclaveCtx.assembleRunStartosisPackageArg(kurtosisYml, runConfig.RelativePathToMainFile, runConfig.MainFunctionName, serializedParams, runConfig.DryRun, runConfig.Parallelism, runConfig.ExperimentalFeatureFlags, runConfig.CloudInstanceId, runConfig.CloudUserId) if err != nil { return nil, nil, stacktrace.Propagate(err, "Error preparing package '%s' for execution", packageRootPath) } @@ -174,14 +185,6 @@ func (enclaveCtx *EnclaveContext) uploadLocalStarlarkPackageDependencies(package return nil } -func isLocalDependencyReplace(replace string) bool { - //TODO replace harcoded values - if strings.HasPrefix(replace, "/") || strings.HasPrefix(replace, ".") { - return true - } - return false -} - // Docs available at https://docs.kurtosis.com/sdk/#runstarlarkpackageblockingstring-packagerootpath-string-serializedparams-boolean-dryrun---starlarkrunresult-runresult-error-error func (enclaveCtx *EnclaveContext) RunStarlarkPackageBlocking( ctx context.Context, @@ -519,7 +522,7 @@ func getErrFromStarlarkRunResult(result *StarlarkRunResult) error { } func (enclaveCtx *EnclaveContext) assembleRunStartosisPackageArg( - packageRootPath string, + kurtosisYaml *KurtosisYaml, relativePathToMainFile string, mainFunctionName string, serializedParams string, @@ -528,14 +531,9 @@ func (enclaveCtx *EnclaveContext) assembleRunStartosisPackageArg( experimentalFeatures []kurtosis_core_rpc_api_bindings.KurtosisFeatureFlag, cloudInstanceId string, cloudUserId string, -) (*kurtosis_core_rpc_api_bindings.RunStarlarkPackageArgs, *KurtosisYaml, error) { - kurtosisYamlFilepath := path.Join(packageRootPath, kurtosisYamlFilename) +) (*kurtosis_core_rpc_api_bindings.RunStarlarkPackageArgs, error) { - kurtosisYaml, err := ParseKurtosisYaml(kurtosisYamlFilepath) - if err != nil { - return nil, nil, stacktrace.Propagate(err, "There was an error parsing the '%v' at '%v'", kurtosisYamlFilename, packageRootPath) - } - return binding_constructors.NewRunStarlarkPackageArgs(kurtosisYaml.PackageName, relativePathToMainFile, mainFunctionName, serializedParams, dryRun, parallelism, experimentalFeatures, cloudInstanceId, cloudUserId), kurtosisYaml, nil + return binding_constructors.NewRunStarlarkPackageArgs(kurtosisYaml.PackageName, relativePathToMainFile, mainFunctionName, serializedParams, dryRun, parallelism, experimentalFeatures, cloudInstanceId, cloudUserId), nil } func (enclaveCtx *EnclaveContext) uploadStarlarkPackage(packageId string, packageRootPath string) error { @@ -572,3 +570,20 @@ func (enclaveCtx *EnclaveContext) uploadStarlarkPackage(packageId string, packag } return nil } + +func getKurtosisYaml(packageRootPath string) (*KurtosisYaml, error) { + kurtosisYamlFilepath := path.Join(packageRootPath, kurtosisYamlFilename) + + kurtosisYaml, err := ParseKurtosisYaml(kurtosisYamlFilepath) + if err != nil { + return nil, stacktrace.Propagate(err, "There was an error parsing the '%v' at '%v'", kurtosisYamlFilename, packageRootPath) + } + return kurtosisYaml, nil +} + +func isLocalDependencyReplace(replace string) bool { + if strings.HasPrefix(replace, osPathSeparatorString) || strings.HasPrefix(replace, dotRelativePathIndicatorString) { + return true + } + return false +} diff --git a/api/typescript/src/core/lib/enclaves/enclave_context.ts b/api/typescript/src/core/lib/enclaves/enclave_context.ts index 5f394c43bd..acc7144293 100644 --- a/api/typescript/src/core/lib/enclaves/enclave_context.ts +++ b/api/typescript/src/core/lib/enclaves/enclave_context.ts @@ -34,7 +34,7 @@ import { GetStarlarkRunResponse, } from "../../kurtosis_core_rpc_api_bindings/api_container_service_pb"; import * as path from "path"; -import {parseKurtosisYaml} from "./kurtosis_yaml"; +import {parseKurtosisYaml, KurtosisYaml} from "./kurtosis_yaml"; import {Readable} from "stream"; import {readStreamContentUntilClosed, StarlarkRunResult} from "./starlark_run_blocking"; import {ServiceIdentifiers} from "../services/service_identifiers"; @@ -44,6 +44,10 @@ export type EnclaveUUID = string; export const KURTOSIS_YAML_FILENAME = "kurtosis.yml"; +const OS_PATH_SEPARATOR_STRING = "/" + +const DOT_RELATIVE_PATH_INDICATOR_STRING = "." + // Docs available at https://docs.kurtosis.com/sdk/#enclavecontext export class EnclaveContext { @@ -142,10 +146,37 @@ export class EnclaveContext { packageRootPath: string, runConfig: StarlarkRunConfig, ): Promise> { - const args = await this.assembleRunStarlarkPackageArg(packageRootPath, runConfig.relativePathToMainFile, runConfig.mainFunctionName, runConfig.serializedParams, runConfig.dryRun, runConfig.cloudInstanceId, runConfig.cloudUserId) + const kurtosisYmlResult = await this.getKurtosisYaml(packageRootPath) + if (kurtosisYmlResult.isErr()) { + return err(new Error(`Unexpected error while getting the Kurtosis yaml file from path '${packageRootPath}'`)) + } + + const kurtosisYaml: KurtosisYaml = kurtosisYmlResult.value + const packageId: string = kurtosisYaml.name + const packageReplaceOptions: Map = kurtosisYaml.packageReplaceOptions + + const args = await this.assembleRunStarlarkPackageArg(kurtosisYaml, runConfig.relativePathToMainFile, runConfig.mainFunctionName, runConfig.serializedParams, runConfig.dryRun, runConfig.cloudInstanceId, runConfig.cloudUserId) if (args.isErr()) { return err(new Error(`Unexpected error while assembling arguments to pass to the Starlark executor \n${args.error}`)) } + + const archiverResponse = await this.genericTgzArchiver.createTgzByteArray(packageRootPath) + if (archiverResponse.isErr()){ + return err(archiverResponse.error) //TODO add msg + } + + const uploadStarlarkPackageResponse = await this.backend.uploadStarlarkPackage(packageId, archiverResponse.value) + if (uploadStarlarkPackageResponse.isErr()){ + return err(uploadStarlarkPackageResponse.error) // TODO add msg + } + + if (packageReplaceOptions.size > 0) { + const uploadLocalStarlarkPackageDependenciesResponse = await this.uploadLocalStarlarkPackageDependencies(packageRootPath, packageReplaceOptions) + if (uploadLocalStarlarkPackageDependenciesResponse.isErr()) { + return err(new Error(`Unexpected error while uploading local Starlark package dependencies '${packageReplaceOptions}' from '${packageRootPath}' \n${uploadLocalStarlarkPackageDependenciesResponse.error}`)) + } + } + const packageRunResult : Result = await this.backend.runStarlarkPackage(args.value) if (packageRunResult.isErr()) { return err(new Error(`Unexpected error happened executing Starlark package \n${packageRunResult.error}`)) @@ -381,7 +412,7 @@ export class EnclaveContext { } private async assembleRunStarlarkPackageArg( - packageRootPath: string, + kurtosisYaml: KurtosisYaml, relativePathToMainFile: string, mainFunctionName: string, serializedParams: string, @@ -389,21 +420,8 @@ export class EnclaveContext { cloudInstanceId: string, cloudUserId: string, ): Promise> { - const kurtosisYamlFilepath = path.join(packageRootPath, KURTOSIS_YAML_FILENAME) - - const resultParseKurtosisYaml = await parseKurtosisYaml(kurtosisYamlFilepath) - if (resultParseKurtosisYaml.isErr()) { - return err(resultParseKurtosisYaml.error) - } - const kurtosisYaml = resultParseKurtosisYaml.value - - const archiverResponse = await this.genericTgzArchiver.createTgzByteArray(packageRootPath) - if (archiverResponse.isErr()){ - return err(archiverResponse.error) - } const args = new RunStarlarkPackageArgs; - args.setLocal(archiverResponse.value) args.setPackageId(kurtosisYaml.name) args.setSerializedParams(serializedParams) args.setDryRun(dryRun) @@ -413,4 +431,47 @@ export class EnclaveContext { args.setCloudUserId(cloudUserId) return ok(args) } + + private async getKurtosisYaml(packageRootPath: string): Promise> { + const kurtosisYamlFilepath = path.join(packageRootPath, KURTOSIS_YAML_FILENAME) + + const resultParseKurtosisYaml = await parseKurtosisYaml(kurtosisYamlFilepath) + if (resultParseKurtosisYaml.isErr()) { + return err(resultParseKurtosisYaml.error) + } + const kurtosisYaml = resultParseKurtosisYaml.value + + return ok(kurtosisYaml) + } + + + private async uploadLocalStarlarkPackageDependencies( + packageRootPath: string, + packageReplaceOptions: Map, + ): Promise> { + for (const [dependencyPackageId, replaceOption] of packageReplaceOptions.entries()) { + if (this.isLocalDependencyReplace(replaceOption)) { + const localPackagePath: string = path.join(packageRootPath, replaceOption) + + const archiverResponse = await this.genericTgzArchiver.createTgzByteArray(localPackagePath) + if (archiverResponse.isErr()){ + return err(archiverResponse.error) + } + + const uploadStarlarkPackageResponse = await this.backend.uploadStarlarkPackage(dependencyPackageId, archiverResponse.value) + if (uploadStarlarkPackageResponse.isErr()){ + return err(uploadStarlarkPackageResponse.error) + } + return ok(null) + } + } + return ok(null) + } + + private isLocalDependencyReplace(replace: string): boolean { + if (replace.startsWith(OS_PATH_SEPARATOR_STRING) || replace.startsWith(DOT_RELATIVE_PATH_INDICATOR_STRING)) { + return true + } + return false + } } diff --git a/api/typescript/src/core/lib/enclaves/generic_api_container_client.ts b/api/typescript/src/core/lib/enclaves/generic_api_container_client.ts index c75e8d02c6..a930f31f0c 100644 --- a/api/typescript/src/core/lib/enclaves/generic_api_container_client.ts +++ b/api/typescript/src/core/lib/enclaves/generic_api_container_client.ts @@ -36,6 +36,7 @@ export interface GenericApiContainerClient { getServices(getServicesArgs: GetServicesArgs): Promise> execCommand(execCommandArgs: ExecCommandArgs): Promise> uploadFiles(name: string, payload: Uint8Array): Promise> + uploadStarlarkPackage(packageId: string, payload: Uint8Array): Promise> storeWebFilesArtifact(storeWebFilesArtifactArgs: StoreWebFilesArtifactArgs): Promise> downloadFilesArtifact(downloadFilesArtifactArgs: DownloadFilesArtifactArgs): Promise> getExistingAndHistoricalServiceIdentifiers(): Promise> diff --git a/api/typescript/src/core/lib/enclaves/grpc_node_api_container_client.ts b/api/typescript/src/core/lib/enclaves/grpc_node_api_container_client.ts index d7c6de5ea4..2186c1e2a0 100644 --- a/api/typescript/src/core/lib/enclaves/grpc_node_api_container_client.ts +++ b/api/typescript/src/core/lib/enclaves/grpc_node_api_container_client.ts @@ -202,6 +202,50 @@ export class GrpcNodeApiContainerClient implements GenericApiContainerClient { return ok(uploadFilesArtifactResponse) } + public async uploadStarlarkPackage(packageId: string, payload: Uint8Array): Promise> { + const uploadStarlarkPackagePromise: Promise> = new Promise((resolve, _unusedReject) => { + const clientStream = this.client.uploadStarlarkPackage((error: ServiceError | null, response?: google_protobuf_empty_pb.Empty) => { + if (error === null) { + if (!response) { + resolve(err(new Error("No error was encountered but the response was still falsy; this should never happen"))); + } else { + resolve(ok(null)); + } + } else { + resolve(err(error)); + } + }) + + const constantChunkMetadata = new DataChunkMetadata() + .setName(packageId) + + let previousChunkHash = "" + for (let idx = 0; idx < payload.length; idx += DATA_STREAM_CHUNK_SIZE) { + const currentChunk = payload.subarray(idx, Math.min(idx + DATA_STREAM_CHUNK_SIZE, payload.length)) + + const dataChunk = new StreamedDataChunk() + .setData(currentChunk) + .setPreviousChunkHash(previousChunkHash) + .setMetadata(constantChunkMetadata) + clientStream.write(dataChunk, (streamErr: Error | null | undefined) => { + if (streamErr != undefined) { + resolve(err(new Error(`An error occurred sending data through the stream:\n${streamErr.message}`))) + } + }) + previousChunkHash = this.computeHexHash(currentChunk) + } + clientStream.end() // close the stream, no more data will be sent + }); + + const uploadStarlarkPackageResponseResult: Result = await uploadStarlarkPackagePromise; + if(uploadStarlarkPackageResponseResult.isErr()){ + return err(uploadStarlarkPackageResponseResult.error) + } + + const uploadStarlarkPackageResponse = uploadStarlarkPackageResponseResult.value + return ok(uploadStarlarkPackageResponse) + } + public async storeWebFilesArtifact(storeWebFilesArtifactArgs: StoreWebFilesArtifactArgs): Promise> { const storeWebFilesArtifactPromise: Promise> = new Promise((resolve, _unusedReject) => { this.client.storeWebFilesArtifact(storeWebFilesArtifactArgs, (error: ServiceError | null, response?: StoreWebFilesArtifactResponse) => { diff --git a/api/typescript/src/core/lib/enclaves/kurtosis_yaml.ts b/api/typescript/src/core/lib/enclaves/kurtosis_yaml.ts index 8a681ffe6c..6ac2986970 100644 --- a/api/typescript/src/core/lib/enclaves/kurtosis_yaml.ts +++ b/api/typescript/src/core/lib/enclaves/kurtosis_yaml.ts @@ -7,7 +7,9 @@ const PACKAGES_URL = "https://docs.kurtosis.com/concepts-reference/packages"; export class KurtosisYaml { constructor( - public readonly name: string, + public readonly name: string, + public readonly description: string, + public readonly packageReplaceOptions: Map, ){} } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 76671db699..6c6e1ec836 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -38,6 +38,8 @@ const ( packageDocLink = "https://docs.kurtosis.com/concepts-reference/packages" osPathSeparatorString = string(os.PathSeparator) + dotRelativePathIndicatorString = "." + onlyOneReplace = 1 ) @@ -548,8 +550,7 @@ func getKurtosisYamlPathForFileUrlInternal(absPathToFile string, packagesDir str } func isLocalDependencyReplace(replace string) bool { - //TODO replace harcoded values - if strings.HasPrefix(replace, "/") || strings.HasPrefix(replace, ".") { + if strings.HasPrefix(replace, osPathSeparatorString) || strings.HasPrefix(replace, dotRelativePathIndicatorString) { return true } return false From 238f8e28a3fc2528b9d6fcefaaed02c2d7d59780 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Wed, 11 Oct 2023 09:25:56 -0300 Subject: [PATCH 05/14] error msgs added --- api/typescript/src/core/lib/enclaves/enclave_context.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/typescript/src/core/lib/enclaves/enclave_context.ts b/api/typescript/src/core/lib/enclaves/enclave_context.ts index acc7144293..a058e6429a 100644 --- a/api/typescript/src/core/lib/enclaves/enclave_context.ts +++ b/api/typescript/src/core/lib/enclaves/enclave_context.ts @@ -162,12 +162,12 @@ export class EnclaveContext { const archiverResponse = await this.genericTgzArchiver.createTgzByteArray(packageRootPath) if (archiverResponse.isErr()){ - return err(archiverResponse.error) //TODO add msg + return err(new Error(`Unexpected error while creating the package's tgs file from '${packageRootPath}'\n${archiverResponse.error}`)) } const uploadStarlarkPackageResponse = await this.backend.uploadStarlarkPackage(packageId, archiverResponse.value) if (uploadStarlarkPackageResponse.isErr()){ - return err(uploadStarlarkPackageResponse.error) // TODO add msg + return err(new Error(`Unexpected error while uploading Starlark package '${packageId}'\n${uploadStarlarkPackageResponse.error}`)) } if (packageReplaceOptions.size > 0) { From 0f84cc4337543b7d83957ae29eeb4c2507976889 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Wed, 11 Oct 2023 09:48:34 -0300 Subject: [PATCH 06/14] unused var removed --- .../package_replace_options_repository.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go index d055733d7a..0a47d40760 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go @@ -9,7 +9,6 @@ import ( var ( packageReplaceOptionsBucketName = []byte("package-replace-options-repository") - emptyValue = []byte{} ) type packageReplaceOptionsRepository struct { From a8838acf43ff678983fb468c00dab62484a11665 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Wed, 11 Oct 2023 11:06:07 -0300 Subject: [PATCH 07/14] fix --- api/typescript/src/core/lib/enclaves/enclave_context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/typescript/src/core/lib/enclaves/enclave_context.ts b/api/typescript/src/core/lib/enclaves/enclave_context.ts index a058e6429a..d380359134 100644 --- a/api/typescript/src/core/lib/enclaves/enclave_context.ts +++ b/api/typescript/src/core/lib/enclaves/enclave_context.ts @@ -170,7 +170,7 @@ export class EnclaveContext { return err(new Error(`Unexpected error while uploading Starlark package '${packageId}'\n${uploadStarlarkPackageResponse.error}`)) } - if (packageReplaceOptions.size > 0) { + if (packageReplaceOptions !== undefined && packageReplaceOptions.size > 0) { const uploadLocalStarlarkPackageDependenciesResponse = await this.uploadLocalStarlarkPackageDependencies(packageRootPath, packageReplaceOptions) if (uploadLocalStarlarkPackageDependenciesResponse.isErr()) { return err(new Error(`Unexpected error while uploading local Starlark package dependencies '${packageReplaceOptions}' from '${packageRootPath}' \n${uploadLocalStarlarkPackageDependenciesResponse.error}`)) From e8248e68c170ea5b44f30be0697dc8dcbca90af0 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 10:45:49 -0300 Subject: [PATCH 08/14] Replacing historical expression with existing --- .../git_package_content_provider.go | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 177d5cc34d..493844ff8f 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -236,30 +236,30 @@ func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( func (provider *GitPackageContentProvider) CloneReplacedPackagesIfNeeded(currentPackageReplaceOptions map[string]string) *startosis_errors.InterpretationError { - historicalPackageReplaceOptions, err := provider.packageReplaceOptionsRepository.Get() + existingPackageReplaceOptions, err := provider.packageReplaceOptionsRepository.Get() if err != nil { - return startosis_errors.WrapWithInterpretationError(err, "An error occurred getting the historical package replace options from the repository") + return startosis_errors.WrapWithInterpretationError(err, "An error occurred getting the existing package replace options from the repository") } - for packageId, historicalReplace := range historicalPackageReplaceOptions { + for packageId, existingReplace := range existingPackageReplaceOptions { shouldClonePackage := false - isHistoricalLocalReplace := isLocalDependencyReplace(historicalReplace) - logrus.Debugf("historicalReplace '%v' isHistoricalLocalReplace? '%v', ", historicalReplace, isHistoricalLocalReplace) + isExistingLocalReplace := isLocalDependencyReplace(existingReplace) + logrus.Debugf("existingReplace '%v' isExistingLocalReplace? '%v', ", existingReplace, isExistingLocalReplace) currentReplace, isCurrentReplace := currentPackageReplaceOptions[packageId] if isCurrentReplace { - // the package will be cloned if the current replace is remote and the historical is local + // the package will be cloned if the current replace is remote and the existing is local isCurrentRemoteReplace := !isLocalDependencyReplace(currentReplace) logrus.Debugf("currentReplace '%v' isCurrentRemoteReplace? '%v', ", isCurrentRemoteReplace, currentReplace) - if isCurrentRemoteReplace && isHistoricalLocalReplace { + if isCurrentRemoteReplace && isExistingLocalReplace { shouldClonePackage = true } } // there is no current replace for this dependency but the version in the cache is local - if !isCurrentReplace && isHistoricalLocalReplace { + if !isCurrentReplace && isExistingLocalReplace { shouldClonePackage = true } @@ -270,13 +270,13 @@ func (provider *GitPackageContentProvider) CloneReplacedPackagesIfNeeded(current } } - // upgrade the historical-replace list with the new values + // upgrade the existing-replace list with the new values for packageId, currentReplace := range currentPackageReplaceOptions { - historicalPackageReplaceOptions[packageId] = currentReplace + existingPackageReplaceOptions[packageId] = currentReplace } - if err = provider.packageReplaceOptionsRepository.Save(historicalPackageReplaceOptions); err != nil { - return startosis_errors.WrapWithInterpretationError(err, "An error occurred saving the historical package replace options from the repository") + if err = provider.packageReplaceOptionsRepository.Save(existingPackageReplaceOptions); err != nil { + return startosis_errors.WrapWithInterpretationError(err, "An error occurred saving the existing package replace options from the repository") } return nil } From f54fcda6e7667cfc326736bc673f779c52287daf Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 10:46:22 -0300 Subject: [PATCH 09/14] removed unused handler --- .../dependency_handler/dependency_handler.go | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go b/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go deleted file mode 100644 index d71794b768..0000000000 --- a/core/server/api_container/server/startosis_engine/startosis_packages/dependency_handler/dependency_handler.go +++ /dev/null @@ -1,15 +0,0 @@ -package dependency_handler - -import ( - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository" -) - -type DependencyHandler struct { - packageReplaceOptionsRepository *package_replace_options_repository.PackageReplaceOptionsRepository - gitPackageContentProvider *git_package_content_provider.GitPackageContentProvider -} - -func NewDependencyHandler(packageReplaceOptionsRepository *package_replace_options_repository.PackageReplaceOptionsRepository, gitPackageContentProvider *git_package_content_provider.GitPackageContentProvider) *DependencyHandler { - return &DependencyHandler{packageReplaceOptionsRepository: packageReplaceOptionsRepository, gitPackageContentProvider: gitPackageContentProvider} -} From ab7cb53663d5167019c72149ac5f760bd0034157 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 11:09:45 -0300 Subject: [PATCH 10/14] refactor in findPackageReplace --- .../git_package_content_provider.go | 70 +++---------------- .../git_package_content_provider/locators.go | 69 ++++++++++++++++++ 2 files changed, 77 insertions(+), 62 deletions(-) create mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 493844ff8f..4c9d16df56 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -38,8 +38,6 @@ const ( packageDocLink = "https://docs.kurtosis.com/concepts-reference/packages" osPathSeparatorString = string(os.PathSeparator) - dotRelativePathIndicatorString = "." - onlyOneReplace = 1 ) @@ -205,14 +203,14 @@ func (provider *GitPackageContentProvider) StorePackageContents(packageId string return packageAbsolutePathOnDisk, nil } -func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( +func (provider *GitPackageContentProvider) GetAbsoluteLocatorForxRelativeLocator( parentModuleId string, maybeRelativeLocator string, packageReplaceOptions map[string]string, ) (string, *startosis_errors.InterpretationError) { var absoluteLocator string - if isLocalAbsoluteLocator(maybeRelativeLocator, parentModuleId) { + if isSamePackageLocalAbsoluteLocator(maybeRelativeLocator, parentModuleId) { return "", startosis_errors.NewInterpretationError("The locator '%s' set in attribute is not a 'local relative locator'. Local absolute locators are not allowed you should modified it to be a valid 'local relative locator'", maybeRelativeLocator) } @@ -229,7 +227,10 @@ func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( absoluteLocator = parsedParentModuleId.getAbsoluteLocatorRelativeToThisURL(maybeRelativeLocator) } - replacedAbsoluteLocator := replaceAbsoluteLocator(absoluteLocator, packageReplaceOptions) + replacedAbsoluteLocator, interpretationErr := replaceAbsoluteLocator(absoluteLocator, packageReplaceOptions) + if interpretationErr != nil { + return "", interpretationErr + } return replacedAbsoluteLocator, nil } @@ -245,13 +246,13 @@ func (provider *GitPackageContentProvider) CloneReplacedPackagesIfNeeded(current shouldClonePackage := false - isExistingLocalReplace := isLocalDependencyReplace(existingReplace) + isExistingLocalReplace := isLocalLocator(existingReplace) logrus.Debugf("existingReplace '%v' isExistingLocalReplace? '%v', ", existingReplace, isExistingLocalReplace) currentReplace, isCurrentReplace := currentPackageReplaceOptions[packageId] if isCurrentReplace { // the package will be cloned if the current replace is remote and the existing is local - isCurrentRemoteReplace := !isLocalDependencyReplace(currentReplace) + isCurrentRemoteReplace := !isLocalLocator(currentReplace) logrus.Debugf("currentReplace '%v' isCurrentRemoteReplace? '%v', ", isCurrentRemoteReplace, currentReplace) if isCurrentRemoteReplace && isExistingLocalReplace { shouldClonePackage = true @@ -379,50 +380,6 @@ func (provider *GitPackageContentProvider) atomicClone(parsedURL *ParsedGitURL) return nil } -func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) string { - if absoluteLocator == "" { - return absoluteLocator - } - - found, packageToBeReplaced, replaceWithPackage := findPackageReplace(absoluteLocator, packageReplaceOptions) - if found { - // we skip if it's a local replace because we will use the same absolute locator - // due the file was already uploaded in the enclave's package cache - if isLocalDependencyReplace(replaceWithPackage) { - return absoluteLocator - } - replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) - logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) - return replacedAbsoluteLocator - } - - return absoluteLocator -} - -func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string) { - pathToAnalyze := absoluteLocator - for { - numberSlashes := strings.Count(pathToAnalyze, urlPathSeparator) - - // check for the minimal path e.g.: github.com/org/package - if numberSlashes < minimumSubPathsForValidGitURL { - break - } - lastIndex := strings.LastIndex(pathToAnalyze, urlPathSeparator) - - packageToBeReplaced := pathToAnalyze[:lastIndex] - replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] - if ok { - logrus.Debugf("dependency replace found for '%s', package '%s' will be replaced with '%s'", absoluteLocator, packageToBeReplaced, replaceWithPackage) - return true, packageToBeReplaced, replaceWithPackage - } - - pathToAnalyze = packageToBeReplaced - } - - return false, "", "" -} - // methods checks whether the root of the package is same as repository root // or it is a sub-folder under it func getPathToPackageRoot(parsedPackagePath *ParsedGitURL) string { @@ -552,14 +509,3 @@ func getKurtosisYamlPathForFileUrlInternal(absPathToFile string, packagesDir str } return filePathToKurtosisYamlNotFound, nil } - -func isLocalAbsoluteLocator(locator string, parentPackageId string) bool { - return strings.HasPrefix(locator, parentPackageId) -} - -func isLocalDependencyReplace(replace string) bool { - if strings.HasPrefix(replace, osPathSeparatorString) || strings.HasPrefix(replace, dotRelativePathIndicatorString) { - return true - } - return false -} diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go new file mode 100644 index 0000000000..17ff9319a2 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go @@ -0,0 +1,69 @@ +package git_package_content_provider + +import ( + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors" + "github.com/sirupsen/logrus" + "strings" +) + +const ( + dotRelativePathIndicatorString = "." +) + +func isLocalLocator(locator string) bool { + if strings.HasPrefix(locator, osPathSeparatorString) || strings.HasPrefix(locator, dotRelativePathIndicatorString) { + return true + } + return false +} + +func isSamePackageLocalAbsoluteLocator(locator string, parentPackageId string) bool { + return strings.HasPrefix(locator, parentPackageId) +} + +func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) (string, *startosis_errors.InterpretationError) { + if absoluteLocator == "" { + return absoluteLocator, nil + } + + found, packageToBeReplaced, replaceWithPackage, interpretationErr := findPackageReplace(absoluteLocator, packageReplaceOptions) + if interpretationErr != nil { + return "", interpretationErr + } + if found { + // we skip if it's a local replace because we will use the same absolute locator + // due the file was already uploaded in the enclave's package cache + if isLocalLocator(replaceWithPackage) { + return absoluteLocator, nil + } + replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) + logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) + return replacedAbsoluteLocator, nil + } + + return absoluteLocator, nil +} + +func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string, *startosis_errors.InterpretationError) { + urlToAnalyze, interpretationErr := parseGitURL(absoluteLocator) + if interpretationErr != nil { + return false, "", "", interpretationErr + } + gitUrl := urlToAnalyze.gitURL + + for { + + lastIndex := strings.LastIndex(gitUrl, urlPathSeparator) + + packageToBeReplaced := gitUrl[:lastIndex] + replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] + if ok { + logrus.Debugf("dependency replace found for '%s', package '%s' will be replaced with '%s'", absoluteLocator, packageToBeReplaced, replaceWithPackage) + return true, packageToBeReplaced, replaceWithPackage, nil + } + + gitUrl = packageToBeReplaced + } + + return false, "", "", nil +} From 32fcf2daebf849f65ace35415b1c909a240a1fff Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 11:16:39 -0300 Subject: [PATCH 11/14] Removed unused package --- .../git_package_content_provider.go | 2 +- .../package_replace_options_repository.go | 2 +- .../package_replace_options_repository_test.go | 2 +- .../startosis_packages/package_content_provider.go | 4 ++-- .../package_replace_options_repository.go | 12 ------------ 5 files changed, 5 insertions(+), 17 deletions(-) delete mode 100644 core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 4c9d16df56..526362a005 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -51,7 +51,7 @@ func NewGitPackageContentProvider(moduleDir string, tmpDir string, enclaveDb *en return &GitPackageContentProvider{ packagesDir: moduleDir, packagesTmpDir: tmpDir, - packageReplaceOptionsRepository: NewPackageReplaceOptionsRepository(enclaveDb), + packageReplaceOptionsRepository: newPackageReplaceOptionsRepository(enclaveDb), } } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go index 0a47d40760..74bf117e38 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository.go @@ -15,7 +15,7 @@ type packageReplaceOptionsRepository struct { enclaveDb *enclave_db.EnclaveDB } -func NewPackageReplaceOptionsRepository( +func newPackageReplaceOptionsRepository( enclaveDb *enclave_db.EnclaveDB, ) *packageReplaceOptionsRepository { return &packageReplaceOptionsRepository{ diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go index b10aaef371..04072e988f 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go @@ -63,7 +63,7 @@ func getPackageReplaceOptionsRepositoryForTest(t *testing.T) *packageReplaceOpti enclaveDb := &enclave_db.EnclaveDB{ DB: db, } - repository := NewPackageReplaceOptionsRepository(enclaveDb) + repository := newPackageReplaceOptionsRepository(enclaveDb) return repository } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go index 48812e2461..d9868b059f 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/package_content_provider.go @@ -29,8 +29,8 @@ type PackageContentProvider interface { // ClonePackage clones the package with the given id and returns the absolute path on disk ClonePackage(packageId string) (string, *startosis_errors.InterpretationError) - // GetAbsoluteLocatorForRelativeLocator returns the absolute package path for a relative module path and replace the package path if - // there is a valid option in the noPackageReplaceOptions map + // GetAbsoluteLocatorForRelativeLocator returns the absolute package path for a relative file path and replace the package path if + // there is a valid option in the packageReplaceOptions map GetAbsoluteLocatorForRelativeLocator(packageId string, relativeOrAbsoluteLocator string, packageReplaceOptions map[string]string) (string, *startosis_errors.InterpretationError) // GetKurtosisYaml returns the package kurtosis.yml file content diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go b/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go deleted file mode 100644 index 99a49623d1..0000000000 --- a/core/server/api_container/server/startosis_engine/startosis_packages/package_replace_options_repository/package_replace_options_repository.go +++ /dev/null @@ -1,12 +0,0 @@ -package package_replace_options_repository - -type PackageReplaceOptionsRepository struct { -} - -func NewPackageReplaceOptionsRepository() *PackageReplaceOptionsRepository { - return &PackageReplaceOptionsRepository{} -} - -func (repository *PackageReplaceOptionsRepository) Get(key string) map[string]string { - panic("Implement me") -} From 564d6dd4fadb0317f9f0a490277e1fe7cdde90cb Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 14:16:09 -0300 Subject: [PATCH 12/14] unit test added for testing overwritte --- .../git_package_content_provider.go | 2 +- ...package_replace_options_repository_test.go | 33 ++++++++++++++++--- .../startosis_replace_testsuite_test.go | 4 +-- .../replace-with-local/main.star | 1 - 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index 526362a005..f1323c673c 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -203,7 +203,7 @@ func (provider *GitPackageContentProvider) StorePackageContents(packageId string return packageAbsolutePathOnDisk, nil } -func (provider *GitPackageContentProvider) GetAbsoluteLocatorForxRelativeLocator( +func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( parentModuleId string, maybeRelativeLocator string, packageReplaceOptions map[string]string, diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go index 04072e988f..f21ed220ed 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/package_replace_options_repository_test.go @@ -24,15 +24,40 @@ func TestSaveAnGet_Success(t *testing.T) { require.Equal(t, allPackageReplaceOptionsForTest, historicalReplacePackageOptions) } +func TestSaveAnGet_OverwriteSuccess(t *testing.T) { + repository := getPackageReplaceOptionsRepositoryForTest(t) + + err := repository.Save(allPackageReplaceOptionsForTest) + require.NoError(t, err) + + randomReplaceOptionsForTest := map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "github.com/kurtosis-tech/random-package", + "github.com/kurtosis-tech/avalanche-package": "github.com/another-org/avalanche-package", + } + + err = repository.Save(randomReplaceOptionsForTest) + require.NoError(t, err) + + expectedReplacePackageOptions := map[string]string{ + "github.com/kurtosis-tech/sample-dependency-package": "github.com/kurtosis-tech/random-package", + "github.com/kurtosis-tech/avalanche-package": "github.com/another-org/avalanche-package", + "github.com/kurtosis-tech/ethereum-package": "github.com/another-org/ethereum-package", + } + + existingReplacePackageOptions, err := repository.Get() + require.NoError(t, err) + require.Equal(t, expectedReplacePackageOptions, existingReplacePackageOptions) +} + func TestSaveAnGet_SuccessForNoReplacePackageOptions(t *testing.T) { repository := getPackageReplaceOptionsRepositoryForTest(t) err := repository.Save(noPackageReplaceOptions) require.NoError(t, err) - historicalReplacePackageOptions, err := repository.Get() + existingReplacePackageOptions, err := repository.Get() require.NoError(t, err) - require.Equal(t, noPackageReplaceOptions, historicalReplacePackageOptions) + require.Equal(t, noPackageReplaceOptions, existingReplacePackageOptions) } func TestSave_ErrorWhenSavingNil(t *testing.T) { @@ -45,9 +70,9 @@ func TestSave_ErrorWhenSavingNil(t *testing.T) { func TestGet_SuccessEmptyRepository(t *testing.T) { repository := getPackageReplaceOptionsRepositoryForTest(t) - historicalReplacePackageOptions, err := repository.Get() + existingReplacePackageOptions, err := repository.Get() require.NoError(t, err) - require.Equal(t, noPackageReplaceOptions, historicalReplacePackageOptions) + require.Equal(t, noPackageReplaceOptions, existingReplacePackageOptions) } func getPackageReplaceOptionsRepositoryForTest(t *testing.T) *packageReplaceOptionsRepository { diff --git a/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go index b09c474b35..9636c6194d 100644 --- a/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go +++ b/internal_testsuites/golang/testsuite/startosis_replace_test/startosis_replace_testsuite_test.go @@ -39,8 +39,8 @@ func (suite *StartosisReplaceTestSuite) SetupTest() { } func (suite *StartosisReplaceTestSuite) TearDownTest() { - //err := suite.destroyEnclaveFunc() - //require.NoError(suite.T(), err, "Destroying the test suite's enclave process has failed, you will have to remove it manually") + err := suite.destroyEnclaveFunc() + require.NoError(suite.T(), err, "Destroying the test suite's enclave process has failed, you will have to remove it manually") } func (suite *StartosisReplaceTestSuite) RunPackage(ctx context.Context, packageRelativeDirpath string) (*enclaves.StarlarkRunResult, error) { diff --git a/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star b/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star index f9c88c2406..481d4a1675 100644 --- a/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star +++ b/internal_testsuites/starlark/packages-with-replace/replace-with-local/main.star @@ -6,7 +6,6 @@ EXPECTED_MSG_FROM_LOCAL_PACKAGE_MAIN = "msg-loaded-from-local-dependency" MSG_ORIGIN_MAIN = "main" MSG_ORIGIN_LOCAL_DEPENDENCY = "local" -# TODO remove https://github.com/kurtosis-tech/sample-startosis-load/tree/main/sample-package if it's not used def run(plan, message_origin=MSG_ORIGIN_MAIN): plan.print("Replace with local package loaded.") From b452e98a4888e6970916e3675c0357fc4bfe062b Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 14:34:08 -0300 Subject: [PATCH 13/14] fix findPackageReplace --- cli/cli/go.mod | 1 - .../git_package_content_provider/locators.go | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cli/cli/go.mod b/cli/cli/go.mod index fba601c46b..162300c783 100644 --- a/cli/cli/go.mod +++ b/cli/cli/go.mod @@ -55,7 +55,6 @@ require ( github.com/mholt/archiver v3.1.1+incompatible github.com/xlab/treeprint v1.2.0 golang.org/x/exp v0.0.0-20230725093048-515e97ebf090 - gopkg.in/segmentio/analytics-go.v3 v3.1.0 k8s.io/api v0.27.2 ) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go index 17ff9319a2..28af38637e 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go @@ -8,6 +8,7 @@ import ( const ( dotRelativePathIndicatorString = "." + subStrNotPresentIndicator = -1 ) func isLocalLocator(locator string) bool { @@ -45,6 +46,10 @@ func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[st } func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string, *startosis_errors.InterpretationError) { + if len(packageReplaceOptions) == 0 { + return false, "", "", nil + } + urlToAnalyze, interpretationErr := parseGitURL(absoluteLocator) if interpretationErr != nil { return false, "", "", interpretationErr @@ -55,6 +60,9 @@ func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string lastIndex := strings.LastIndex(gitUrl, urlPathSeparator) + if len(gitUrl) <= lastIndex || lastIndex == subStrNotPresentIndicator { + break + } packageToBeReplaced := gitUrl[:lastIndex] replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] if ok { From 491d41a4df16f9cfdfdbe0b43ca73dc1876c18ce Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Thu, 12 Oct 2023 14:52:25 -0300 Subject: [PATCH 14/14] reverted findPackageReplace function --- .../git_package_content_provider.go | 5 +-- .../git_package_content_provider/locators.go | 44 ++++++++----------- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go index f1323c673c..337ae2910b 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/git_package_content_provider.go @@ -227,10 +227,7 @@ func (provider *GitPackageContentProvider) GetAbsoluteLocatorForRelativeLocator( absoluteLocator = parsedParentModuleId.getAbsoluteLocatorRelativeToThisURL(maybeRelativeLocator) } - replacedAbsoluteLocator, interpretationErr := replaceAbsoluteLocator(absoluteLocator, packageReplaceOptions) - if interpretationErr != nil { - return "", interpretationErr - } + replacedAbsoluteLocator := replaceAbsoluteLocator(absoluteLocator, packageReplaceOptions) return replacedAbsoluteLocator, nil } diff --git a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go index 28af38637e..5429424e01 100644 --- a/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go +++ b/core/server/api_container/server/startosis_engine/startosis_packages/git_package_content_provider/locators.go @@ -1,7 +1,6 @@ package git_package_content_provider import ( - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors" "github.com/sirupsen/logrus" "strings" ) @@ -22,56 +21,51 @@ func isSamePackageLocalAbsoluteLocator(locator string, parentPackageId string) b return strings.HasPrefix(locator, parentPackageId) } -func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) (string, *startosis_errors.InterpretationError) { +func replaceAbsoluteLocator(absoluteLocator string, packageReplaceOptions map[string]string) string { if absoluteLocator == "" { - return absoluteLocator, nil + return absoluteLocator } - found, packageToBeReplaced, replaceWithPackage, interpretationErr := findPackageReplace(absoluteLocator, packageReplaceOptions) - if interpretationErr != nil { - return "", interpretationErr - } + found, packageToBeReplaced, replaceWithPackage := findPackageReplace(absoluteLocator, packageReplaceOptions) + if found { // we skip if it's a local replace because we will use the same absolute locator // due the file was already uploaded in the enclave's package cache if isLocalLocator(replaceWithPackage) { - return absoluteLocator, nil + return absoluteLocator } replacedAbsoluteLocator := strings.Replace(absoluteLocator, packageToBeReplaced, replaceWithPackage, onlyOneReplace) logrus.Debugf("absoluteLocator '%s' replaced with '%s'", absoluteLocator, replacedAbsoluteLocator) - return replacedAbsoluteLocator, nil + return replacedAbsoluteLocator } - return absoluteLocator, nil + return absoluteLocator } -func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string, *startosis_errors.InterpretationError) { +func findPackageReplace(absoluteLocator string, packageReplaceOptions map[string]string) (bool, string, string) { if len(packageReplaceOptions) == 0 { - return false, "", "", nil - } - - urlToAnalyze, interpretationErr := parseGitURL(absoluteLocator) - if interpretationErr != nil { - return false, "", "", interpretationErr + return false, "", "" } - gitUrl := urlToAnalyze.gitURL + pathToAnalyze := absoluteLocator for { + numberSlashes := strings.Count(pathToAnalyze, urlPathSeparator) - lastIndex := strings.LastIndex(gitUrl, urlPathSeparator) - - if len(gitUrl) <= lastIndex || lastIndex == subStrNotPresentIndicator { + // check for the minimal path e.g.: github.com/org/package + if numberSlashes < minimumSubPathsForValidGitURL { break } - packageToBeReplaced := gitUrl[:lastIndex] + lastIndex := strings.LastIndex(pathToAnalyze, urlPathSeparator) + + packageToBeReplaced := pathToAnalyze[:lastIndex] replaceWithPackage, ok := packageReplaceOptions[packageToBeReplaced] if ok { logrus.Debugf("dependency replace found for '%s', package '%s' will be replaced with '%s'", absoluteLocator, packageToBeReplaced, replaceWithPackage) - return true, packageToBeReplaced, replaceWithPackage, nil + return true, packageToBeReplaced, replaceWithPackage } - gitUrl = packageToBeReplaced + pathToAnalyze = packageToBeReplaced } - return false, "", "", nil + return false, "", "" }