feat(core): context lookup errors are reported to CX app (aws#3772)

Instead of the toolkit failing and stopping when a context provider
lookup fails, the error is reported to the CDK app, which can then
decide what to do with it.

In practice, it will attach a (localized) error to the Construct tree.
This has the following advantages:

* Dependent context resolution can proceed in a stepwise fashion,
  without the toolkit stopping at the first failure. For example,
  a VPC lookup from a value found in SSM will work.
* Stacks in multiple accounts that need context lookup can be used
  in a single app with cold context. Without this change they couldn't,
  because you could only have credentials for a single account at a
  time available so lookup for one of the accounts was bound to fail.

Fixes aws#3654.

packages/@aws-cdk/aws-ec2/lib/vpc.ts

@@ -805,7 +805,7 @@ export class Vpc extends VpcBase {
       provider: cxapi.VPC_PROVIDER,
       props: { filter } as cxapi.VpcContextQuery,
       dummyValue: undefined
-    });
+    }).value;
 
     return new ImportedVpc(scope, id, attributes || DUMMY_VPC_PROPS, attributes === undefined);
 

packages/@aws-cdk/aws-route53/lib/hosted-zone.ts

@@ -87,7 +87,7 @@ export class HostedZone extends Resource implements IHostedZone {
       provider: cxapi.HOSTED_ZONE_PROVIDER,
       dummyValue: DEFAULT_HOSTED_ZONE,
       props: query
-    });
+    }).value;
 
     // CDK handles the '.' at the end, so remove it here
     if (response.Name.endsWith('.')) {

packages/@aws-cdk/aws-ssm/lib/parameter.ts

@@ -238,7 +238,7 @@ export class StringParameter extends ParameterBase implements IStringParameter {
       provider: cxapi.SSM_PARAMETER_PROVIDER,
       props: { parameterName },
       dummyValue: `dummy-value-for-${parameterName}`
-    });
+    }).value;
 
     return value;
   }

packages/@aws-cdk/core/lib/context-provider.ts

@@ -1,3 +1,4 @@
+import cxapi = require('@aws-cdk/cx-api');
 import { Construct } from './construct';
 import { Stack } from './stack';
 import { Token } from './token';
@@ -81,7 +82,7 @@ export class ContextProvider {
     };
   }
 
