Skip to content

Calamari .NET Framework cleanup#1883

Open
sathvikkumar-octo wants to merge 6 commits intomainfrom
sk/netfx-cleanup
Open

Calamari .NET Framework cleanup#1883
sathvikkumar-octo wants to merge 6 commits intomainfrom
sk/netfx-cleanup

Conversation

@sathvikkumar-octo
Copy link
Copy Markdown
Contributor

Noticed some .NET framework fixes and ended up in a rabbit hole of cleaning up bits that are no longer relevant in .NET 8 land.

  • Remove obsolete polyfill NuGet packages not required for .NET 8
  • Remove TLS 1.2 workarounds (default on net8.0)
  • Remove RunResolvePackageDependencies MSBuild workaround
  • Remove dead MSBuild conditions, code, and variables
  • Delete source/Directory.Build.props (moved sole usage inline)
  • Resolve TODOs that were previously blocked by .NET Framework constraints
  • Update stale comments referencing .NET Framework

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes legacy .NET Framework-era compatibility code and build workarounds now that Calamari targets .NET 8, simplifying runtime behavior and project/build configuration.

Changes:

  • Removed obsolete polyfill NuGet packages and dead framework conditionals across projects.
  • Removed TLS/security-protocol workarounds and associated test setup/code paths.
  • Simplified MSBuild targets by removing the RunResolvePackageDependencies workaround and inlining the only Directory.Build.props usage.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
source/Directory.Build.props Deleted now-unused IsWindowsNetCoreBuild property definition.
source/Calamari/Kubernetes/Commands/KubernetesDeploymentCommandBase.cs Removed outdated .NET Framework comment and simplified default async implementation.
source/Calamari/Calamari.csproj Dropped .NET Framework reference assemblies + other obsolete package refs; removed MSBuild workaround target dependency.
source/Calamari.Tests/GlobalTestSetup.cs Removed TLS setup hook that bypassed main entrypoint during tests.
source/Calamari.Tests/Calamari.Tests.csproj Removed obsolete package reference; removed MSBuild workaround; removed net462-only copy steps.
source/Calamari.Testing/Calamari.Testing.csproj Removed obsolete RuntimeInformation package reference.
source/Calamari.Terraform.Tests/CommandsFixture.cs Replaced local chmod helper with shared ExecutableHelper and removed duplicated implementation.
source/Calamari.Shared/Integration/Packages/NuGet/InternalNuGetPackageDownloader.cs Removed .NET Framework-only warning about OctopusNugetHttpTimeout.
source/Calamari.Shared/Integration/Packages/Download/NpmPackageDownloader.cs Removed explicit TLS 1.2 ServicePointManager tweak.
source/Calamari.Shared/Integration/Packages/Download/MavenPackageDownloader.cs Removed explicit TLS 1.2 ServicePointManager tweak.
source/Calamari.Shared/Calamari.Shared.csproj Removed obsolete polyfill package references.
source/Calamari.GoogleCloudScripting/Calamari.GoogleCloudScripting.csproj Removed RunResolvePackageDependencies workaround from tooling extraction target.
source/Calamari.GoogleCloudScripting.Tests/Calamari.GoogleCloudScripting.Tests.csproj Removed obsolete RuntimeInformation package reference.
source/Calamari.Common/Plumbing/Variables/KnownVariables.cs Removed unused/legacy NugetHttpTimeout variable constant.
source/Calamari.Common/Plumbing/SecurityProtocols.cs Deleted legacy TLS enabling helper.
source/Calamari.Common/Plumbing/Extensions/ScriptingEnvironment.cs Removed legacy framework-detection helper (IsNet45OrNewer).
source/Calamari.Common/CalamariFlavourProgramAsync.cs Removed startup TLS workaround invocation.
source/Calamari.Common/CalamariFlavourProgram.cs Removed startup TLS workaround invocation.
source/Calamari.Common/Calamari.Common.csproj Removed obsolete polyfill package references.
source/Calamari.AzureWebApp/Calamari.AzureWebApp.csproj Removed RunResolvePackageDependencies workaround from tooling extraction target.
source/Calamari.AzureWebApp.Tests/DeployAzureWebCommandFixture.cs Replaced stringly-typed account variable key with SpecialVariables.
source/Calamari.AzureWebApp.Tests/Calamari.AzureWebApp.Tests.csproj Inlined OS-platform check now that Directory.Build.props is removed.
source/Calamari.AzureScripting/Calamari.AzureScripting.csproj Removed RunResolvePackageDependencies workaround from tooling extraction target.
source/Calamari.AzureResourceGroup/Calamari.AzureResourceGroup.csproj Removed RunResolvePackageDependencies workaround from tooling extraction target.
source/Calamari.AzureResourceGroup.Tests/AzureResourceGroupActionHandlerFixture.cs Modernized async file IO usage in tests; removed stale framework-related comment.
source/Calamari.AzureAppService/Calamari.AzureAppService.csproj Removed RunResolvePackageDependencies workaround from tooling extraction target.
source/Calamari.Azure/Kubernetes/Discovery/AzureKubernetesDiscoverer.cs Updated stale comment to a current TODO about SDK migration.
credits/credits.md Removed credit entry for removed .NET Framework reference assemblies package.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sathvikkumar-octo sathvikkumar-octo marked this pull request as ready for review April 14, 2026 06:46
Copy link
Copy Markdown
Contributor

@APErebus APErebus left a comment

Choose a reason for hiding this comment

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

Look good to me. I do think we should run a full chain server branch with this PR before merging, just to be sure

var discoveredClusters = new List<KubernetesCluster>();

// There appears to be an issue where the azure client returns stale data
// We need to upgrade this to use the newer SDK, but we need to upgrade to .NET 4.6.2 to support that.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe Rob has a PR floating around that does this

Copy link
Copy Markdown
Contributor

@Jtango18 Jtango18 left a comment

Choose a reason for hiding this comment

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

This looks legit to me. Similar to other PR would like a 2nd set of eyes on it @APErebus

This also prompts the question - should just kill the AzureWebAppShim project all togteher now that we're pure net8?

@sathvikkumar-octo
Copy link
Copy Markdown
Contributor Author

sathvikkumar-octo commented Apr 14, 2026

Look good to me. I do think we should run a full chain server branch with this PR before merging, just to be sure

Found WhenRunningAdHocScript in server but this uses a mock script service so it doesn't execute Calamari
Added a consolidation test to check if the dotnet-script/ path is in the package but unsure if its valuable.

This also prompts the question - should just kill the AzureWebAppShim project all togteher now that we're pure net8?

We need the shim because the MS web deploy library is framework only unfortunately.

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