From 4323ea0f7eb18799b7e4146394a66f853a497976 Mon Sep 17 00:00:00 2001 From: GuillaumeM <86654731+GuillaumeM-2ISA@users.noreply.github.com> Date: Fri, 3 Mar 2023 14:53:54 +0100 Subject: [PATCH] Fix: Bug-When device model is delete some Azure resources are still there #1832 (#1837) * fix delete device model + unit tests * update unit tests + services * update unit tests azure device registry provider --- .../Providers/IDeviceRegistryProvider.cs | 2 + .../Services/IConfigService.cs | 2 + .../Providers/AzureDeviceRegistryProvider.cs | 19 ++++++++ .../Services/ConfigService.cs | 25 ++++++---- .../Services/DeviceModelService.cs | 5 ++ .../Pages/Devices/CreateDevicePageTests.cs | 1 - .../AzureDeviceRegistryProviderTests.cs | 48 +++++++++++++++++++ .../Server/Services/ConfigServiceTests.cs | 26 ++++++++++ .../Services/DeviceModelServiceTests.cs | 12 +++++ 9 files changed, 129 insertions(+), 11 deletions(-) diff --git a/src/AzureIoTHub.Portal.Application/Providers/IDeviceRegistryProvider.cs b/src/AzureIoTHub.Portal.Application/Providers/IDeviceRegistryProvider.cs index 37e1b156a..f932c4e69 100644 --- a/src/AzureIoTHub.Portal.Application/Providers/IDeviceRegistryProvider.cs +++ b/src/AzureIoTHub.Portal.Application/Providers/IDeviceRegistryProvider.cs @@ -41,5 +41,7 @@ public interface IDeviceRegistryProvider Task GetEnrollmentCredentialsAsync(string deviceId, string modelId); Task DeleteEnrollmentGroupAsync(EnrollmentGroup enrollmentGroup, CancellationToken cancellationToken); + + Task DeleteEnrollmentGroupByDeviceModelIdAsync(string deviceModelId, CancellationToken cancellationToken); } } diff --git a/src/AzureIoTHub.Portal.Application/Services/IConfigService.cs b/src/AzureIoTHub.Portal.Application/Services/IConfigService.cs index c04247fe9..047f797db 100644 --- a/src/AzureIoTHub.Portal.Application/Services/IConfigService.cs +++ b/src/AzureIoTHub.Portal.Application/Services/IConfigService.cs @@ -17,6 +17,8 @@ public interface IConfigService Task RollOutDeviceModelConfiguration(string modelId, Dictionary desiredProperties); + Task DeleteDeviceModelConfigurationByConfigurationNamePrefix(string configurationNamePrefix); + Task RollOutEdgeModelConfiguration(IoTEdgeModel edgeModel); Task RollOutDeviceConfiguration(string modelId, Dictionary desiredProperties, string configurationId, Dictionary targetTags, int priority = 0); diff --git a/src/AzureIoTHub.Portal.Infrastructure/Providers/AzureDeviceRegistryProvider.cs b/src/AzureIoTHub.Portal.Infrastructure/Providers/AzureDeviceRegistryProvider.cs index 1ef6c7e62..144b71368 100644 --- a/src/AzureIoTHub.Portal.Infrastructure/Providers/AzureDeviceRegistryProvider.cs +++ b/src/AzureIoTHub.Portal.Infrastructure/Providers/AzureDeviceRegistryProvider.cs @@ -80,6 +80,25 @@ public async Task DeleteEnrollmentGroupAsync(EnrollmentGroup enrollmentGroup, Ca await this.dps.DeleteEnrollmentGroupAsync(enrollmentGroup, cancellationToken); } + public async Task DeleteEnrollmentGroupByDeviceModelIdAsync(string deviceModelId, CancellationToken cancellationToken = default) + { + var enrollmentGroupName = ComputeEnrollmentGroupName(deviceModelId); + EnrollmentGroup? enrollmentGroup; + + try + { + enrollmentGroup = await this.dps.GetEnrollmentGroupAsync(enrollmentGroupName); + } + catch (HttpRequestException e) + when (e.StatusCode == System.Net.HttpStatusCode.NotFound) + { + // Nothing to do, the enrollement group does not exist. + return; + } + + await this.dps.DeleteEnrollmentGroupAsync(enrollmentGroup, cancellationToken); + } + /// /// this function get the attestation mechanism of the DPS. /// diff --git a/src/AzureIoTHub.Portal.Server/Services/ConfigService.cs b/src/AzureIoTHub.Portal.Server/Services/ConfigService.cs index d94829bc3..95dbb2708 100644 --- a/src/AzureIoTHub.Portal.Server/Services/ConfigService.cs +++ b/src/AzureIoTHub.Portal.Server/Services/ConfigService.cs @@ -202,14 +202,27 @@ public async Task DeleteConfiguration(string configId) public async Task RollOutDeviceModelConfiguration(string modelId, Dictionary desiredProperties) { - var configurations = await this.registryManager.GetConfigurationsAsync(0); - #pragma warning disable CA1308 // Normalize strings to uppercase var configurationNamePrefix = modelId?.Trim() .ToLowerInvariant() .Replace(" ", "-", StringComparison.OrdinalIgnoreCase); #pragma warning restore CA1308 // Normalize strings to uppercase + await DeleteDeviceModelConfigurationByConfigurationNamePrefix(configurationNamePrefix); + + var newConfiguration = new Configuration($"{configurationNamePrefix}-{DateTime.UtcNow.Ticks}"); + + newConfiguration.Labels.Add("created-by", "Azure IoT hub Portal"); + newConfiguration.TargetCondition = $"tags.modelId = '{modelId}'"; + newConfiguration.Content.DeviceContent = desiredProperties; + + _ = await this.registryManager.AddConfigurationAsync(newConfiguration); + } + + public async Task DeleteDeviceModelConfigurationByConfigurationNamePrefix(string configurationNamePrefix) + { + var configurations = await this.registryManager.GetConfigurationsAsync(0); + foreach (var item in configurations) { if (!item.Id.StartsWith(configurationNamePrefix, StringComparison.OrdinalIgnoreCase)) @@ -219,14 +232,6 @@ public async Task RollOutDeviceModelConfiguration(string modelId, Dictionary command.DeviceModelId.Equals(deviceModelId, StringComparison.Ordinal)).ToList(); diff --git a/src/AzureIoTHub.Portal.Tests.Unit/Client/Pages/Devices/CreateDevicePageTests.cs b/src/AzureIoTHub.Portal.Tests.Unit/Client/Pages/Devices/CreateDevicePageTests.cs index 7bc5ded18..87863f6c1 100644 --- a/src/AzureIoTHub.Portal.Tests.Unit/Client/Pages/Devices/CreateDevicePageTests.cs +++ b/src/AzureIoTHub.Portal.Tests.Unit/Client/Pages/Devices/CreateDevicePageTests.cs @@ -23,7 +23,6 @@ namespace AzureIoTHub.Portal.Tests.Unit.Client.Pages.Devices using UnitTests.Mocks; using AutoFixture; using AzureIoTHub.Portal.Shared.Models.v10.Filters; - using AzureIoTHub.Portal.Shared.Models.v10; using System.Linq; using AzureIoTHub.Portal.Shared.Models; diff --git a/src/AzureIoTHub.Portal.Tests.Unit/Infrastructure/Providers/AzureDeviceRegistryProviderTests.cs b/src/AzureIoTHub.Portal.Tests.Unit/Infrastructure/Providers/AzureDeviceRegistryProviderTests.cs index 867e7f6cb..81d768c97 100644 --- a/src/AzureIoTHub.Portal.Tests.Unit/Infrastructure/Providers/AzureDeviceRegistryProviderTests.cs +++ b/src/AzureIoTHub.Portal.Tests.Unit/Infrastructure/Providers/AzureDeviceRegistryProviderTests.cs @@ -3,8 +3,10 @@ namespace AzureIoTHub.Portal.Tests.Unit.Infrastructure.Providers { + using System; using System.Net; using System.Net.Http; + using System.Threading; using System.Threading.Tasks; using AzureIoTHub.Portal.Application.Wrappers; using AzureIoTHub.Portal.Infrastructure; @@ -330,5 +332,51 @@ public async Task WhenEnrollmentGoupsNotExistGetEnrollmentCredentialsAsyncShould this.mockRepository.VerifyAll(); } + + [Test] + public async Task DeleteEnrollmentGroupByDeviceModelIdAsyncShouldDeleteEnrollmentGroup() + { + // Arrange + var manager = CreateManager(); + var deviceModelId = Guid.NewGuid().ToString(); + var attestation = new SymmetricKeyAttestation("aaa", "bbb"); + + _ = this.mockProvisioningServiceClient + .Setup(c => c.GetEnrollmentGroupAsync(It.Is(x => x == deviceModelId))) + .ReturnsAsync(new EnrollmentGroup(deviceModelId, attestation) + { + Capabilities = new DeviceCapabilities + { + IotEdge = true + } + }); + + _ = this.mockProvisioningServiceClient.Setup(c => c.DeleteEnrollmentGroupAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + // Act + await manager.DeleteEnrollmentGroupByDeviceModelIdAsync(deviceModelId); + + // Assert + this.mockRepository.VerifyAll(); + } + + [Test] + public async Task DeleteEnrollmentGroupByDeviceModelIdAsyncShouldBeNotFound() + { + // Arrange + var manager = CreateManager(); + var deviceModelId = Guid.NewGuid().ToString(); + + _ = this.mockProvisioningServiceClient + .Setup(c => c.GetEnrollmentGroupAsync(It.Is(x => x == deviceModelId))) + .Throws(new HttpRequestException(null, null, HttpStatusCode.NotFound)); + + // Act + await manager.DeleteEnrollmentGroupByDeviceModelIdAsync(deviceModelId); + + // Assert + this.mockRepository.VerifyAll(); + } } } diff --git a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/ConfigServiceTests.cs b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/ConfigServiceTests.cs index 789194cef..d599d7477 100644 --- a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/ConfigServiceTests.cs +++ b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/ConfigServiceTests.cs @@ -963,5 +963,31 @@ public void WhenAddGonfigurationFailedRollOutEdgeModelConfigurationShouldThrowIn this.mockRepository.VerifyAll(); this.mockRegistryManager.Verify(c => c.GetConfigurationsAsync(It.IsAny()), Times.Once()); } + + [Test] + public async Task WhenConfigurationExistsDeleteDeviceModelConfigurationByConfigurationNamePrefixShouldRemoveIt() + { + // Arrange + var configsServices = CreateConfigsServices(); + var configurationPrefix = Guid.NewGuid().ToString(); + var suffix = Guid.NewGuid().ToString(); + + _ = this.mockRegistryManager.Setup(c => c.GetConfigurationsAsync(It.IsAny())) + .ReturnsAsync(new Configuration[] + { + new Configuration($"{configurationPrefix}-{suffix}"), + new Configuration($"null-{suffix}") + }); + + _ = this.mockRegistryManager.Setup(c => c.RemoveConfigurationAsync(It.Is(x => x == $"{configurationPrefix}-{suffix}"))) + .Returns(Task.CompletedTask); + + // Act + await configsServices.DeleteDeviceModelConfigurationByConfigurationNamePrefix(configurationPrefix); + + // Assert + this.mockRegistryManager.Verify(c => c.GetConfigurationsAsync(It.IsAny()), Times.Once()); + this.mockRegistryManager.Verify(c => c.RemoveConfigurationAsync(It.IsAny()), Times.Once()); + } } } diff --git a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelServiceTests.cs b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelServiceTests.cs index 4ff2c54f7..5f7797356 100644 --- a/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelServiceTests.cs +++ b/src/AzureIoTHub.Portal.Tests.Unit/Server/Services/DeviceModelServiceTests.cs @@ -341,6 +341,12 @@ public async Task DeleteDeviceModelShouldDeleteDeviceModel() Items = Array.Empty() }); + _ = this.mockRegistryProvider.Setup(repository => repository.DeleteEnrollmentGroupByDeviceModelIdAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + _ = this.mockConfigService.Setup(repository => repository.DeleteDeviceModelConfigurationByConfigurationNamePrefix(It.IsAny())) + .Returns(Task.CompletedTask); + _ = this.mockDeviceModelCommandRepository.Setup(repository => repository.GetAll()) .Returns(commands); @@ -399,6 +405,12 @@ public async Task DeleteDeviceModelShouldThrowAnErrorExceptionWhenDDbUpdateExcep Items = Array.Empty() }); + _ = this.mockRegistryProvider.Setup(repository => repository.DeleteEnrollmentGroupByDeviceModelIdAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + _ = this.mockConfigService.Setup(repository => repository.DeleteDeviceModelConfigurationByConfigurationNamePrefix(It.IsAny())) + .Returns(Task.CompletedTask); + _ = this.mockDeviceModelCommandRepository.Setup(repository => repository.GetAll()) .Returns(commands);