Atomic Remediation with Exception Handling and Compliance Reporting

[ankit2995]

Note: The PR should be merged in dev/v3.x

Implement a robust remediation framework that ensures remediation actions are applied atomically, with comprehensive exception handling and detailed reporting. This approach minimizes partial updates, improves reliability, and provides clear visibility into remediation outcomes, failures, and compliance status.

fix: `fcli aviator ssc apply-remediations`: Atomic Remediation with Exception Handling and Compliance Reporting
fix: `fcli fod aviator apply-remediations`: Atomic Remediation with Exception Handling and Compliance Reporting

[rsenden]

Wrong indentation for this and various other method declarations

[rsenden]

To understand the exception handling in this method, developers need to check writeSourceFile to understand that this converts IOException to SkipRemediationException. Also, this doesn't roll back on any other exceptions (which are unlikely but not impossible), whereas other methods explicitly catch Exception.

[rsenden]

Given that this situation requires special processing (command termination instead of skipping single remediation), it would be better to use a dedicated exception type like RollbackRemediationException or a more generic TerminateRemediationProcessingException or something similar. This would avoid unintended command termination if any other code throws AviatorTechnicalException.

[rsenden]

See comment on applyRemediation method to have that method define flow and exception handling for applying individual remediations. Ultimate goal would be to have this method simply do the following for every individual remediation, without try/catch block:

  if (processRemediation(...)) {
    appliedRemediations++;
  }

[rsenden]

Ideally, to better document the overall process for applying an individual remediation, and to centralize exception handling, this method should effectively do something like the following:

private boolean processRemediation(...) {
  try {
    var pendingWrites = prepareFileChanges(...); // May throw RemediationSkipExceptions only; either explicitly or through generic try/catch to indicate prepare error. If this already reads source files, result object can include both original contents (for rollback if needed) and the changes to be applied, otherwise original file contents may be collected before updating each source file in commitChanges method.
    try {
      commitChanges(pendingWrites); // May throw RemediationCommitException only, listing any files that were already updated
      return true; // Indicate to caller that remediation was applied successfully
    } catch ( RemediationCommitException e ) {
      rollBackChanges(e.getRollbacks()); // Throws exception in case rollback was unsuccessful (ideally a dedicated exception type), causing command termination. RemediationCommitException::getRollbacks returns file paths and original contents for rollback.
    }
  } catch (RemediationSkipException e) {
    // Log warning, update skippedByReason
  }
  return false; // Indicate to caller that remediation was not applied successfully
}

Or alternatively, instead of boolean to report status, just propagate exceptions to the caller, i.e.:

private void processRemediation(...) {
  var pendingWrites = prepareFileChanges(...); // May throw RemediationSkipExceptions only; either explicitly or through generic try/catch to indicate prepare error. If this already reads source files, result object can include both original contents (for rollback if needed) and the changes to be applied, otherwise original file contents may be collected before updating each source file in commitChanges method.
  try {
    commitChanges(pendingWrites); // May throw RemediationCommitException only, listing any files that were already updated
  } catch ( RemediationCommitException e ) {
    rollBackChanges(e.getRollbacks()); // Throws exception in case rollback was unsuccessful (ideally a dedicated exception type), causing command termination. RemediationCommitException::getRollbacks returns file paths and original contents for rollback.
    throw new RemediationSkipException(...); // Indicate remediation was (successfully) rolled back due to errors
  }
}

With the latter, processRemediationsXML method would only catch RemediationSkipException to log warning & update skippedByReason, all other exceptions (like error during rollback) would result in command termination.

The above likely requires more work, main point is that it clearly encapsulates both processing (prepare, commit, rollback if needed) and exception handling (explicit skip reasons, technical errors, rollback errors, ...).

For some of these exception types, it may make sense to declare them as checked exceptions instead on RuntimeExceptions, as they are integral part of the overall workflow, and to explicitly document what exceptions may be thrown by each method.