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

Index.json in Consolidated zip stores full path, not just hash #1416

Open
wants to merge 2 commits into
base: release/27.3.5
Choose a base branch
from

Conversation

rain-on
Copy link
Collaborator

@rain-on rain-on commented Dec 17, 2024

At the moment the index.json in consolidated calamari contains a list of hashes (aka directories) which must be consolidated in order for a given flavour of calamari to execute.

Thus when octopus is trying to construct a calamari to run, it copies ALL files from each of the listed directories into a single zip (with pathing) - and sends that over to the target.

Up until now, that works correctly.

As part of the migration to netcore - AzureWebApp required a "shim app" to wrap the capabilities in Microsoft.Deployments.Web (which does not have a netcore counterpart) - this shim-app is a netfx app - and uses newtonsoft.

When a consolidated calamari is created - the AzureWebApp now list in its hashes:

  • Directories containing required files for the netcore capabilities
  • Directories containing required files for the shim-app.

Unfortunately, this means that Newtonsoft is pulled in 3 times:

  • 👍 Once, in root directory for netcore
  • 👍 Once, in shim-app subdirectory for netfx
  • 👎 Once again in root directory, due to nature of "copy everything in hash-dir"/.

Thus - the netcore version of newtonsoft in the root-directory is overwritten with the netfx version - meaning Calamari.AzureWebApp fails to start when it is reconstructed from a consolidated calamari.

================

The solution is to replace the hashes, with a full-blown file-reference (See example below).

NOTE: Linux path separators are used as they are valid cross platform.

{ "Packages": { "Calamari": { "PackageId": "Calamari", "Version": "27.3.5-pullrequest1416-0028", "IsNupkg": true, "PlatformHashes": { "netfx": [ "01656ee57f64634592a6f674815ad8b6/Cloud/Microsoft.Azure.Management.Network.Fluent.dll", "01656ee57f64634592a6f674815ad8b6/Microsoft.Azure.Management.Network.Fluent.dll", "022cfe9926214ee1ca1326f7582bb06c/Octopus.Versioning.dll", "0589b3488d5f50ef7bffca1f6e4d5740/Calamari.CloudAccounts.pdb", "0589b3488d5f50ef7bffca1f6e4d5740/Cloud/Calamari.CloudAccounts.pdb", "05d1b950c470ea8b0aa357f9a59cf264/Cloud/System.Resources.Writer.dll",

@rain-on rain-on changed the base branch from main to release/27.3.5 December 17, 2024 04:29
@rain-on rain-on changed the title Backport of index.json changes to callout specific file Index.json in Consolidated zip stores full path, not just hash Dec 17, 2024
Copy link
Contributor

@IsaacCalligeros95 IsaacCalligeros95 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -39,7 +39,8 @@ public IReadOnlyList<SourceFile> GetSourceFiles(ILogger log)
IsNupkg = false,
FullNameInDestinationArchive = string.Join("/", parts.Skip(1)),
FullNameInSourceArchive = entry.FullName,
Hash = hasher.Hash(entry)
Hash = hasher.Hash(entry),
FileName = parts.Last()
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW entry.Name should give you the file name

var entryName = Path.Combine(uniqueFile.Hash, uniqueFile.FullNameInDestinationArchive);
var entry = zip.CreateEntry(entryName, CompressionLevel.Fastest);

var entry = zip.CreateEntry(uniqueFile.EntryNameInConsolidationArchive(), CompressionLevel.Fastest);
Copy link
Contributor

Choose a reason for hiding this comment

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

could CompressionLevel.SmallestSize be a good way to go? We can afford extra creation time if it makes it a smaller archive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants