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.

Sign up for GitHub

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

Jump to bottom

PDS prepare upload remote data to shared storage #3131

Merged
merged 29 commits into from
Jun 13, 2024
Merged

PDS prepare upload remote data to shared storage #3131

merged 29 commits into from
Jun 13, 2024

Conversation

lorriborri
Copy link
Member

@lorriborri lorriborri commented May 15, 2024 •
edited by de-jcup

@lorriborri lorriborri requested a review from de-jcup May 21, 2024 14:53
@lorriborri lorriborri marked this pull request as ready for review May 21, 2024 14:53
de-jcup
de-jcup requested changes May 23, 2024
View reviewed changes
Copy link
Member

@de-jcup de-jcup left a comment

Choose a reason for hiding this comment

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

Good job. but there a things to change/discuss.

sechub-wrapper-prepare/build.gradle Outdated Show resolved Hide resolved
...rapper-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/GitContext.java Outdated Show resolved Hide resolved
...rapper-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/GitContext.java Outdated Show resolved Hide resolved
...apper-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/JGitAdapter.java Outdated Show resolved Hide resolved
...apper-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/JGitAdapter.java Outdated Show resolved Hide resolved
...c/test/java/com/mercedesbenz/sechub/wrapper/prepare/modules/PrepareWrapperModuleGitTest.java Outdated Show resolved Hide resolved
...repare/src/test/java/com/mercedesbenz/sechub/wrapper/prepare/upload/FileNameSupportTest.java Outdated Show resolved Hide resolved
...repare/src/test/java/com/mercedesbenz/sechub/wrapper/prepare/upload/FileNameSupportTest.java Outdated Show resolved Hide resolved
...om/mercedesbenz/sechub/wrapper/prepare/upload/PrepareWrapperSharedVolumePropertiesSetup.java Outdated Show resolved Hide resolved
...om/mercedesbenz/sechub/wrapper/prepare/upload/PrepareWrapperSharedVolumePropertiesSetup.java Outdated Show resolved Hide resolved
@lorriborri lorriborri force-pushed the feature-3026-pds-prepare-upload-service branch from 9407d38 to 311dbee Compare May 24, 2024 13:53
@lorriborri lorriborri force-pushed the feature-3026-pds-prepare-upload-service branch from 311dbee to 43a6c11 Compare May 24, 2024 13:57
@lorriborri lorriborri requested review from de-jcup and winzj May 28, 2024 15:30
winzj
winzj requested changes May 29, 2024
View reviewed changes
Copy link
Member

@winzj winzj left a comment

Choose a reason for hiding this comment

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

Looking good, well done 👍

I just found some smaller parts, please have a look at my comments.

sechub-pds/src/main/java/com/mercedesbenz/sechub/pds/config/PDSConfigService.java Outdated Show resolved Hide resolved
sechub-pds/src/main/java/com/mercedesbenz/sechub/pds/job/PDSFileUploadJobService.java Outdated Show resolved Hide resolved
sechub-pds/src/test/java/com/mercedesbenz/sechub/pds/config/PDSConfigServiceTest.java Show resolved Hide resolved
...re/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/AbstractInputValidator.java Outdated Show resolved Hide resolved
...r-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/git/JGitAdapter.java Outdated Show resolved Hide resolved
...apper-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/ToolContext.java Outdated Show resolved Hide resolved
...er-prepare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/git/GitContext.java Outdated Show resolved Hide resolved
...c/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/git/PrepareWrapperModuleGit.java Outdated Show resolved Hide resolved
.../java/com/mercedesbenz/sechub/wrapper/prepare/modules/skopeo/PrepareWrapperModuleSkopeo.java Outdated Show resolved Hide resolved
...pare/src/main/java/com/mercedesbenz/sechub/wrapper/prepare/modules/skopeo/SkopeoContext.java Outdated Show resolved Hide resolved
@lorriborri lorriborri requested a review from winzj May 31, 2024 10:13
@de-jcup de-jcup force-pushed the feature-3026-pds-prepare-upload-service branch from ffa77bd to 0a135df Compare June 6, 2024 16:43
@de-jcup
Refactoring + changes #3026
8561617 
- validation now without inheritance by introducing input validation
  support class
- renamed some classes - e.g. WrapperSkopeo -> SkopeoWrapper etc.
- changed tests for validation
- removed package prepare.prepare
- moved parts which were used only internally by implementations
  to abstract base class + changed visibility to protected only
- reduced amount of packages, technical parts like factory moved to
  semantic packages or to main prepare package if there are was
  no special semantic
