Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(api): add source and qa-skip as parameters for plan and outputs endpoints #155

Merged

Conversation

gabriel-farache
Copy link
Contributor

In order to allow users to run Move2Kube with the API in a non-interactive mode, I added the remote-source query parameter in the plan endpoint and the skip-qa query parameter in the outputs endpoints.

remote-source is expected a git URL that will be used by the cmd.

skip-qa will set the the flag qa-skip to true when executing the transformation cmd.

This will allow CI pipeline to test the usage of M2K without any interaction or pre-configured file.

Needs konveyor/move2kube#1109 to be merged as it fixes a missing default value for SSHKeys; when skipping the QA, it causes an error.

Copy link

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

@github-actions github-actions bot added the feat label Nov 28, 2023
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (2e2bd55) 14.55% compared to head (f895e95) 14.48%.

Files Patch % Lines
internal/common/utils.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #155      +/-   ##
==========================================
- Coverage   14.55%   14.48%   -0.07%     
==========================================
  Files           2        2              
  Lines         213      214       +1     
==========================================
  Hits           31       31              
- Misses        178      179       +1     
  Partials        4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HarikrishnanBalagopal
Copy link
Contributor

HarikrishnanBalagopal commented Nov 29, 2023

@gabriel-farache Thanks for making the PR! A couple of questions:

  1. Is there a way to sanitize/validate the remote source/git URL?
  2. Is there a way to limit the size of the cloned repo to respect the max-upload-size flag
    rootCmd.Flags().Int("max-upload-size", 100*1024*1024, "Max size (in bytes) for file uploads.")
    ?

It might require some changes in https://github.com/konveyor/move2kube to use something like https://confluence.atlassian.com/bbkb/how-to-check-your-repository-s-size-and-identify-large-files-1178867192.html $ git count-objects -vH to limit the size.

We can return an error message if the size is too large.

@gabriel-farache
Copy link
Contributor Author

@gabriel-farache Thanks for making the PR! A couple of questions:

  1. Is there a way to sanitize the remote source URL and make sure that it is a real git URL?
  2. Is there a way to limit the size of the cloned repo to respect the max-upload-size flag
    rootCmd.Flags().Int("max-upload-size", 100*1024*1024, "Max size (in bytes) for file uploads.")

    ?

It might require some changes in https://github.com/konveyor/move2kube to use something like https://confluence.atlassian.com/bbkb/how-to-check-your-repository-s-size-and-identify-large-files-1178867192.html $ git count-objects -vH to limit the size

@HarikrishnanBalagopal For the 1st point, yes it's doable

For the 2nd point it would be more complicated
To do it we would have to clone the repo then compute its size; so the storage will be taken for that amount of time.
If I am not wrong, the CLI is cloning the repo in /tmp then it deletes it.
So what is the requirements behind max-upload-size?

@seshapad
Copy link
Member

@gabriel-farache The DCO for the PR has failed. To pass this test, could you please sign your commits?

@gabriel-farache
Copy link
Contributor Author

@seshapad Sorry, I forgot the -s in my commit command, it's done

@seshapad
Copy link
Member

@seshapad Sorry, I forgot the -s in my commit command, it's done

No worries. Thank you for addressing it.

@gabriel-farache gabriel-farache force-pushed the feat/add_query_params branch 3 times, most recently from 6522279 to 8d537b2 Compare December 1, 2023 08:20
Signed-off-by: gabriel-farache <gfarache@redhat.com>
Signed-off-by: gabriel-farache <gfarache@redhat.com>
@HarikrishnanBalagopal
Copy link
Contributor

HarikrishnanBalagopal commented Dec 2, 2023

@gabriel-farache Have implemented the size limit in CLI konveyor/move2kube#1111
Added a --max-clone-size flag to both the plan and transform commands.
It is the maximum size in bytes. Default is -1 where negative values mean infinite bytes.

Copy link
Contributor

@HarikrishnanBalagopal HarikrishnanBalagopal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HarikrishnanBalagopal HarikrishnanBalagopal merged commit dc14c8d into konveyor:main Dec 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants