fix(management): validate site gRPC addresses on create/update (review 2026-10-21 03/R-1)
This commit is contained in:
@@ -131,7 +131,7 @@ The whole folder surface is reachable from the CLI as `template folder list|crea
|
||||
### Sites
|
||||
|
||||
- **ListSites** / **GetSite**: Query site definitions.
|
||||
- **CreateSite** / **UpdateSite** / **DeleteSite**: Manage site definitions.
|
||||
- **CreateSite** / **UpdateSite** / **DeleteSite**: Manage site definitions. Create/Update validate `GrpcNodeAAddress` / `GrpcNodeBAddress` at the write boundary with `GrpcEndpointRules.TryValidate` (both optional; a non-empty value must be an absolute `http://` or `https://` URL with a host, e.g. `http://host:8083`) and reject a malformed value with a readable `COMMAND_FAILED` error naming the field — a scheme-less address used to be persisted and then throw inside central's channel build on the next site-address refresh (review 2026-10-21 03/R-1).
|
||||
- **ListAreas** / **CreateArea** / **UpdateArea** / **DeleteArea**: Manage area hierarchies per site.
|
||||
- **MoveArea** (`AreaId`, `NewParentAreaId?`): Re-parent an area within its own site; a null `NewParentAreaId` moves it to the site root. Delegates every rule to `AreaService.MoveAreaAsync` — rejects self-parent, descendant-parent (cycle), cross-site parent, and sibling name collision at the target level; the service writes the `"Move"` audit row. Same any-of `[Designer, Deployer]` gate as the other area mutations. CLI: `site area move`.
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Validation;
|
||||
using ZB.MOM.WW.ScadaBridge.DeploymentManager;
|
||||
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
|
||||
using ZB.MOM.WW.ScadaBridge.Communication;
|
||||
@@ -1885,8 +1886,23 @@ public class ManagementActor : ReceiveActor
|
||||
return site;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Rejects a malformed site gRPC address at the write boundary (review 2026-10-21
|
||||
/// 03/R-1). Both addresses are optional, but a non-empty value must be an absolute
|
||||
/// http(s) URL — anything else makes the central channel build throw inside
|
||||
/// <c>SitePairChannelProvider.UpdateSite</c> on the next address refresh.
|
||||
/// </summary>
|
||||
private static void ValidateSiteGrpcAddresses(string? grpcNodeAAddress, string? grpcNodeBAddress)
|
||||
{
|
||||
if (!GrpcEndpointRules.TryValidate(grpcNodeAAddress, allowEmpty: true, out var errorA))
|
||||
throw new ManagementCommandException($"GrpcNodeAAddress: {errorA}");
|
||||
if (!GrpcEndpointRules.TryValidate(grpcNodeBAddress, allowEmpty: true, out var errorB))
|
||||
throw new ManagementCommandException($"GrpcNodeBAddress: {errorB}");
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleCreateSite(IServiceProvider sp, CreateSiteCommand cmd, string user)
|
||||
{
|
||||
ValidateSiteGrpcAddresses(cmd.GrpcNodeAAddress, cmd.GrpcNodeBAddress);
|
||||
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||
var site = new Site(cmd.Name, cmd.SiteIdentifier)
|
||||
{
|
||||
@@ -1906,6 +1922,7 @@ public class ManagementActor : ReceiveActor
|
||||
|
||||
private static async Task<object?> HandleUpdateSite(IServiceProvider sp, UpdateSiteCommand cmd, string user)
|
||||
{
|
||||
ValidateSiteGrpcAddresses(cmd.GrpcNodeAAddress, cmd.GrpcNodeBAddress);
|
||||
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||
var site = await repo.GetSiteByIdAsync(cmd.SiteId)
|
||||
?? throw new ManagementCommandException($"Site with ID {cmd.SiteId} not found.");
|
||||
|
||||
+80
@@ -0,0 +1,80 @@
|
||||
using Akka.Actor;
|
||||
using Akka.TestKit.Xunit2;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NSubstitute;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Review 2026-10-21 03/R-1: a malformed site gRPC address used to be persisted
|
||||
/// unvalidated and then throw inside <c>SitePairChannelProvider.UpdateSite</c>.
|
||||
/// Create/Update must reject it at the write boundary with a readable error.
|
||||
/// </summary>
|
||||
public class SiteGrpcEndpointValidationTests : TestKit
|
||||
{
|
||||
private readonly ISiteRepository _siteRepo = Substitute.For<ISiteRepository>();
|
||||
private readonly ServiceCollection _services;
|
||||
|
||||
/// <summary>Initializes the harness.</summary>
|
||||
public SiteGrpcEndpointValidationTests()
|
||||
{
|
||||
_services = new ServiceCollection().AddTestConfigurationTransaction();
|
||||
_services.AddScoped(_ => _siteRepo);
|
||||
_services.AddScoped(_ => Substitute.For<IAuditService>());
|
||||
_siteRepo.GetSiteByIdAsync(7, Arg.Any<CancellationToken>())
|
||||
.Returns(new Site("S", "S1") { Id = 7 });
|
||||
}
|
||||
|
||||
private IActorRef CreateActor() =>
|
||||
Sys.ActorOf(Props.Create(() => new ManagementActor(
|
||||
_services.BuildServiceProvider(), NullLogger<ManagementActor>.Instance)));
|
||||
|
||||
private static ManagementEnvelope Admin(object command) =>
|
||||
new(new AuthenticatedUser("admin", "Admin", new[] { "Administrator" }, Array.Empty<string>()),
|
||||
command, Guid.NewGuid().ToString("N"));
|
||||
|
||||
/// <summary>Create with a scheme-less node-A address is rejected and nothing is saved.</summary>
|
||||
[Theory]
|
||||
[InlineData("scadabridge-site-a-node-a")]
|
||||
[InlineData("site-a-node-a:8083")]
|
||||
[InlineData("ftp://host:8083")]
|
||||
public void CreateSite_MalformedGrpcAddress_ReturnsReadableError(string bad)
|
||||
{
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Admin(new CreateSiteCommand("S", "S1", null, GrpcNodeAAddress: bad)));
|
||||
|
||||
var error = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
Assert.Contains("GrpcNodeAAddress", error.Error);
|
||||
Assert.Contains("http://", error.Error);
|
||||
_siteRepo.DidNotReceive().AddSiteAsync(Arg.Any<Site>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
/// <summary>Update with a malformed node-B address is rejected and nothing is saved.</summary>
|
||||
[Fact]
|
||||
public void UpdateSite_MalformedGrpcNodeB_ReturnsReadableError()
|
||||
{
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Admin(new UpdateSiteCommand(7, "S", null,
|
||||
GrpcNodeAAddress: "http://a:8083", GrpcNodeBAddress: "b:8083")));
|
||||
|
||||
var error = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
Assert.Contains("GrpcNodeBAddress", error.Error);
|
||||
_siteRepo.DidNotReceive().UpdateSiteAsync(Arg.Any<Site>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
/// <summary>Both gRPC addresses are optional; valid http(s) URLs (and blanks) are accepted.</summary>
|
||||
[Fact]
|
||||
public void CreateSite_ValidOrEmptyGrpcAddresses_Succeeds()
|
||||
{
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Admin(new CreateSiteCommand("S", "S1", null,
|
||||
GrpcNodeAAddress: "http://site-a-node-a:8083", GrpcNodeBAddress: null)));
|
||||
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user