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

S3 MFA-Delete and Object Lock support #5247

Open
SolDavidCloud opened this issue Nov 28, 2019 · 9 comments
Open

S3 MFA-Delete and Object Lock support #5247

SolDavidCloud opened this issue Nov 28, 2019 · 9 comments
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@SolDavidCloud
Copy link

I don't see how to enforce MFA Delete and Object Locking in an S3 Bucket.

I can't find MFA-Delete in CloudFormation, but ObjectLock is:

Type: AWS::S3::Bucket
Properties: 
  ObjectLockConfiguration: 
    ObjectLockConfiguration
  ObjectLockEnabled: Boolean

Use Case

Define Buckets with MFA-Delete and Object Lock enabled from the start.

Proposed Solution

Add the properties to the S3.Bucket object.
Workaround: Doing it manually after creation.

Other

Maybe it already exists, or another workaround exists, and I didn't find it. Thanks.


This is a 🚀 Feature Request

@SolDavidCloud SolDavidCloud added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 28, 2019
@SomayaB SomayaB added the @aws-cdk/aws-s3 Related to Amazon S3 label Nov 29, 2019
@eladb eladb assigned iliapolo and unassigned eladb Jan 23, 2020
@iliapolo
Copy link
Contributor

iliapolo commented Mar 9, 2020

@Corsario-Mexico Thanks for reporting this.

Until we push a fix for this, have a look at the Modifying the AWS CloudFormation Resource behind AWS Constructs section in the Escape Hatches Guide.

This allows you to set any CFN supported property on your resource even if it isn't yet supported by the encompassing CDK L2 construct. It is meant as a workaround for exactly these types of scenarios.

@iliapolo iliapolo added the effort/small Small work item – less than a day of effort label Mar 9, 2020
@iliapolo
Copy link
Contributor

iliapolo commented Mar 9, 2020

@Corsario-Mexico Regarding MFA delete, thats a bit more tricky because you are right, there is no official CloudFormation support for that.

This needs to be implemented using custom resources. We will put this on the docket.

btw, if you'd like to contribute this feature that would be awesome. We will of course provide support and guidance all the way. Just let us know :)

@iliapolo iliapolo added effort/medium Medium work item – several days of effort and removed effort/small Small work item – less than a day of effort labels Mar 9, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label May 19, 2020
@iliapolo iliapolo added the p2 label Aug 29, 2020
@iliapolo iliapolo removed their assignment Jun 27, 2021
@ryparker
Copy link
Contributor

ryparker commented Sep 29, 2021

Came across this issue when looking for ways to configure Object Lock. Here's my workaround that uses escape hatches:

import { Bucket, CfnBucket } from '@aws-cdk/aws-s3'

const bucket = new Bucket(stack, 'Bucket', {
  ...
  versioned: true, // Bucket versioning is required when enabling ObjectLock
});

// Get the CloudFormation resource
const cfnBucket = bucket.node.defaultChild as CfnBucket;
// Add the ObjectLockConfiguration prop to the Bucket's CloudFormation output.
cfnBucket.addPropertyOverride('ObjectLockConfiguration.ObjectLockEnabled', 'Enabled');

ObjectLockConfiguration CloudFormation docs

@trompx
Copy link

trompx commented Jan 19, 2022

Hey @ryparker,

The problem is that object lock must be set when the bucket is created.

By doing what you suggests, the addPropertyOverride applies on an already existing bucket, thus failing with the following error:
Object Lock configuration cannot be enabled on existing buckets (Service: Amazon S3; Status Code: 409; Error Code: InvalidBucketState; Request ID: XXX; S3 Extended Request ID: YYY=; Proxy: null)

Or maybe I am doing something wrong...

EDIT:
@iliapolo is there a way to create the CfnBucket (L1 construct) then augments its properties with a Bucket (L2 construct) ? In fact the only way I see to have object lock is to first create it with CfnBucket, but then we cannot use other useful features brought by s3.Bucket L2 construct like autoDeleteObjects.

@ryparker
Copy link
Contributor

ryparker commented Jan 19, 2022

If you're importing the bucket using a method such as Bucket.fromBucketName() then you wouldn't be able to change the resource at all. This applies to all resources that are imported into CDK not just Buckets.

Although you can use an imported resource anywhere, you cannot modify the imported resource.

importing resources in CDK


If you're creating a new bucket in your CDK code but already deployed without the object lock override then an attempt to update your CDK code with this override will fail with the mentioned error:

Object Lock configuration cannot be enabled on existing buckets

As far as I know your only option is to delete the bucket and redeploy with the desired property overrides.

@trompx
Copy link

trompx commented Jan 21, 2022

Thanks for your input @ryparker
But I tried again to cdk deploy after a cdk destroy, making sure the buckets were not existing, with the following setup and am still having the error.

const bucket = new s3.Bucket(this, id, {
  bucketName: bucketName,
  blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
  versioned: true,
  publicReadAccess: false,
  autoDeleteObjects: isDev ? true : false,
});
const cfnBucket = bucket.node.defaultChild as s3.CfnBucket;
cfnBucket.addPropertyOverride(
  "ObjectLockConfiguration.ObjectLockEnabled",
  "Enabled"
);

@EugRomanchenko
Copy link

EugRomanchenko commented Jan 28, 2022

Thanks for your input @ryparker But I tried again to cdk deploy after a cdk destroy, making sure the buckets were not existing, with the following setup and am still having the error.

const bucket = new s3.Bucket(this, id, {
  bucketName: bucketName,
  blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
  versioned: true,
  publicReadAccess: false,
  autoDeleteObjects: isDev ? true : false,
});
const cfnBucket = bucket.node.defaultChild as s3.CfnBucket;
cfnBucket.addPropertyOverride(
  "ObjectLockConfiguration.ObjectLockEnabled",
  "Enabled"
);

One more parameter overwrite is missing to enable Object Lock (https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.CfnBucketProps.html):

cfnBucket.addPropertyOverride(
  "ObjectLockEnabled",
  true
);
cfnBucket.addPropertyOverride(
  "ObjectLockConfiguration.ObjectLockEnabled",
  "Enabled"
);

@zachgoll
Copy link

zachgoll commented Dec 9, 2022

To summarize the current working solution from all the threads above:

(I've also added a retention period - seems like the CDK stack will get "stuck" without this config)

import { Bucket, CfnBucket } from 'aws-cdk-lib/aws-s3'

const bucket = new Bucket(this, 'ObjLockExampleBucket', {
    versioned: true
})

const bucketResource = bucket.node.defaultChild as CfnBucket
bucketResource.addPropertyOverride('ObjectLockEnabled', true)
bucketResource.addPropertyOverride('ObjectLockConfiguration.ObjectLockEnabled', 'Enabled')

// Can be `Years` or `Days`, not both (integer value)
bucketResource.addPropertyOverride('ObjectLockConfiguration.Rule.DefaultRetention.Years', 5)

// Can be `GOVERNANCE` or `COMPLIANCE` - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-defaultretention.html
bucketResource.addPropertyOverride('ObjectLockConfiguration.Rule.DefaultRetention.Mode', 'GOVERNANCE')

mergify bot pushed a commit that referenced this issue Feb 6, 2023
S3 Object Lock allows configuring various retention holds, for legal and compliance purposes, on an S3 bucket. This enables a write-once-read-many model. Object Lock can only be enabled on new buckets via the CloudFormation (and therefore via the CDK). Updates to an existing bucket will result in a CloudFormation update failure.

This behavior is possible today using Escape Hatches to modify the L1 construct (with the same limitations):

```ts
cfnBucket.addPropertyOverride("ObjectLockEnabled", true);
```

Providing L2 wrappers around this configuration can aleviate some common and easy-to-make mistakes, such as providing `ObjectLockConfiguration` without providing `ObjectLockEnabled` or specifying `"Governance"` instead of `"GOVERNANCE"` for the compliance mode.

It is possible to enable Object Lock without specifying a default duration. Therefore, there needs to be a means to set `ObjectLockEnabled`. This is done with the `ObjectLoc.enabled` property. Since this is a boolean, it can theoretically be set to `false`. If `false` and a `defaultRetention` is provided, an error is thrown.

CloudFormation allows specifying `Days` or `Years` for retention; for simplicity, this implementation always converts to `Days`. Because CloudFormation requires that to be a positive integer, this implementation also proactively performs that validation at synthesis time.

Further, CloudFormation does not allow omitting `ObjectLockEnabled` within `ObjectLockConfiguration`. The following template would result in a validation error that the input does not match the schema:

```yaml
Bucket:
  Type: AWS::S3::Bucket
  Properties:
    ObjectLockEnabled: true
    ObjectLockConfiguration:
      Rule:
        DefaultRetention:
          Days: 1
          Mode: GOVERNANCE
```

Therefore, this implementation also always sets
`ObjectLockConfiguration.ObjectLockEnabled` to `"Enabled"`.

Additionally, it seems that the behavior of doing

```yaml
Bucket:
  Type: AWS::S3::Bucket
  Properties:
    ObjectLockEnabled: true
    ObjectLockConfiguration:
      ObjectLockEnabled: 'Enabled'
```

causes CloudFormation to create the buckets with Object Lock enabled and then just wait and wait and wait. Frankly I didn't wait for the operation to time out so I don't know whether that would succeed or fail, but in any case, that would be a duplicate of specifying only `ObjectLockEnabled: true` (without nested in `ObjectLockConfiguration`) so this implementation prefers the shorter variant, which CloudFormation/S3 also seem to prefer, when Object Lock is enabled without default retention.

Unfortunately, there isn't a way to check during synthesis whether the bucket already exists, so there's not really a way to detect that pitfall. Users will just get the typical CloudFormation error for this situation and a stack rollback.

More variants of Object Lock configuration in S3 and descriptions of what CloudFormation does with them can be found at: https://gist.github.com/788df029f121af14645f31152ff54e32

This _partially_ addresses #5247 (nothing here handles MFA delete).
This follows up on #21738 which has been marked as abandoned.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

As of v2.64.0 you can now enable object lock using:

new s3.Bucket(stack, 'Bucket', {
  objectLockEnabled: true,
});

or

new s3.Bucket(stack, 'Bucket', {
  objectLockDefaultRetention: s3.ObjectLockRetention.governance(cdk.Duration.days(5 * 365)),
});

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3-readme.html#object-lock-configuration

It looks like MFA Delete is still not supported by the underlying CloudFormation resources (so no progress there).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants