-
Notifications
You must be signed in to change notification settings - Fork 82
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
RFC: 18 Open context provider framework
- Loading branch information
Daniel Neilson
committed
May 27, 2020
1 parent
c0beb93
commit 685ed07
Showing
1 changed file
with
142 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
--- | ||
feature name: Open context provider framwork | ||
start date: 2020-05-27 | ||
rfc pr: | ||
related issue: https://github.com/aws/aws-cdk-rfcs/issues/18 | ||
--- | ||
|
||
# Summary | ||
|
||
The CDK has a super-handy feature in the form of the ContextProvider ( https://docs.aws.amazon.com/cdk/latest/guide/context.html ). These ContextProviders are currently hard-coded into the CDK CLI, and additional ContextProviders cannot be added by external sources. This proposal is to make the ContextProvider mechanism pluggable so that 3rd parties can create ContextProviders without adding them to the CDK repository, and users of the CDK can use these 3rd party ContextProviders. | ||
|
||
# Motivation | ||
|
||
Development within the CDK Construct library itself benefits greatly from this capability. When a new ContextProvider is needed, then a CDK contributor can just create it. Unfortunately, since the providers are hard-coded into the CDK, no 3rd party construct library can create their own ContextProvider; other than by contributing it to the CDK, of course. Some ContextProviders may be too specialized for reasonable consideration as a CDK contribution. | ||
|
||
Instead 3rd party libraries are required to implement workarounds, like CfnCustomResources that resolve during deployment instead of during synthesis. This increases the size of the Cfn template generated by the app, increases the time required to deploy the stack, adds unnecessary extra resources to the user's account, and makes it possible for a value generated by the CfnCustomResource to differ from one deployment to the next. | ||
|
||
Unfortunately, these CfnCustomResources required by 3rd party construct libraries may also be a security risk to the user. The lambda that backs them persists as long as the stack is deployed, and that lambda is granted permissions into the account that may be possible to leverage by an attacker that gains access to the lambda; depending on the lambda's purpose, of course. | ||
|
||
# Basic Example | ||
|
||
``` | ||
// Allow the provider to be an arbitrary string | ||
const someValues: thirdpartlib.DoSomethingContextReponse = ContextProvider.getValue(scope, { | ||
provider: 'thidparty-do-something', | ||
props: { | ||
// whatever needed | ||
}, | ||
dummyValue: undefined, | ||
}).value; | ||
// do something with someValues -- which has been materialized at synth time. | ||
// Still retain the existing functionality (ex: Vpc.fromLookup()) | ||
const attributes: cxapi.VpcContextResponse = ContextProvider.getValue(scope, { | ||
provider: cxschema.ContextProvider.VPC_PROVIDER, | ||
props: { | ||
filter, | ||
returnAsymmetricSubnets: true, | ||
subnetGroupNameTag: options.subnetGroupNameTag, | ||
} as cxschema.VpcContextQuery, | ||
dummyValue: undefined, | ||
}).value; | ||
``` | ||
|
||
# Design Summary | ||
|
||
Leverage the existing `Plugin` mechanism that is implemeted into the CDK CLI, and currently only used to register `CredentialProvider`s. Extend the `PluginHost` class to provide a method for registering a `ContextProvider` plugin. | ||
|
||
# Detailed Design | ||
|
||
## Background -- Understanding the current CDK structure for ContextProvider | ||
|
||
CDK CLI: `packages/aws-cdk` | ||
|
||
CDK CLI entrypoint: `bin/cdk.ts`: `initCommandLine()` | ||
1. Load a configuration (`Configuration().load()` -- `lib/settings.ts`) -- merges key:value pairs from cdk.json into settings | ||
2. Load the plugins from the settings['plugin'] key (i.e. 'plugin' key from cdk.json) | ||
|
||
Loading plugins -- happens in `initCommandLine()` | ||
* Resolves the plugin module, and then invokes `PluginHost.instance.load(plugin)` ( `lib/plugin.ts` ) | ||
* `lib/plugin.ts` defines `interface Plugin { version: '1'; init?: (host: PluginHost) => void; }` | ||
* `PluginHost.load(moduleSpec: string)` -- if arg is a plugin, then run `plugin.init(PluginHost.instance)`. | ||
* `plugin.init(PluginHost.instance)` can invoke `PluginHost.instance.registerCredentialProviderSource` to register itself | ||
* Currently, only registration method is `registerCredentialProviderSource` | ||
|
||
ContextProvider -- `context-providers/` | ||
* All of the context provider implementations are baked into the CDK CLI app. | ||
* Base: `interface ContextProviderPlugin { getValue(args: {[key: string]: any}): Promise<any>; }` (in `context-providers/provider.ts`) | ||
* Values for context providers are resolved via `provideContextValues()` in `context-providers/index.ts` | ||
* This uses a list of `availableContextProviders` which is currently hard-coded in the same file, but there is a `registerContextProvider()` function | ||
that adds to that list; though the function is marked as "only for testing") | ||
* `@aws-cdk/cloud-assembly-schema/lib/context-queries.ts` contains: | ||
* `ContextProvider` -- an enum of each of the provider types (ex: `VPC_PROVIDER = 'vpc-provider'`) | ||
* Query parameter interfaces for each of the context providers. | ||
* `type ContextQueryProperties` -- union of all of the query interfaces for all context providers. | ||
* `@aws-cdk/core/context-provider.ts` contains `ContextProvider` construct definition. | ||
* API is via `ContextProvider.getValue()` | ||
* The `provider` key is a string type. | ||
* This encodes the `provider` key with the provider-specific properties (i.e. the query), and passes that encoding and the query arguments to `scope.node.tryGetContext()` (i.e. `ContructNode.tryGetContext()` -- `@aws-cdk/core/lib/contruct-compat.ts`) | ||
* `Construct.tryGetContext()` just calls `this._actualNode.tryGetContext()` (i.e. `constructs.Node.tryGetContext()` -- which is in `https://github.com/aws/constructs`) | ||
* On error, this calls `stack.reportMissingContext()` with one of the args being a cast of the `provider` from a string to a `cxschema.ContextProvider` | ||
* `reportMissingContext()` throws an error if the `provider` isn't from the built-in enum, and then pushes the error as a `cxschema.MissingContext` onto | ||
`this._missingContext` | ||
* `MissingContext` defines the `provider` as type `ContextProvider` | ||
|
||
## Proposed changes | ||
|
||
### In CDK CLI | ||
|
||
2. Add field `readonly identifier: string` to `interface ContextProviderPlugin` | ||
3. Modify all existing implementations of `ContextProviderPlugin` as follows: | ||
1. Define the `identifier` field with appropriate value from `cxschema.ContextProvider` enum. | ||
2. Add a call to `registerContextProvider()` to the bottom of each definition file. | ||
* ex: In `context-provider/vpc.ts` we add the call `registerContextProvider(VpcNetworkContextProviderPlugin.identifier, VpcNetworkContextProviderPlugin);` | ||
3. Modify the definition of `availableContextProviders` in `context-providers/index.ts` to be initially empty. | ||
4. Modify the implementation of `registerContextProvider` in `context-providers/index.ts` to throw an error if trying to re-register a name. | ||
5. Use the existing `PluginHost` mechanism to import/register ; extend `PluginHost` with a `registerContextProviderSource()` method. | ||
2. In `registerContextProviderSource()` we: | ||
1. Check that the given module implements `interface ContextProviderPlugin` as best we can, and throw an error if it does not. | ||
2. Check whether the plugin has already been registered. If it has, then should we throw an error here, or a warning, or... ? | ||
2. Invoke `registerContextProvider()` to register the plugin | ||
|
||
### In CDK modules | ||
|
||
A saving grace here for the Construct API is that even though `enum ContextProvider` in `@aws-cdk/cloud-assembly-schema/lib/context-queries.ts` is a static enum that defines the names of each of the built-in `ContextProvider`s, the `interface GetContextKeyOptions` defines the `provider` field as being a string. So, we shouldn't require any API changes here. | ||
|
||
A challenge that I can see is that the call to `stack.reportMissingContext()` in `ContextProvider.getValue()` casts the `provider` value to a `cxschema.ContextProvider` (i.e. to the hard-coded enum), and then the call-chain under that requires that the `provider` be of the `cxschema.ContextProvider` type. For a plugable `ContextProvider` to work, all of the `providers` in this call-chain would have to have their type changed to `string`. I don't *think* that this will negatively impact the `CloudAssembly` -- the value of the `provider` is a string in the `cdk.out` json file, so changing it to a string internally should not impact that. | ||
|
||
# Drawbacks | ||
|
||
This change would alter the internal API in that some fields that are currently defined as `cxschema.ContextProvider` would need to change to `string` type. In practice, this means that the `ContextProvider` plugins would have a hard requirement of requiring at minimum a specific version of the `@aws-cdk/cloud-assembly-schema` and `@aws-cdk/core` modules. | ||
|
||
# Rationale and Alternatives | ||
|
||
Other options may be possible. This is one proposal by someone not intimately familiar with the internals of the CDK toolchain. | ||
|
||
> - Why is this design the best in the space of possible designs? | ||
> - What other designs have been considered and what is the rationale for not | ||
> choosing them? | ||
> - What is the impact of not doing this? | ||
# Adoption Strategy | ||
|
||
> If we implement this proposal, how will existing CDK developers adopt it? Is | ||
> this a breaking change? How can we assist in adoption? | ||
# Unresolved questions | ||
|
||
* Is there a precidence for coupling a minumum version of the CDK CLI with the a minimum version of CDK construct library? | ||
|
||
# Future Possibilities | ||
|
||
> Think about what the natural extension and evolution of your proposal would be | ||
> and how it would affect CDK as whole. Try to use this section as a tool to more | ||
> fully consider all possible interactions with the project and ecosystem in your | ||
> proposal. Also consider how this fits into the roadmap for the project. | ||
> | ||
> This is a good place to "dump ideas", if they are out of scope for the RFC you | ||
> are writing but are otherwise related. | ||
> | ||
> If you have tried and cannot think of any future possibilities, you may simply | ||
> state that you cannot think of anything. |