Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources #2764

christian-andersen-msft · 2025-01-14T12:31:37Z

Fixes AB#535826

…erification for AOAI services to develop and run on CAPI/managed AI resources

…ces using new method

…ess-verification-rebranch

…access-verification-rebranch

…rganized function

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

src/System Application/App/AI/src/Azure OpenAI/AzureOpenAI.Codeunit.al

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

src/System Application/App/AI/src/Azure OpenAI/AOAIAccountVerificationLog.Table.al

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

…uggable, Updated table name with spaces

src/System Application/App/AI/src/Azure OpenAI/AOAIAccountVerificationLog.Table.al

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

src/System Application/App/AI/src/Azure OpenAI/AOAIAccountVerificationLog.Table.al

…cations, fixed minor issues with record fetching and saving

…ugging features removed.

encimita · 2025-01-28T11:34:34Z

src/System Application/App/AI/src/Azure OpenAI/AzureOpenAI.Codeunit.al

@@ -100,11 +100,26 @@ codeunit 7771 "Azure OpenAI"
    /// Deployment would look like: gpt-35-turbo-16k
    /// </remarks>
    [NonDebuggable]
+    [Obsolete('Using Managed AI resources now requires different input parameters. Use the other overload for SetManagedResourceAuthorization instead.', '26.0')]


You also need to wrap this procedure (including docs comments) within preprocessor symbols:

#if not CLEAN26 <obsoleted procedure code> #endif

This will make sure our automations remove the code after the obsoletion period has passed. It's also the root cause of the failure you see in the automated tests for this PR.

encimita · 2025-01-28T11:51:19Z

src/System Application/App/AI/src/Azure OpenAI/AzureOpenAIImpl.Codeunit.al

@@ -207,6 +207,21 @@ codeunit 7772 "Azure OpenAI Impl"
        end;


Since you are obsoleting a function in the facade codeunit, you are leaving some code in this codeunit unused after the obsoletion period has passed.
In particular the old overload of SetManagedResourceAuthorization will not be called by anyone anymore and it should hence be removed after the obsoletion period has passed.

This codeunit is marked with access=internal, which means you don't need to explicitly obsolete the function with an [Obsolete()] tag because noone can reference this code outside of its own AL extension.
But you still need to make sure we don't leave unused code in the repos after the automations clean up the obsoleted code.

The way to do it here is to wrap the old overload of SetManagedResourceAuthorization in the tags:

#if not CLEAN26 <code for the old overload of SetManagedResourceAuthorization> #endif

This way, when we remove the code from the other codeunit because it's wrapped in CLEAN tags (see my other comment), this will also be removed (our automations will just remove whatever is inside the CLEAN tags, there is no smartness there, so it's up to the developer to decide what to put inside these tags).

encimita · 2025-01-28T12:17:24Z

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

@@ -57,6 +77,19 @@ codeunit 7767 "AOAI Authorization"
        Deployment := NewDeployment;
        ApiKey := NewApiKey;
        ManagedResourceDeployment := NewManagedResourceDeployment;
+        MicrosoftManagedAuthorizationWithDeployment := true;


The other overload of SetMicrosoftManagedAuthorization is now unused after the function in the other codeunit is removed.

So you need to wrap the old overload of SetMicrosoftManagedAuthorization into

#if not CLEAN26 <old overload> #endif

encimita · 2025-01-28T12:42:09Z

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

+                end
+                else
+                    if MicrosoftManagedAuthorizationWithDeployment then
+                        exit(AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls());


With this change, we are no longer checking anywhere that the variables are not empty.

I suggest we don't add the 4 new booleans at all, and instead we rely on the existence of account name or not (for example).

Example pseudo-code:

Enum::"AOAI Resource Utilization"::"Microsoft Managed": if (AOAIAccountName <> '') and (ManagedResourceDeployment <> '') and (not ApiKey.IsEmpty()) then exit(VerifyAOAIAccount(AOAIAccountName, ApiKey) and AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls()) else exit((Deployment <> '') and (Endpoint <> '') and (not ApiKey.IsEmpty()) and (ManagedResourceDeployment <> '') and AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls());

You could even go one step further and make sure the old verification code is cleaned up automatically after the obsoletion period has passed

Example pseudo-code:

#if CLEAN26 Enum::"AOAI Resource Utilization"::"Microsoft Managed": exit((AOAIAccountName <> '') and (ManagedResourceDeployment <> '') and (not ApiKey.IsEmpty()) and VerifyAOAIAccount(AOAIAccountName, ApiKey) and AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls()); #else Enum::"AOAI Resource Utilization"::"Microsoft Managed": if (AOAIAccountName <> '') and (ManagedResourceDeployment <> '') and (not ApiKey.IsEmpty()) then exit(VerifyAOAIAccount(AOAIAccountName, ApiKey) and AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls()) else exit((Deployment <> '') and (Endpoint <> '') and (not ApiKey.IsEmpty()) and (ManagedResourceDeployment <> '') and AzureOpenAiImpl.IsTenantAllowlistedForFirstPartyCopilotCalls()); #endif

encimita · 2025-01-28T12:56:13Z

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

+            if VerificationLog.Get(TruncatedAccountName) then
+                RemainingGracePeriod := GracePeriod - (CurrentDateTime - VerificationLog.LastSuccessfulVerification)
+            else
+                RemainingGracePeriod := GracePeriod;


If there is no entry in verification log, then the remaining grace period should be 0.

It means that azure account was never verified and hence they are not entitled to grace period.

encimita · 2025-01-28T13:00:14Z

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

+            '0000AA1', // Event ID
+            StrSubstNo(LogMessage, AccountName, VerificationDate),
+            Verbosity::Warning,
+            DataClassification::SystemMetadata,


DataClassification::CustomerContent

The account name is customer content

encimita · 2025-01-28T13:01:00Z

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al

+            // Within GRACE period
+            if IsAccountVerifiedWithinPeriod(TruncatedAccountName, GracePeriod) then begin
+                ShowUserNotification(StrSubstNo(AuthFailedWithinGracePeriodUserNotificationLbl, FormatDurationAsDays(RemainingGracePeriod)));
+                LogTelemetry(AccountName, Today, StrSubstNo(AuthFailedWithinGracePeriodLogMessageLbl, AccountName, Today, FormatDurationAsDays(RemainingGracePeriod)));


you are doing StrSubstNo inside LogTelemetry already. It seems like we don't need it here.

Christian Andersen and others added 17 commits November 26, 2024 14:00

Slice 535826: [AI][Public Preview]Copilot Toolkit: Automatic access v…

1702f14

…erification for AOAI services to develop and run on CAPI/managed AI resources

Restored SetManagedResourceAuthorization with old parameters

09f9d04

Removed unnessecary variables when verifying Microsoft managed resour…

4119ef8

…ces using new method

Merge commit '9fc5c8aac7c0c57e78b1f75d0830ee8ea718f9cb' into aoai-acc…

9ed666f

…ess-verification-rebranch

Merge branch 'main' of https://github.com/microsoft/BCApps into aoai-…

da93eee

…access-verification-rebranch

Merge branch 'main' of https://github.com/microsoft/BCApps into aoai-…

0264589

…access-verification-rebranch

Simplified configuration check and made it more flexible

22af527

Merge branch 'main' into aoai-access-verification-rebranch

f9abbb7

Merge branch 'main' into aoai-access-verification-rebranch

5fc3ec2

Temporary Database and notification templates added

2b18667

tmp template changes

8a08eb2

Grace period with caching added

d6c54fe

Added notification and telemetry to verification logic as well as reo…

54ad50a

…rganized function

corercted comments

02bd29c

added debugging messages

16e7169

Merge branch 'main' into aoai-access-verification-rebranch

e3680d2

removed wrong reference

8c41f6d

christian-andersen-msft requested a review from a team as a code owner January 14, 2025 12:31

github-actions bot added the AL: System Application label Jan 14, 2025

christian-andersen-msft changed the title ~~Aoai access verification rebranch~~ Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources Jan 14, 2025

github-actions bot added this to the Version 26.0 milestone Jan 14, 2025

Added execution permissions to table and increased test timeouts

10ec3e8

encimita reviewed Jan 15, 2025

View reviewed changes

darjoo reviewed Jan 15, 2025

View reviewed changes

msft-sam reviewed Jan 15, 2025

View reviewed changes

src/System Application/App/AI/src/Azure OpenAI/AOAIAuthorization.Codeunit.al Show resolved Hide resolved

christian-andersen-msft added 5 commits January 15, 2025 17:18

added additional debugging messages

e6ce450

Added obsolete to old SetManagedResourceAuthorization, Removed NonDeb…

b6d0848

…uggable, Updated table name with spaces

Updated debugging messages to handle duration

91c85c7

fixed errors in debugging formatting

8d03c7f

Merge branch 'main' into aoai-access-verification-rebranch

efd3c2e

NKarolak suggested changes Jan 20, 2025

View reviewed changes

christian-andersen-msft added 7 commits January 21, 2025 12:03

Merge branch 'main' into aoai-access-verification-rebranch

8f292ec

Updated quality and functionality of logging messages and user notifi…

b7750b2

…cations, fixed minor issues with record fetching and saving

Merge branch 'main' into aoai-access-verification-rebranch

e711ea4

Notification text improved. Logic regarding grace period updated. Deb…

69ae41a

…ugging features removed.

Variables sorted alphabetically

a1bf072

field caption clarified

41f99e6

Description of parameters improved

2df6e9a

NKarolak approved these changes Jan 23, 2025

View reviewed changes

Added session telemetry

1d2a760

christian-andersen-msft requested a review from encimita January 23, 2025 15:15

Added telemetry tags

09e7ac7

encimita requested changes Jan 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources #2764

Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources #2764

christian-andersen-msft commented Jan 14, 2025 •

edited by github-actions bot

Loading

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

encimita Jan 28, 2025

Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources #2764

Are you sure you want to change the base?

Automatic access verification for AOAI services to develop and run on CAPI/managed AI resources #2764

Conversation

christian-andersen-msft commented Jan 14, 2025 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christian-andersen-msft commented Jan 14, 2025 •

edited by github-actions bot

Loading