Skip to content

Commit

Permalink
Python: Azure OpenAI setting fix: Don't fetch AD token if API is pres…
Browse files Browse the repository at this point in the history
…ent in environment variable. (#8985)

### Motivation and Context

While working on #8951 I found that interacting with an Azure OpenAI
instance I had an API key for, but my identity didn't have IAM access to
the API for, that I was getting a permission error:

```
semantic_kernel.exceptions.service_exceptions.ServiceResponseException: ("<class 'semantic_kernel.connectors.ai.open_ai.services.azure_text_embedding.AzureTextEmbedding'> service failed to generate embeddings", AuthenticationError("Error code: 401 - {'error': {'code': 'PermissionDenied', 'message': 'Principal does not have access to API/Operation.'}}"))
```

I found that this was because, though my API key was set in the
environment, the codepath only recognized an explicitly passed in API
key when deciding whether to issue an AD token while initializing an
AzureTextEmbedding instance.

This PR fixes that by looking at the setting object explicitly, which
may pull the API key from the environment.


### Contribution Checklist

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
  • Loading branch information
lossyrob authored Sep 25, 2024
1 parent 679df70 commit 771ac2c
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,21 @@ def __init__(

# If the api_key is none, and the ad_token is none, and the ad_token_provider is none,
# then we will attempt to get the ad_token using the default endpoint specified in the Azure OpenAI settings.
if api_key is None and ad_token_provider is None and azure_openai_settings.token_endpoint and ad_token is None:
if (
azure_openai_settings.api_key is None
and ad_token_provider is None
and azure_openai_settings.token_endpoint
and ad_token is None
and async_client is None
):
ad_token = azure_openai_settings.get_azure_openai_auth_token(
token_endpoint=azure_openai_settings.token_endpoint
)

if not azure_openai_settings.api_key and not ad_token and not ad_token_provider:
raise ServiceInitializationError("Please provide either api_key, ad_token or ad_token_provider")
if not azure_openai_settings.api_key and not ad_token and not ad_token_provider and not async_client:
raise ServiceInitializationError(
"Please provide either api_key, ad_token, ad_token_provider, or a custom client."
)

super().__init__(
deployment_name=azure_openai_settings.text_deployment_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,22 @@ def __init__(

# If the api_key is none, and the ad_token is none, and the ad_token_provider is none,
# then we will attempt to get the ad_token using the default endpoint specified in the Azure OpenAI settings.
if api_key is None and ad_token_provider is None and azure_openai_settings.token_endpoint and ad_token is None:
if (
azure_openai_settings.api_key is None
and ad_token_provider is None
and azure_openai_settings.token_endpoint
and ad_token is None
and async_client is None
):
ad_token = azure_openai_settings.get_azure_openai_auth_token(
token_endpoint=azure_openai_settings.token_endpoint
)

if not azure_openai_settings.api_key and not ad_token and not ad_token_provider and not async_client:
raise ServiceInitializationError(
"Please provide either api_key, ad_token, ad_token_provider, or a custom client"
)

super().__init__(
deployment_name=azure_openai_settings.embedding_deployment_name,
endpoint=azure_openai_settings.endpoint,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
OpenAITextPromptExecutionSettings,
)
from semantic_kernel.connectors.ai.open_ai.services.azure_text_completion import AzureTextCompletion
from semantic_kernel.connectors.ai.open_ai.settings.azure_open_ai_settings import AzureOpenAISettings
from semantic_kernel.connectors.ai.text_completion_client_base import TextCompletionClientBase
from semantic_kernel.contents.text_content import TextContent
from semantic_kernel.exceptions import ServiceInitializationError
Expand Down Expand Up @@ -54,6 +55,20 @@ def test_init_with_custom_header(azure_openai_unit_test_env) -> None:
assert azure_text_completion.client.default_headers[key] == value


def test_azure_text_embedding_generates_no_token_with_api_key_in_env(azure_openai_unit_test_env) -> None:
with (
patch(
f"{AzureOpenAISettings.__module__}.{AzureOpenAISettings.__qualname__}.get_azure_openai_auth_token",
) as mock_get_token,
):
mock_get_token.return_value = "test_token"
azure_text_completion = AzureTextCompletion()

assert azure_text_completion.client is not None
# API key is provided in env var, so the ad_token should be None
assert mock_get_token.call_count == 0


@pytest.mark.parametrize("exclude_list", [["AZURE_OPENAI_TEXT_DEPLOYMENT_NAME"]], indirect=True)
def test_init_with_empty_deployment_name(monkeypatch, azure_openai_unit_test_env) -> None:
monkeypatch.delenv("AZURE_OPENAI_TEXT_DEPLOYMENT_NAME", raising=False)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from semantic_kernel.connectors.ai.embeddings.embedding_generator_base import EmbeddingGeneratorBase
from semantic_kernel.connectors.ai.open_ai.services.azure_text_embedding import AzureTextEmbedding
from semantic_kernel.connectors.ai.open_ai.settings.azure_open_ai_settings import AzureOpenAISettings
from semantic_kernel.exceptions.service_exceptions import ServiceInitializationError


Expand Down Expand Up @@ -74,6 +75,20 @@ def test_azure_text_embedding_init_with_from_dict(azure_openai_unit_test_env) ->
assert azure_text_embedding.client.default_headers[key] == value


def test_azure_text_embedding_generates_no_token_with_api_key_in_env(azure_openai_unit_test_env) -> None:
with (
patch(
f"{AzureOpenAISettings.__module__}.{AzureOpenAISettings.__qualname__}.get_azure_openai_auth_token",
) as mock_get_token,
):
mock_get_token.return_value = "test_token"
azure_text_embedding = AzureTextEmbedding()

assert azure_text_embedding.client is not None
# API key is provided in env var, so the ad_token should be None
assert mock_get_token.call_count == 0


@pytest.mark.asyncio
@patch.object(AsyncEmbeddings, "create", new_callable=AsyncMock)
async def test_azure_text_embedding_calls_with_parameters(mock_create, azure_openai_unit_test_env) -> None:
Expand Down

0 comments on commit 771ac2c

Please sign in to comment.