From 08ec261a3cd34e7225f471cbeab8cef605ac61fc Mon Sep 17 00:00:00 2001 From: Geoff Baskwill Date: Sun, 23 Feb 2020 16:45:14 -0500 Subject: [PATCH] feat: Support `resource.extensions` for safe resource extensions (#7352) Merge logic for extending resources with the existing framework using `resources.Resources` has some surprising behaviours. This introduces a more formalized definition for extending resources, where `Metadata` and `Properties` will be shallowly merged (the values for existing keys will be replaced instead of merged), the `DependsOn` list will be added to, and the values for `CreationPolicy`, `DeletionPolicy`, `UpdatePolicy`, and `UpdateReplacePolicy` will be replaced if they exist in the extension. Extension of other resource attributes is not supported at this time. --- docs/providers/aws/guide/resources.md | 24 ++- docs/providers/aws/guide/serverless.yml.md | 7 + .../lib/mergeCustomProviderResources.js | 61 ++++++ .../lib/mergeCustomProviderResources.test.js | 178 ++++++++++++++++++ 4 files changed, 266 insertions(+), 4 deletions(-) diff --git a/docs/providers/aws/guide/resources.md b/docs/providers/aws/guide/resources.md index 11e2a7331..ecca627ba 100644 --- a/docs/providers/aws/guide/resources.md +++ b/docs/providers/aws/guide/resources.md @@ -48,7 +48,9 @@ resources: # CloudFormation template syntax WriteCapacityUnits: 1 ``` -You can overwrite/attach any kind of resource to your CloudFormation stack. You can add `Resources`, `Outputs` or even overwrite the `Description`. You can also use [Serverless Variables](./variables.md) for sensitive data or reusable configuration in your resources templates. Please be cautious as overwriting existing parts of your CloudFormation stack might introduce unexpected behavior. +You can attach any kind of resource to your CloudFormation stack. You can add `Resources`, `Outputs`. You can also use [Serverless Variables](./variables.md) for sensitive data or reusable configuration in your resources templates. + +Note: By suppling your resources at `resources.Resources` you may accidentally override resources as generated by the framework. To intentionally extend such resources, please use `resources.extensions`, see [Override AWS CloudFormation Resource](#override-aws-cloudformation-resource) section for more info. ## AWS CloudFormation Resource Reference @@ -95,7 +97,7 @@ If you are unsure how a resource is named, that you want to reference from your ## Override AWS CloudFormation Resource -You can override the specific CloudFormation resource to apply your own options. For example, if you want to set `AWS::Logs::LogGroup` retention time to 30 days, override it with above table's `Name Template`. +You can override the specific CloudFormation resource to apply your own options (place all such extensions at `resources.extensions` section). For example, if you want to set `AWS::Logs::LogGroup` retention time to 30 days, override it with above table's `Name Template`. When you override basic resources, there are two things to keep in mind when it comes to `normalizedFunctionName`: @@ -115,9 +117,23 @@ functions: cors: true resources: - Resources: + extensions: WriteDashPostLogGroup: - Type: AWS::Logs::LogGroup Properties: RetentionInDays: '30' ``` + +Here's how the extension logic is defined: + +| Resource attribute | Operation | +| --------------------- | -------------------------------------------------------------------------------------------------------------------------- | +| `CreationPolicy` | Set to extension value if present. | +| `DeletionPolicy` | Set to extension value if present. | +| `DependsOn` | Merge. The extension value will be added to the resource's `DependsOn` list. | +| `Metadata` | Merge. If a metadata key with the same name exists in the resource, the value will be _replaced_ with the extension value. | +| `Properties` | Merge. If a property with the same name exists in the resource, the value will be _replaced_ with the extension value. | +| `UpdatePolicy` | Set to extension value if present. | +| `UpdateReplacePolicy` | Set to extension value if present. | +| _other_ | Not supported. An error will be thrown if you try to extend an unsupported attribute. | + +Extending using `resources.extensions` only works on the `Resources` part of the CloudFormation template. diff --git a/docs/providers/aws/guide/serverless.yml.md b/docs/providers/aws/guide/serverless.yml.md index a85ad0a4c..e208abadd 100644 --- a/docs/providers/aws/guide/serverless.yml.md +++ b/docs/providers/aws/guide/serverless.yml.md @@ -413,6 +413,13 @@ resources: ProvisionedThroughput: ReadCapacityUnits: 1 WriteCapacityUnits: 1 + extensions: + # override Properties or other attributes of Framework-created resources. + # See https://serverless.com/framework/docs/providers/aws/guide/resources#override-aws-cloudformation-resource for more details + UsersCreateLogGroup: + Properties: + RetentionInDays: '30' + # The "Outputs" that your AWS CloudFormation Stack should produce. This allows references between services. Outputs: UsersTableArn: diff --git a/lib/plugins/aws/package/lib/mergeCustomProviderResources.js b/lib/plugins/aws/package/lib/mergeCustomProviderResources.js index 414b098f4..61d1bc10f 100644 --- a/lib/plugins/aws/package/lib/mergeCustomProviderResources.js +++ b/lib/plugins/aws/package/lib/mergeCustomProviderResources.js @@ -12,11 +12,72 @@ module.exports = { this.serverless.service.resources.Outputs = {}; } + const extensions = this.serverless.service.resources + ? this.serverless.service.resources.extensions + : null; + if (extensions) { + delete this.serverless.service.resources.extensions; + } + _.merge( this.serverless.service.provider.compiledCloudFormationTemplate, this.serverless.service.resources ); + if (extensions) { + const template = this.serverless.service.provider.compiledCloudFormationTemplate; + + for (const [resourceName, resourceDefinition] of _.entries(extensions)) { + for (const [extensionAttributeName, value] of _.entries(resourceDefinition)) { + if (!template.Resources[resourceName]) { + template.Resources[resourceName] = {}; + } + + switch (extensionAttributeName) { + case 'CreationPolicy': + case 'DeletionPolicy': + case 'UpdatePolicy': + case 'UpdateReplacePolicy': + template.Resources[resourceName][extensionAttributeName] = value; + break; + + case 'Properties': + if (!template.Resources[resourceName].Properties) { + template.Resources[resourceName].Properties = {}; + } + + Object.assign(template.Resources[resourceName].Properties, value); + break; + + case 'DependsOn': + if (!template.Resources[resourceName].DependsOn) { + template.Resources[resourceName].DependsOn = []; + } + + template.Resources[resourceName].DependsOn.push(...value); + break; + + case 'Metadata': + if (!template.Resources[resourceName].Metadata) { + template.Resources[resourceName].Metadata = {}; + } + + Object.assign(template.Resources[resourceName].Metadata, value); + break; + + // default includes any future attributes we don't know about yet. + default: + throw new Error( + `${resourceName}: Sorry, extending the ${extensionAttributeName} resource ` + + 'attribute at this point is not supported. Feel free to propose support ' + + 'for it in the Framework issue tracker: ' + + 'https://github.com/serverless/serverless/issues' + ); + } + } + } + } + return BbPromise.resolve(); }, }; diff --git a/lib/plugins/aws/package/lib/mergeCustomProviderResources.test.js b/lib/plugins/aws/package/lib/mergeCustomProviderResources.test.js index 8ecfc7b97..1ebd3a1a7 100644 --- a/lib/plugins/aws/package/lib/mergeCustomProviderResources.test.js +++ b/lib/plugins/aws/package/lib/mergeCustomProviderResources.test.js @@ -170,5 +170,183 @@ describe('mergeCustomProviderResources', () => { ).to.deep.equal(coreCloudFormationTemplate.Outputs.ServerlessDeploymentBucketName); }); }); + + it('should create non-existent resource / attributes for resources.extensions.*', () => { + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources = {}; + + awsPackage.serverless.service.resources = { + extensions: { + SampleResource: { + Properties: { + PropertyA: 'new', + }, + DependsOn: ['new'], + Metadata: { + key: 'value', + anotherKey: { + key2: 'value2', + }, + }, + }, + }, + }; + + return awsPackage.mergeCustomProviderResources().then(() => { + expect(awsPackage.serverless.service.provider.compiledCloudFormationTemplate.extensions).to + .not.exist; + + expect( + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources + ).to.deep.equal({ + SampleResource: { + Properties: { + PropertyA: 'new', + }, + DependsOn: ['new'], + Metadata: { + key: 'value', + anotherKey: { + key2: 'value2', + }, + }, + }, + }); + }); + }); + + it('should overwrite for resources.extensions.*.{CreationPolicy,DeletionPolicy,UpdatePolicy,UpdateReplacePolicy}', () => { + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources = { + SampleResource: { + CreationPolicy: { + AutoScalingCreationPolicy: { + MinSuccessfulInstancesPercent: 10, + }, + ResourceSignal: { + Count: 3, + Timeout: 'PT5M', + }, + }, + DeletionPolicy: 'Retain', + UpdatePolicy: { + AutoScalingReplacingUpdate: { + WillReplace: false, + }, + }, + UpdateReplacePolicy: 'Retain', + }, + }; + + // note: it's quite likely that these test values don't make sense; it's up to the user + // to provide the values they want. This just verifies that the properties are overwritten + // as documented. + awsPackage.serverless.service.resources = { + extensions: { + SampleResource: { + CreationPolicy: { + ResourceSignal: { + Count: 3, + Timeout: 'PT5M', + }, + }, + DeletionPolicy: 'Snapshot', + UpdatePolicy: {}, + UpdateReplacePolicy: 'Snapshot', + }, + }, + }; + + return awsPackage.mergeCustomProviderResources().then(() => { + expect( + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources + .SampleResource + ).to.deep.equal({ + CreationPolicy: { + ResourceSignal: { + Count: 3, + Timeout: 'PT5M', + }, + }, + DeletionPolicy: 'Snapshot', + UpdatePolicy: {}, + UpdateReplacePolicy: 'Snapshot', + }); + }); + }); + + it('should merge with overwrite for resources.extensions.*.Properties', () => { + // this shows that PropertyA will get overwritten, not merged + // and both PropertyB and PropertyC will exist in the final result + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources = { + SampleResource: { + Properties: { + PropertyA: { an: 'object' }, + PropertyB: 'b', + }, + }, + }; + + awsPackage.serverless.service.resources = { + extensions: { + SampleResource: { + Properties: { + PropertyA: { another: 'object' }, + PropertyC: 'new', + }, + }, + }, + }; + + return awsPackage.mergeCustomProviderResources().then(() => { + expect( + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources + .SampleResource.Properties + ).to.deep.equal({ + PropertyA: { another: 'object' }, + PropertyB: 'b', + PropertyC: 'new', + }); + }); + }); + + it('should append for resources.extensions.*.DependsOn', () => { + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources = { + SampleResource: { + DependsOn: ['a'], + }, + }; + + awsPackage.serverless.service.resources = { + extensions: { + SampleResource: { + DependsOn: ['b'], + }, + }, + }; + + return awsPackage.mergeCustomProviderResources().then(() => { + expect( + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources + .SampleResource.DependsOn + ).to.deep.equal(['a', 'b']); + }); + }); + + it('should throw error for unsupported resources.extensions.*.*', () => { + awsPackage.serverless.service.provider.compiledCloudFormationTemplate.Resources = { + SampleResource: {}, + }; + + awsPackage.serverless.service.resources = { + extensions: { + SampleResource: { + unsupported: 'property', + }, + }, + }; + + expect(() => awsPackage.mergeCustomProviderResources()).to.throw( + /SampleResource: Sorry, extending the unsupported resource attribute at this point is not supported/ + ); + }); }); });