Skip to content

Commit

Permalink
chore(cxapi): plugin context provider limited by cx schema (#18709)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
rix0rrr authored Feb 4, 2022
1 parent e89688e commit 1eca946
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -473,4 +495,6 @@ export type ContextQueryProperties = AmiContextQuery
| LoadBalancerContextQuery
| LoadBalancerListenerContextQuery
| SecurityGroupContextQuery
| KeyContextQuery;
| KeyContextQuery
| PluginContextQuery;

Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,9 @@
},
{
"$ref": "#/definitions/KeyContextQuery"
},
{
"$ref": "#/definitions/PluginContextQuery"
}
]
}
Expand All @@ -472,6 +475,7 @@
"key-provider",
"load-balancer",
"load-balancer-listener",
"plugin",
"security-group",
"ssm",
"vpc-provider"
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"16.0.0"}
{"version":"16.0.0"}
15 changes: 10 additions & 5 deletions packages/@aws-cdk/core/lib/context-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/core/test/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:');
});
});

/**
Expand Down
18 changes: 17 additions & 1 deletion packages/aws-cdk/lib/context-providers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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.`);
Expand Down Expand Up @@ -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
*
Expand Down
20 changes: 17 additions & 3 deletions packages/aws-cdk/lib/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
}
}
11 changes: 6 additions & 5 deletions packages/aws-cdk/test/context-providers/generic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<any> {
called = true;
return '';
Expand All @@ -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<any> {
return 'yay';
},
Expand All @@ -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
Expand Down

0 comments on commit 1eca946

Please sign in to comment.