From b24410c985dd3125e25fcabc021e6870d1a7cfd4 Mon Sep 17 00:00:00 2001 From: Leandro Poroli Date: Mon, 9 Oct 2023 17:58:51 -0300 Subject: [PATCH] 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