Add opt-in version-check exclusions for unversioned orchestrations - #1408
wangbill (YunchuWang) wants to merge 9 commits into
Conversation
Allow hosts to exempt exact registered infrastructure orchestration names from worker version checks only for null or empty execution versions. Preserve existing policies, execution identities, and constructor signatures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Move both test files into the active lowercase test project so they are compiled and executed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds opt-in exclusions for unversioned infrastructure orchestrations while preserving existing version policies.
Changes:
- Adds ordinal, instance-scoped orchestration exclusions.
- Bypasses checks only for excluded null/empty-version executions.
- Adds documentation and dispatcher/settings tests.
| File | Summary |
|---|---|
Test/DurableTask.Core.Tests/VersionSettingsTests.cs |
Adds settings tests. Moderate (3 votes): outside the active test project, so tests are not compiled or run. |
Test/DurableTask.Core.Tests/TaskOrchestrationDispatcherVersioningTests.cs |
Adds dispatcher tests. Moderate (3 votes): outside the active test project, so tests are not compiled or run. |
src/DurableTask.Core/TaskOrchestrationDispatcher.cs |
Applies exclusions at the version gate. |
src/DurableTask.Core/Settings/VersioningSettings.cs |
Adds exclusion configuration. |
docs/features/versioning.md |
Documents configuration and behavior. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Move the shared purge models into Core with their existing null-only token checks and disposition values. Add an optional BCL-only service client interface without changing existing client contracts or the version exemption. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document that DateTime.MaxValue leaves the deadline unspecified by the caller and that the backing service may apply a default. Setting auto-purge records the choice without managing a runner. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
changed the title
Keep Core free of purge contracts and SDK Client dependencies. Add the standalone interface package, SDK model identity and Core boundary tests, and standard build/sign/pack wiring. Require an explicitly supplied compatible SDK package until the model release is available. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The dedicated purge contract stage already builds its product project. Leave existing Core, Azure Storage, and Emulator validation independent of the SDK release prerequisite. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Core test coverage is not wired into the active project, and the boundary test references the wrong tombstone type.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (3)
Resolved since last review (1)
Remove the standalone contract project, extraction-only tests, and SDK dependency/release wiring from this repository. Restore all non-versioning files to the original feature baseline while preserving the generic opt-in version exemption unchanged. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
changed the title
Move the two versioning test files into the existing lowercase test project directory so case-sensitive checkouts include them. Preserve both file blobs unchanged. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Blob auto-purge: staged merge and release orderThis PR is stage 1 of the cross-repository integration. Review can proceed in parallel, but merge/release must follow the dependency order:
Gate: completing four consecutive merges is not sufficient. Each upstream package must actually be published and its complete dependency closure restorable before the next consumer is merged. Local validation artifacts or matching version numbers alone are not release-readiness evidence. Keep auto-purge explicitly disabled until the compatible worker/provider/backend/storage rollout is verified. |
|
I don't know if this is something that we should do. The more ways we add to ignore a system built to accept/deny work the more likely it is to be mis-used. I also think, from a CX side, it's odd that we can allow users to enable features that require infrastructure orchestrations but then also require they add them manually to this filter. What I would do instead is just keep a constant list of all the infrastructure orchestrations that exist since we own them. We then compare against that constant list instead of having users manually add to the exclusion list. You could also just have the feature registration add to the list, but I'd rather not have that be specifiable since it's a very specific case we're accounting for. You could also keep this, but make it internal and then have the extension methods enabling the feature add it to this. Though I'd prefer if we didn't keep it around in general. |
Make VersioningSettings.ExcludedOrchestrationNames internal instead of public; it was never shipped (only in this open PR) and is reserved for the Durable Task Framework's own approved integrations, not a customer-configurable allow list. Grant friend access to the exact Azure Functions Durable Task in-process host assembly via InternalsVisibleTo, using its full public key when Core is signed (Release) and simple name when Core is unsigned (Debug/test), matching the existing test-friend convention. No business version policy, dispatcher behavior, or other public API changes. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the inaccurate '(in-process worker)' wording from the InternalsVisibleTo comment; the WebJobs extension is a separate host, not the language worker. Copilot-Session: 883b4cbd-e93c-4d4e-8cb8-ffcf69525cfa Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| }; | ||
| if (configureExclusion) | ||
| { | ||
| settings.ExcludedOrchestrationNames.Add(InternalName); |
| PropertyInfo internalProperty = typeof(VersioningSettings).GetProperty( | ||
| "ExcludedOrchestrationNames", BindingFlags.NonPublic | BindingFlags.Instance); | ||
| Assert.IsNotNull(internalProperty, "ExcludedOrchestrationNames must remain accessible internally."); | ||
| Assert.IsTrue(internalProperty.GetMethod.IsAssembly, "The getter must be internal, not public or private."); |
|
halspang Addressed the public API/CX concern in this PR and the companion Functions PR #3556.
Changes: Core This uses the internal feature-registration alternative from your comment, rather than a public bypass knob or SDK-specific constants in Core. IVT exposes the host to Core's internal surface and adds signing/internal-ABI release coupling; it is not a security boundary. Compatible Core publication and real downstream dependency updates remain pre-merge gates; no production package or PR merge was performed. |
Summary
Keep worker-version exemptions for automatically registered, unversioned infrastructure orchestrations internal to Core and its Functions host integration.
The orchestration dispatcher applies worker version policies before middleware and orchestration execution. A versioned worker therefore cannot use middleware to execute an unversioned infrastructure orchestration without rejecting or failing it, or disabling version checks for business orchestrations.
VersioningSettings.ExcludedOrchestrationNamesis an internal, instance-scoped, initially emptyISet<string>using exact, case-sensitive ordinal name matching. It is not a public customer-configurable allow list. An exclusion applies only when the execution version is null or empty. Nonempty versions, including whitespace, remain subject to the existing matching and failure policies even when their name is in the set.Scope and compatibility
Infrastructure identities remain owned and validated by their integrations, not hardcoded in Core. Core grants the exact WebJobs Functions host friend access using its full signing public key; normal consumers cannot access the internal collection. Older workers continue applying their existing version policies; deployments must account for that before routing unversioned infrastructure executions to them. Friend access exposes Core's internal surface to the host and adds internal-ABI/signing compatibility requirements; it is not an authentication or security boundary.
The net change in this PR is limited to the internal generic Core version exemption, the signed Functions-host friendship, tests, and documentation. The optional large payload purge interface package is owned by the SDK repository in microsoft/durabletask-dotnet#805, not by this repository. Core contains no purge contract models or type forwarders and has no dependency on that package or SDK Client. The solution, dependency manifest, and CI/release wiring have no net changes here.
Validation
Focused tests exercise the real dispatcher, middleware, and executor, with an in-memory backend test double recording completion and abandonment.
No cloud, storage, SQL, or mixed-worker deployment suites were run for this Core change.
October 6 internal registration follow-up
Core commits
1ac92281and31b40089remove the public exclusion surface and add the exactMicrosoft.Azure.WebJobs.Extensions.DurableTaskfriend declaration. Signed Release uses the host's full .NET Foundation public key (host token014045d636e89289, distinct from Core's unchangedd53979610a6e89dd); unsigned Core builds retain the existing test friends and declare the host by simple name. The dispatcher guard is unchanged. No SDK orchestration name or dependency was added to Core.The companion Functions source in Azure/azure-functions-durable-extension#3556, commit
bba3cf82, retains automatic registration of the exact marked .NET isolated infrastructure orchestration. Actual host Debug and Release builds are both signed and compiled against the freshly packed Core3.10.1-local.ivt.20261006.31b40089. The host's full key matches the packaged Core friend metadata. Public reflection does not expose the collection; an ordinary consumer can compile the unchanged public versioning API but cannot compile collection access (actual Roslyn diagnostic CS1061).Fresh local scoped results: 40 Core cases on each of net8.0/net48, plus 73 host and 78 worker cases on each of net8.0/net10.0. Package hashes, all resolved Core DLL copies, and the validated source bytes were checked. These are local verification artifacts, not production package releases or a claim that hosted CI is green. Actual compatible Core publication and downstream dependency updates remain merge/release prerequisites.