Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.57k stars 4.82k forks source link

[BUG] ResourceManager.CosmosDB not using PATCH semantics #39030

Closed chassq closed 8 months ago

chassq commented 1 year ago

Library name and version

Azure.ResourceManager.CosmosDB 1.4.0-beta3

Describe the bug

See https://developercommunity.visualstudio.com/t/Using-CosmosDBAccountPatch-object-always/10478789

When trying to use the azure Management SDK for Cosmos DB every time we use the CosmosDBAccountPatch object to say set IPRules no matter what we set the Allow Access from Azure Portal flag is set to false/disabled.

For Example:

var ri = CosmosDBAccountResource.CreateResourceIdentifier(IaCConfiguration.AzureSubscriptionId.ToString()
                                                     , resourceConfig.ResourceGroupName
                                                     , resourceConfig.ResourceName);

var armResource = ArmClient.GetCosmosDBAccountResource(ri);

if (armResource == null)
 {
     continue;
 }

var armResourceData = await armResource.GetAsync(cancellationToken: cancellationToken). ConfigureAwait(false);
 if (armResourceData?. Value?. Data?. IPRules == null)
 {
     continue;
 }

var firewallRuleCollection = armResourceData.Value.Data.IPRules;

if (firewallRuleCollection == null)
 {
     continue;
 }

//Clear all existing rules
 firewallRuleCollection.Clear();

firewallRuleCollection.Add(new CosmosDBIPAddressOrRange {  IPAddressOrRange = '<IP Address>'});

var updateParameters = new CosmosDBAccountPatch();

updateParameters.NetworkAclBypass = armResourceData.Value.Data.NetworkAclBypass;
 updateParameters.PublicNetworkAccess = armResourceData.Value.Data.PublicNetworkAccess;

foreach (var ipRule in firewallRuleCollection)
 {
     updateParameters.IPRules.Add(ipRule);
 }

var updateOperation = await armResource.UpdateAsync(WaitUntil.Completed, updateParameters, cancellationToken). ConfigureAwait(false);

Expected behavior

The Allow Access from Azure Portal flag would not be mutated unless we explicitly set it.

Actual behavior

The Allow Access from Azure Portal flag is mutated back to its default of disabled it seems..

Reproduction Steps

See code above in description

Environment

Windows 11 (latest patches) desktop

Microsoft Visual Studio Enterprise 2022 Version 17.8.0 Preview 2.0 VisualStudio.17.Preview/17.8.0-pre.2.0+34112.27 Microsoft .NET Framework Version 4.8.09032

Installed Version: Enterprise

Visual C++ 2022 00476-80000-00000-AA741 Microsoft Visual C++ 2022

ADL Tools Service Provider 1.0 This package contains services used by Data Lake tools

ASA Service Provider 1.0

ASP.NET and Web Tools 17.8.226.21692 ASP.NET and Web Tools

Azure App Service Tools v3.0.0 17.8.226.21692 Azure App Service Tools v3.0.0

Azure Data Lake Tools for Visual Studio 2.6.5000.0 Microsoft Azure Data Lake Tools for Visual Studio

Azure Functions and Web Jobs Tools 17.8.226.21692 Azure Functions and Web Jobs Tools

Azure Stream Analytics Tools for Visual Studio 2.6.5000.0 Microsoft Azure Stream Analytics Tools for Visual Studio

Bundler & Minifier 2.9.9 Adds support for bundling and minifying JavaScript, CSS and HTML files in any project.

C# Tools 4.8.0-2.23429.7+44555193fd1135b5d53a2099f76fec91e0d1ebde C# components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

Common Azure Tools 1.10 Provides common services for use by Azure Mobile Services and Microsoft Azure Tools.

Entity Framework Core Power Tools 2.5 Adds useful design-time EF Core DbContext features to the Visual Studio Solution Explorer context menu.

Extensibility Message Bus 1.4.39 (main@e8108eb) Provides common messaging-based MEF services for loosely coupled Visual Studio extension components communication and integration.

GitHub Copilot 1.110.0.0 (v1.110.0.0@9f24d0f53) GitHub Copilot is an AI pair programmer that helps you write code faster and with less work.

GitHub Copilot Agent 1.110.389 (v1.110.0)

