-
Notifications
You must be signed in to change notification settings - Fork 4.1k
[Az.Functions] Fix cloud portability by using dynamic endpoints for storage #29035
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
base: main
Are you sure you want to change the base?
[Az.Functions] Fix cloud portability by using dynamic endpoints for storage #29035
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this 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 pull request addresses cloud portability issues in Az.Functions by replacing hardcoded endpoint values with dynamic endpoint resolution for storage connection strings and Application Insights. The changes ensure that New-AzFunctionApp can work correctly across Azure Public Cloud and custom/sovereign Azure cloud environments.
Key Changes
- Modified
GetEndpointSuffixfunction to useStorageEndpointSuffixproperty from Azure context instead of hardcoded switch statement - Updated Application Insights endpoint construction in WhatIf scenario to dynamically retrieve endpoint from Azure context
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Functions/Functions.Autorest/custom/HelperFunctions.ps1 | Refactored GetEndpointSuffix to use StorageEndpointSuffix property from Azure environment context, replacing hardcoded endpoint mappings for different cloud environments |
| src/Functions/Functions.Autorest/custom/New-AzFunctionApp.ps1 | Updated WhatIf scenario to dynamically construct Application Insights endpoints using AzureApplicationInsightsEndpointResourceId from Azure environment context instead of hardcoded "applicationinsights.azure.com" domain |
|
@wyunchi-ms - Would it be possible to include this fix in the Az 15.2.0 release? Thank you. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
isra-fel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please update the ChangeLog.md file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
|
@isra-fel - Thank you for the review. The change log has been updated. |
|
LGTM |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Hi @Francisco-Gamino , the code freeze date is Jan 6, so we cannot accept this PR in this release. I will re-target this to main branch. Let's release it in the next release. |
| public X509Certificate2 GetCert() { return _cert; } | ||
|
|
||
| static RSAParameters RsaParamsFromPem(string path, string password) // CodeQL [SM02205] BouncyCastle is the only API available because we need to be at netstandard2.0 | ||
| static RSAParameters RsaParamsFromPem(string path, string password) //CodeQL [SM02205] BouncyCastle is the only API available because we need to be at netstandard2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the change here.
src/Functions/Functions/ChangeLog.md
Outdated
| --> | ||
| ## Upcoming Release | ||
|
|
||
| * Fix cloud portability by using dynamic endpoints for storage [#29034] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Fix cloud portability by using dynamic endpoints for storage [#29034] | |
| * Fixed cloud portability by using dynamic endpoints for storage [#29034] |
|
This PR was labeled "needs-revision" because it has unresolved review comments or CI failures. |
092b548 to
69366db
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| --> | ||
| ## Upcoming Release | ||
|
|
||
| * Fixed cloud portability by using dynamic endpoints for storage [#29034] |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog entry could be more descriptive for users. Consider expanding it to explain the impact and what specific issue was fixed. For example: "Fixed 'New-AzFunctionApp' cmdlet to work in non-public Azure clouds by dynamically retrieving storage endpoints instead of using hardcoded values. This enables full cloud portability across Azure Public, Government, China, and custom cloud environments."
| $storageEndpointSuffix = (Get-AzContext).Environment.StorageEndpointSuffix | ||
|
|
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function could throw a null reference exception if Get-AzContext returns null. Consider adding error handling to check if the context exists before accessing its properties. For example: '$context = Get-AzContext; if ($null -eq $context) { return '' }; $storageEndpointSuffix = $context.Environment.StorageEndpointSuffix'
| $storageEndpointSuffix = (Get-AzContext).Environment.StorageEndpointSuffix | |
| $context = Get-AzContext | |
| if ($null -eq $context -or $null -eq $context.Environment) | |
| { | |
| return '' | |
| } | |
| $storageEndpointSuffix = $context.Environment.StorageEndpointSuffix |
Description
This PR fixes issues #29034 where New-AzFunctionApp fails in non-public Azure clouds due to hardcoded endpoint values for storage connection strings. The change ensure full cloud portability across Azure Public and custom cloud environments.
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.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.