feat(toolkit-lib): diagnose hook failures in the central stack diagnoser

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/diagnose/cdk-diagnose-guard-hook-failure-displays-hook-details.integtest.ts

@@ -0,0 +1,32 @@
+import { integTest, withSpecificFixture } from '../../../lib';
+
+jest.setTimeout(2 * 60 * 60_000);
+
+integTest(
+  'cdk diagnose after guard hook failure displays hook failure details',
+  withSpecificFixture('guard-hook-app', async (fixture) => {
+    // Deploy the setup stack which creates the Guard Hook via CloudFormation
+    await fixture.cdkDeploy('guard-hook-setup');
+
+    // Attempt to deploy the non-compliant stack; it fails due to the Guard Hook.
+    // --no-rollback leaves the stack in CREATE_FAILED so diagnose can inspect it.
+    const deployOutput = await fixture.cdkDeploy('guard-hook-test', {
+      options: ['--no-rollback'],
+      allowErrExit: true,
+    });
+    expect(deployOutput).toContain('CREATE_FAILED');
+
+    // Run cdk diagnose on the failed stack
+    const diagnoseOutput = await fixture.cdk(
+      ['--unstable=diagnose', 'diagnose', fixture.fullStackName('guard-hook-test')],
+      { allowErrExit: true },
+    );
+
+    // diagnose has no live activity stream, so the hook failure details must have
+    // been fetched via the GetHookResult API and attached to the diagnosis
+    expect(diagnoseOutput).toContain("Hook 'Private::Guard::TestHook' failed: NonCompliant Rules:");
+    expect(diagnoseOutput).toContain('[AWS_S3_Bucket_AccessControl]');
+    expect(diagnoseOutput).toContain('• Check was not compliant as property [/Resources/NonCompliantBucket/Properties/AccessControl[L:0,C:91]] existed.');
+    expect(diagnoseOutput).toContain('Remediation: AccessControl is deprecated');
+  }),
+);

packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts

@@ -12,7 +12,7 @@ import type { EnvironmentResources } from '../environment';
 import type { IoHelper } from '../io/private/io-helper';
 import type { ISourceTracer } from '../source-tracing/private/source-tracing';
 import { PollRange, StackEventPoller } from '../stack-events';
-import { formatHookResultDetails, isFailedHookStatus } from '../stack-events/hook-result-details';
+import { fetchHookResultDetails, formatHookResultDetails, isFailedHookStatus, normalizeHookMessage } from '../stack-events/hook-result-details';
 import type { ResourceError, ResourceErrors } from '../stack-events/resource-errors';
 import { StackStatus } from '../stack-events/stack-status';
 
@@ -36,6 +36,18 @@ export interface CloudFormationStackDiagnoserProps {
    * level and their information is simply not added to the output.
    */
   readonly additionalExplorationSdkProvider?: SdkProvider;
+
+  /**
+   * Whether to fetch the failure details of hooks that failed resources, via the
+   * `GetHookResult` API, and attach them to the diagnosis.
+   *
+   * During a deployment the activity stream already shows these details live, so
+   * `cdk deploy` leaves this off to avoid repeating them. `cdk diagnose` has no
+   * live stream and enables it.
+   *
+   * @default false
+   */
+  readonly fetchHookFailureDetails?: boolean;
 }
 
 export type SdkProvider = () => Promise<SDK>;
@@ -417,14 +429,12 @@ export class CloudFormationStackDiagnoser {
    */
   private async _hookResultDetails(hook: HookResultSummary): Promise<string | undefined> {
     if (hook.HookResultId) {
-      try {
-        const result = await this.cfn.getHookResult({ HookResultId: hook.HookResultId });
-        const details = formatHookResultDetails(result);
-        if (details) {
-          return details;
-        }
-      } catch (e: any) {
-        await this.props.ioHelper.defaults.debug(`Could not fetch hook result ${hook.HookResultId}: ${e.message}`);
+      const details = await fetchHookResultDetails(this.cfn, hook.HookResultId, {
+        ioHelper: this.props.ioHelper,
+        envResources: this.props.envResources,
+      });
+      if (details) {
+        return details;
       }
     }
     return formatHookResultDetails(hook);
@@ -443,19 +453,55 @@ export class CloudFormationStackDiagnoser {
       : this.props.sourceTracer.traceStack(err.stackArn, err.parentStackLogicalIds);
 
     // eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism
-    const [sourceTrace, additionalContext] = await Promise.all([
+    const [sourceTrace, additionalContext, hookContext] = await Promise.all([
       sourceTracePromise,
       this.investigateResourceBestEffort(err),
+      this.hookFailureContext(err),
     ]);
 
+    const allContext = [...hookContext, ...additionalContext];
     return {
       ...err,
       sourceTrace,
       topLevelStackHierarchicalId: this.props.topLevelStackHierarchicalId,
-      ...(additionalContext.length > 0 ? { additionalContext } : {}),
+      ...(allContext.length > 0 ? { additionalContext: allContext } : {}),
     };
   }
 
+  /**
+   * Fetch the failure details of the hooks that caused the given resource error.
+   *
+   * The resource's own status reason only names the hooks that failed it (e.g.
+   * "The following hook(s) failed: [X]"); the actual failure details live in the
+   * hook results API. Falls back to the hook event's status reason if the fetch
+   * fails or returns no details.
+   *
+   * Only active when `fetchHookFailureDetails` is set (i.e. for `cdk diagnose`);
+   * during a deployment the activity stream already shows these details.
+   */
+  private async hookFailureContext(err: ResourceError): Promise<AdditionalDiagnosticContext[]> {
+    if (!this.props.fetchHookFailureDetails) {
+      return [];
+    }
+    const ret: AdditionalDiagnosticContext[] = [];
+    for (const hook of err.hookFailures ?? []) {
+      let details = hook.hookInvocationId
+        ? await fetchHookResultDetails(this.cfn, hook.hookInvocationId, {
+          ioHelper: this.props.ioHelper,
+          envResources: this.props.envResources,
+        })
+        : undefined;
+      details = details ?? (hook.hookStatusReason ? normalizeHookMessage(hook.hookStatusReason) : undefined);
+      if (details) {
+        ret.push({
+          source: `CloudFormation Hook (${hook.hookType})`,
+          messages: details.split('\n').map((line, i) => i === 0 ? `Hook '${hook.hookType}' failed: ${line}` : line),
+        });
+      }
+    }
+    return ret;
+  }
+
   private async investigateResourceBestEffort(err: ResourceError) {
     const sdk = await this.additionalExplorationSdk();
     if (!sdk) {

packages/@aws-cdk/toolkit-lib/lib/api/stack-events/hook-result-details.ts

@@ -1,4 +1,9 @@
+import * as util from 'node:util';
 import type { GetHookResultCommandOutput, HookResultSummary } from '@aws-sdk/client-cloudformation';
+import type { ICloudFormationClient } from '../aws-auth/private';
+import { isAccessDeniedError } from '../aws-auth/util';
+import type { EnvironmentResources } from '../environment';
+import type { IoHelper } from '../io/private';
 
 /**
  * The relevant parts of a hook result, common to `GetHookResult` outputs and
@@ -53,6 +58,70 @@ export function isFailedHookStatus(status: string | undefined): boolean {
   return status === 'HOOK_COMPLETE_FAILED' || status === 'HOOK_FAILED';
 }
 
+export interface FetchHookResultDetailsOptions {
+  /**
+   * The IoHelper used to warn when the fetch fails.
+   */
+  readonly ioHelper: IoHelper;
+
+  /**
+   * Environment resources, used to look up the bootstrap toolkit version when
+   * diagnosing hook result fetch failures caused by missing permissions.
+   *
+   * @default - Bootstrap version is not reported in error messages
+   */
+  readonly envResources?: EnvironmentResources;
+}
+
+/**
+ * Fetch a hook invocation's failure details via the `GetHookResult` API and format
+ * them into a human-readable string.
+ *
+ * For Guard Hooks the details come from the failed annotations; for other hooks
+ * (e.g. Lambda Hooks) they come from the hook result's own status reason.
+ * Returns undefined if the fetch fails (emitting a warning, with a bootstrap
+ * upgrade hint if the failure looks permissions-related) or the result carries
+ * no failure details.
+ */
+export async function fetchHookResultDetails(
+  cfn: ICloudFormationClient,
+  hookInvocationId: string,
+  options: FetchHookResultDetailsOptions,
+): Promise<string | undefined> {
+  try {
+    const result = await cfn.getHookResult({ HookResultId: hookInvocationId });
+    return formatHookResultDetails(result);
+  } catch (e: any) {
+    const errorMessage = e instanceof Error ? e.message : String(e);
+
+    const isPermissionsError =
+      isAccessDeniedError(e) ||
+      (typeof errorMessage === 'string' && errorMessage.toLowerCase().includes('not authorized to perform: cloudformation:gethookresult'));
+
+    if (isPermissionsError && options.envResources) {
+      let currentVersion: number | undefined = undefined;
+      try {
+        currentVersion = (await options.envResources.lookupToolkit()).version;
+      } catch {
+        // ignore errors looking up the bootstrap version
+      }
+
+      await options.ioHelper.defaults.warn(
+        `Failed to fetch result details for Hook invocation ${hookInvocationId}: ${errorMessage}. ` +
+        'Make sure you have permissions to call the GetHookResult API, or re-bootstrap your environment ' +
+        "by running 'cdk bootstrap' to update the Bootstrap CDK Toolkit stack. " +
+        `Bootstrap toolkit stack version 31 or later is needed; current version: ${currentVersion ?? 'unknown'}.`,
+      );
+    } else {
+      await options.ioHelper.defaults.warn(
+        util.format('Failed to fetch Hook details for invocation %s: %s', hookInvocationId, errorMessage),
+      );
+    }
+
+    return undefined;
+  }
+}
+
 /**
  * Trims leading/trailing whitespace, collapses all internal whitespace
  * (including newlines) to a single space, and truncates to `maxChars`

packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts

@@ -1,8 +1,33 @@
 import type { StackEvent } from '@aws-sdk/client-cloudformation';
+import { isFailedHookStatus } from './hook-result-details';
 import type { ResourceEvent } from './stack-event-poller';
 import { DeploymentErrorCodes } from '../../toolkit/toolkit-error';
 import { isCancellationEvent, isErrorEvent, isRegularResourceEvent } from '../../util/cloudformation';
 
+/**
+ * A failed CloudFormation Hook invocation that caused a resource error.
+ */
+export interface ResourceHookFailure {
+  /**
+   * The type name of the hook that failed (e.g. `Private::Guard::TestHook`)
+   */
+  readonly hookType: string;
+
+  /**
+   * The unique identifier of the hook invocation.
+   *
+   * Can be used to fetch the full failure details via the `GetHookResult` API.
+   */
+  readonly hookInvocationId?: string;
+
+  /**
+   * The status reason reported on the hook's stack event.
+   *
+   * This is usually a terse summary; the `GetHookResult` API has richer details.
+   */
+  readonly hookStatusReason?: string;
+}
+
 export interface ResourceError {
   /**
    * The stack this resource error occurred in
@@ -56,6 +81,15 @@ export interface ResourceError {
    * and early-validation errors.
    */
   readonly timestamp?: Date;
+
+  /**
+   * CloudFormation Hook failures that caused this resource error, if any.
+   *
+   * Hook failures are reported on separate stack events from the resource failure
+   * itself; they are correlated to the resource error by logical ID and by the
+   * hook type appearing in the resource's status reason.
+   */
+  readonly hookFailures?: ResourceHookFailure[];
 }
 
 /**
@@ -69,6 +103,16 @@ export class ResourceErrors {
    */
   private readonly _errors: ResourceError[] = [];
 
+  /**
+   * Failed hook invocations seen so far, keyed by stack ARN + logical ID of the
+   * resource they were invoked on.
+   *
+   * Hook failures are reported on their own events (usually with an `IN_PROGRESS`
+   * resource status), before the resource failure event they cause. We remember
+   * them here so they can be attached to that resource error when it arrives.
+   */
+  private readonly seenHookFailures = new Map<string, ResourceHookFailure[]>();
+
   public isEmpty() {
     return this._errors.length === 0;
   }
@@ -86,16 +130,44 @@ export class ResourceErrors {
    */
   public update(...events: ResourceEvent[]) {
     for (const event of events) {
+      this.trackHookFailure(event.event);
       if (isErrorEvent(event.event)) {
         // Cancelled is not an interesting failure reason, nor is the stack message (stack
         // message will just say something like "stack failed to update")
         if (!isCancellationEvent(event.event) && isRegularResourceEvent(event.event)) {
-          this._errors.push(errorFromEvent(event));
+          this._errors.push(errorFromEvent(event, this.hookFailuresFor(event.event)));
         }
       }
     }
   }
 
+  private trackHookFailure(event: StackEvent) {
+    if (!isFailedHookStatus(event.HookStatus) || !event.HookType || !event.LogicalResourceId || !event.StackId) {
+      return;
+    }
+    const key = hookFailureKey(event);
+    const failures = this.seenHookFailures.get(key) ?? [];
+    failures.push({
+      hookType: event.HookType,
+      hookInvocationId: event.HookInvocationId,
+      hookStatusReason: event.HookStatusReason,
+    });
+    this.seenHookFailures.set(key, failures);
+  }
+
+  /**
+   * Return the hook failures that caused the given resource failure, if any.
+   *
+   * A hook failure belongs to a resource failure when it was recorded for the same
+   * resource, and its hook type is mentioned in the resource's status reason (e.g.
+   * "The following hook(s) failed: [Private::Guard::TestHook]").
+   */
+  private hookFailuresFor(event: StackEvent): ResourceHookFailure[] {
+    const recorded = this.seenHookFailures.get(hookFailureKey(event)) ?? [];
+    const reason = event.ResourceStatusReason ?? '';
+    return recorded.filter((h) => reason.includes(h.hookType));
+  }
+
   /**
    * Take our best guess at the error code of the root cause
    *
@@ -123,9 +195,7 @@ export class ResourceErrors {
   }
 }
 
-function errorFromEvent(ev: ResourceEvent): ResourceError {
-  // FIXME: Check hooks
-
+function errorFromEvent(ev: ResourceEvent, hookFailures: ResourceHookFailure[]): ResourceError {
   return {
     logicalId: ev.event.LogicalResourceId ?? '',
     message: ev.event.ResourceStatusReason ?? '',
@@ -135,9 +205,14 @@ function errorFromEvent(ev: ResourceEvent): ResourceError {
     errorCode: extractErrorCode(ev.event),
     physicalId: ev.event.PhysicalResourceId,
     timestamp: ev.event.Timestamp,
+    ...(hookFailures.length > 0 ? { hookFailures } : {}),
   };
 }
 
+function hookFailureKey(event: StackEvent): string {
+  return `${event.StackId ?? ''}|${event.LogicalResourceId ?? ''}`;
+}
+
 /**
  * Extract an error code from the given stack event.
  *