- refactored tests to make easier to read - e.g. same setup parts
  to beforeEach method etc.
@de-jcup
Reduced log output and speed up integration tests #3197, #3198
ba0f3ab 
- log outputs reduced /enhanced
- integration test reconnection of executor configurations on local
  test restarts do now only load necessary data
@de-jcup de-jcup force-pushed the feature-3026-pds-prepare-upload-service branch from 0a135df to 8029f32 Compare June 7, 2024 09:02
@de-jcup de-jcup force-pushed the feature-3026-pds-prepare-upload-service branch from 8029f32 to 2dd0cb7 Compare June 7, 2024 10:48
@de-jcup
Refactoring and bugfixes #3026
083b221 
- renamed ToolWrapper to AbstractToolWrapper
- fixed failure in javadoc of Scenario 22
- renamed some parts inside skopeo preparation module
- dropped getProcess() inside ProcessAdapter (avoid that anybody
  uses direct process access - all shall be done by adapter)
- removed pepare wrapper environment variable class, was not used
  any more
- extracted Git location converter and wrote tests
- url conversion method now returns an URL instead of a string
@de-jcup
Skopeo call changes #3026
6b535c4 
- replaced process builder call for cleanup on skopeo with
  java file api variant (delete auth file)
- skopeo now called with stdin input for password
- introduced process builder factory + tests
- process adapter supports now user input per stdin
- added test for process adapter support (stdin)
- moved logic for skopeo location conversion to own class
  (SkopeoLocationConverter) and wrote tests, afterwards
  implementation was changed
@de-jcup de-jcup force-pushed the feature-3026-pds-prepare-upload-service branch from 5f4a52e to d8bcdf8 Compare June 10, 2024 15:14
@de-jcup
Changed directory cleanup from process to plain java #3026
63ce627 
- introduced DirectoryAndFileSupport, wrote tests
- introduced AutoCleanupGitFilesFilter, wrote tests
- updated GitWrapperTest
- separated former git auto clean environment entry to
  KEY_PDS_PREPARE_MODULE_GIT_REMOVE_GIT_FILES_BEFORE_UPLOAD
  KEY_PDS_PREPARE_MODULE_GIT_CLONE_WITHOUT_GIT_HISTORY
- changed environment entries for skopeo as well, everything from
  a module is now prefixed "pds.prepare.module." etc.
@de-jcup de-jcup force-pushed the feature-3026-pds-prepare-upload-service branch from d8bcdf8 to 63ce627 Compare June 11, 2024 08:34
@de-jcup
Changed git prepare module cleanup behavior and documentatino #3026
30e7e64 
- when git history is not wanted, the .git folder is now
  always removed
- additional gitfiles are treated extra
- pds config file in pds prepare solution has now the parameters
  inside as optional
@de-jcup
Merge branch 'develop' into feature-3026-pds-prepare-upload-service
a4ac055
winzj
winzj requested changes Jun 12, 2024
View reviewed changes
Copy link
Member

@winzj winzj left a comment

Choose a reason for hiding this comment

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

Looking good 👍

I just had some things that are not clear to me.

sechub-pds-solutions/prepare/helm/pds-prepare/templates/deployment.yaml Outdated Show resolved Hide resolved
...-pds/src/main/java/com/mercedesbenz/sechub/pds/execution/PDSExecutionEnvironmentPrepare.java Outdated Show resolved Hide resolved
...s-archive/src/main/java/com/mercedesbenz/sechub/commons/archive/DirectoryAndFileSupport.java Show resolved Hide resolved
@de-jcup
Review changes #3026
87d2b9c 
- added PDS_WORKSPACE_AUTOCLEAN_DISABLED handling in deployment.yaml
  again
@de-jcup
Merge branch 'develop' into feature-3026-pds-prepare-upload-service
2e87924
@de-jcup
Review changes #3026
a1b1809 
- dropped unnecessary class
- added missing licence header
- small refactoring inside DirectoryAndFileSupport
winzj
winzj approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@winzj winzj left a comment

Choose a reason for hiding this comment

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

Looking good 👍

@de-jcup de-jcup merged commit 1617be0 into develop Jun 13, 2024
2 checks passed
@de-jcup de-jcup deleted the feature-3026-pds-prepare-upload-service branch June 13, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@de-jcup de-jcup de-jcup approved these changes

@winzj winzj winzj approved these changes

Assignees

@de-jcup de-jcup

@lorriborri lorriborri

@winzj winzj

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@lorriborri @de-jcup @winzj