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

Jump to bottom

Using ManagedIdentityCredential in Microsoft.DotNet.Monitoring.Sdk #4172

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+41 −63
Draft

Using ManagedIdentityCredential in Microsoft.DotNet.Monitoring.Sdk #4172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4ed5dd
Adding ChainedTokenCredential
fhnaseer Sep 26, 2024
ee9541a
Removing fields,
fhnaseer Sep 26, 2024
2e04b25
more,
fhnaseer Sep 26, 2024
87f455f
Merge branch 'main' into fhnaseer/monitoring-managed-identity
fhnaseer Sep 30, 2024
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
4 changes: 0 additions & 4 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,6 @@ extends:
GrafanaHost: https://dotnet-eng-grafana-staging.westus2.cloudapp.azure.com
GrafanaKeyVault: dotnet-grafana-staging
GrafanaVariableGroup: Dotnet-Grafana-Staging
ServiceConnectionClientId: 57f299da-15de-4117-b8f6-7c10451926f0
ServiceConnectionId: 7829de7e-fb4e-4118-8370-475d6bc61905
${{ else }}:
DeploymentEnvironment: Production
DotNetStatusAppName: dotneteng-status
Expand All @@ -269,5 +267,3 @@ extends:
GrafanaHost: https://dotnet-eng-grafana.westus2.cloudapp.azure.com
GrafanaKeyVault: dotnet-grafana
GrafanaVariableGroup: Dotnet-Grafana-Production
ServiceConnectionClientId: fc1eb341-aea4-4a11-8f80-d14b8775b2ba
ServiceConnectionId: 4a511f6f-b538-48e6-a389-207e430634d1
10 changes: 1 addition & 9 deletions eng/deploy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
parameters:
- name: ServiceConnectionName
type: string
- name: ServiceConnectionClientId
type: string
- name: ServiceConnectionId
type: string
- name: PublishProfile
type: string
- name: DotNetStatusAppName
Expand Down Expand Up @@ -194,10 +190,6 @@ stages:
value: ${{ parameters.GrafanaHost }}
- name: GrafanaKeyVault
value: ${{ parameters.GrafanaKeyVault }}
- name: GrafanaClientId
value: ${{ parameters.ServiceConnectionClientId }}
- name: GrafanaServiceConnectionId
value: ${{ parameters.ServiceConnectionId }}
jobs:
- job: notifyEndDeployment
displayName: Notify deployment end
Expand All @@ -220,7 +212,7 @@ stages:
useGlobalJson: true
- script: dotnet publish $(Build.SourcesDirectory)\src\Monitoring\Sdk\Microsoft.DotNet.Monitoring.Sdk.csproj -f net6.0
displayName: Build Monitoring SDK
- script: dotnet build $(Build.SourcesDirectory)\src\Monitoring\Monitoring.ArcadeServices\Monitoring.ArcadeServices.proj -t:PublishGrafana -p:GrafanaAccessToken=$(grafana-admin-api-key) -p:GrafanaHost=$(GrafanaHost) -p:GrafanaKeyVaultName=$(GrafanaKeyVault) -p:ClientId=$(GrafanaClientId) -p:ServiceConnectionId=$(GrafanaServiceConnectionId) -p:SystemAccessToken=$(System.AccessToken) -p:GrafanaEnvironment=$(DeploymentEnvironment) -p:ParametersFile=parameters.json -v:normal
- script: dotnet build $(Build.SourcesDirectory)\src\Monitoring\Monitoring.ArcadeServices\Monitoring.ArcadeServices.proj -t:PublishGrafana -p:GrafanaAccessToken=$(grafana-admin-api-key) -p:GrafanaHost=$(GrafanaHost) -p:GrafanaKeyVaultName=$(GrafanaKeyVault) -p:GrafanaEnvironment=$(DeploymentEnvironment) -p:ParametersFile=parameters.json -v:normal
displayName: Publish Grafana Dashboards

- stage: validateDeployment
Expand Down
47 changes: 2 additions & 45 deletions src/Monitoring/Sdk/MonitoringPublish.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,7 @@ public class MonitoringPublish : BuildTask
[Required]
public string KeyVaultName { get; set; }

// For azure pipeline service connection authentication
public string ClientId { get; set; }
public string ServiceConnectionId { get; set; }
public string SystemAccessToken { get; set; }

// For client secret authentication
public string KeyVaultServicePrincipalId { get; set; }
public string KeyVaultServicePrincipalSecret { get; set; }
public string ManagedIdentityId { get; set; }

[Required]
public string Tag { get; set; }
Expand All @@ -58,43 +51,7 @@ public sealed override bool Execute()

