From 1eca94686a209699833c5fa2aa6fe72ac58a63e0 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 4 Feb 2022 11:05:00 +0100 Subject: [PATCH] chore(cxapi): plugin context provider limited by cx schema (#18709) During internal trialing of context provider plugins, we discovered that the JSON schema validation of the Cloud Assembly is throwing soot into our food: any properties we add aren't exactly those of another context provider, and therefore not allowed. Add a new branch of the context provider schema, a `PluginContextQuery`, which allows any set of properties. The actual plugin name is now merely a property passed to the "plugin" context provider. When the handler is registered or the query is handled, we combine the key in the lookup table with the actual provider name as such: `plugin:${pluginName}`. This is an implementation detail of the CLI, and not part of the contract (can be changed easily whenver necessary). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/cloud-assembly/context-queries.ts | 26 ++++++++++++++++++- .../schema/cloud-assembly.schema.json | 18 +++++++++++++ .../schema/cloud-assembly.version.json | 2 +- .../@aws-cdk/core/lib/context-provider.ts | 15 +++++++---- packages/@aws-cdk/core/test/context.test.ts | 5 ++++ .../aws-cdk/lib/context-providers/index.ts | 18 ++++++++++++- packages/aws-cdk/lib/plugin.ts | 20 +++++++++++--- .../test/context-providers/generic.test.ts | 11 ++++---- 8 files changed, 99 insertions(+), 16 deletions(-) diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts index 97ede25af2498..e82f8ee6f5ee3 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/context-queries.ts @@ -53,6 +53,11 @@ export enum ContextProvider { * KMS Key Provider */ KEY_PROVIDER = 'key-provider', + + /** + * A plugin provider (the actual plugin name will be in the properties) + */ + PLUGIN = 'plugin', } /** @@ -461,7 +466,24 @@ export interface KeyContextQuery { * Alias name used to search the Key */ readonly aliasName: string; +} + +/** + * Query input for plugins + * + * This alternate branch is necessary because it needs to be able to escape all type checking + * we do on on the cloud assembly -- we cannot know the properties that will be used a priori. + */ +export interface PluginContextQuery { + /** + * The name of the plugin + */ + readonly pluginName: string; + /** + * Arbitrary other arguments for the plugin + */ + [key: string]: any; } export type ContextQueryProperties = AmiContextQuery @@ -473,4 +495,6 @@ export type ContextQueryProperties = AmiContextQuery | LoadBalancerContextQuery | LoadBalancerListenerContextQuery | SecurityGroupContextQuery -| KeyContextQuery; +| KeyContextQuery +| PluginContextQuery; + diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json index 5fe1f4fb4321a..b94e9e27433c2 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json @@ -452,6 +452,9 @@ }, { "$ref": "#/definitions/KeyContextQuery" + }, + { + "$ref": "#/definitions/PluginContextQuery" } ] } @@ -472,6 +475,7 @@ "key-provider", "load-balancer", "load-balancer-listener", + "plugin", "security-group", "ssm", "vpc-provider" @@ -834,6 +838,20 @@ "region" ] }, + "PluginContextQuery": { + "description": "Query input for plugins\n\nThis alternate branch is necessary because it needs to be able to escape all type checking\nwe do on on the cloud assembly -- we cannot know the properties that will be used a priori.", + "type": "object", + "additionalProperties": {}, + "properties": { + "pluginName": { + "description": "The name of the plugin", + "type": "string" + } + }, + "required": [ + "pluginName" + ] + }, "RuntimeInfo": { "description": "Information about the application's runtime components.", "type": "object", diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json index 5bdbc9d33c3b3..ae7a33e962d0b 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json @@ -1 +1 @@ -{"version":"16.0.0"} \ No newline at end of file +{"version":"16.0.0"} diff --git a/packages/@aws-cdk/core/lib/context-provider.ts b/packages/@aws-cdk/core/lib/context-provider.ts index b8d6c16abac75..a74623f087802 100644 --- a/packages/@aws-cdk/core/lib/context-provider.ts +++ b/packages/@aws-cdk/core/lib/context-provider.ts @@ -17,6 +17,13 @@ export interface GetContextKeyOptions { * Provider-specific properties. */ readonly props?: { [key: string]: any }; + + /** + * Whether to include the stack's account and region automatically. + * + * @default true + */ + readonly includeEnvironment?: boolean; } /** @@ -60,11 +67,9 @@ export class ContextProvider { public static getKey(scope: Construct, options: GetContextKeyOptions): GetContextKeyResult { const stack = Stack.of(scope); - const props = { - account: stack.account, - region: stack.region, - ...options.props || {}, - }; + const props = options.includeEnvironment ?? true + ? { account: stack.account, region: stack.region, ...options.props } + : (options.props ?? {}); if (Object.values(props).find(x => Token.isUnresolved(x))) { throw new Error( diff --git a/packages/@aws-cdk/core/test/context.test.ts b/packages/@aws-cdk/core/test/context.test.ts index b052e74e8f1b7..7df1ce97cebac 100644 --- a/packages/@aws-cdk/core/test/context.test.ts +++ b/packages/@aws-cdk/core/test/context.test.ts @@ -166,6 +166,11 @@ describe('context', () => { }); + + test('can skip account/region from attach to context', () => { + const stack = new Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); + expect(ContextProvider.getKey(stack, { provider: 'asdf', includeEnvironment: false }).key).toEqual('asdf:'); + }); }); /** diff --git a/packages/aws-cdk/lib/context-providers/index.ts b/packages/aws-cdk/lib/context-providers/index.ts index fe643ca946c83..dcf34a5af0ed8 100644 --- a/packages/aws-cdk/lib/context-providers/index.ts +++ b/packages/aws-cdk/lib/context-providers/index.ts @@ -18,6 +18,8 @@ import { VpcNetworkContextProviderPlugin } from './vpcs'; export type ContextProviderFactory = ((sdk: SdkProvider) => ContextProviderPlugin); export type ProviderMap = {[name: string]: ContextProviderFactory}; +const PLUGIN_PROVIDER_PREFIX = 'plugin'; + /** * Iterate over the list of missing context values and invoke the appropriate providers from the map to retrieve them */ @@ -28,7 +30,12 @@ export async function provideContextValues( for (const missingContext of missingValues) { const key = missingContext.key; - const factory = availableContextProviders[missingContext.provider]; + + const providerName = missingContext.provider === cxschema.ContextProvider.PLUGIN + ? `${PLUGIN_PROVIDER_PREFIX}:${(missingContext.props as cxschema.PluginContextQuery).pluginName}` + : missingContext.provider; + + const factory = availableContextProviders[providerName]; if (!factory) { // eslint-disable-next-line max-len throw new Error(`Unrecognized context provider name: ${missingContext.provider}. You might need to update the toolkit to match the version of the construct library.`); @@ -70,6 +77,15 @@ export function registerContextProvider(name: string, provider: ContextProviderP availableContextProviders[name] = () => provider; } +/** + * Register a plugin context provider + * + * A plugin provider cannot reuse the SDKs authentication mechanisms. + */ +export function registerPluginContextProvider(name: string, provider: ContextProviderPlugin) { + registerContextProvider(`${PLUGIN_PROVIDER_PREFIX}:${name}`, provider); +} + /** * Register a context provider factory * diff --git a/packages/aws-cdk/lib/plugin.ts b/packages/aws-cdk/lib/plugin.ts index cde0b8c63bf32..3efa2920270d6 100644 --- a/packages/aws-cdk/lib/plugin.ts +++ b/packages/aws-cdk/lib/plugin.ts @@ -2,7 +2,7 @@ import { inspect } from 'util'; import * as chalk from 'chalk'; import { CredentialProviderSource } from './api/aws-auth/credentials'; -import { registerContextProvider } from './context-providers'; +import { registerPluginContextProvider } from './context-providers'; import { ContextProviderPlugin, isContextProviderPlugin } from './context-providers/provider'; import { error } from './logging'; @@ -106,12 +106,26 @@ export class PluginHost { * This feature is experimental, and only intended to be used internally at Amazon * as a trial. * + * After registering with 'my-plugin-name', the provider must be addressed as follows: + * + * ```ts + * const value = ContextProvider.getValue(this, { + * providerName: 'plugin', + * props: { + * pluginName: 'my-plugin-name', + * myParameter1: 'xyz', + * }, + * includeEnvironment: true | false, + * dummyValue: 'what-to-return-on-the-first-pass', + * }) + * ``` + * * @experimental */ - public registerContextProviderAlpha(providerName: string, provider: ContextProviderPlugin) { + public registerContextProviderAlpha(pluginProviderName: string, provider: ContextProviderPlugin) { if (!isContextProviderPlugin(provider)) { throw new Error(`Object you gave me does not look like a ContextProviderPlugin: ${inspect(provider)}`); } - registerContextProvider(providerName, provider); + registerPluginContextProvider(pluginProviderName, provider); } } diff --git a/packages/aws-cdk/test/context-providers/generic.test.ts b/packages/aws-cdk/test/context-providers/generic.test.ts index ad26b22689171..24ba0d105b26a 100644 --- a/packages/aws-cdk/test/context-providers/generic.test.ts +++ b/packages/aws-cdk/test/context-providers/generic.test.ts @@ -6,6 +6,7 @@ import { MockSdkProvider } from '../util/mock-sdk'; const mockSDK = new MockSdkProvider(); const TEST_PROVIDER: any = 'testprovider'; +const PLUGIN_PROVIDER: any = 'plugin'; test('errors are reported into the context value', async () => { // GIVEN @@ -84,7 +85,7 @@ test('context provider can be registered using PluginHost', async () => { let called = false; // GIVEN - PluginHost.instance.registerContextProviderAlpha(TEST_PROVIDER, { + PluginHost.instance.registerContextProviderAlpha('prov', { async getValue(_: {[key: string]: any}): Promise { called = true; return ''; @@ -94,16 +95,16 @@ test('context provider can be registered using PluginHost', async () => { // WHEN await contextproviders.provideContextValues([ - { key: 'asdf', props: { account: '1234', region: 'us-east-1' }, provider: TEST_PROVIDER }, + { key: 'asdf', props: { account: '1234', region: 'us-east-1', pluginName: 'prov' }, provider: PLUGIN_PROVIDER }, ], context, mockSDK); // THEN - error is marked transient expect(called).toEqual(true); }); -test('context provider can be called without account/region', async () => { +test('plugin context provider can be called without account/region', async () => { // GIVEN - PluginHost.instance.registerContextProviderAlpha(TEST_PROVIDER, { + PluginHost.instance.registerContextProviderAlpha('prov', { async getValue(_: {[key: string]: any}): Promise { return 'yay'; }, @@ -112,7 +113,7 @@ test('context provider can be called without account/region', async () => { // WHEN await contextproviders.provideContextValues([ - { key: 'asdf', props: { banana: 'yellow' } as any, provider: TEST_PROVIDER }, + { key: 'asdf', props: { banana: 'yellow', pluginName: 'prov' } as any, provider: PLUGIN_PROVIDER }, ], context, mockSDK); // THEN - error is marked transient