chore(s3): additional documentation for addToResourcePolicy. (aws#15761)

This is to clarify that in some cases the policy will not be added and the result should be checked.

Closes aws#6548 and aws#7370.

----

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

packages/@aws-cdk/aws-s3/README.md

@@ -92,13 +92,36 @@ A bucket policy will be automatically created for the bucket upon the first call
 
 ```ts
 const bucket = new Bucket(this, 'MyBucket');
-bucket.addToResourcePolicy(new iam.PolicyStatement({
+const result = bucket.addToResourcePolicy(new iam.PolicyStatement({
   actions: ['s3:GetObject'],
   resources: [bucket.arnForObjects('file.txt')],
   principals: [new iam.AccountRootPrincipal()],
 }));
 ```
 
+If you try to add a policy statement to an existing bucket, this method will 
+not do anything:
+
+```ts
+const bucket = Bucket.fromBucketName(this, 'existingBucket', 'bucket-name');
+
+// Nothing will change here
+const result = bucket.addToResourcePolicy(new iam.PolicyStatement({
+  ...
+}));
+```
+
+That's because it's not possible to tell whether the bucket 
+already has a policy attached, let alone to re-use that policy to add more 
+statements to it. We recommend that you always check the result of the call:
+
+```ts
+const result = bucket.addToResourcePolicy(...)
+if (!result.statementAdded) {
+  // Uh-oh! Someone probably made a mistake here.
+}
+```
+
 The bucket policy can be directly accessed after creation to add statements or
 adjust the removal policy.
 
@@ -150,7 +173,7 @@ const bucket = Bucket.fromBucketAttributes(this, 'ImportedBucket', {
 });
 
 // now you can just call methods on the bucket
-bucket.grantReadWrite(user);
+bucket.addEventNotification(EventType.OBJECT_CREATED, ...);
 ```
 
 Alternatively, short-hand factories are available as `Bucket.fromBucketName` and

packages/@aws-cdk/aws-s3/lib/bucket.ts

@@ -82,9 +82,23 @@ export interface IBucket extends IResource {
 
   /**
    * Adds a statement to the resource policy for a principal (i.e.
-   * account/role/service) to perform actions on this bucket and/or it's
+   * account/role/service) to perform actions on this bucket and/or its
    * contents. Use `bucketArn` and `arnForObjects(keys)` to obtain ARNs for
    * this bucket or objects.
+   *
+   * Note that the policy statement may or may not be added to the policy.
+   * For example, when an `IBucket` is created from an existing bucket,
+   * it's not possible to tell whether the bucket already has a policy
+   * attached, let alone to re-use that policy to add more statements to it.
+   * So it's safest to do nothing in these cases.
+   *
+   * @param permission the policy statement to be added to the bucket's
+   * policy.
+   * @returns metadata about the execution of this method. If the policy
+   * was not added, the value of `statementAdded` will be `false`. You
+   * should always check this value to make sure that the operation was
+   * actually carried out. Otherwise, synthesis and deploy will terminate
+   * silently, which may be confusing.
    */
   addToResourcePolicy(permission: iam.PolicyStatement): iam.AddToResourcePolicyResult;
 
@@ -546,9 +560,23 @@ export abstract class BucketBase extends Resource implements IBucket {
 
   /**
    * Adds a statement to the resource policy for a principal (i.e.
-   * account/role/service) to perform actions on this bucket and/or it's
+   * account/role/service) to perform actions on this bucket and/or its
    * contents. Use `bucketArn` and `arnForObjects(keys)` to obtain ARNs for
    * this bucket or objects.
+   *
+   * Note that the policy statement may or may not be added to the policy.
+   * For example, when an `IBucket` is created from an existing bucket,
+   * it's not possible to tell whether the bucket already has a policy
+   * attached, let alone to re-use that policy to add more statements to it.
+   * So it's safest to do nothing in these cases.
+   *
+   * @param permission the policy statement to be added to the bucket's
+   * policy.
+   * @returns metadata about the execution of this method. If the policy
+   * was not added, the value of `statementAdded` will be `false`. You
+   * should always check this value to make sure that the operation was
+   * actually carried out. Otherwise, synthesis and deploy will terminate
+   * silently, which may be confusing.
    */
   public addToResourcePolicy(permission: iam.PolicyStatement): iam.AddToResourcePolicyResult {
     if (!this.policy && this.autoCreatePolicy) {
@@ -720,6 +748,9 @@ export abstract class BucketBase extends Resource implements IBucket {
    *     const grant = bucket.grantPublicAccess();
    *     grant.resourceStatement!.addCondition(‘IpAddress’, { “aws:SourceIp”: “54.240.143.0/24” });
    *
+   * Note that if this `IBucket` refers to an existing bucket, possibly not
+   * managed by CloudFormation, this method will have no effect, since it's
+   * impossible to modify the policy of an existing bucket.
    *
    * @param keyPrefix the prefix of S3 object keys (e.g. `home/*`). Default is "*".
    * @param allowedActions the set of S3 actions to allow. Default is "s3:GetObject".