[Az.RecoveryServices.Backup] Fix AFS Restore Command Bug #28302
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -241,7 +241,71 @@ public RestAzureNS.AzureOperationResponse TriggerRestore() | |
| targetFolder = "/"; | ||
| } | ||
|
|
||
| GenericResource storageAccountResource = null; | ||
| string sourceResourceId = null; | ||
| string storageAccountLocation = vaultLocation; // Default to vault location | ||
|
|
||
| try | ||
| { | ||
| storageAccountResource = ServiceClientAdapter.GetStorageAccountResource(recoveryPoint.ContainerName.Split(';')[2]); | ||
| if (storageAccountResource != null) | ||
| { | ||
| sourceResourceId = storageAccountResource.Id; | ||
| storageAccountLocation = storageAccountResource.Location; | ||
| Logger.Instance.WriteDebug("Successfully retrieved storage account resource"); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.Instance.WriteDebug($"Failed to get storage account resource: {ex.Message}. Will attempt to retrieve from protected item."); | ||
| storageAccountResource = null; | ||
| } | ||
|
|
||
| // If storage account retrieval failed, fall back to getting it from protected item | ||
| if (storageAccountResource == null || string.IsNullOrEmpty(sourceResourceId)) | ||
| { | ||
| try | ||
| { | ||
| Dictionary<UriEnums, string> keyValueDict = HelperUtils.ParseUri(recoveryPoint.Id); | ||
| string containerUri = HelperUtils.GetContainerUri(keyValueDict, recoveryPoint.Id); | ||
| string protectedItemUri = HelperUtils.GetProtectedItemUri(keyValueDict, recoveryPoint.Id); | ||
|
|
||
| var protectedItemResponse = ServiceClientAdapter.GetProtectedItem( | ||
| containerUri, | ||
| protectedItemUri, | ||
| null, | ||
| vaultName: vaultName, | ||
| resourceGroupName: resourceGroupName); | ||
|
|
||
| if (protectedItemResponse?.Body?.Properties is AzureFileshareProtectedItem afsProtectedItem) | ||
| { | ||
| sourceResourceId = afsProtectedItem.SourceResourceId; | ||
| Logger.Instance.WriteDebug("Retrieved source resource ID from protected item"); | ||
|
|
||
| // Create a minimal resource object with the source resource ID | ||
| if (!string.IsNullOrEmpty(sourceResourceId)) | ||
| { | ||
| storageAccountResource = new GenericResource | ||
|
There was a problem hiding this comment. Creating a new GenericResource with minimal properties could lead to issues if other parts of the code expect additional properties to be populated. Consider documenting this limitation or ensuring all required properties are set. Copilot uses AI. Check for mistakes. |
||
| { | ||
| Id = sourceResourceId, | ||
|
There was a problem hiding this comment. initiliazing this way might not work. please confirm |
||
| Location = storageAccountLocation | ||
| }; | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.Instance.WriteDebug($"Failed to get protected item: {ex.Message}"); | ||
| } | ||
| } | ||
|
|
||
| // Validate that we have a source resource ID | ||
| if (storageAccountResource == null || string.IsNullOrEmpty(storageAccountResource.Id)) | ||
| { | ||
| throw new ArgumentException(string.Format(Resources.AzureFileShareNotFound, | ||
|
There was a problem hiding this comment. The error message uses 'AzureFileShareNotFound' resource which may not accurately describe the actual problem - missing source resource ID rather than a missing file share. Consider using a more specific error message that indicates the source storage account resource ID could not be retrieved. Copilot uses AI. Check for mistakes. |
||
| recoveryPoint.ContainerName)); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. can we add a test case for this. also have we thoroughly tested this change? |
||
| GenericResource targetStorageAccountResource = null; | ||
| string targetStorageAccountLocation = null; | ||
| if (targetStorageAccountName != null) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 assumption that the storage account location should default to vault location may not always be correct. Consider adding a comment explaining why this is a reasonable fallback or implement logic to determine the actual storage account location.
Copilot uses AI. Check for mistakes.