Added package hash validation on restore #380
Conversation
ilchenkob
requested review from
bplasmeijer and
jeanfrancoislarente
as code owners
| $fileName = ($filePath.Replace(("{0}" -f $destinationPath), "")) -replace "([\/\\])", "" | ||
| $package = $packages.$fileName | ||
|
|
||
| if (Test-Path $filePath -PathType Leaf) | ||
| { | ||
| $requiredFile = Get-Item -Path $filePath | ||
| $requiredFileHash = $(Get-FileHash -Path $filePath).Hash | ||
|
|
||
| if ($requiredFile.Length -gt 0) | ||
| if ($package.hash -eq $requiredFileHash) |
There was a problem hiding this comment.
What happens in the event where the url and hash are not provided in the sitecore-packages.json file?
Do we need to add the hash for all files in the sitecore-packages.json file?
ie:
"Data Exchange Framework 5.0.0 rev. 01466.scwdp.zip": {
"url": "",
"hash": ""
}
There was a problem hiding this comment.
This is a good point. Should I add check to skip hash if it's empty?
For me it sounds better to put hashes for all files in sitecore-packages.json, but I understand that might be tricky to do it.
There was a problem hiding this comment.
It's up to you. Ideally the hash for all files exists, but it's probably more realistic to skip if hash is empty. Otherwise someone has to download the zip, convert it using SAT and then update the hash.
It's something that can be done over time and perhaps we can enforce future PRs to include the hash for items added to the json (guilty as charged) and as modifications are made, we can add the hash.
I'd say for now just add logic to skip if empty.
There was a problem hiding this comment.
@jeanfrancoislarente
I updated my changes: if hash in sitecore-packages.json is empty, then old check will be used, otherwise file hash will be verified.
ilchenkob
force-pushed
the
feature/downloaded-package-check
branch
from
873b3b1
to
6db0430
Compare
|
FYI, the reason why we haven't done this before is that it takes for ever to do if your packages are stored on for example a Azure Storage file share. Have you tested how long time it tasks to check all packages using a remote file share? |
|
@pbering |
|
Hoping someone who is using a remote fileshare can take this for a test-drive. |
jeanfrancoislarente
added
testing-help-needed
|
@jeanfrancoislarente / @pbering just want to clarify that Remote file share is the equivalent of Azure File Share. I can help testing this next week if this is still required. |
This PR contains updated version of
Invoke-PackageRestore.ps1script: it checks hashes of downloaded packages.It helps to prevent build failure on a later stage if some package was not fully downloaded or is corrupted