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.

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

Remove deprecated Azure SDK's from Calamari.CloudAccounts #1356

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

APErebus
Copy link
Contributor

@APErebus APErebus commented Oct 4, 2024

Removes the use of Microsoft.Rest and Microsoft.Azure.Management.Fluent from Calamari.CloudAccounts as these are deprecated libraries.

The main code changes is to move the logic for retrieving the access token on to the IAzureAccount implementations, rather than on extension methods

Comment on lines -27 to -30
public static readonly AzureKnownEnvironment Global = new AzureKnownEnvironment("AzureGlobalCloud");
public static readonly AzureKnownEnvironment AzureChinaCloud = new AzureKnownEnvironment("AzureChinaCloud");
public static readonly AzureKnownEnvironment AzureUSGovernment = new AzureKnownEnvironment("AzureUSGovernment");
public static readonly AzureKnownEnvironment AzureGermanCloud = new AzureKnownEnvironment("AzureGermanCloud");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this was unused. The only one was Global, but we only needed the string, so let's just use it.

Comment on lines -76 to +78
var token = !jwt.IsNullOrEmpty()
? await new AzureOidcAccount(variables).Credentials(CancellationToken.None)
: await new AzureServicePrincipalAccount(variables).Credentials();
var resourcesClient = new ResourceManagementClient(token, AuthHttpClientFactory.ProxyClientHandler())
{
SubscriptionId = subscriptionId,
BaseUri = new Uri(resourceManagementEndpoint),
};
resourcesClient.HttpClient.DefaultRequestHeaders.Add("Authorization", "Bearer " + token);
resourcesClient.HttpClient.BaseAddress = new Uri(resourceManagementEndpoint);
return resourcesClient;
};

await CreateDeployment(createArmClient, resourceGroupName, deploymentName, deploymentMode, template, parameters);
{
var account = AzureAccountFactory.Create(variables);
var token = new TokenCredentials(await account.GetAuthorizationToken(log, CancellationToken.None));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidating around the AzureAccountFactory and the new IAzureAccount.GetAuthorizationToken method

public string ClientId { get; }
public string TenantId { get; }
public string Jwt { get; }
public string AzureEnvironment { get; }
public string ResourceManagementEndpointBaseUri { get; }
public string ActiveDirectoryEndpointBaseUri { get; }

internal static string GetDefaultScope(string environmentName)
public async Task<string> GetAuthorizationToken(ILog log, CancellationToken cancellationToken)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this code was in the two extension method classes (one for each type), but changed to instance methods so they can be accessed through the IAzureAccount

@APErebus APErebus changed the title Add NET 6.0 support and start rewriting stuff Remove deprecated SDK's from Calamari.CloudAccounts Oct 7, 2024
@APErebus APErebus changed the title Remove deprecated SDK's from Calamari.CloudAccounts Remove deprecated Azure SDK's from Calamari.CloudAccounts Oct 7, 2024
@APErebus APErebus force-pushed the ap/remove-deprecated-api-from-cloud-accounts branch 3 times, most recently from 2a80b23 to d47d44f Compare October 14, 2024 01:08
Base automatically changed from ap/net-core-azurewebapp to main October 28, 2024 23:31
@APErebus APErebus force-pushed the ap/remove-deprecated-api-from-cloud-accounts branch from d47d44f to 4c8f8f8 Compare October 28, 2024 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant