Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,18 @@ public static Command CreateCommand(
cleanupCommand.AddOption(configOption);
cleanupCommand.AddOption(verboseOption);

// Generate correlation ID at workflow entry point
var correlationId = HttpClientFactory.GenerateCorrelationId();

// Set default handler for 'a365 cleanup' (without subcommand) - cleans up everything
cleanupCommand.SetHandler(async (configFile, verbose) =>
{
// Generate correlation ID at workflow entry point
var correlationId = HttpClientFactory.GenerateCorrelationId();
logger.LogInformation("Starting cleanup (CorrelationId: {CorrelationId})", correlationId);

await ExecuteAllCleanupAsync(logger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService, configFile, correlationId: correlationId);
}, configOption, verboseOption);

// Add subcommands for granular control
cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, federatedCredentialService, correlationId: correlationId));
cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, federatedCredentialService));
cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor));
cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor));

Expand All @@ -63,8 +64,7 @@ private static Command CreateBlueprintCleanupCommand(
IBotConfigurator botConfigurator,
CommandExecutor executor,
AgentBlueprintService agentBlueprintService,
FederatedCredentialService federatedCredentialService,
string? correlationId = null)
FederatedCredentialService federatedCredentialService)
{
var command = new Command("blueprint", "Remove Entra ID blueprint application and service principal");

Expand All @@ -91,6 +91,10 @@ private static Command CreateBlueprintCleanupCommand(
{
try
{
// Generate correlation ID at workflow entry point
var correlationId = HttpClientFactory.GenerateCorrelationId();
logger.LogInformation("Starting blueprint cleanup (CorrelationId: {CorrelationId})", correlationId);

var config = await LoadConfigAsync(configFile, logger, configService);
if (config == null) return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ await UpdateEndpointAsync(
logger: logger,
configService: configService,
botConfigurator: botConfigurator,
platformDetector: platformDetector);
platformDetector: platformDetector,
correlationId: correlationId);
}
catch (Agent365Exception ex)
{
Expand Down Expand Up @@ -1859,13 +1860,15 @@ await ProjectSettingsSyncHelper.ExecuteAsync(
/// <param name="configService">Configuration service</param>
/// <param name="botConfigurator">Bot configurator service</param>
/// <param name="platformDetector">Platform detector service</param>
/// <param name="correlationId">Optional correlation ID for tracing</param>
public static async Task UpdateEndpointAsync(
string configPath,
string newEndpointUrl,
ILogger logger,
IConfigService configService,
IBotConfigurator botConfigurator,
PlatformDetector platformDetector)
PlatformDetector platformDetector,
string? correlationId = null)
{
var setupConfig = await configService.LoadAsync(configPath);

Expand All @@ -1887,6 +1890,12 @@ public static async Task UpdateEndpointAsync(
logger.LogInformation("Updating messaging endpoint...");
logger.LogInformation("");

// Normalize location once; used by both Step 1 and Step 1.5.
// Null-coalescing is intentional: Location is only validated inside the Step 1 block (not here),
// so it may still be null at this point. The empty-string fallback is never passed to any API —
// Step 1 throws before using it, and Step 1.5 guards on !IsNullOrWhiteSpace(Location).
var normalizedLocation = setupConfig.Location?.Replace(" ", "").ToLowerInvariant() ?? string.Empty;

// Step 1: Delete existing endpoint if it exists
if (!string.IsNullOrWhiteSpace(setupConfig.MessagingEndpoint) || !string.IsNullOrWhiteSpace(setupConfig.BotName))
{
Expand Down Expand Up @@ -1916,12 +1925,11 @@ public static async Task UpdateEndpointAsync(
endpointName = Services.Helpers.EndpointHelper.GetEndpointName(setupConfig.BotName);
}

var normalizedLocation = setupConfig.Location.Replace(" ", "").ToLowerInvariant();

var deleted = await botConfigurator.DeleteEndpointWithAgentBlueprintAsync(
endpointName,
normalizedLocation,
setupConfig.AgentBlueprintId);
setupConfig.AgentBlueprintId,
correlationId: correlationId);

if (!deleted)
{
Expand All @@ -1936,12 +1944,29 @@ public static async Task UpdateEndpointAsync(
logger.LogInformation("No existing endpoint found. Proceeding with registration.");
}

// Step 1.5: Pre-create cleanup of the target endpoint name.
// If a previous --update-endpoint failed during the create step, Azure may have
// partially provisioned the new endpoint and left it in a bad state that blocks
// subsequent creates with InternalServerError. Delete it now to ensure a clean slate.
if (!setupConfig.NeedDeployment && !string.IsNullOrWhiteSpace(setupConfig.Location))
{
var targetEndpointName = Services.Helpers.EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, setupConfig.AgentBlueprintId);
logger.LogInformation("Removing target endpoint '{EndpointName}' (derived from {Url}) to ensure a clean state before registration.", targetEndpointName, newEndpointUrl);
var preCleanupDeleted = await botConfigurator.DeleteEndpointWithAgentBlueprintAsync(targetEndpointName, normalizedLocation, setupConfig.AgentBlueprintId, correlationId: correlationId);
if (!preCleanupDeleted)
{
// Not fatal — proceed and let Step 2 surface the error if the partially-provisioned
// endpoint is still blocking. The warning helps diagnose production issues.
logger.LogWarning("Pre-create cleanup for '{EndpointName}' did not confirm deletion. Proceeding anyway.", targetEndpointName);
}
}

// Step 2: Register new endpoint with the provided URL
logger.LogInformation("");
logger.LogInformation("Registering new messaging endpoint...");

var (endpointRegistered, _) = await SetupHelpers.RegisterBlueprintMessagingEndpointAsync(
setupConfig, logger, botConfigurator, newEndpointUrl);
setupConfig, logger, botConfigurator, newEndpointUrl, correlationId: correlationId);

if (!endpointRegistered)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ public async Task<bool> DeleteEndpointWithAgentBlueprintAsync(
string? correlationId = null)
{
_logger.LogInformation("Deleting endpoint with Agent Blueprint Identity...");
_logger.LogDebug(" Endpoint Name: {EndpointName}", endpointName);
_logger.LogDebug(" Agent Blueprint ID: {AgentBlueprintId}", agentBlueprintId);
_logger.LogInformation(" Endpoint Name: {EndpointName}", endpointName);
_logger.LogInformation(" Agent Blueprint ID: {AgentBlueprintId}", agentBlueprintId);

if (string.IsNullOrWhiteSpace(location))
{
Expand Down Expand Up @@ -288,6 +288,12 @@ public async Task<bool> DeleteEndpointWithAgentBlueprintAsync(

// Call the endpoint
_logger.LogInformation("Making request to delete endpoint (Location: {Location}).", normalizedLocation);
_logger.LogInformation("Delete request payload:");
_logger.LogInformation(" AzureBotServiceInstanceName: {Name}", endpointName);
_logger.LogInformation(" AppId: {AppId}", agentBlueprintId);
_logger.LogInformation(" TenantId: {TenantId}", tenantId);
_logger.LogInformation(" Location: {Location}", normalizedLocation);
_logger.LogInformation(" Environment: {Environment}", EndpointHelper.GetDeploymentEnvironment(config.Environment));

using var request = new HttpRequestMessage(HttpMethod.Delete, deleteEndpointUrl);
request.Content = new StringContent(deleteEndpointBody.ToJsonString(), System.Text.Encoding.UTF8, "application/json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands;
using Microsoft.Agents.A365.DevTools.Cli.Models;
using Microsoft.Agents.A365.DevTools.Cli.Services;
using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers;
using Microsoft.Extensions.Logging;
using NSubstitute;
using System.CommandLine;
Expand Down Expand Up @@ -1462,7 +1463,7 @@ await _mockBotConfigurator.DidNotReceive().CreateEndpointWithAgentBlueprintAsync
}

[Fact]
public async Task UpdateEndpointAsync_WithNoExistingEndpoint_ShouldSkipDeleteAndRegister()
public async Task UpdateEndpointAsync_WithNoExistingOldEndpoint_ShouldOnlyCallPreCreateCleanup()
{
// Arrange - Config without BotName (no existing endpoint)
var config = new Agent365Config
Expand Down Expand Up @@ -1490,6 +1491,10 @@ public async Task UpdateEndpointAsync_WithNoExistingEndpoint_ShouldSkipDeleteAnd
_mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>(), Arg.Any<string>())
.Returns(Task.CompletedTask);

_mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
.Returns(Task.FromResult(true)); // NotFound = success for pre-create cleanup

_mockBotConfigurator.CreateEndpointWithAgentBlueprintAsync(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
.Returns(EndpointRegistrationResult.Created);
Expand All @@ -1503,13 +1508,100 @@ await BlueprintSubcommand.UpdateEndpointAsync(
_mockBotConfigurator,
_mockPlatformDetector);

// Assert - Should NOT call delete (no existing endpoint)
await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync(
// Assert - Step 1 (delete old) is skipped — no existing endpoint to delete.
// Step 1.5 (pre-create cleanup) still calls delete exactly once with the TARGET endpoint name,
// so there is exactly one delete call total.
var expectedTargetName = EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, config.AgentBlueprintId);
await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
expectedTargetName,
"eastus",
config.AgentBlueprintId);

// Should still register the new endpoint
await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Any<string>());
newEndpointUrl,
Arg.Any<string>(),
config.AgentBlueprintId);
}
finally
{
if (File.Exists(generatedPath)) File.Delete(generatedPath);
if (File.Exists(configPath)) File.Delete(configPath);
}
}

// Should still register the new endpoint
[Fact]
public async Task UpdateEndpointAsync_WithExistingOldEndpointAndPartiallyProvisionedTarget_ShouldCallDeleteTwice()
{
// Regression: when a prior --update-endpoint failed during the create step, Azure may have
// partially provisioned the new endpoint. On the next run, BOTH Step 1 (delete old) and
// Step 1.5 (pre-create cleanup of target) must fire, targeting different endpoint names.
var currentlyRegisteredUrl = "https://currently-registered-3979.inc1.devtunnels.ms/api/messages";
var newEndpointUrl = "https://newtunnel-3979.inc1.devtunnels.ms/api/messages";

var config = new Agent365Config
{
TenantId = "00000000-0000-0000-0000-000000000000",
AgentBlueprintId = "blueprint-123",
MessagingEndpoint = currentlyRegisteredUrl, // static config (original tunnel)
BotMessagingEndpoint = currentlyRegisteredUrl, // generated config (last successful registration)
Location = "eastus",
NeedDeployment = false,
DeploymentProjectPath = Path.GetTempPath()
};

var testId = Guid.NewGuid().ToString();
var configPath = Path.Combine(Path.GetTempPath(), $"test-config-{testId}.json");
var generatedPath = Path.Combine(Path.GetTempPath(), $"a365.generated.config-{testId}.json");

await File.WriteAllTextAsync(generatedPath, "{}");

try
{
_mockConfigService.LoadAsync(Arg.Any<string>(), Arg.Any<string>())
.Returns(Task.FromResult(config));

_mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>(), Arg.Any<string>())
.Returns(Task.CompletedTask);

_mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
.Returns(Task.FromResult(true));

_mockBotConfigurator.CreateEndpointWithAgentBlueprintAsync(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
.Returns(EndpointRegistrationResult.Created);

// Act
await BlueprintSubcommand.UpdateEndpointAsync(
configPath,
newEndpointUrl,
_mockLogger,
_mockConfigService,
_mockBotConfigurator,
_mockPlatformDetector);

// Assert — exactly two delete calls with distinct endpoint names
var oldEndpointName = EndpointHelper.GetEndpointNameFromUrl(currentlyRegisteredUrl, config.AgentBlueprintId);
var targetEndpointName = EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, config.AgentBlueprintId);

oldEndpointName.Should().NotBe(targetEndpointName, "Step 1 and Step 1.5 must target different endpoints");

// Step 1: delete the currently-registered (old) endpoint
await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
oldEndpointName, "eastus", config.AgentBlueprintId);

// Step 1.5: pre-create cleanup of the partially-provisioned target endpoint
await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
targetEndpointName, "eastus", config.AgentBlueprintId);

// Total: exactly two delete calls
await _mockBotConfigurator.Received(2).DeleteEndpointWithAgentBlueprintAsync(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>());

// Step 2: register the new endpoint
await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,23 @@ public void GetEndpointNameFromUrl_WithNgrokUrl_ReturnsExpectedName()
result.Should().Be(expected);
}

[Fact]
public void GetEndpointNameFromUrl_DifferentDevTunnelUrls_ProduceDifferentNames()
{
// Regression: second --update-endpoint must delete the currently registered endpoint,
// not the original MessagingEndpoint. Different URLs must yield different names so
// the delete step targets the right Azure resource.
var originalUrl = "https://x23kz7ll-3979.inc1.devtunnels.ms/api/messages"; // MessagingEndpoint (static)
var updatedUrl = "https://w3jv62pr-3979.inc1.devtunnels.ms/api/messages"; // BotMessagingEndpoint (after first update)
var blueprintId = "e412c0fa-7127-4615-9bef-1a7ba99e76af";

var originalName = EndpointHelper.GetEndpointNameFromUrl(originalUrl, blueprintId);
var updatedName = EndpointHelper.GetEndpointNameFromUrl(updatedUrl, blueprintId);

updatedName.Should().NotBe(originalName, "deletion must target the currently registered URL, not the static MessagingEndpoint");
updatedName.Should().Be("w3jv62pr-3979-inc1-devtunnels-ms-e412c0fa");
}

// GetEndpointNameFromHost tests

[Fact]
Expand Down
Loading