private async Task<bool> ExecuteAsync()
{
string msftTenantId = "72f988bf-86f1-41af-91ab-2d7cd011db47";
TokenCredential tokenCredential;

if (ClientId == null && ServiceConnectionId == null && SystemAccessToken == null && KeyVaultServicePrincipalId == null && KeyVaultServicePrincipalSecret == null)
{
tokenCredential = new DefaultAzureCredential(
new DefaultAzureCredentialOptions()
{
TenantId = msftTenantId
}
);
}
else if (ClientId != null || ServiceConnectionId != null || SystemAccessToken != null)
{
if (ClientId == null || ServiceConnectionId == null || SystemAccessToken == null)
{
Log.LogError("Invalid login combination. Set ClientId, ServiceConnectionId and SystemAccessToken for CI, or none for local user authentication.");
return false;
}
else
{
tokenCredential = new AzurePipelinesCredential(msftTenantId, ClientId, ServiceConnectionId, SystemAccessToken);
}
}
else
{
if (KeyVaultServicePrincipalId == null || KeyVaultServicePrincipalSecret == null)
{
Log.LogError("Invalid login combination. Set KeyVaultServicePrincipalId and KeyVaultServicePrincipalSecret for Client Secret authentication.");
return false;
}
else
{
tokenCredential = new ClientSecretCredential(msftTenantId, KeyVaultServicePrincipalId, KeyVaultServicePrincipalSecret);
}
}

ChainedTokenCredential tokenCredential = TokenCredentialHelper.GetChainedTokenCredential(ManagedIdentityId);
Copy link
Member Author

@fhnaseer fhnaseer Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@garath @mmitche How can I get ManagedIdentityId here? Should it be passed in a similar fashion like ServiceConnectionId/KeyVaultServicePrincipalId?

Copy link
Member

Choose a reason for hiding this comment

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

It depends on what the token credential is actually used for here. @garath may know more, but it looks like it's used to access a KV and replace secret sentinels in a config file with the actual secrets.

If this is the case, then most likely we want to use workload identity federation. This is the pattern where you execute the deployment set in AzureCLI with a service connection that is attached to a managed identity that has access to the vault. Looks sorta like: https://github.com/dotnet/arcade/blob/main/eng/publishing/v3/publish.yml#L87-L95

In that case, then I think the managed identity would be the value provided by the AzureCLI: https://github.com/dotnet/arcade/blob/main/eng/publishing/v3/publish.yml#L111

You may not need that though. You may just need AzureCliCredential and _defaultCredential as part of the chain.

Copy link
Member

Choose a reason for hiding this comment

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

The user assigned managed identity would be placed alongside the service in Azure, and FR knows how to set up the service connection using federated credentials if you don't.

Copy link
Member

@garath garath Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I believe you want to keep the AzurePipelinesCredential use. This tool is most often run during an Azure Pipeline run, so managed identities won't work.

The DefaultAzureCredential is meant to cover use by a dev on their own machine (which is also a common way its run).

If you actually need to configure use for managed identity, I recommend following the current pattern of seeing which flags are passed in to the app and then configuring a Credential instance specifically for it. (Though, I'm not sure about cases where MI would be helpful here. Please describe the use case.)

using (var client = new GrafanaClient(Host, AccessToken))
using (var deploy = new DeployPublisher(
grafanaClient: client,
Expand Down
37 changes: 37 additions & 0 deletions src/Monitoring/Sdk/TokenCredentialHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
using System.Collections.Concurrent;
using Azure.Identity;

namespace Microsoft.DotNet.Monitoring.Sdk;

internal static class TokenCredentialHelper
{
private static readonly ChainedTokenCredential _defaultCredential = new(
new DefaultAzureCredential(
new DefaultAzureCredentialOptions
{
ExcludeEnvironmentCredential = true
}
)
);

private static ConcurrentDictionary<string, ChainedTokenCredential> CredentialCache { get; } = new ConcurrentDictionary<string, ChainedTokenCredential>();

public static ChainedTokenCredential GetChainedTokenCredential(string managedIdentityId)
{
if (managedIdentityId == null)
{
return _defaultCredential;
}

if (CredentialCache.TryGetValue(managedIdentityId, out var chainedTokenCredential))
{
return chainedTokenCredential;
}

var credential = new ChainedTokenCredential(new ManagedIdentityCredential(managedIdentityId), new AzureCliCredential(), _defaultCredential);

CredentialCache.TryAdd(managedIdentityId, credential);

return credential;
}
}
6 changes: 1 addition & 5 deletions src/Monitoring/Sdk/sdk/Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@
Host="$(GrafanaHost)"
AccessToken="$(GrafanaAccessToken)"
KeyVaultName="$(GrafanaKeyVaultName)"
ClientId="$(ClientId)"
ServiceConnectionId="$(ServiceConnectionId)"
SystemAccessToken="$(SystemAccessToken)"
KeyVaultServicePrincipalId="$(GrafanaKeyVaultServicePrincipalId)"
KeyVaultServicePrincipalSecret="$(GrafanaKeyVaultServicePrincipalSecret)"
MangedIdentityId="$(MangedIdentityId)"
Tag="$(GrafanaDashboardTag)"
Environment="$(GrafanaEnvironment)"
ParametersFile="$(ParametersFile)"
Expand Down