feat: allow git remote paths to be provided as source to move2kube #998

venkatbandarupalli · 2023-03-26T19:20:29Z

Details

Git remote path format

SSH

git@github.com git remote URL ^ branch,tag,commit ^ path within the repo
git@github.com git remote URL^ branch_branchname ^ path within the repo

HTTPS

https:// git remote URL ^ branch,tag,commit ^ path within the repo
https:// git remote URL ^ branch_ branchname ^ path within the repo

Example Usage

move2kube transform -s https://github.com/konveyor/move2kube-demos.git^branch_main^samples/enterprise-app --overwrite

github-actions · 2023-03-26T19:20:40Z

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

kmehant

Thanks for the PR, Venkat, can you look at my comment and also please update this branch. Thanks.

m2kqacache.yaml

HarikrishnanBalagopal · 2023-03-27T08:29:49Z

Thanks for the PR, Venkat, can you look at my comment and also please update this branch. Thanks.

Please consider this alternative, adding the feature behind a flag

$ move2kube transform --source-repo github.com/myaccount/mysourcerepo.git --cust-repo github.com/myaccount/mycustrepo.git

Make it easier to integrate with the UI later and it avoids breaking some assumptions we have made in the CLI, API and UI.

ashokponkumar

Nice work Venkat! A few suggestions.

ashokponkumar · 2023-03-27T09:12:16Z

cmd/transform.go

+		if err != nil {
+			logrus.Errorf("Error while getting current user's home directory: %s", err)
+		}
+		remoteSourcesFolderAbs, err := filepath.Abs(filepath.Join(homeDir, types.AppName, common.RemoteSourcesFolder))


we can use

move2kube/common/pathsofinterest.go

Line 53 in 6156882

TempPath = TempDirPrefix + "temp"

path to store data during the run.

Please make this configurable with a flag. We can keep temp dir as default if the flag is not provided.

$ move2kube transform \ --source-repo github.com/myaccount/mysourcerepo.git \ --source-repo-dir data/workspaces/1/projects/1/expanded/sources/1 --cust-repo github.com/myaccount/mycustrepo.git \ --cust-repo-dir data/workspaces/1/projects/1/expanded/customizations/1

ashokponkumar · 2023-03-27T09:14:29Z

common/githelper/githelper.go

+	// reference format - git+ssh://<git remote URL>^<branch,tag,commit>^<path within the repo>
+	// reference format - git+https://<git remote URL>^<branch,tag,commit>^<path within the repo>
+	// reference format - git+ssh://<git remote URL>^branch_<branchname>^<path within the repo>
+	// reference format - git+https://<git remote URL>^branch_<branchname>^<path within the repo>


Why not reuse existing formats like https://pip.pypa.io/en/stable/topics/vcs-support/?

Thanks for the PR, Venkat, can you look at my comment and also please update this branch. Thanks.

Please consider this alternative, adding the feature behind a flag

$ move2kube transform --source-repo github.com/myaccount/mysourcerepo.git --cust-repo github.com/myaccount/mycustrepo.git

Make it easier to integrate with the UI later and it avoids breaking some assumptions we have made in the CLI, API and UI.

Why would it impact the UI, it would just become an additional feature supported, isn't it? Having a single flag will help avoiding coordinating between the various ways we can get input, isn't it?

Why would it impact the UI, it would just become an additional feature supported, isn't it? Having a single flag will help avoiding coordinating between the various ways we can get input, isn't it?

Would like expose this feature in the API and UI so that users can provide the URL instead of a zip file for sources and customizations.
If we use a temporary folder it will keep cloning the same repo for every transformation. Making it configurable allows us to clone just once (clone the repo again only if the user tells us to pull some new change).

Having a single flag will help avoiding coordinating between the various ways we can get input, isn't it?

The format you mentioned https://pip.pypa.io/en/stable/topics/vcs-support/ is also valid.
I suggested using a separate flag for simplicity of implementation. It also makes it less likely to accidentally break the default flows we have right now.

HarikrishnanBalagopal · 2023-03-27T11:09:22Z

common/githelper/githelper.go

+	destpath := filepath.Join(basepath, destFolder)
+	_, err := os.Stat(destpath)
+	if err == nil {
+		return filepath.Join(destpath, grp.PathWithinRepo), err


return nil if there is no error

venkatbandarupalli · 2023-03-27T16:33:05Z

Hello @kmehant I have made the changes suggested by you.

Hello @ashokponkumar. I thank you for the suggestions.

The package (go git) which I am currently utilising in this work does not support the schema that you have mentioned here #998 (comment). It matches against this regex expression (https://github.com/go-git/go-git/blob/bf3471db54b0255ab5b159005069f37528a151b7/internal/url/url.go#L9) and returns an error as unsupported scheme "git+https". Hence, I conclude to use the formats mentioned in the code comment (https://github.com/konveyor/move2kube/pull/998/files#diff-97583a50e7076356ce7fbb5920570a45e7dd64aa07c2b32ca5c70546262f023aR37-R40).

#998 (comment) @ashokponkumar Certainly, I didn't know that we had that in the code, I will utilise that.

@HarikrishnanBalagopal I thank you for the suggestions.

@HarikrishnanBalagopal and @ashokponkumar I have understood both of your approaches for the flag. However, I don't see any conclusion derived from both of your comments for me to work on changes on using the existing -s flag or new flag for this feature. Which one should I move forward with?

kmehant · 2023-03-28T08:25:44Z

@venkatbandarupalli

Please use the format here https://pip.pypa.io/en/stable/topics/vcs-support/ which should essentially be the input format for the user. Then you could extract components out of it and use it for your clone operation.

Please use the existing -s flag to take the git remote path as input. Also, force clone everytime irrespective of the repository being cloned already or not. As Ashok mentioned, use the temp folder path where you would clone the folder.

Can you please extend this work for plan subcommand and also when we use plan and then transform usecase?
Refer : Involved usage and one step usage slides from https://move2kube.konveyor.io/presentation

Once you are ready with the work, can you as well post a video showing the working of the following?

move2kube transform usecase
move2kube plan usecase
move2kube plan and then transform usecase

cc @ashokponkumar @HarikrishnanBalagopal

kmehant · 2023-03-29T09:54:07Z

@venkatbandarupalli Please be careful while adding comments in Golang. Refer to this https://stackoverflow.com/questions/53004291/exported-type-should-have-comment-or-be-unexported-golang-vs-code.

Also, please run make test-style to check linting errors and correct them.

Thank you.

codecov · 2023-03-31T18:22:26Z

Codecov Report

Patch coverage has no change and project coverage change: -0.07 ⚠️

Comparison is base (205a4f5) 17.66% compared to head (8fa519d) 17.59%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #998      +/-   ##
==========================================
- Coverage   17.66%   17.59%   -0.07%     
==========================================
  Files          63       63              
  Lines        5249     5268      +19     
==========================================
  Hits          927      927              
- Misses       4074     4093      +19     
  Partials      248      248

Impacted Files	Coverage Δ
common/utils.go	`1.00% <0.00%> (-0.02%)`	⬇️
lib/planner.go	`0.00% <0.00%> (ø)`
types/plan/planutils.go	`0.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

venkatbandarupalli · 2023-03-31T18:40:36Z

@kmehant

Plan and then transform usecase

Transform usecase

venkatbandarupalli · 2023-03-31T18:45:53Z

cmd/plan.go

@@ -67,7 +67,8 @@ func planHandler(cmd *cobra.Command, flags planFlags) {
 	planfile := flags.planfile
 	srcpath := flags.srcpath
 	name := flags.name
-
+	srcPathStruct := common.GetInputClonedPath(srcpath, common.RemoteSourcesFolder, true)


Line 70 means the following

srcpath could be path on filesystem or git remote path

third argument is meant for overwrite and it is set to true

returned value srcPathStruct is Input struct that holds two values cloned filesystem path and git remote path.

this implementation is backward compatible. If the input is filesystem path from the user, then the input struct will hold the same filesystem path that user provided and remote git path will be an empty string.

venkatbandarupalli · 2023-03-31T18:51:25Z

common/pathconverters/pathconverters.go

+		keys := make([]string, 0, len(mapping))
+		for k := range mapping {
+			keys = append(keys, k)
+		}
+
+		sort.Slice(keys, func(i, j int) bool {
+			return len(keys[i]) > len(keys[j])
+		})
+
+		for _, src := range keys {
+			dest := mapping[src]


This change was discussed with @kmehant.

I have stumbled across a blocker for plan and then transform usecase. The problem was with the usage of temp directory to store the remote sources that are cloned. In the code here https://github.com/konveyor/move2kube/blob/6aee6e37fdb268e5cf0f25f35ffeba00291cc725/common/pathconverters/pathconverters.go#L191 a function is used to convert absolute paths in the plan file to relative paths before being written into plan file. To this function both temp directory and sources directory are sent. The implementation tries to get the relative path with any of the provided parent paths to the paths in the plan file. In the case of using remote move2kube sources in the temp dir makes both of these directories as potential parents for the paths in the plan file. Due to this reason, I have worked on a workaround where we consider the longest parent paths first and then move to shorter ones. So that the most matching parents paths are chosen. This workaround worked for me.

@venkatbandarupalli
This might fail in the below case

If these are the two inputs to the function

/a/v/temp/somefolder /a/v/temp/anotherfolder

If these are the paths in the plan file

/a/v/temp/anotherfolder/someconfig.yaml

Then ordering the paths based on length might result in

/a/v/temp/somefolder /a/v/temp/anotherfolder

As I see the first one is chosen always if its a parent then the relative path will become

..

However, the relative path should be

.

we might better iterate over all the paths and then choose the one have the highest prefix match in that case in the above example, the chosen path will be /a/v/temp/anotherfolder and relative path will be .

lets get inputs from @ashokponkumar and @HarikrishnanBalagopal as well.

venkatbandarupalli · 2023-03-31T18:52:30Z

common/vcs/git.go

+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+


This file implements the git vcs feature

venkatbandarupalli · 2023-03-31T18:53:18Z

common/vcs/git.go

+func isGitCommitHash(commithash string) bool {
+	gitCommitHashRegex := regexp.MustCompile(`^[a-fA-F0-9]{40}$`)
+	return gitCommitHashRegex.MatchString(commithash)
+}
+
+func isGitBranch(branch string) bool {
+	gitBranchRegex := regexp.MustCompile(`^(main|development|master|(features|tests|(bug|hot)fix)((\/|-)[a-zA-Z0-9]+([-_][a-zA-Z0-9]+)*){1,2}|release(\/|-)[0-9]+(\.[0-9]+)*(-(alpha|beta|rc)[0-9]*)?)$`)
+	return gitBranchRegex.MatchString(branch)
+}
+
+func isGitTag(tag string) bool {
+	gitTagRegex := regexp.MustCompile(`^v[0-9]+(\.[0-9]+)?(\.[0-9]+)?$`)
+	return gitTagRegex.MatchString(tag)
+}


Used regex for effective matching and extraction of parts of information from the input.

venkatbandarupalli · 2023-03-31T18:54:38Z

common/vcs/vcs.go

+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+


This file defines an interface for all the VCS kind of systems that could implement this interface in the future.

Signed-off-by: venkatbandarupalli <vbandarupalli19@gmail.com>

…kube Signed-off-by: venkatbandarupalli <vbandarupalli19@gmail.com>

kmehant

LGTM

github-actions bot added the feat label Mar 26, 2023

kmehant added the lfx-project https://lfx.linuxfoundation.org/tools/mentorship/ label Mar 26, 2023

kmehant requested changes Mar 26, 2023

View reviewed changes

m2kqacache.yaml Outdated Show resolved Hide resolved

venkatbandarupalli force-pushed the feat-git-src branch 2 times, most recently from 00b0c9f to 2475da3 Compare March 26, 2023 19:31

kmehant requested a review from ashokponkumar March 27, 2023 05:02

ashokponkumar requested changes Mar 27, 2023

View reviewed changes

HarikrishnanBalagopal reviewed Mar 27, 2023

View reviewed changes

venkatbandarupalli force-pushed the feat-git-src branch from 60f5a43 to b0a3f51 Compare March 31, 2023 18:08

venkatbandarupalli commented Mar 31, 2023

View reviewed changes

kmehant requested review from HarikrishnanBalagopal and ashokponkumar April 3, 2023 09:08

venkatbandarupalli force-pushed the feat-git-src branch from b0a3f51 to d7c307d Compare April 9, 2023 21:45

venkatbandarupalli added 3 commits April 9, 2023 16:49

feat: allow git remote paths to be provided as source to move2kube

00e02ef

Signed-off-by: venkatbandarupalli <vbandarupalli19@gmail.com>

feat: allow git remote paths to be provided as source to move2kube

63fbe31

Signed-off-by: venkatbandarupalli <vbandarupalli19@gmail.com>

fixup! feat: allow git remote paths to be provided as source to move2…

8fa519d

…kube Signed-off-by: venkatbandarupalli <vbandarupalli19@gmail.com>

venkatbandarupalli force-pushed the feat-git-src branch from 43c5a6e to 8fa519d Compare April 9, 2023 21:49

kmehant self-requested a review April 10, 2023 06:07

kmehant approved these changes Apr 10, 2023

View reviewed changes

ashokponkumar approved these changes Apr 10, 2023

View reviewed changes

kmehant removed the request for review from HarikrishnanBalagopal April 10, 2023 07:53

ashokponkumar merged commit 4bfde1d into konveyor:main Apr 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow git remote paths to be provided as source to move2kube #998

feat: allow git remote paths to be provided as source to move2kube #998

venkatbandarupalli commented Mar 26, 2023

github-actions bot commented Mar 26, 2023

kmehant left a comment

HarikrishnanBalagopal commented Mar 27, 2023 •

edited

Loading

ashokponkumar left a comment

ashokponkumar Mar 27, 2023

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

ashokponkumar Mar 27, 2023

ashokponkumar Mar 27, 2023

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

venkatbandarupalli commented Mar 27, 2023

kmehant commented Mar 28, 2023

kmehant commented Mar 29, 2023

codecov bot commented Mar 31, 2023 •

edited

Loading

venkatbandarupalli commented Mar 31, 2023

venkatbandarupalli Mar 31, 2023

venkatbandarupalli Mar 31, 2023

kmehant Apr 3, 2023

kmehant Apr 3, 2023

kmehant Apr 3, 2023

venkatbandarupalli Mar 31, 2023

venkatbandarupalli Mar 31, 2023

venkatbandarupalli Mar 31, 2023

kmehant left a comment

feat: allow git remote paths to be provided as source to move2kube #998

feat: allow git remote paths to be provided as source to move2kube #998

Conversation

venkatbandarupalli commented Mar 26, 2023

Details

Git remote path format

SSH

HTTPS

Example Usage

github-actions bot commented Mar 26, 2023

kmehant left a comment

Choose a reason for hiding this comment

HarikrishnanBalagopal commented Mar 27, 2023 • edited Loading

ashokponkumar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HarikrishnanBalagopal Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HarikrishnanBalagopal Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

HarikrishnanBalagopal Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

venkatbandarupalli commented Mar 27, 2023

kmehant commented Mar 28, 2023

kmehant commented Mar 29, 2023

codecov bot commented Mar 31, 2023 • edited Loading

Codecov Report

venkatbandarupalli commented Mar 31, 2023

Plan and then transform usecase

Transform usecase

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmehant left a comment

Choose a reason for hiding this comment

HarikrishnanBalagopal commented Mar 27, 2023 •

edited

Loading

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

HarikrishnanBalagopal Mar 27, 2023 •

edited

Loading

codecov bot commented Mar 31, 2023 •

edited

Loading