winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.75k stars 187 forks source link

Missing properties for Winglibs/DynamoDB #6753

Open thoroc opened 1 week ago

thoroc commented 1 week ago

Use Case

I am trying to replicate work done in AWS CDK, and I have found that the following properties are not present on the equivalent in winglibs:

import { RemovalPolicy } from 'aws-cdk-lib'
import { BillingMode } from 'aws-cdk-lib/aws-dynamodb'

new Table(this, 'MyTable', {
  tableName: 'MyTable',
  partitionKey: { name: 'id', type: AttributeType.STRING },
  billingMode: BillingMode.PAY_PER_REQUEST, // no equivalent in winglibs/dynamodb
  removalPolicy: RemovalPolicy.RETAIN, // no equivalent in winglibs/dynamodb
  pointInTimeRecovery: true,
  deletionProtection: true, // no equivalent in winglibs/dynamodb
});

Those are properties set in production, and without them implemented, it is a non starter for adoption atm.

Proposed Solution

No response

Implementation Notes

No response

Component

Libraries

Community Notes

Chriscbr commented 1 week ago

Thanks for bringing this to our attention @thoroc! I think the ideal fix would be to add these missing properties to the TableProps type in @winglibs/dynamodb here and handle the new props in the implementation accordingly.

Pull requests are welcome!

thoroc commented 1 week ago

@Chriscbr I am open to create a PR for that, but I might need some guidance for the implementation.

Chriscbr commented 1 week ago

That would be great - I'll assign the issue to you.

The simulator implementation of the Table class uses DynamoDB local (a version of DynamoDB you can run on a container), which ought to support most of the same props as the AWS Terraform provider. For any of the props that expect an enum like BillingMode or RemovalPolicy, I might suggest defining them as new enums in https://github.com/winglang/winglibs/blob/23d298a87242f793eaa2203f3e39fe1c3048dcc5/dynamodb/dynamodb-types.w to avoid taking on a dependency on aws-cdk-lib.

Happy to help provide any more pointers as needed 👍

thoroc commented 1 week ago

Just to confirm, I cannot see RemovalPolicy or the DeletionProtection in the cdktf-aws: https://github.com/hashicorp/cdktf-aws-cdk/blob/main/docs/dynamodbTable.md. Am I looking at the wrong place?

thoroc commented 1 week ago

Just opened a draft PR, but I need to investigate a bit more terraform to see if the options are available: https://github.com/winglang/winglibs/pull/272

Chriscbr commented 1 week ago

Ah - RemovalPolicy is an idiom specific to AWS CDK because it's related to CloudFormation specific behavior. See here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html.

I'm not sure there's a clear way to map this into the Terraform implementation. Terraform has a "lifecycle" hook that lets you specify that a resource shouldn't be destroyed, but it's not as full featured:

resource "aws_s3_bucket" "MyPreciousBucket" {
  lifecycle {
    prevent_destroy = true
  }
}

One option could be to say that if deletionProtection is enabled, then we should also add that to your Terraform. It could be added like this:

new tfaws.dynamodbTable.DynamodbTable({
  ...
  lifecycle: {
    preventDestroy: true,
  }
});

Deletion protection should be available (I see it listed in the Terraform Provider [here](https://registry.terraform.io/providers/hashicorp/aws/latest/doc. It's not available in the cdktf-aws-cdk repo you linked but it is in the cdktf-provider-aws repo - see here: https://github.com/cdktf/cdktf-provider-aws/blob/main/docs/dynamodbTable.typescript.md#dynamodbtableconfig-

thoroc commented 1 week ago

Deletion protection should be available (I see it listed in the Terraform Provider

I was just playing with cdktf locally and I can confirm there is a props that seems to map to the same in aws cdk: deletionProtectionEnabled

I'll have a look at the solution you've suggested for removalPolicy.

thoroc commented 1 week ago

Just a question about testing. I am unsure how I would go about unit testing the new properties here.

For the tf side of things, should I expect a synthesised tf template?

For the sim side of things, I need to think about it.

Chriscbr commented 1 week ago

Yeah, that's a good question. I think the best option on the cloud side would be if we could just make some assertions about the configuration that's generated, similar to https://developer.hashicorp.com/terraform/cdktf/test/unit-tests#write-assertions. I'm not sure if it's possible to reuse these utilities in Wing at the moment though.

On the sim side maybe you can call the DescribeTable API inflight to confirm that the properties are set?