[ChangeSafety] Phase 1: AcquirePolicyToken Support #28711

notyashhh · 2025-10-17T03:00:55Z

Description

Mandatory Checklist

Please choose the target release of Azure PowerShell. (⚠️Target release is a different concept from API readiness. Please click below links for details.)
- General release
- Public preview
- Private preview
- Engineering build
- No need for a release
Check this box to confirm: I have read the Submitting Changes section of CONTRIBUTING.md and reviewed the following information:

SHOULD update ChangeLog.md file(s) appropriately
- Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
  - A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
- Should not change ChangeLog.md if no new release is required, such as fixing test case only.
SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
SHOULD have proper test coverage for changes in pull request.
SHOULD NOT adjust version of module manually in pull request

azure-client-tools-bot-prd · 2025-10-17T03:00:57Z

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copilot

Pull Request Overview

Introduces Phase 1 support for Azure Policy Change Safety by adding a configurable EnablePolicyToken flag and plumbing dynamic parameters (e.g. -AcquirePolicyToken) through selected cmdlets. Key changes include adding the EnablePolicyToken config and test, refactoring dynamic parameter handling in storage file content cmdlets, and replacing prior package references with extensive project references to azure-powershell-common to surface new shared functionality.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tools/Common.Netcore.Dependencies.targets	Comments out prior package references (now relying on project references in individual csproj files).
tools/AzDev/src/AzDev.csproj	Downgrades select packages to .NET 8-compatible versions.
src/shared/ConfigKeys.cs	Adds new EnablePolicyToken config key.
src/Storage/Storage/Storage.csproj	Adds project references to common libraries for policy token support.
src/Storage/Storage/File/Cmdlet/SetAzureStorageFileContent.cs	Refactors dynamic parameter merging to include new base parameters.
src/Storage/Storage/File/Cmdlet/GetAzureStorageFileContent.cs	Same refactor for dynamic parameter merging on get cmdlet.
src/Storage/Storage.common/Storage.common.csproj	Adds project references to shared modules.
src/Storage/Storage.Management/Storage.Management.csproj	Adds project references to shared modules.
src/Storage/Storage.Management.Sdk/Storage.Management.Sdk.csproj	Adds project references to shared modules.
src/Resources/Tags/Tags.csproj	Adds project references to shared modules.
src/Resources/ResourceManager/ResourceManager.csproj	Adds project references to shared modules.
src/Resources/Authorization.Management.Sdk/Authorization.Management.Sdk.csproj	Adds project references to shared modules.
src/Accounts/Authentication/Config/Definitions/EnablePolicyTokenConfig.cs	Introduces the EnablePolicyToken config definition.
src/Accounts/Authentication/Config/ConfigInitializer.cs	Registers the new config.
src/Accounts/Authentication/Authentication.csproj	Adds project references for shared functionality access.
src/Accounts/AssemblyLoading/AssemblyLoading.csproj	Adds Common project reference.
src/Accounts/Accounts.sln	Adds solution entries for referenced common projects.
src/Accounts/Accounts.Test/UnitTest/EnablePolicyTokenConfigTest.cs	Adds unit test validating config defaults/key.
documentation/change-safety.md	Adds feature documentation (Phase 1) for change safety tokens.

Copilot · 2025-10-17T03:02:50Z

src/Storage/Storage/File/Cmdlet/SetAzureStorageFileContent.cs

+            // Non-Windows: just pass through whatever base produced (could be null or already a dictionary including AcquirePolicyToken)
+            return baseResult;
        }
        private WindowsOnlyParameters context;


The previous implementation relied on returning the WindowsOnlyParameters object so parameter binding populated context.PreserveSMBAttribute; now a RuntimeDefinedParameterDictionary is returned and the bound value will reside in dict["PreserveSMBAttribute"].Value, leaving context.PreserveSMBAttribute at its default and breaking downstream logic that reads context. Either (a) set up a post-binding synchronization (e.g. override BeginProcessing to copy from the runtime defined parameter value into context) or (b) preserve prior behavior by returning an IDynamicParameters object that encapsulates both base and Windows-only parameters. For a minimal fix, after adding rd, attach a handler or in processing retrieve value from dict instead of context.

Suggested change

private WindowsOnlyParameters context;

private WindowsOnlyParameters context;

protected override void BeginProcessing()

{

base.BeginProcessing();

// Synchronize PreserveSMBAttribute from bound parameters to context

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && context != null)

{

object value;

if (this.MyInvocation.BoundParameters.TryGetValue("PreserveSMBAttribute", out value))

{

context.PreserveSMBAttribute = value is bool b ? b : false;

}

}

}

Copilot · 2025-10-17T03:02:51Z

src/Storage/Storage/File/Cmdlet/GetAzureStorageFileContent.cs

+        public override object GetDynamicParameters()
        {
+            var baseResult = base.GetDynamicParameters();
+            var dict = baseResult as RuntimeDefinedParameterDictionary;
+
            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
            {
                context = new WindowsOnlyParameters();
-                return context;
+
+                if (dict == null)
+                {
+                    dict = new RuntimeDefinedParameterDictionary();
+                }
+
+                var winParamProp = typeof(WindowsOnlyParameters).GetProperty("PreserveSMBAttribute");
+                if (winParamProp != null && !dict.ContainsKey(winParamProp.Name))
+                {
+                    var attrs = winParamProp.GetCustomAttributes(true).OfType<Attribute>().ToArray();
+                    var attrCollection = new Collection<Attribute>(attrs.ToList());
+                    var rd = new RuntimeDefinedParameter(winParamProp.Name, winParamProp.PropertyType, attrCollection);
+                    dict.Add(winParamProp.Name, rd);
+                }
+
+                return dict;
            }
-            else return null;
+
+            return baseResult;
        }


Same binding issue as in SetAzureStorageFileContent: returning a dictionary prevents automatic population of context.PreserveSMBAttribute, so any later usage of context will see the default value. Apply the same fix: copy dict["PreserveSMBAttribute"].Value into context.PreserveSMBAttribute before it's consumed, or revert to returning an object that exposes the property directly for binding.

Copilot · 2025-10-17T03:02:51Z

src/Storage/Storage/File/Cmdlet/SetAzureStorageFileContent.cs


-        public object GetDynamicParameters()
+        // Override to cooperatively merge base dynamic parameters (e.g. AcquirePolicyToken) with existing Windows-only ones.
+        public override object GetDynamicParameters()


[nitpick] The dynamic parameter merging logic introduced here is duplicated almost verbatim in GetAzureStorageFileContent; consider extracting a shared helper (e.g. a protected method in a common base class) to reduce duplication and ensure future changes (additional Windows-only parameters or new base dynamic parameters) stay consistent.

Copilot · 2025-10-17T03:02:51Z

src/Storage/Storage/File/Cmdlet/SetAzureStorageFileContent.cs

+
+            // Non-Windows: just pass through whatever base produced (could be null or already a dictionary including AcquirePolicyToken)
+            return baseResult;
        }


[nitpick] The dynamic parameter merging logic introduced here is duplicated almost verbatim in GetAzureStorageFileContent; consider extracting a shared helper (e.g. a protected method in a common base class) to reduce duplication and ensure future changes (additional Windows-only parameters or new base dynamic parameters) stay consistent.

Copilot · 2025-10-17T03:02:51Z

src/Storage/Storage/File/Cmdlet/GetAzureStorageFileContent.cs

        }
-        public object GetDynamicParameters()
+        // Override to merge base dynamic parameters (e.g. AcquirePolicyToken) with existing Windows-only parameter.
+        public override object GetDynamicParameters()


[nitpick] Duplicate dynamic parameter merge logic; refactor into a reusable method or utility to avoid divergence and simplify future parameter additions.

