Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQS: support bucket notifications #564

Merged
merged 1 commit into from
Aug 14, 2018
Merged

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Aug 14, 2018

Allow sqs.Queue to be used as a destination for bucket notifications. Also supports queues that are server-side encrypted using a custom KMS key.

Added support for specifying KMS keys in QueueRef, including export/import support.

Added support to kms.EncryptionKey to allow forbidding "no-op" when adding policy statements. This is needed in order to fail the user from subscribing a queue to receive bucket notifications without the ability to allow S3 to read the confirmation messages from the queue as described
in the S3 documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

Allow sqs.Queue to be used as a destination for bucket notifications.
Also supports queues that are server-side encrypted using a custom KMS key.

Added support for specifying KMS keys in `QueueRef`, including export/import support.

Added support to `kms.EncryptionKey` to allow forbidding "no-op" when
adding policy statements. This is needed in order to fail the user from
subscribing a queue to receive bucket notifications without the ability
to allow S3 to read the confirmation messages from the queue as described
in the S3 documentation [1]



[1]: https://docs.aws.amazon.com/AmazonS3/latest/dev/ways-to-add-notification-config-to-bucket.html
@@ -1,4 +1,4 @@
import { Construct, DeletionPolicy, Output, PolicyDocument, PolicyStatement } from '@aws-cdk/cdk';
import { Construct, DeletionPolicy, Output, PolicyDocument, PolicyStatement, resolve } from '@aws-cdk/cdk';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point I would import cdk = require('@aws-cdk/cdk');.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll forgive me - I forgot to push this change and merged. We'll clean this up later.

@@ -54,10 +54,15 @@ export abstract class EncryptionKeyRef extends Construct {

/**
* Adds a statement to the KMS key resource policy.
* @param statement The policy statement to add
* @param allowNoOp If this is set to `false` and there is no policy
* defined (i.e. external key), the operation will fail. Otherwise, it will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the text for the @param, or your Sphinx output will be all funked-up!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What?! how is this related to my sphinx output?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my sphinx output:

   .. py:method:: addToResourcePolicy(statement, [allowNoOp])

      Adds a statement to the KMS key resource policy.


      :param statement: The policy statement to add
      :type statement: :py:class:`@aws-cdk/cdk.PolicyStatement`
      :param allowNoOp: If this is set to `false` and there is no policy defined (i.e. external key), the operation will fail. Otherwise, it will no-op.
      :type allowNoOp: boolean or undefined

Looks fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. I had bunch of cases where this generated invalid rst. Maybe typescript changed how it parses jsdoc tags?

@eladb eladb merged commit 9699428 into master Aug 14, 2018
@eladb eladb deleted the benisrae/sqs-bucket-notifications branch August 14, 2018 17:00
rix0rrr added a commit that referenced this pull request Aug 15, 2018
### Features

* __@aws-cdk/cdk__: Tokens can now be transparently embedded into
  strings and encoded into JSON without losing their semantics. This
  makes it possible to treat late-bound (deploy-time) values as if they
  were regular strings ([@rix0rrr] in
  [#518](#518)).
* __@aws-cdk/aws-s3__: add support for bucket notifications to Lambda,
  SNS, and SQS targets ([@eladb] in
  [#201](#201),
  [#560](#560),
  [#561](#561),
  [#564](#564))
* __@aws-cdk/cdk__: non-alphanumeric characters can now be used as
  construct identifiers ([@eladb] in
  [#556](#556))
* __@aws-cdk/aws-iam__: add support for `maxSessionDuration` for Roles
  ([@eladb] in [#545](#545)).

### Changes

* __@aws-cdk/aws-lambda__ (_**BREAKING**_): most classes renamed to be
  shorter and more in line with official service naming (`Lambda`
  renamed to `Function` or ommitted) ([@eladb] in
  [#550](#550))
* __@aws-cdk/aws-codepipeline__ (_**BREAKING**_): move all CodePipeline
  actions from `@aws-cdk/aws-xxx-codepipeline` packages into the regular
  `@aws-cdk/aws-xxx` service packages ([@skinny85] in
  [#459](#459)).
* __@aws-cdk/aws-custom-resources__ (_**BREAKING**_): package was
  removed, and the Custom Resource construct added to the
  __@aws-cdk/aws-cloudformation__ package ([@rix0rrr] in
  [#513](#513))

### Fixes

* __@aws-cdk/aws-lambda__: Lambdas that are triggered by CloudWatch
  Events now show up in the console, and can only be triggered the
  indicated Event Rule. _**BREAKING**_ for middleware writers (as this
  introduces an API change), but transparent to regular consumers
  ([@eladb] in [#558](#558))
* __@aws-cdk/aws-codecommit__: fix a bug where `pollForSourceChanges`
  could not be set to `false` ([@maciejwalkowiak] in
  [#534](#534))
* __aws-cdk__: don't fail if the `~/.aws/credentials` file is missing
  ([@RomainMuller] in
  [#541](#541))
* __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions
  to correctly support TemplateConfiguration ([@mindstorms6] in
  [#571](#571)).
* __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions
  to correctly support ParameterOverrides ([@mindstorms6] in
  [#574](#574)).

### Known Issues

* `cdk init` will try to init a `git` repository and fail if no global
  `user.name` and `user.email` have been configured.
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants