Skip to content
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.

Sign up for GitHub

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

Upgrade Resource & Storage SDK to T2 #1050

Merged
merged 13 commits into from
Feb 4, 2022
Merged

Upgrade Resource & Storage SDK to T2 #1050

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
36ee042
Update packages
nturinski Jan 26, 2022
98212e0
Convert resource sdk to T2 and use new creds
nturinski Jan 27, 2022
67fe10e
Bump package version
nturinski Jan 27, 2022
e7a9385
PR fixes
nturinski Jan 27, 2022
a25d6b8
Add credentials2 prop to use T2 clients
nturinski Jan 27, 2022
4d29f11
Remove credential2 from ISubscriptionContext, use GenericCred where n…
nturinski Feb 2, 2022
716dc75
Enforce some service cred stuff, use generic creds
nturinski Feb 2, 2022
c758ef2
export uiUtils.listAllIterator
nturinski Feb 3, 2022
5514380
Export type for index.d.ts
nturinski Feb 3, 2022
6219682
Factor out BasicAuth and TokenCreds from index.d.ts
nturinski Feb 4, 2022
bb3d53c
Update ui/index.d.ts
nturinski Feb 4, 2022
e55dca3
Add newlines to files
nturinski Feb 4, 2022
2b5f0ef
Merge branch 'nat/t2packages' of https://github.com/microsoft/vscode-…
nturinski Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions ui/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*--------------------------------------------------------------------------------------------*/

import type { SubscriptionModels } from '@azure/arm-resources-subscriptions';
import type { ExtendedLocation } from '@azure/arm-resources/esm/models';
import type { ResourceManagementModels } from '@azure/arm-resources';
import type { StorageManagementModels } from '@azure/arm-storage';
import type { ExtendedLocation, ResourceGroup } from '@azure/arm-resources';
import type * as coreClient from '@azure/core-client';
import type { PagedAsyncIterableIterator } from "@azure/core-paging";
import type { Environment } from '@azure/ms-rest-azure-env';
import type { HttpOperationResponse, RequestPrepareOptions, ServiceClient } from '@azure/ms-rest-js';
import type { StorageAccount } from '@azure/arm-storage';
import { Disposable, Event, ExtensionContext, FileChangeEvent, FileChangeType, FileStat, FileSystemProvider, FileType, InputBoxOptions, MarkdownString, MessageItem, MessageOptions, OpenDialogOptions, OutputChannel, Progress, QuickPickItem, QuickPickOptions, TextDocumentShowOptions, ThemeIcon, TreeDataProvider, TreeItem, Uri } from 'vscode';
import { TargetPopulation } from 'vscode-tas-client';
import { AzureExtensionApi, AzureExtensionApiProvider } from './api';
Expand Down Expand Up @@ -139,12 +140,19 @@ export abstract class SubscriptionTreeItemBase extends AzExtParentTreeItem {
constructor(parent: AzExtParentTreeItem, subscription: ISubscriptionContext);
}

/**
* Loose type to use for T1 and T2 versions of "@azure/ms-rest-js". The Azure Account extension returns
* credentials that will satisfy both T1 and T2 requirements
*/
export type AzExtServiceClientCredentials = AzExtServiceClientCredentialsT1 | AzExtServiceClientCredentialsT2;
nturinski marked this conversation as resolved.
Show resolved Hide resolved

/**
* Loose interface to allow for the use of different versions of "@azure/ms-rest-js"
* There's several cases where we don't control which "credentials" interface gets used, causing build errors even though the functionality itself seems to be compatible
* For example: https://github.com/Azure/azure-sdk-for-js/issues/10045
* Used specifically for T1 Azure SDKs
*/
export interface AzExtServiceClientCredentials {
export interface AzExtServiceClientCredentialsT1 {
/**
* Signs a request with the Authentication header.
*
Expand All @@ -154,6 +162,25 @@ export interface AzExtServiceClientCredentials {
signRequest(webResource: any): Promise<any>;
}

/**
* Loose interface to allow for the use of different versions of "@azure/ms-rest-js"
* Used specifically for T2 Azure SDKs
*/
export interface AzExtServiceClientCredentialsT2 {

/**
* Gets the token provided by this credential.
*
* This method is called automatically by Azure SDK client libraries. You may call this method
* directly, but you must also handle token caching and token refreshing.
*
* @param scopes - The list of scopes for which the token will have access.
* @param options - The options used to configure any requests this
* TokenCredential implementation might make.
*/
getToken(scopes?: string | string[], options?: any): Promise<any | null>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this returns Promise<any | null> instead of Promise<TokenResponse | AccessToken | null>? Even though we do the same thing for signRequest above and return Promise<any>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to keep it more loose so that other versions of credentials could be accepted. For example, BasicAuthenticationCredentials returns WebResourceLike whereas the ServiceClientCredentials returns WebResource. We originally did that for signRequest because different versions of "@azure/ms-rest-js" would change the credential interface. I figured the same could happen with getToken.

Here's an example with the typing screwing up here: Azure/azure-sdk-for-js#10045

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that version of signRequest that returns WebResource is older? It looks like the current version returns WebResourceLike: https://github.com/Azure/ms-rest-js/blob/f6f0d92d79a1dfa8d92ff0891b88bc6b7a349e69/lib/credentials/serviceClientCredentials.ts

}

/**
* Information specific to the Subscription
*/
Expand Down Expand Up @@ -1162,13 +1189,13 @@ export interface IResourceGroupWizardContext extends ILocationWizardContext, IRe
* If an existing resource group is picked, this value will be defined after `ResourceGroupListStep.prompt` occurs
* If a new resource group is picked, this value will be defined after the `execute` phase of the 'create' subwizard
*/
resourceGroup?: ResourceManagementModels.ResourceGroup;
resourceGroup?: ResourceGroup;

/**
* The task used to get existing resource groups.
* By specifying this in the context, we can ensure that Azure is only queried once for the entire wizard
*/
resourceGroupsTask?: Promise<ResourceManagementModels.ResourceGroup[]>;
resourceGroupsTask?: Promise<ResourceGroup[]>;

newResourceGroupName?: string;

Expand All @@ -1185,7 +1212,7 @@ export declare class ResourceGroupListStep<T extends IResourceGroupWizardContext
* Used to get existing resource groups. By passing in the context, we can ensure that Azure is only queried once for the entire wizard
* @param wizardContext The context of the wizard.
*/
public static getResourceGroups<T extends IResourceGroupWizardContext>(wizardContext: T): Promise<ResourceManagementModels.ResourceGroup[]>;
public static getResourceGroups<T extends IResourceGroupWizardContext>(wizardContext: T): Promise<ResourceGroup[]>;

/**
* Checks existing resource groups in the wizard's subscription to see if the name is available.
Expand Down Expand Up @@ -1218,7 +1245,7 @@ export interface IStorageAccountWizardContext extends IResourceGroupWizardContex
* If an existing storage account is picked, this value will be defined after `StorageAccountListStep.prompt` occurs
* If a new storage account is picked, this value will be defined after the `execute` phase of the 'create' subwizard
*/
storageAccount?: StorageManagementModels.StorageAccount;
storageAccount?: StorageAccount;

newStorageAccountName?: string;
}
Expand Down Expand Up @@ -1409,7 +1436,11 @@ export interface IMinimumServiceClientOptions {
requestPolicyFactories?: any[] | ((defaultRequestPolicyFactories: any[]) => (void | any[]));
}

export type AzExtGenericClientInfo = AzExtServiceClientCredentials | { credentials: AzExtServiceClientCredentials; environment: Environment; } | undefined;
/**
* Credential type to be used for creating generic http rest clients
*/
export type AzExtGenericCredentials = AzExtServiceClientCredentialsT1 | AzExtServiceClientCredentialsT2 | AzExtServiceClientCredentials;
export type AzExtGenericClientInfo = AzExtGenericCredentials | { credentials: AzExtGenericCredentials; environment: Environment; } | undefined;

/**
* Creates a generic http rest client (i.e. for non-Azure calls or for Azure calls that the available sdks don't support), ensuring best practices are followed. For example:
Expand Down Expand Up @@ -1650,3 +1681,7 @@ export declare namespace AzExtFsExtra {
export function writeFile(resource: Uri | string, contents: string): Promise<void>;
export function pathExists(resource: Uri | string): Promise<boolean>;
}

export declare namespace uiUtils {
export function listAllIterator<T>(list: (options?: coreClient.OperationOptions) => PagedAsyncIterableIterator<T>, options?: coreClient.OperationOptions)
}
Loading