Microsoft Azure Hive Query Language Service 2.6.5000.0 Language service for Hive query

Microsoft Azure Stream Analytics Language Service 2.6.5000.0 Language service for Azure Stream Analytics

Microsoft Azure Tools for Visual Studio 2.9 Support for Azure Cloud Services projects

Microsoft JVM Debugger 1.0 Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines

Mono Debugging for Visual Studio 17.8.14 (0c9914e) Support for debugging Mono processes with Visual Studio.

NuGet Package Manager 6.8.0 NuGet Package Manager in Visual Studio. For more information about NuGet, visit https://docs.nuget.org/

Razor (ASP.NET Core) 17.8.2.2345506+ade90399d42c1a7bf92191b1c067816c0ae1c311 Provides languages services for ASP.NET Core Razor.

Solution Colors 1.1.44 Allows you to associate a color with a solution and display it in various locations within Visual Studio. Inspired by the Peacock extension for VS Code.

SQL Server Data Tools 17.8.64.0 Microsoft SQL Server Data Tools

Syntax Visualizer 1.0 An extension for visualizing Roslyn SyntaxTrees.

Test Adapter for Boost.Test 1.0 Enables Visual Studio's testing tools with unit tests written for Boost.Test. The use terms and Third Party Notices are available in the extension installation directory.

Test Adapter for Google Test 1.0 Enables Visual Studio's testing tools with unit tests written for Google Test. The use terms and Third Party Notices are available in the extension installation directory.

ToolWindowHostedEditor 1.0 Hosting json editor into a tool window

TypeScript Tools 17.0.20830.2001 TypeScript Tools for Microsoft Visual Studio

Visual Basic Tools 4.8.0-2.23429.7+44555193fd1135b5d53a2099f76fec91e0d1ebde Visual Basic components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

Visual F# Tools 17.8.0-beta.23425.10+0d3549fa5b8b6387ade191d76768405cefed8229 Microsoft Visual F# Tools

Visual Studio IntelliCode 2.2 AI-assisted development for Visual Studio.

VisualStudio.DeviceLog 1.0 Information about my package

VisualStudio.Mac 1.0 Mac Extension for Visual Studio

VSPackage Extension 1.0 VSPackage Visual Studio Extension Detailed Info

Xamarin 17.8.0.118 (main@35c256f) Visual Studio extension to enable development for Xamarin.iOS and Xamarin.Android.

Xamarin Designer 17.8.1.11 (remotes/origin/d17-8@13ef934098) Visual Studio extension to enable Xamarin Designer tools in Visual Studio.

Xamarin Templates 17.8.16 (830b56a) Templates for building iOS, Android, and Windows apps with Xamarin and Xamarin.Forms.

Xamarin.Android SDK 13.2.1.2 (d17-5/a8a26c7) Xamarin.Android Reference Assemblies and MSBuild support. Mono: d9a6e87 Java.Interop: xamarin/java.interop/d17-5@149d70fe SQLite: xamarin/sqlite/3.40.1@68c69d8 Xamarin.Android Tools: xamarin/xamarin-android-tools/d17-5@ca1552d

Xamarin.iOS and Xamarin.Mac SDK 16.4.0.16 (b5972410d) Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support.

jsquire commented 1 year ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

chassq commented 1 year ago

Just to let you know we are running into this as well on Azure Cognitive Services. For example, this code removes the virtual network rules we assign via bicep earlier in our release cycle. Doe now seem to be a PATCH just on the IPRules in this case.

        foreach (var resourceConfig in _resourceConfigurationService.ResourceConfigurations.Where(rc => rc.ResourceConfigurationType == ResourceConfigurationTypeEnum.AzureTranslators))
        {
            var ri = CognitiveServicesAccountResource.CreateResourceIdentifier(IaCConfiguration.AzureSubscriptionId.ToString()
                                                            , resourceConfig.ResourceGroupName
                                                            , resourceConfig.ResourceName
                                                            );

            var armResource = ArmClient.GetCognitiveServicesAccountResource(ri);

            if (armResource == null)
            {
                continue;
            }

            var armResourceData = await armResource.GetAsync(cancellationToken: cancellationToken).ConfigureAwait(false);
            if (armResourceData?.Value?.Data?.Properties?.NetworkAcls?.IPRules == null)
            {
                continue;
            }

            var location = armResourceData?.Value?.Data?.Location.Name;
            if(string.IsNullOrWhiteSpace(location))
            {
                throw new ArgumentException("Location could not be found for Azure Translation Service.");
            }

            var firewallRuleCollection = armResourceData?.Value?.Data?.Properties?.NetworkAcls?.IPRules ?? new List<CognitiveServicesIPRule>();

            if (firewallRuleCollection == null)
            {
                continue;
            }

            //Clear all existing rules
            firewallRuleCollection.Clear();

            //Add Allowed IPs
            foreach (var allowed in IaCConfiguration.FirewallRules.AllowedIPAddresses)
            {
                if (allowed.IpAddressStart.Equals(allowed.IpAddressEnd))
                {
                    firewallRuleCollection.Add(new CognitiveServicesIPRule(allowed.IpAddressStart));
                }
                else
                {
                    firewallRuleCollection.Add(new CognitiveServicesIPRule(allowed.IpAddressStart));
                    firewallRuleCollection.Add(new CognitiveServicesIPRule(allowed.IpAddressEnd));
                }
            }

            var updateParameters = new CognitiveServicesAccountData(location)
            {
                 Properties = new CognitiveServicesAccountProperties()
                 {
                     NetworkAcls = new CognitiveServicesNetworkRuleSet()                      
                 }
            };

            foreach (var ipRule in firewallRuleCollection)
            {
                updateParameters.Properties.NetworkAcls.IPRules.Add(ipRule);
            }

            await armResource.UpdateAsync(WaitUntil.Completed, updateParameters, cancellationToken).ConfigureAwait(false);
        }
mcgallan commented 9 months ago

@chassq I have reproduced the bug you mentioned based on version 1.4.0-beta.6 and found no update issues related to IpRules. The relevant code for reproduction is as follows:

var updateOptions = new CosmosDBAccountPatch()
{
    IsVirtualNetworkFilterEnabled = false,
    EnableAutomaticFailover = true,
    DisableKeyBasedMetadataWriteAccess = true,
    EnableBurstCapacity = true,
    EnablePriorityBasedExecution = true,
    DefaultPriorityLevel = DefaultPriorityLevel.Low,
};
updateOptions.Tags.Add("key3", "value3");
updateOptions.Tags.Add("key4", "value4");
foreach (var ipRule in account.Data.IPRules)
{
    updateOptions.IPRules.Add(ipRule);
}
updateOptions.IPRules.Add(new CosmosDBIPAddressOrRange()
{
    IPAddressOrRange = "23.43.230.122"
});
updateOptions.IPRules.Add(new CosmosDBIPAddressOrRange()
{
    IPAddressOrRange = "23.43.230.123"
});
await account.UpdateAsync(WaitUntil.Completed, updateOptions);

Nevertheless, the bug you mentioned has been fixed in the new version. Please wait patiently for the release of the new version. Thank you for your feedback.

github-actions[bot] commented 9 months ago

Hi @chassq. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

chassq commented 9 months ago

I am sorry but I do not see a question for me above.

mcgallan commented 9 months ago

@chassq , The example I provided is to demonstrate that in the latest version, the Update patch for IP Rules has no issues. However, I may not have understood what you meant by ‘the Allow Access from Azure Portal flag is set to false/disabled’. Is this an error message you encountered while using the SDK? Or is it an operation you were unable to perform in the Azure portal? Or is it the name of a property? If possible, I would like to learn more about the issue described in this sentence. If you have any screenshots to help me understand, that would be even better.

github-actions[bot] commented 9 months ago

Hi @chassq. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

chassq commented 9 months ago

@mcgallan So, there was a bug, and it was fixed. Thank you! I do not think anything was meant by ‘the Allow Access from Azure Portal flag is set to false/disabled’ other than the PATCH semantic was not working no matter how the value was set in the Portal. Seems if the bug is fixed this is good!

github-actions[bot] commented 9 months ago

Hi @chassq. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

chassq commented 9 months ago

@mcgallan Also to note we do not seem to have access to beta 6 (or whichever later version this was fixed in) in the public nuget feed. So cannot confirm from this side. All we see is beta 5.

github-actions[bot] commented 8 months ago

Hi @chassq, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.