-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add early storage validation and improve error messaging for Red Team scans #44389
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
Open
Copilot
wants to merge
14
commits into
main
Choose a base branch
from
copilot/update-storage-account-error-messaging
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
a33f90b
Initial plan
Copilot 7565bd6
Add early storage validation and improve error messages
Copilot 0aa1aa8
Add unit tests for storage validation
Copilot b565b5b
Address code review comments - move imports to top of files
Copilot 8395f53
Remove unnecessary f-string
Copilot c6f4d41
Change error blame to SYSTEM_ERROR for storage connectivity failures
Copilot 9152f75
Clean up code style: move imports to top, remove unnecessary f-strings
Copilot e7fad22
Optimize storage validation: use BytesIO and unique test IDs
Copilot ec3b07b
Merge branch 'main' into copilot/update-storage-account-error-messaging
slister1001 92feed3
Update sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_common…
slister1001 3ca2341
Update sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_common…
slister1001 a8cd762
Update sdk/evaluation/azure-ai-evaluation/tests/unittests/test_redtea…
slister1001 57fb140
Update sdk/evaluation/azure-ai-evaluation/tests/unittests/test_redtea…
slister1001 44fdca9
fix tox black checker
slister1001 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
153 changes: 153 additions & 0 deletions
153
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_redteam/test_storage_validation.py
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 |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| # --------------------------------------------------------- | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # --------------------------------------------------------- | ||
| """Unit tests for storage account validation functionality.""" | ||
|
|
||
| import os | ||
| import tempfile | ||
| import unittest | ||
| from unittest.mock import Mock, patch | ||
| from azure.ai.evaluation._common.evaluation_onedp_client import EvaluationServiceOneDPClient | ||
| from azure.ai.evaluation._common.utils import upload | ||
| from azure.ai.evaluation._exceptions import EvaluationException | ||
| from azure.ai.evaluation.red_team._mlflow_integration import MLflowIntegration | ||
|
|
||
|
|
||
| class TestStorageValidation(unittest.TestCase): | ||
| """Test cases for storage account validation.""" | ||
|
|
||
| def setUp(self): | ||
| """Set up test fixtures.""" | ||
| self.mock_credential = Mock() | ||
| self.test_endpoint = "https://test-endpoint.azure.com" | ||
|
|
||
| @patch("azure.ai.evaluation._common.evaluation_onedp_client.RestEvaluationServiceClient") | ||
| def test_test_storage_upload_success(self, mock_rest_client_class): | ||
| """Test successful storage upload validation.""" | ||
| # Arrange | ||
| mock_rest_client = Mock() | ||
| mock_rest_client_class.return_value = mock_rest_client | ||
|
|
||
| mock_pending_upload_response = Mock() | ||
| mock_pending_upload_response.blob_reference_for_consumption = Mock() | ||
| mock_pending_upload_response.blob_reference_for_consumption.credential = Mock() | ||
| mock_pending_upload_response.blob_reference_for_consumption.credential.sas_uri = ( | ||
| "https://test.blob.core.windows.net/container?sas_token" | ||
| ) | ||
|
|
||
| mock_rest_client.evaluation_results.start_pending_upload.return_value = mock_pending_upload_response | ||
|
|
||
| # Mock ContainerClient | ||
| with patch( | ||
| "azure.ai.evaluation._common.evaluation_onedp_client.ContainerClient" | ||
| ) as mock_container_client_class: | ||
| mock_container_client = Mock() | ||
| mock_container_client.__enter__ = Mock(return_value=mock_container_client) | ||
| mock_container_client.__exit__ = Mock(return_value=None) | ||
| mock_container_client_class.from_container_url.return_value = mock_container_client | ||
|
|
||
| # Mock the upload_blob method to succeed | ||
| mock_container_client.upload_blob = Mock() | ||
| # Act | ||
| client = EvaluationServiceOneDPClient(self.test_endpoint, self.mock_credential) | ||
| result = client.test_storage_upload() | ||
|
|
||
| # Assert | ||
| self.assertTrue(result) | ||
| mock_container_client.upload_blob.assert_called_once() | ||
|
|
||
| @patch("azure.ai.evaluation._common.evaluation_onedp_client.RestEvaluationServiceClient") | ||
| def test_test_storage_upload_failure(self, mock_rest_client_class): | ||
| """Test storage upload validation failure.""" | ||
| # Arrange | ||
| mock_rest_client = Mock() | ||
| mock_rest_client_class.return_value = mock_rest_client | ||
|
|
||
| # Simulate upload failure | ||
| mock_rest_client.evaluation_results.start_pending_upload.side_effect = Exception( | ||
| "Storage account not accessible" | ||
| ) | ||
|
|
||
| # Act & Assert | ||
| client = EvaluationServiceOneDPClient(self.test_endpoint, self.mock_credential) | ||
| with self.assertRaises(EvaluationException) as context: | ||
| client.test_storage_upload() | ||
|
|
||
| # Verify error message contains helpful guidance | ||
| self.assertIn("storage", str(context.exception).lower()) | ||
| self.assertIn("permissions", str(context.exception).lower()) | ||
|
|
||
| def test_mlflow_integration_test_storage_upload_onedp(self): | ||
| """Test MLflowIntegration storage validation for OneDP projects.""" | ||
| # Arrange | ||
| mock_logger = Mock() | ||
| mock_generated_rai_client = Mock() | ||
| mock_evaluation_client = Mock() | ||
| mock_evaluation_client.test_storage_upload.return_value = True | ||
| mock_generated_rai_client._evaluation_onedp_client = mock_evaluation_client | ||
|
|
||
| mlflow_integration = MLflowIntegration( | ||
| logger=mock_logger, generated_rai_client=mock_generated_rai_client, one_dp_project=True | ||
| ) | ||
|
|
||
| # Act | ||
| result = mlflow_integration.test_storage_upload() | ||
|
|
||
| # Assert | ||
| self.assertTrue(result) | ||
| mock_evaluation_client.test_storage_upload.assert_called_once() | ||
|
|
||
| def test_mlflow_integration_test_storage_upload_non_onedp(self): | ||
| """Test MLflowIntegration storage validation for non-OneDP projects.""" | ||
| # Arrange | ||
| mock_logger = Mock() | ||
| mock_generated_rai_client = Mock() | ||
|
|
||
| mlflow_integration = MLflowIntegration( | ||
| logger=mock_logger, generated_rai_client=mock_generated_rai_client, one_dp_project=False | ||
| ) | ||
|
|
||
| # Act | ||
| result = mlflow_integration.test_storage_upload() | ||
|
|
||
| # Assert | ||
| # For non-OneDP projects, we skip the test and return True | ||
| self.assertTrue(result) | ||
|
|
||
|
|
||
| class TestUploadErrorMessages(unittest.TestCase): | ||
| """Test cases for improved upload error messages.""" | ||
|
|
||
| @patch("azure.ai.evaluation._common.utils.ContainerClient") | ||
| def test_upload_error_message_includes_guidance(self, mock_container_client_class): | ||
| """Test that upload errors include helpful troubleshooting guidance.""" | ||
| # Arrange | ||
| mock_container_client = Mock() | ||
| mock_container_client.account_name = "testaccount" | ||
| mock_container_client.upload_blob.side_effect = Exception("Connection refused") | ||
|
|
||
| # Create a temporary test file | ||
| with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as tmp_file: | ||
| tmp_file.write("test content") | ||
| test_file_path = tmp_file.name | ||
|
|
||
| # Act & Assert | ||
| try: | ||
| with self.assertRaises(EvaluationException) as context: | ||
| upload(path=test_file_path, container_client=mock_container_client) | ||
|
|
||
| # Verify error message contains helpful guidance | ||
| error_message = str(context.exception) | ||
| self.assertIn("storage account", error_message.lower()) | ||
| self.assertIn("permissions", error_message.lower()) | ||
| self.assertIn("verify", error_message.lower()) | ||
| finally: | ||
| # Clean up the temporary file | ||
| try: | ||
| os.unlink(test_file_path) | ||
| except Exception: | ||
| pass | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
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.
Uh oh!
There was an error while loading. Please reload this page.