-  public static getValue(scope: Construct, options: GetContextValueOptions): any {
+  public static getValue(scope: Construct, options: GetContextValueOptions): GetContextValueResult {
     const stack = Stack.of(scope);
 
     if (Token.isUnresolved(stack.account) || Token.isUnresolved(stack.region)) {
@@ -93,19 +94,35 @@ export class ContextProvider {
 
     const { key, props } = this.getKey(scope, options);
     const value = scope.node.tryGetContext(key);
+    const providerError = extractProviderError(value);
 
-    // if context is missing, report and return a dummy value
-    if (value === undefined) {
+    // if context is missing or an error occurred during context retrieval,
+    // report and return a dummy value.
+    if (value === undefined || providerError !== undefined) {
       stack.reportMissingContext({ key, props, provider: options.provider, });
-      return options.dummyValue;
+
+      if (providerError !== undefined) {
+        scope.node.addError(providerError);
+      }
+      return { value: options.dummyValue };
     }
 
-    return value;
+    return { value };
   }
 
   private constructor() { }
 }
 
+/**
+ * If the context value represents an error, return the error message
+ */
+function extractProviderError(value: any): string | undefined {
+  if (typeof value === 'object' && value !== null) {
+    return value[cxapi.PROVIDER_ERROR_KEY];
+  }
+  return undefined;
+}
+
 /**
  * Quote colons in all strings so that we can undo the quoting at a later point
  *

packages/@aws-cdk/core/lib/stack.ts

@@ -417,7 +417,7 @@ export class Stack extends Construct implements ITaggable {
     const value = ContextProvider.getValue(this, {
       provider: cxapi.AVAILABILITY_ZONE_PROVIDER,
       dummyValue: ['dummy1a', 'dummy1b', 'dummy1c'],
-    });
+    }).value;
 
     if (!Array.isArray(value)) {
       throw new Error(`Provider ${cxapi.AVAILABILITY_ZONE_PROVIDER} expects a list`);

packages/@aws-cdk/core/test/test.context.ts

@@ -1,5 +1,5 @@
 import { Test } from 'nodeunit';
-import { ConstructNode, Stack } from '../lib';
+import { Construct, ConstructNode, Stack } from '../lib';
 import { ContextProvider } from '../lib/context-provider';
 
 export = {
@@ -111,6 +111,34 @@ export = {
 
     test.done();
   },
+
+  'context provider errors are attached to tree'(test: Test) {
+    const contextProps = { provider: 'bloop' };
+    const contextKey = 'bloop:account=12345:region=us-east-1';  // Depends on the mangling algo
+
+    // GIVEN
+    const stack = new Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } });
+
+    // NOTE: error key is inlined here because it's part of the CX-API
+    // compatibility surface.
+    stack.node.setContext(contextKey, { $providerError: 'I had a boo-boo' });
+    const construct = new Construct(stack, 'Child');
+
+    // Verify that we got the right hardcoded key above, give a descriptive error if not
+    test.equals(ContextProvider.getKey(construct, contextProps).key, contextKey);
+
+    // WHEN
+    ContextProvider.getValue(construct, {
+      ...contextProps,
+      dummyValue: undefined,
+    });
+
+    // THEN
+    const error = construct.node.metadata.find(m => m.type === 'aws:cdk:error');
+    test.equals(error && error.data, 'I had a boo-boo');
+
+    test.done();
+  },
 };
 
 /**

packages/@aws-cdk/core/test/test.synthesis.ts

@@ -78,7 +78,7 @@ export = {
     ]);
     test.deepEqual(readJson(session.directory, 'foo.json'), { bar: 123 });
     test.deepEqual(session.manifest, {
-      version: '0.36.0',
+      version: cxapi.CLOUD_ASSEMBLY_VERSION,
       artifacts: {
         'my-random-construct': {
           type: 'aws:cloudformation:stack',

packages/@aws-cdk/cx-api/lib/cxapi.ts

@@ -36,3 +36,8 @@ export const DISABLE_ASSET_STAGING_CONTEXT = 'aws:cdk:disable-asset-staging';
  * Omits stack traces from construct metadata entries.
  */
 export const DISABLE_METADATA_STACK_TRACE = 'aws:cdk:disable-stack-trace';
+
+/**
+ * If a context value is an object with this key, it indicates an error
+ */
+export const PROVIDER_ERROR_KEY = '$providerError';

packages/@aws-cdk/cx-api/lib/index.ts

@@ -9,3 +9,5 @@ export * from './cloud-assembly';
 export * from './assets';
 export * from './environment';
 export * from './metadata';
+
+export { CLOUD_ASSEMBLY_VERSION } from './versioning';

packages/aws-cdk/lib/api/cxapp/stacks.ts

@@ -200,18 +200,32 @@ export class AppStacks {
     const trackVersions: boolean = this.props.configuration.settings.get(['versionReporting']);
 
     // We may need to run the cloud executable multiple times in order to satisfy all missing context
+    let previouslyMissingKeys: Set<string> | undefined;
     while (true) {
       const assembly = await this.props.synthesizer(this.props.aws, this.props.configuration);
 
       if (assembly.manifest.missing) {
-        debug(`Some context information is missing. Fetching...`);
+        const missingKeys = missingContextKeys(assembly.manifest.missing);
 
-        await contextproviders.provideContextValues(assembly.manifest.missing, this.props.configuration.context, this.props.aws);
+        let tryLookup = true;
+        if (previouslyMissingKeys && setsEqual(missingKeys, previouslyMissingKeys)) {
+          debug(`Not making progress trying to resolve environmental context. Giving up.`);
+          tryLookup = false;
+        }
+
+        previouslyMissingKeys = missingKeys;
+
+        if (tryLookup) {
+          debug(`Some context information is missing. Fetching...`);
 
-        // Cache the new context to disk
-        await this.props.configuration.saveContext();
+          await contextproviders.provideContextValues(assembly.manifest.missing, this.props.configuration.context, this.props.aws);
 
-        continue;
+          // Cache the new context to disk
+          await this.props.configuration.saveContext();
+
+          // Execute again
+          continue;
+        }
       }
 
       if (trackVersions && assembly.runtime) {
@@ -414,6 +428,21 @@ export interface Tag {
   readonly Value: string;
 }
 
+/**
+ * Return all keys of misisng context items
+ */
+function missingContextKeys(missing?: cxapi.MissingContext[]): Set<string> {
+  return new Set((missing || []).map(m => m.key));
+}
+
+function setsEqual<A>(a: Set<A>, b: Set<A>) {
+  if (a.size !== b.size) { return false; }
+  for (const x of a) {
+    if (!b.has(x)) { return false; }
+  }
+  return true;
+}
+
 function _makeCdkMetadataAvailableCondition() {
   return _fnOr(RegionInfo.regions
     .filter(ri => ri.cdkMetadataResourceAvailable)

packages/aws-cdk/lib/context-providers/index.ts

@@ -1,7 +1,7 @@
 import cxapi = require('@aws-cdk/cx-api');
 import { ISDK } from '../api/util/sdk';
 import { debug } from '../logging';
-import { Context } from '../settings';
+import { Context, TRANSIENT_CONTEXT_KEY } from '../settings';
 import { AZContextProviderPlugin } from './availability-zones';
 import { HostedZoneContextProviderPlugin } from './hosted-zones';
 import { ContextProviderPlugin } from './provider';
@@ -18,6 +18,7 @@ export async function provideContextValues(
   missingValues: cxapi.MissingContext[],
   context: Context,
   sdk: ISDK) {
+
   for (const missingContext of missingValues) {
     const key = missingContext.key;
     const constructor = availableContextProviders[missingContext.provider];
@@ -28,12 +29,28 @@ export async function provideContextValues(
 
     const provider = new constructor(sdk);
 
-    const value = await provider.getValue(missingContext.props);
+    let value;
+    try {
+      value = await provider.getValue(missingContext.props);
+    } catch (e) {
+      // Set a specially formatted provider value which will be interpreted
+      // as a lookup failure in the toolkit.
+      value = { [cxapi.PROVIDER_ERROR_KEY]: e.message, [TRANSIENT_CONTEXT_KEY]: true };
+    }
     context.set(key, value);
     debug(`Setting "${key}" context to ${JSON.stringify(value)}`);
   }
 }
 
+/**
+ * Register a context provider
+ *
+ * (Only available for testing right now).
+ */
+export function registerContextProvider(name: string, provider: ProviderConstructor) {
+  availableContextProviders[name] = provider;
+}
+
 const availableContextProviders: ProviderMap = {
   [cxapi.AVAILABILITY_ZONE_PROVIDER]: AZContextProviderPlugin,
   [cxapi.SSM_PARAMETER_PROVIDER]: SSMContextProviderPlugin,

packages/aws-cdk/lib/settings.ts

@@ -11,6 +11,11 @@ export const PROJECT_CONFIG = 'cdk.json';
 export const PROJECT_CONTEXT = 'cdk.context.json';
 export const USER_DEFAULTS = '~/.cdk.json';
 
+/**
+ * If a context value is an object with this key set to a truthy value, it won't be saved to cdk.context.json
+ */
+export const TRANSIENT_CONTEXT_KEY = '$dontSaveContext';
+
 const CONTEXT_KEY = 'context';
 
 export type Arguments = { readonly [name: string]: unknown };
@@ -284,8 +289,7 @@ export class Settings {
 
   public async save(fileName: string): Promise<this> {
     const expanded = expandHomeDir(fileName);
-    await fs.writeJson(expanded, this.settings, { spaces: 2 });
-
+    await fs.writeJson(expanded, stripTransientValues(this.settings), { spaces: 2 });
     return this;
   }
 
@@ -362,3 +366,25 @@ function expandHomeDir(x: string) {
   }
   return x;
 }
+
+/**
+ * Return all context value that are not transient context values
+ */
+function stripTransientValues(obj: {[key: string]: any}) {
+  const ret: any = {};
+  for (const [key, value] of Object.entries(obj)) {
+    if (!isTransientValue(value)) {
+      ret[key] = value;
+    }
+  }
+  return ret;
+}
+
+/**
+ * Return whether the given value is a transient context value
+ *
+ * Values that are objects with a magic key set to a truthy value are considered transient.
+ */
+function isTransientValue(value: any) {
+  return typeof value === 'object' && value !== null && (value as any)[TRANSIENT_CONTEXT_KEY];
+}

packages/aws-cdk/test/api/test.stacks.ts

@@ -2,6 +2,7 @@ import cxapi = require('@aws-cdk/cx-api');
 import { Test, testCase } from 'nodeunit';
 import { SDK } from '../../lib';
 import { AppStacks, DefaultSelection } from '../../lib/api/cxapp/stacks';
+import { registerContextProvider } from '../../lib/context-providers';
 import { Configuration } from '../../lib/settings';
 import { testAssembly } from '../util';
 
@@ -126,6 +127,35 @@ export = testCase({
       test.done();
     }
   },
+
+  async 'stop executing if context providers are not making progress'(test: Test) {
+    registerContextProvider('testprovider', class {
+      public async getValue(_: {[key: string]: any}): Promise<any> {
+        return 'foo';
+      }
+    });
+
+    const stacks = new AppStacks({
+      configuration: new Configuration(),
+      aws: new SDK(),
+      synthesizer: async () => testAssembly({
+        stacks: [ {
+          stackName: 'thestack',
+          template: { resource: 'noerrorresource' },
+        }],
+        // Always return the same missing keys, synthesis should still finish.
+        missing: [
+          { key: 'abcdef', props: {}, provider: 'testprovider' }
+        ]
+      }),
+    });
+
+    // WHEN
+    await stacks.selectStacks(['thestack'], { defaultBehavior: DefaultSelection.AllStacks });
+
+    // THEN: the test finishes normally
+    test.done();
+  },
 });
 
 function testStacks({ env, versionReporting = true }: { env?: string, versionReporting?: boolean } = {}) {
@@ -136,22 +166,24 @@ function testStacks({ env, versionReporting = true }: { env?: string, versionRep
     configuration,
     aws: new SDK(),
     synthesizer: async () => testAssembly({
-      stackName: 'withouterrors',
-      env,
-      template: { resource: 'noerrorresource' },
-    },
-    {
-      stackName: 'witherrors',
-      env,
-      template: { resource: 'errorresource' },
-      metadata: {
-        '/resource': [
-          {
-            type: cxapi.ERROR_METADATA_KEY,
-            data: 'this is an error'
-          }
-        ]
+      stacks: [{
+        stackName: 'withouterrors',
+        env,
+        template: { resource: 'noerrorresource' },
       },
+      {
+        stackName: 'witherrors',
+        env,
+        template: { resource: 'errorresource' },
+        metadata: {
+          '/resource': [
+            {
+              type: cxapi.ERROR_METADATA_KEY,
+              data: 'this is an error'
+            }
+          ]
+        },
+      }]
     }),
   });
 }