diff --git a/Agent/Agent.Api/Controllers/DmnController.cs b/Agent/Agent.Api/Controllers/DmnController.cs index db19274a..b6cf5e8a 100644 --- a/Agent/Agent.Api/Controllers/DmnController.cs +++ b/Agent/Agent.Api/Controllers/DmnController.cs @@ -1,6 +1,7 @@ -using Agent.Api.Services; +using Agent.Api.Services; using Credentials.Models.Models.Zeebe; using Credentials.Models.Services; +using FiveSafesTes.Core.Constants; using FiveSafesTes.Core.Models; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -20,7 +21,7 @@ public class DmnController : ControllerBase private readonly IServicedZeebeClient _zeebeClient; private readonly ILogger _logger; private readonly DmnPath _DmnPath; - private readonly string path; + public DmnController( IDmnService dmnService, IServicedZeebeClient zeebeClient, @@ -31,33 +32,48 @@ public DmnController( _dmnService = dmnService; _zeebeClient = zeebeClient; _logger = logger; - _DmnPath = DmnPath; + } - // Get DMN file path from configuration or use default - var configuredPath = _DmnPath.Path; - + /// + /// The table a request is for. Omitting it gives the environment variables table, + /// so the endpoints answer exactly as they did before this controller could address + /// more than one. + /// + private DmnTable TableFor(string? table) + { + var resolved = DmnFiles.Resolve(table); - if (!string.IsNullOrEmpty(DmnPath.Path)) + if (!string.IsNullOrWhiteSpace(table) + && !string.Equals(resolved.Slug, table.Trim(), StringComparison.OrdinalIgnoreCase)) { - // Use configured path - make it absolute if relative - if (Path.IsPathRooted(DmnPath.Path)) - { - path = Path.Combine(DmnPath.Path, "credentials.dmn"); - } - else - { - var projectDirectory = Path.GetFullPath(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "..", "..", "..")); - path = Path.GetFullPath(Path.Combine(projectDirectory, DmnPath.Path, "credentials.dmn")); - } - } - else - { - var projectDirectory = Path.GetFullPath(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "..", "..", "..")); - path = Path.GetFullPath(Path.Combine(projectDirectory, "..", "..", "Credentials","Credentials.Models","ProcessModels", "credentials.dmn")); + _logger.LogWarning("Unknown DMN table '{Requested}', falling back to {Slug}", table, resolved.Slug); } + return resolved; + } + + /// + /// Where the editable copy of a table lives. On a deployed stack that is the + /// persistent volume, so admin edits survive a restart. + /// + private string PathFor(string? table) + { + var path = DmnFiles.ResolvePath(_DmnPath.Path, TableFor(table).FileName); _logger.LogInformation($"DMN file path resolved to: {path}"); + return path; + } + + /// + /// The tables this controller can manage, so the admin UI can offer them without + /// hardcoding the list. + /// + [HttpGet("tables")] + [SwaggerOperation(Summary = "List the DMN tables", Description = "The decision tables a TRE Admin can view and edit")] + [ProducesResponseType(typeof(IReadOnlyList), 200)] + public IActionResult GetTables() + { + return Ok(DmnFiles.Tables); } /// @@ -68,8 +84,10 @@ public DmnController( [SwaggerOperation(Summary = "Get DMN decision table", Description = "Retrieves the complete DMN decision table including all inputs, outputs, and rules")] [ProducesResponseType(typeof(DmnDecisionTable), 200)] [ProducesResponseType(400)] - public async Task GetDmnTable() + public async Task GetDmnTable([FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + try { var table = await _dmnService.LoadDmnTableAsync(path); @@ -90,8 +108,10 @@ public async Task GetDmnTable() [SwaggerOperation(Summary = "Get all DMN rules", Description = "Retrieves all rules from the DMN decision table")] [ProducesResponseType(typeof(List), 200)] [ProducesResponseType(400)] - public async Task GetRules() + public async Task GetRules([FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + try { var table = await _dmnService.LoadDmnTableAsync(path); @@ -113,8 +133,10 @@ public async Task GetRules() [SwaggerOperation(Summary = "Add new DMN rule", Description = "Creates a new rule in the DMN decision table")] [ProducesResponseType(typeof(DmnOperationResult), 200)] [ProducesResponseType(400)] - public async Task AddRule([FromBody] CreateDmnRuleRequest request) + public async Task AddRule([FromBody] CreateDmnRuleRequest request, [FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + // We use a temp file to prevent invalid rules from contaminating the real DMN file. // The rule must be added before validation because ValidateDmnAsync validates the // entire DMN file structure (not individual values). The real content validation @@ -179,8 +201,10 @@ public async Task AddRule([FromBody] CreateDmnRuleRequest request [SwaggerOperation(Summary = "Update DMN rule", Description = "Updates an existing rule in the DMN decision table")] [ProducesResponseType(typeof(DmnOperationResult), 200)] [ProducesResponseType(400)] - public async Task UpdateRule([FromBody] UpdateDmnRuleRequest request) + public async Task UpdateRule([FromBody] UpdateDmnRuleRequest request, [FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + var _tempPath = System.IO.Path.GetTempFileName() + ".dmn"; try { @@ -237,8 +261,10 @@ public async Task UpdateRule([FromBody] UpdateDmnRuleRequest requ [SwaggerOperation(Summary = "Delete DMN rule", Description = "Deletes a rule from the DMN decision table")] [ProducesResponseType(typeof(DmnOperationResult), 200)] [ProducesResponseType(400)] - public async Task DeleteRule(string ruleId) + public async Task DeleteRule(string ruleId, [FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + try { await _dmnService.DeleteRuleAsync(path, ruleId); @@ -274,8 +300,10 @@ public async Task DeleteRule(string ruleId) [SwaggerOperation(Summary = "Validate DMN", Description = "Validates the DMN file structure and rules")] [ProducesResponseType(typeof(DmnOperationResult), 200)] [ProducesResponseType(400)] - public async Task ValidateDmn() + public async Task ValidateDmn([FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + try { await _dmnService.ValidateDmnAsync(path); @@ -305,7 +333,7 @@ public async Task ValidateDmn() [SwaggerOperation(Summary = "Test DMN evaluation", Description = "Tests the DMN with provided input variables and returns matching rules")] [ProducesResponseType(typeof(DmnTestResponse), 200)] [ProducesResponseType(400)] - public async Task TestDmn([FromBody] DmnTestRequest request) + public async Task TestDmn([FromBody] DmnTestRequest request, [FromQuery(Name = "table")] string? dmnTable = null) { try { @@ -317,7 +345,7 @@ public async Task TestDmn([FromBody] DmnTestRequest request) // Create DMN request for Zeebe var dmnRequest = new DmnRequest { - DecisionId = "CredentialsDMN", + DecisionId = TableFor(dmnTable).DecisionId, Variables = request.InputVariables }; @@ -350,8 +378,10 @@ public async Task TestDmn([FromBody] DmnTestRequest request) [SwaggerOperation(Summary = "Deploy DMN to Zeebe", Description = "Deploys the DMN file to the Zeebe workflow engine")] [ProducesResponseType(typeof(DmnOperationResult), 200)] [ProducesResponseType(400)] - public async Task DeployDmn() + public async Task DeployDmn([FromQuery(Name = "table")] string? dmnTable = null) { + var path = PathFor(dmnTable); + try { await _dmnService.DeployDmnToZeebeAsync(path); diff --git a/Agent/Agent.Api/DoAgentWork.cs b/Agent/Agent.Api/DoAgentWork.cs index ceca943b..b1f79511 100644 --- a/Agent/Agent.Api/DoAgentWork.cs +++ b/Agent/Agent.Api/DoAgentWork.cs @@ -1,4 +1,4 @@ -using System.Diagnostics; +using System.Diagnostics; using System.Net; using System.Net.Http.Headers; using System.Text; @@ -59,6 +59,7 @@ public class DoAgentWork : IDoAgentWork private readonly IConfiguration _config; // Used to start Camunda process instances directly via Zeebe gRPC, replacing the previous HTTP calls to CredentialsController private readonly Credentials.Models.Services.IServicedZeebeClient _zeebeClient; + private readonly IImageCodeResolver _imageCodeResolver; private readonly IOptionsMonitor _onboardingConfig; @@ -81,6 +82,7 @@ public DoAgentWork(IServiceProvider serviceProvider, IVaultCredentialsService vaultService, IConfiguration config, Credentials.Models.Services.IServicedZeebeClient zeebeClient, + IImageCodeResolver imageCodeResolver, IOptionsMonitor configSettings ) { @@ -113,6 +115,7 @@ IOptionsMonitor configSettings _vaultService = vaultService; _config = config; _zeebeClient = zeebeClient; + _imageCodeResolver = imageCodeResolver; _onboardingConfig = configSettings; } @@ -488,10 +491,17 @@ public async Task Execute() { foreach (var cancelsubproj in cancelsubprojs) { + // Carry whatever reason put the submission into RequestCancellation + // through to the terminal status. Passing an empty description here + // overwrote it, so a credential failure ended up as a bare "Cancelled" + // with nothing to explain it. + var cancelReason = cancelsubproj.StatusDescription ?? string.Empty; + _subHelper.UpdateStatusForTre(cancelsubproj.Id.ToString(), StatusType.CancellationRequestSent, - ""); + cancelReason); //TODO Do we need to call Hutch or other stuff to cancel and do other cancel stuff - _subHelper.CloseSubmissionForTre(cancelsubproj.Id.ToString(), StatusType.Cancelled, "", ""); + _subHelper.CloseSubmissionForTre(cancelsubproj.Id.ToString(), StatusType.Cancelled, + cancelReason, ""); } } @@ -547,8 +557,7 @@ public async Task Execute() StatusType.AgentTransferringToPod, ""); } - Dictionary> credentials = - new Dictionary>(); + SubmissionCredentials credentials = new SubmissionCredentials(); if (await _features.IsEnabledAsync(FeatureFlags.EphemeralCredentials)) { @@ -603,7 +612,8 @@ await TriggerStartCredentialsAsync(aSubmission.Id, project, aSubmission.SubmittedBy.Id, treProjectForCreds?.SubmissionBucketTre, treProjectForCreds?.OutputBucketTre, - _config["MinioTRESettings:Url"]); + _config["MinioTRESettings:Url"], + FindSubmissionImageCodes(aSubmission)); Log.Information("Triggered credentials for submission {SubId}", aSubmission.Id); } catch (Exception ex) @@ -667,14 +677,45 @@ await TriggerStartCredentialsAsync(aSubmission.Id, project, if (anyErrored) { - Log.Error("Credential process errored for submission {SubId}.", aSubmission.Id); + // Name the handler that failed and why. "Credential process + // failed" on its own leaves a researcher, and whoever they + // ask, with nothing to go on. + var failures = credsRowforParentKey + .Where(c => c.SuccessStatus == SuccessStatus.Error + && !string.IsNullOrWhiteSpace(c.ErrorMessage)) + .Select(c => $"{c.CredentialType}: {c.ErrorMessage}") + .Distinct() + .ToList(); + + var reason = failures.Count > 0 + ? "Credential process failed - " + string.Join("; ", failures) + : "Credential process failed"; + + Log.Error("{Reason} for submission {SubId}.", reason, aSubmission.Id); var latestRow = creds.First(); - latestRow.ErrorMessage = "Credential process failed"; + latestRow.ErrorMessage = reason; await _credsDbContext.SaveChangesAsync(); + // Handlers run in sequence, so an failure part-way leaves the + // earlier ones' accounts and vault paths behind. Nothing else + // collects them: there is no credential sweeper, nothing scans + // for unexpired rows, and Vault has no expiry on the KV mount. + // Revoke immediately rather than after the usual delay. + try + { + await TriggerRevokeCredentialsAsync(aSubmission.Id, + aSubmission.Project.Name, aSubmission.SubmittedBy.Id, 0); + } + catch (Exception ex) + { + Log.Error(ex, + "Failed to revoke credentials after a failed credential process for submission {SubId}", + aSubmission.Id); + } + _subHelper.UpdateStatusForTre(aSubmission.Id.ToString(), - StatusType.RequestCancellation, "Credential process failed"); + StatusType.RequestCancellation, reason); continue; } @@ -913,6 +954,8 @@ await _minioTreHelper.CopyObjectToDestination(InputBucket, NewCleanedInput, Log.Information("looking for _AgentSettings.ImageNameToAddToToken > " + _AgentSettings.ImageNameToAddToToken); + var unresolvedImageCodes = new List(); + foreach (var Executor in tesMessage.Executors) { if (Executor.Env == null) @@ -920,6 +963,23 @@ await _minioTreHelper.CopyObjectToDestination(InputBucket, NewCleanedInput, Executor.Env = new Dictionary(); } + // Resolve {{@imageCode}} first: everything below reads the image + // field, and it needs to see the real container image. + var imageResolution = await _imageCodeResolver.ResolveExecutorImageAsync( + Executor.Image, aSubmission.Project.Name, + aSubmission.SubmittedBy.Id.ToString()); + + Executor.Image = imageResolution.Image; + var executorImageCodes = imageResolution.ImageCodes; + + foreach (var unresolved in imageResolution.Unresolved) + { + if (!unresolvedImageCodes.Contains(unresolved, StringComparer.Ordinal)) + { + unresolvedImageCodes.Add(unresolved); + } + } + Executor.Env["SCHEMA"] = aSubmission.Project.Name; Executor.Env["CATALOG"] = _AgentSettings.CATALOG; @@ -944,22 +1004,15 @@ await _minioTreHelper.CopyObjectToDestination(InputBucket, NewCleanedInput, Log.Information( $"Injecteing credentials into environment variables for {aSubmission.Id} nub > {credentials.Count}"); - if (credentials != null && credentials.Count > 0) + if (credentials != null && credentials.Any) { - foreach (var outerKey in credentials) + // Shared variables go to every executor; ones scoped to an + // image code only to the executors running that image. + foreach (var credential in credentials.ForImageCodes(executorImageCodes)) { - if (outerKey.Value is IDictionary - innerDict) //The format is dictionary within a dictionary - { - foreach (var inner in innerDict) - { - var key = inner.Key; - var value = inner.Value?.ToString() ?? string.Empty; - Log.Information("Injected credentials with Key " + key); - - Executor.Env[key] = value; - } - } + Log.Information("Injected credentials with Key " + credential.Key); + + Executor.Env[credential.Key] = credential.Value; } Log.Information( @@ -971,11 +1024,11 @@ await _minioTreHelper.CopyObjectToDestination(InputBucket, NewCleanedInput, // the workload container picks them up without bespoke wiring. The // generic loop above still injects the raw accessKey/secretKey/ // endPoint/bucket keys for tools that read those directly. - if (credentials != null && - credentials.TryGetValue("s3", out var s3Creds) && s3Creds != null) + var s3Creds = credentials?.OfType("s3"); + if (s3Creds != null && s3Creds.Count > 0) { string S3Val(string k) => - s3Creds.TryGetValue(k, out var v) ? v?.ToString() ?? string.Empty + s3Creds.TryGetValue(k, out var v) ? v ?? string.Empty : string.Empty; var s3AccessKey = S3Val("accessKey"); @@ -1045,11 +1098,31 @@ string S3Val(string k) => _dbContext.SaveChanges(); - if (tesMessage is not null) + if (unresolvedImageCodes.Count > 0) + { + // One unresolvable image code fails the whole job rather than + // dispatching a message that still carries a literal placeholder + // in an image field, which would fail obscurely at pull time. + var reason = + $"Could not resolve image code(s): {string.Join(", ", unresolvedImageCodes)}"; + + Log.Error("{Function} {Reason} for sub {SubId}", "Execute", reason, + aSubmission.Id); + + _subHelper.UpdateStatusForTre(aSubmission.Id.ToString(), StatusType.Failed, + reason); + + processedOK = false; + } + else if (tesMessage is not null) { var stringdata = JsonConvert.SerializeObject(tesMessage); - Log.Information("{Function} tesMessage is not null runhing CreateTESK {tesMessage}", - "Execute", stringdata); + + // Only the shape of the message, never its contents: by this point + // the executor env holds ephemeral credentials and any secrets + // resolved from vault, and these logs ship to Seq. + Log.Information("{Function} sending to CreateTESK {TesMessage}", + "Execute", DescribeTesMessageForLog(tesMessage)); CreateTesk(stringdata, aSubmission.Id, aSubmission.Project.Id, aSubmission.SubmittedBy.Id, aSubmission.TesId, OutputBucket, @@ -1099,11 +1172,45 @@ public void ClearJob(string jobname) } } + /// + /// The image codes across all of a submission's executors, read from the TES message + /// before any of them are resolved. Null when the message cannot be read, which leaves + /// the tre credential handler checking every image code's rows as it did before. + /// + private IReadOnlyList? FindSubmissionImageCodes(Submission submission) + { + try + { + if (string.IsNullOrWhiteSpace(submission.TesJson)) + { + return null; + } + + var tesTask = JsonConvert.DeserializeObject(submission.TesJson); + + if (tesTask is null) + { + return null; + } + + return (tesTask.Executors ?? new List()) + .SelectMany(executor => _imageCodeResolver.FindImageCodes(executor.Image)) + .Distinct(StringComparer.Ordinal) + .ToList(); + } + catch (Exception ex) + { + Log.Warning(ex, "Could not read the image codes for submission {SubId}", submission.Id); + return null; + } + } + // Starts the Start_Credentials Camunda process for the given submission. // InputCollections mirrors the top-level variables as a list because the BPMN multi-instance // subprocess iterates over InputCollections to fan out credential creation per item. private async Task TriggerStartCredentialsAsync(int submissionId, string projectName, int userId, - string? submissionBucket = null, string? outputBucket = null, string? endPoint = null) + string? submissionBucket = null, string? outputBucket = null, string? endPoint = null, + IReadOnlyList? imageCodes = null) { var variables = new Dictionary { @@ -1124,17 +1231,26 @@ private async Task TriggerStartCredentialsAsync(int submissionId, string project } }; + // The credentials DMN has no image code input, so it returns the rows for every + // image the TRE has set up. This lets the tre handler drop the ones this submission + // does not run, so a broken row for one image cannot fail every submission. + if (imageCodes != null) + { + variables["imageCodes"] = imageCodes; + } + await _zeebeClient.CreateProcessInstanceAsync("Start_Credentials", variables); Log.Information("Camunda StartCredentials triggered successfully for submission {SubmissionId}", submissionId); } - private async Task>> WaitForAndFetchCredentialsAsync( + private async Task WaitForAndFetchCredentialsAsync( int submissionId, TimeSpan? timeout = null) { var maxWaitTime = timeout ?? TimeSpan.FromMinutes(5); var pollInterval = TimeSpan.FromSeconds(5); //Reduced polling interval for faster fetch - var fetchedCredentials = new Dictionary>(); + var fetchedCredentials = new SubmissionCredentials(); + var fetchedPaths = new HashSet(StringComparer.Ordinal); var stopwatch = System.Diagnostics.Stopwatch.StartNew(); Log.Information($"Starting to wait for credentials for submission {submissionId}."); @@ -1150,21 +1266,46 @@ private async Task>> WaitForAndFet foreach (var record in credentialRecord) { - if (!fetchedCredentials.ContainsKey(record.CredentialType) && - !string.IsNullOrEmpty(record.VaultPath)) + // Keyed on the vault path, not the credential type. A credential type + // can own several paths - the tre handler writes one per image code - + // and keying on the type would read the first and silently drop the rest. + if (string.IsNullOrEmpty(record.VaultPath) || !fetchedPaths.Add(record.VaultPath)) { - Log.Information( - $"Found {record.CredentialType} credentials for submission {submissionId} at vault path: {record.VaultPath}"); + continue; + } + Log.Information( + $"Found {record.CredentialType} credentials for submission {submissionId} at vault path: {record.VaultPath}"); + + // Scoped to the one path. A read that throws has to put the path back + // the same way an empty read does, or it stays in fetchedPaths, every + // later poll skips it, and the wait spins to its timeout. Catching here + // also keeps one bad path from abandoning the rest of this pass. + try + { var credentials = await _vaultService.GetCredentialAsync(record.VaultPath); if (credentials != null && credentials.Count > 0) { - fetchedCredentials[record.CredentialType] = credentials; + AddToSubmissionCredentials(fetchedCredentials, record.CredentialType, + record.VaultPath, credentials); record.IsProcessed = true; Log.Information( $"Successfully fetched {record.CredentialType} credentials for submission {submissionId}"); } + else + { + // Nothing read, so allow a later poll to try this path again. + fetchedPaths.Remove(record.VaultPath); + } + } + catch (Exception ex) + { + fetchedPaths.Remove(record.VaultPath); + + Log.Error(ex, + "Error reading vault path {VaultPath} for submission {SubmissionId}; will retry", + record.VaultPath, submissionId); } } @@ -1173,12 +1314,26 @@ private async Task>> WaitForAndFet await _credsDbContext.SaveChangesAsync(); } - if (fetchedCredentials.Count > 0) + // Only done once every path this pass saw has actually been read. Returning + // on the first success would leave a path that failed to read unretried, and + // a missing image-scoped path means one executor silently runs without its + // variables. By this point the credentials process has completed, so every + // row already exists and a short wait is better than a quiet gap. + var unread = credentialRecord.Count(r => !string.IsNullOrEmpty(r.VaultPath) && !r.IsProcessed); + + if (fetchedCredentials.Any && unread == 0) { Log.Information($"Successfully fetched all credentials for submission {submissionId}"); return fetchedCredentials; } + if (unread > 0) + { + Log.Warning( + "{Unread} vault path(s) for submission {SubmissionId} could not be read yet; retrying", + unread, submissionId); + } + await Task.Delay(pollInterval); } catch (Exception ex) @@ -1193,6 +1348,76 @@ private async Task>> WaitForAndFet throw new TimeoutException(errorMsg); } + /// + /// A description of a TES message safe to log: its shape, its images, and the names + /// of the environment variables, but none of their values. + /// + /// The executor env holds ephemeral credentials and secrets resolved from vault by + /// the time the message is dispatched, and these logs are shipped to Seq, so the + /// message itself must never be serialised into them. + /// + private static string DescribeTesMessageForLog(TesTask tesMessage) + { + var executors = tesMessage.Executors ?? new List(); + + var described = executors.Select((executor, index) => + { + var names = executor.Env is null || executor.Env.Count == 0 + ? "none" + : string.Join(", ", executor.Env.Keys.OrderBy(name => name, StringComparer.Ordinal)); + + return $"[{index}] image={executor.Image} env={{{names}}}"; + }); + + return $"id={tesMessage.Id} name={tesMessage.Name} executors={executors.Count} " + + string.Join(" ", described); + } + + /// + /// Files one vault path's variables as shared or as belonging to an image code. + /// + /// Every handler builds its path as {tag}/{user}/{submissionId}/{project}, and the + /// tre handler appends the image code when a variable is scoped to one. So a fifth + /// segment means image-scoped, and is the code itself. + /// + private static void AddToSubmissionCredentials(SubmissionCredentials destination, + string? credentialType, string vaultPath, Dictionary values) + { + var segments = vaultPath.Split('/', StringSplitOptions.RemoveEmptyEntries); + var imageCode = segments.Length > 4 ? segments[^1] : null; + + // Fall back to the first path segment, which is where the credential type comes + // from when the handler builds the path. + var type = string.IsNullOrWhiteSpace(credentialType) + ? (segments.Length > 0 ? segments[0] : "unknown") + : credentialType; + + var target = SelectGroup(destination, type, imageCode); + + foreach (var value in values) + { + target[value.Key] = value.Value?.ToString() ?? string.Empty; + } + } + + private static Dictionary SelectGroup(SubmissionCredentials destination, + string credentialType, string? imageCode) + { + var groups = string.IsNullOrEmpty(imageCode) + ? destination.SharedByType + : destination.ByImageCode; + + var key = string.IsNullOrEmpty(imageCode) ? credentialType : imageCode; + + if (!groups.TryGetValue(key, out var group)) + { + group = new Dictionary(StringComparer.Ordinal); + groups[key] = group; + } + + return group; + } + // Starts the Credentials_Revoke Camunda process to revoke credentials for the given submission. // timer controls how long (seconds) the revoke process waits before expiring credentials. private async Task TriggerRevokeCredentialsAsync(int submissionId, string projectName, int user, int timer) diff --git a/Agent/Agent.Api/Models/SubmissionCredentials.cs b/Agent/Agent.Api/Models/SubmissionCredentials.cs new file mode 100644 index 00000000..4767daa6 --- /dev/null +++ b/Agent/Agent.Api/Models/SubmissionCredentials.cs @@ -0,0 +1,68 @@ +namespace Agent.Api.Models +{ + /// + /// The environment variables fetched from Vault for one submission, split by who gets them. + /// + /// The credentials process stores a variable either at the base path for its credential + /// type, meaning it applies to every executor, or one level deeper under an image code, + /// meaning it applies only to executors running that image. + /// + public class SubmissionCredentials + { + /// + /// Variables every executor receives, kept grouped by credential type (postgres, + /// trino, s3, tre). The grouping matters because callers look a type up by name - + /// the S3 block reads accessKey and secretKey out of the "s3" group specifically + /// rather than trusting those names to be unique across every type. + /// + public Dictionary> SharedByType { get; } = + new(StringComparer.OrdinalIgnoreCase); + + /// Variables keyed by the image code they belong to. + public Dictionary> ByImageCode { get; } = + new(StringComparer.OrdinalIgnoreCase); + + public bool Any => SharedByType.Count > 0 || ByImageCode.Count > 0; + + /// Total variables across every group, for logging. Never log the values. + public int Count => SharedByType.Values.Sum(group => group.Count) + + ByImageCode.Values.Sum(group => group.Count); + + /// The variables of one credential type, or an empty set if there are none. + public IReadOnlyDictionary OfType(string credentialType) + { + return SharedByType.TryGetValue(credentialType, out var group) + ? group + : new Dictionary(StringComparer.Ordinal); + } + + /// + /// The variables one executor should receive: the shared ones, plus any belonging to + /// the image codes it uses. An image code with no variables of its own is not an + /// error - it just means nothing in the DMN was scoped to it. + /// + public IEnumerable> ForImageCodes(IReadOnlyList imageCodes) + { + foreach (var group in SharedByType.Values) + { + foreach (var pair in group) + { + yield return pair; + } + } + + foreach (var imageCode in imageCodes) + { + if (!ByImageCode.TryGetValue(imageCode, out var scoped)) + { + continue; + } + + foreach (var pair in scoped) + { + yield return pair; + } + } + } + } +} diff --git a/Agent/Agent.Api/Program.cs b/Agent/Agent.Api/Program.cs index af6c5442..1d63de16 100644 --- a/Agent/Agent.Api/Program.cs +++ b/Agent/Agent.Api/Program.cs @@ -186,6 +186,7 @@ await SetUpRabbitMQ.DoItTreAsync(configuration["RabbitMQ:HostAddress"], configur builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddScoped(); var TVP = new TokenValidationParameters { diff --git a/Agent/Agent.Api/Services/ImageCodeResolver.cs b/Agent/Agent.Api/Services/ImageCodeResolver.cs new file mode 100644 index 00000000..73395efd --- /dev/null +++ b/Agent/Agent.Api/Services/ImageCodeResolver.cs @@ -0,0 +1,226 @@ +using System.Text.Json; +using System.Text.RegularExpressions; +using Credentials.Models.Models.Zeebe; +using Credentials.Models.Services; +using FiveSafesTes.Core.Constants; + +namespace Agent.Api.Services +{ + public interface IImageCodeResolver + { + /// + /// The image codes written as {{@code}} in an executor's image field. Usually one, + /// and empty when the researcher gave a literal image name. + /// + IReadOnlyList FindImageCodes(string? image); + + /// + /// Asks the image code DMN what an image code resolves to for this project and user. + /// Returns null when no rule matches, which fails the job rather than running + /// something unintended. + /// + Task ResolveImageAsync(string project, string user, string imageCode, + CancellationToken cancellationToken = default); + + /// + /// Resolves every image code in one executor's image field and substitutes the + /// results, reporting any that could not be resolved. + /// + Task ResolveExecutorImageAsync(string? image, string project, string user, + CancellationToken cancellationToken = default); + } + + /// + /// The outcome of resolving one executor's image field. + /// + public class ImageCodeResolution + { + public ImageCodeResolution(string? image, IReadOnlyList imageCodes, IReadOnlyList unresolved) + { + Image = image; + ImageCodes = imageCodes; + Unresolved = unresolved; + } + + /// The image field with every resolvable code substituted. + public string? Image { get; } + + /// The codes found, whether or not they resolved. Used to pick this executor's variables. + public IReadOnlyList ImageCodes { get; } + + /// Codes with no matching DMN rule. Any entry here fails the whole job. + public IReadOnlyList Unresolved { get; } + + public bool Success => Unresolved.Count == 0; + } + + /// + /// Turns the {{@imageCode}} placeholder in an executor's image field into the container + /// image to actually run, by asking the image code DMN. + /// + /// This is a plain decision lookup rather than a Camunda process: image codes vary per + /// executor, while the credentials process runs once per submission, so there is nothing + /// to orchestrate and nothing to store. + /// + public class ImageCodeResolver : IImageCodeResolver + { + // {{@code}} - the @ is part of the code, and is what distinguishes an image code + // from the general {{code}} syntax that is parked. + private static readonly Regex ImageCodePattern = + new(@"\{\{\s*(@[^{}\s]+)\s*\}\}", RegexOptions.Compiled | RegexOptions.CultureInvariant); + + private readonly IServicedZeebeClient _zeebeClient; + private readonly ILogger _logger; + + public ImageCodeResolver(IServicedZeebeClient zeebeClient, ILogger logger) + { + _zeebeClient = zeebeClient; + _logger = logger; + } + + public IReadOnlyList FindImageCodes(string? image) + { + if (string.IsNullOrWhiteSpace(image)) + { + return Array.Empty(); + } + + return ImageCodePattern.Matches(image) + .Select(match => match.Groups[1].Value.Trim().ToLowerInvariant()) + .Where(code => code.Length > 1) + .Distinct(StringComparer.Ordinal) + .ToList(); + } + + public async Task ResolveImageAsync(string project, string user, string imageCode, + CancellationToken cancellationToken = default) + { + try + { + var response = await _zeebeClient.EvaluateDecisionModelAsync(new DmnRequest + { + DecisionId = DmnFiles.ImageCodesDecisionId, + Variables = new Dictionary + { + ["project"] = project, + ["user"] = user, + ["imageCode"] = imageCode + } + }); + + var image = ReadImage(response); + + if (string.IsNullOrWhiteSpace(image)) + { + _logger.LogError( + "The image code DMN returned no image for {ImageCode} (project {Project}, user {User})", + imageCode, project, user); + + return null; + } + + _logger.LogInformation("Image code {ImageCode} resolved to {Image}", imageCode, image); + return image; + } + catch (Exception exception) + { + _logger.LogError(exception, + "Failed to evaluate the image code DMN for {ImageCode} (project {Project})", + imageCode, project); + + return null; + } + } + + public async Task ResolveExecutorImageAsync(string? image, string project, string user, + CancellationToken cancellationToken = default) + { + var imageCodes = FindImageCodes(image); + + if (imageCodes.Count == 0) + { + // A literal image name. Nothing to do. + return new ImageCodeResolution(image, imageCodes, Array.Empty()); + } + + var resolvedByCode = new Dictionary(StringComparer.Ordinal); + var unresolved = new List(); + + foreach (var imageCode in imageCodes) + { + var resolved = await ResolveImageAsync(project, user, imageCode, cancellationToken); + + if (string.IsNullOrWhiteSpace(resolved)) + { + unresolved.Add(imageCode); + continue; + } + + resolvedByCode[imageCode] = resolved; + } + + // Substituted through the same pattern that found them, so the replacement matches + // however the researcher cased or spaced the placeholder. + var substituted = ImageCodePattern.Replace(image!, match => + { + var code = match.Groups[1].Value.Trim().ToLowerInvariant(); + return resolvedByCode.TryGetValue(code, out var value) ? value : match.Value; + }); + + return new ImageCodeResolution(substituted, imageCodes, unresolved); + } + + /// + /// Pulls the image out of the decision result. + /// + /// ServicedZeebeClient reshapes what Zeebe returns: a decision with a single output + /// column gives a bare value, which it wraps as { "result": value }, while a + /// multi-output decision keeps its real column names. The image code table has one + /// output today, so "result" is the expected shape, but both are handled so a future + /// column on that table cannot quietly break resolution. + /// + private static string? ReadImage(DmnResponse response) + { + if (response.Result is null || response.Result.Count == 0) + { + return null; + } + + foreach (var key in new[] { "image", "result" }) + { + if (response.Result.TryGetValue(key, out var value)) + { + var text = AsText(value); + if (!string.IsNullOrWhiteSpace(text)) + { + return text; + } + } + } + + // Single unnamed value under some other key. + return response.Result.Count == 1 ? AsText(response.Result.Values.First()) : null; + } + + private static string? AsText(object? value) + { + if (value is null) + { + return null; + } + + // Dictionary from System.Text.Json holds JsonElement values. + if (value is JsonElement element) + { + return element.ValueKind switch + { + JsonValueKind.String => element.GetString(), + JsonValueKind.Null or JsonValueKind.Undefined => null, + _ => element.ToString() + }; + } + + return value.ToString(); + } + } +} diff --git a/Agent/Agent.Web/Controllers/DmnController.cs b/Agent/Agent.Web/Controllers/DmnController.cs index ae819ede..1de7c6df 100644 --- a/Agent/Agent.Web/Controllers/DmnController.cs +++ b/Agent/Agent.Web/Controllers/DmnController.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using FiveSafesTes.Core.Constants; using FiveSafesTes.Core.Models; using FiveSafesTes.Core.Services; using Microsoft.AspNetCore.Authorization; @@ -47,118 +49,187 @@ public IActionResult Index() #region API Proxy Methods + /// + /// The part of a failed API call worth showing an admin. + /// + /// BaseClientHelper puts the whole API response inside its exception message, so the + /// useful text - a FEEL parse error, say - ends up wrapped in two layers of JSON. + /// Rules are written by hand, so these errors are read often and need to be legible. + /// + private static string Readable(Exception exception) + { + var message = exception.Message; + var start = message.IndexOf('{'); + + if (start < 0) + { + return message; + } + + try + { + using var document = JsonDocument.Parse(message[start..]); + + if (document.RootElement.TryGetProperty("message", out var inner)) + { + var text = inner.GetString(); + if (!string.IsNullOrWhiteSpace(text)) + { + return text; + } + } + } + catch (JsonException) + { + // Not the shape we expected; the raw message is better than nothing. + } + + return message; + } + + /// + /// Carries the chosen table through to the API as a query parameter. Null means the + /// API picks its default, which is the environment variables table. + /// + private static Dictionary? TableParams(string? table) + { + return string.IsNullOrWhiteSpace(table) + ? null + : new Dictionary { ["table"] = table }; + } + + /// + /// The DMN tables a TRE Admin can manage, so the page does not hardcode the list. + /// + [HttpGet] + [Route("Dmn/GetTables")] + public async Task GetTables() + { + try + { + var result = await _clientHelper.CallAPIWithoutModel>("/api/Dmn/tables"); + return Json(result); + } + catch (Exception ex) + { + _logger.LogError(ex, "Error retrieving the list of DMN tables"); + return Json(new { success = false, message = Readable(ex) }); + } + } + + [HttpGet] [Route("Dmn/GetTable")] - public async Task GetTable() + public async Task GetTable(string? table = null) { try { - var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/table"); + var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/table", TableParams(table)); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error retrieving DMN table"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpGet] [Route("Dmn/GetRules")] - public async Task GetRules() + public async Task GetRules(string? table = null) { try { - var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/rules"); + var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/rules", TableParams(table)); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error retrieving DMN rules"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpPost] [Route("Dmn/AddRule")] - public async Task AddRule([FromBody] CreateDmnRuleRequest request) + public async Task AddRule([FromBody] CreateDmnRuleRequest request, string? table = null) { try { - var result = await _clientHelper.CallAPI("/api/Dmn/rules", request); + var result = await _clientHelper.CallAPI("/api/Dmn/rules", request, TableParams(table)); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error adding DMN rule"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpPut] [Route("Dmn/UpdateRule")] - public async Task UpdateRule([FromBody] UpdateDmnRuleRequest request) + public async Task UpdateRule([FromBody] UpdateDmnRuleRequest request, string? table = null) { try { - var result = await _clientHelper.CallAPI("/api/Dmn/rules", request, usePut: true); + var result = await _clientHelper.CallAPI("/api/Dmn/rules", request, TableParams(table), usePut: true); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error updating DMN rule"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpDelete] [Route("Dmn/DeleteRule/{ruleId}")] - public async Task DeleteRule(string ruleId) + public async Task DeleteRule(string ruleId, string? table = null) { try { - var result = await _clientHelper.CallAPIDelete($"/api/Dmn/rules/{ruleId}"); + var result = await _clientHelper.CallAPIDelete($"/api/Dmn/rules/{ruleId}", TableParams(table)); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error deleting DMN rule"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpGet] [Route("Dmn/ValidateDmn")] - public async Task ValidateDmn() + public async Task ValidateDmn(string? table = null) { try { - var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/validate"); + var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/validate", TableParams(table)); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error validating DMN"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } [HttpPost] [Route("Dmn/DeployDmn")] - public async Task DeployDmn() + public async Task DeployDmn(string? table = null) { try { - var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/deploy", httpMethod: HttpMethod.Post); + var result = await _clientHelper.CallAPIWithoutModel("/api/Dmn/deploy", TableParams(table), httpMethod: HttpMethod.Post); return Json(result); } catch (Exception ex) { _logger.LogError(ex, "Error deploying DMN to Zeebe"); - return BadRequest(new { success = false, message = ex.Message }); + return BadRequest(new { success = false, message = Readable(ex) }); } } diff --git a/Agent/Agent.Web/Views/Dmn/Index.cshtml b/Agent/Agent.Web/Views/Dmn/Index.cshtml index 9223ad32..7808bfc9 100644 --- a/Agent/Agent.Web/Views/Dmn/Index.cshtml +++ b/Agent/Agent.Web/Views/Dmn/Index.cshtml @@ -13,6 +13,19 @@
+ +
+ +
+ +
+
+ +
+
+
@section Scripts { - + } diff --git a/Agent/Agent.Web/wwwroot/js/dmn-manager.js b/Agent/Agent.Web/wwwroot/js/dmn-manager.js index fb3893ab..6c601a39 100644 --- a/Agent/Agent.Web/wwwroot/js/dmn-manager.js +++ b/Agent/Agent.Web/wwwroot/js/dmn-manager.js @@ -1,11 +1,23 @@ -let dmnTable = null; +let dmnTable = null; let dataTable = null; +let currentTable = null; + +/** + * Adds the chosen decision table to a URL. Leaving it off makes the API fall back + * to the environment variables table, which is what this page used to edit. + */ +function withTable(url) { + if (!currentTable) { + return url; + } + return url + (url.includes('?') ? '&' : '?') + 'table=' + encodeURIComponent(currentTable); +} // Initialize on page load $(document).ready(function () { console.log('DMN Manager initialized'); - loadDmnTable(); + loadTables(); // Event Listeners $('#addRuleBtn').on('click', showAddRuleModal); @@ -14,8 +26,108 @@ $(document).ready(function () { $('#deployBtn').on('click', deployDmn); $('#saveRuleBtn').on('click', saveRule); $('#confirmDeleteBtn').on('click', deleteRule); + + $('#tableSelect').on('change', function () { + currentTable = $(this).val(); + updateTableHelp(); + loadDmnTable(); + }); }); +/** + * Turns a failed request into something an admin can act on. + * + * Always returns text: a bare "Error saving rule:" with nothing after it hides the + * reason completely, and has already concealed both an expired session and a FEEL + * syntax error. + */ +function describeAjaxError(xhr, error) { + if (xhr.status === 401 || xhr.status === 403) { + return 'your session may have expired. Please refresh the page and sign in again.'; + } + + if (xhr.status === 0) { + return 'no response from the server. It may still be starting up.'; + } + + return xhr.responseJSON?.message + || error + || (xhr.status ? xhr.status + ' ' + xhr.statusText : 'unknown error'); +} + +/** + * Removes the DataTable instance and the rows it was managing, leaving a plain table + * for the next render to rebuild. Safe to call when there is nothing to destroy. + */ +function destroyDataTable() { + if (!dataTable) { + return; + } + + console.log('[INFO] Destroying existing DataTable'); + dataTable.destroy(); + dataTable = null; + + // destroy() puts back the rows it was given, which belong to the table being + // replaced. Clearing them stops a stale row count surviving the switch. + $('#rulesTable tbody').empty(); +} + +/** + * Load the decision tables this admin can edit, then show the first one. + * The list comes from the API so adding a table does not mean editing this page. + */ +function loadTables() { + $.ajax({ + url: '/Dmn/GetTables', + method: 'GET', + success: function (tables) { + const select = $('#tableSelect'); + select.empty(); + + if (!tables || !tables.length) { + console.warn('[WARN] No decision tables returned; falling back to the default'); + loadDmnTable(); + return; + } + + tables.forEach(function (table) { + select.append( + $('