Copilot · 2025-10-17T03:02:52Z

src/Storage/Storage/File/Cmdlet/GetAzureStorageFileContent.cs

-            else return null;
+
+            return baseResult;
        }


[nitpick] Duplicate dynamic parameter merge logic; refactor into a reusable method or utility to avoid divergence and simplify future parameter additions.

Copilot · 2025-10-17T03:02:52Z

documentation/change-safety.md

+| Phase | Scope | Visible Parameter(s) | Notes |
+|-------|-------|----------------------|-------|
+| Phase 1 (current) | Token acquisition only | `-AcquirePolicyToken` | `-ChangeReference` is suppressed/hidden. |
+| Phase 2 (planned) | Adds change reference association | `-AcquirePolicyToken`, `-ChangeReference` | `-ChangeReference` implies token acquisition. |


The tables use a double leading pipe (||) which breaks standard Markdown table rendering; remove the extra leading pipe so each row begins with a single | to ensure proper formatting.

Copilot · 2025-10-17T03:02:52Z

documentation/change-safety.md

+| Parameter | Type | Phase | Purpose |
+|----------|------|-------|---------|
+| `-AcquirePolicyToken` | Switch | 1+ | Force acquisition of a change-safety policy token for the operation. |
+| `-ChangeReference <string>` | String | 2 | (Planned) Associates the operation with an external change record (e.g., a ChangeState resource ID); implies token acquisition. Not yet surfaced. |


The tables use a double leading pipe (||) which breaks standard Markdown table rendering; remove the extra leading pipe so each row begins with a single | to ensure proper formatting.

Copilot · 2025-10-17T03:02:52Z

documentation/change-safety.md

+| Symptom | Possible Cause | Mitigation |
+|--------|----------------|-----------|
+| Parameters not visible | Feature flag not enabled or common library not updated | Run `Update-AzConfig -EnablePolicyToken $true` and ensure you have a version that includes the handler. |


The tables use a double leading pipe (||) which breaks standard Markdown table rendering; remove the extra leading pipe so each row begins with a single | to ensure proper formatting.

notyashhh · 2025-10-17T03:04:45Z

Related to this change: Azure/azure-powershell-common#449

TODO:

Remove redundant Debug logs
Need to reverse csproj files (only updated for testing) (Run Disconnect-AzCommonDevRepo)
GetAzureStorageFileContent.cs & SetAzureStorageFileContent.cs incorrectly implement GetDynamicParameters() function. So I have temporarily updated it for testing

Added AcquirePolicy Changes + some csproj changes (to be reversed)

9b29fe4

Copilot AI review requested due to automatic review settings October 17, 2025 03:00

notyashhh mentioned this pull request Oct 17, 2025

[ChangeSafety] Phase 1: AcquirePolicyToken Support Azure/azure-powershell-common#449

Draft

Copilot AI reviewed Oct 17, 2025

View reviewed changes

notyashhh marked this pull request as draft October 17, 2025 03:11

-        private WindowsOnlyParameters context;
+        private WindowsOnlyParameters context;
+        protected override void BeginProcessing()
+        {
+            base.BeginProcessing();
+            // Synchronize PreserveSMBAttribute from bound parameters to context
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && context != null)
+            {
+                object value;
+                if (this.MyInvocation.BoundParameters.TryGetValue("PreserveSMBAttribute", out value))
+                {
+                    context.PreserveSMBAttribute = value is bool b ? b : false;
+                }
+            }
+        }

[ChangeSafety] Phase 1: AcquirePolicyToken Support #28711

Are you sure you want to change the base?

[ChangeSafety] Phase 1: AcquirePolicyToken Support #28711

Uh oh!

Conversation

notyashhh commented Oct 17, 2025

Description

Mandatory Checklist

Uh oh!

azure-client-tools-bot-prd bot commented Oct 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

notyashhh commented Oct 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant