From 10add7a13a6e79d1751005e077bbd60de69434fc Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 12 Feb 2024 17:11:53 -0800 Subject: [PATCH 1/2] Throw RequestFailedException for failed backup/restore Fixes #41855 --- .../CHANGELOG.md | 4 + .../src/KeyVaultBackupOperation.cs | 5 +- .../src/RestoreOperationInternal.cs | 7 +- .../tests/BackupOperationTests.cs | 77 +++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md index 1d12b73059edd..7bad732513232 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md +++ b/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md @@ -10,6 +10,10 @@ Changes from both the last release and the last beta include: - The `sasToken` parameter is now optional in `KeyVaultBackupClient.StartRestore` and `StartRestoreAsync`. Managed Identity will be used instead if `sasToken` is null. - The `sasToken` parameter is now optional in `KeyVaultBackupClient.StartSelectiveKeyRestore` and `StartSelectiveKeyRestoreAsync`. Managed Identity will be used instead if `sasToken` is null. +### Breaking Changes + +- `KeyVaultBackupOperation`, `KeyVaultRestoreOperation`, and `KeyVaultSelectiveKeyRestoreOperation` now throw a `RequestFailedException` when the service returns an error response. ([#41855](https://github.com/Azure/azure-sdk-for-net/issues/41855)) + ### Bugs Fixed - When a Key Vault is moved to another tenant, the client is reauthenticated. diff --git a/sdk/keyvault/Azure.Security.KeyVault.Administration/src/KeyVaultBackupOperation.cs b/sdk/keyvault/Azure.Security.KeyVault.Administration/src/KeyVaultBackupOperation.cs index 6888508ac3b53..a2cd70bbc61d1 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Administration/src/KeyVaultBackupOperation.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Administration/src/KeyVaultBackupOperation.cs @@ -151,9 +151,12 @@ await _client.GetBackupDetailsAsync(Id, cancellationToken).ConfigureAwait(false) _requestFailedException = new RequestFailedException("Unexpected failure", ex); throw _requestFailedException; } + if (_value != null && _value.EndTime.HasValue && _value.Error != null) { - _requestFailedException = new RequestFailedException($"{_value.Error.Message}\nInnerError: {_value.Error.InnerError}\nCode: {_value.Error.Code}"); + _requestFailedException = _response != null ? + new RequestFailedException(_response) + : new RequestFailedException($"{_value.Error.Message}\nInnerError: {_value.Error.InnerError}\nCode: {_value.Error.Code}"); throw _requestFailedException; } } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Administration/src/RestoreOperationInternal.cs b/sdk/keyvault/Azure.Security.KeyVault.Administration/src/RestoreOperationInternal.cs index 8e64a543a1e87..dfff9a1866469 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Administration/src/RestoreOperationInternal.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Administration/src/RestoreOperationInternal.cs @@ -119,7 +119,7 @@ internal RestoreOperationInternal(TResponseType value, Response response, KeyVau }; /// - /// The error value resturned by the service call. + /// The error value returned by the service call. /// internal KeyVaultServiceError Error => _value switch { @@ -215,9 +215,12 @@ await _client.GetSelectiveKeyRestoreDetailsAsync(Id, cancellationToken).Configur _requestFailedException = new RequestFailedException("Unexpected Failure.", ex); throw _requestFailedException; } + if (_value != null && EndTime.HasValue && Error?.Code != null) { - _requestFailedException = new RequestFailedException($"{Error.Message}\nInnerError: {Error.InnerError}\nCode: {Error.Code}"); + _requestFailedException = _response != null ? + new RequestFailedException(_response) + : new RequestFailedException($"{Error.Message}\nInnerError: {Error.InnerError}\nCode: {Error.Code}"); throw _requestFailedException; } } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Administration/tests/BackupOperationTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Administration/tests/BackupOperationTests.cs index b2284a86b65ff..a8c6732991b30 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Administration/tests/BackupOperationTests.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Administration/tests/BackupOperationTests.cs @@ -2,7 +2,10 @@ // Licensed under the MIT License. using System; +using System.Text.Json; using System.Threading; +using Azure.Core.Serialization; +using Azure.Core.TestFramework; using Azure.Security.KeyVault.Administration.Models; using Moq; using NUnit.Framework; @@ -120,5 +123,79 @@ public void ValueThrowsWhenOperationIsNotComplete() Assert.That(operation.StartTime, Is.EqualTo(incompleteBackup.StartTime)); Assert.That(operation.EndTime, Is.EqualTo(incompleteBackup.EndTime)); } + + [Test(Description = "https://github.com/Azure/azure-sdk-for-net/issues/41855")] + public void BackupOperationThrowsOnBadRequest() + { + const string jobId = "c79735efd41d4b8a8ef23634b7f3dd0d"; + var response = new MockResponse(200, "OK").WithJson($$""" + { + "endTime": 1704409031, + "error": { + "code": "BadRequest", + "innererror": null, + "message": "Invalid backup: Reason: Cannot read backup status document" + }, + "jobId": "c79735efd41d4b8a8ef23634b7f3dd0d", + "startTime": 1704409030, + "status": "Failed", + "statusDetails": "Invalid backup: Reason: Cannot read backup status document" + } + """); + + using var doc = JsonDocument.Parse(response.Content.ToStream()); + var detail = FullBackupDetailsInternal.DeserializeFullBackupDetailsInternal(doc.RootElement); + + var mockClient = new Mock(); + mockClient + .Setup(m => m.GetBackupDetails(It.IsAny(), It.IsAny())) + .Returns(Response.FromValue(detail, response)); + + var operation = new KeyVaultBackupOperation(mockClient.Object, jobId); + var ex = Assert.Throws(() => operation.WaitForCompletion()); + Assert.AreEqual(ex.Status, 200); + + dynamic error = ex.GetRawResponse()?.Content.ToDynamicFromJson(JsonPropertyNames.UseExact).error; + Assert.NotNull(error); + Assert.AreEqual("BadRequest", (string)error.code); + Assert.AreEqual("Invalid backup: Reason: Cannot read backup status document", (string)error.message); + } + + [Test(Description = "https://github.com/Azure/azure-sdk-for-net/issues/41855")] + public void RestoreOperationThrowsOnBadRequest() + { + const string jobId = "c79735efd41d4b8a8ef23634b7f3dd0d"; + var response = new MockResponse(200, "OK").WithJson($$""" + { + "endTime": 1704409031, + "error": { + "code": "BadRequest", + "innererror": null, + "message": "Invalid backup: Reason: Cannot read backup status document" + }, + "jobId": "c79735efd41d4b8a8ef23634b7f3dd0d", + "startTime": 1704409030, + "status": "Failed", + "statusDetails": "Invalid backup: Reason: Cannot read backup status document" + } + """); + + using var doc = JsonDocument.Parse(response.Content.ToStream()); + var detail = RestoreDetailsInternal.DeserializeRestoreDetailsInternal(doc.RootElement); + + var mockClient = new Mock(); + mockClient + .Setup(m => m.GetRestoreDetails(It.IsAny(), It.IsAny())) + .Returns(Response.FromValue(detail, response)); + + var operation = new KeyVaultRestoreOperation(mockClient.Object, jobId); + var ex = Assert.Throws(() => operation.WaitForCompletion()); + Assert.AreEqual(ex.Status, 200); + + dynamic error = ex.GetRawResponse()?.Content.ToDynamicFromJson(JsonPropertyNames.UseExact).error; + Assert.NotNull(error); + Assert.AreEqual("BadRequest", (string)error.code); + Assert.AreEqual("Invalid backup: Reason: Cannot read backup status document", (string)error.message); + } } } From 087a66455b153472f0c50bdaa16670f42969ac68 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 14 Feb 2024 15:22:05 -0800 Subject: [PATCH 2/2] Update release dates --- .../Azure.Security.KeyVault.Administration/CHANGELOG.md | 4 ++-- .../Azure.Security.KeyVault.Certificates/CHANGELOG.md | 2 +- sdk/keyvault/Azure.Security.KeyVault.Keys/CHANGELOG.md | 2 +- sdk/keyvault/Azure.Security.KeyVault.Secrets/CHANGELOG.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md index 7bad732513232..9b5997716fed5 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md +++ b/sdk/keyvault/Azure.Security.KeyVault.Administration/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 4.4.0 (2024-02-13) +## 4.4.0 (2024-02-14) Changes from both the last release and the last beta include: @@ -12,7 +12,7 @@ Changes from both the last release and the last beta include: ### Breaking Changes -- `KeyVaultBackupOperation`, `KeyVaultRestoreOperation`, and `KeyVaultSelectiveKeyRestoreOperation` now throw a `RequestFailedException` when the service returns an error response. ([#41855](https://github.com/Azure/azure-sdk-for-net/issues/41855)) +- `KeyVaultBackupOperation`, `KeyVaultRestoreOperation`, and `KeyVaultSelectiveKeyRestoreOperation` now throw a `RequestFailedException` with a different error message - but a raw `Response` with more details - when the service returns an error response. ([#41855](https://github.com/Azure/azure-sdk-for-net/issues/41855)) ### Bugs Fixed diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md index 01c3663cbbf22..e5a45ef8a18d9 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md +++ b/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 4.6.0 (2024-02-13) +## 4.6.0 (2024-02-14) Changes from both the last release and the last beta include: diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Keys/CHANGELOG.md index ab8103f2134bf..6850dc16c098c 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/CHANGELOG.md +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 4.6.0 (2024-02-13) +## 4.6.0 (2024-02-14) Changes from both the last release and the last beta include: diff --git a/sdk/keyvault/Azure.Security.KeyVault.Secrets/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Secrets/CHANGELOG.md index e3df7481fd787..10b3ebe7cbc63 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Secrets/CHANGELOG.md +++ b/sdk/keyvault/Azure.Security.KeyVault.Secrets/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 4.6.0 (2024-02-13) +## 4.6.0 (2024-02-14) Changes from both the last release and the last beta include: