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

aws-cloudfront needs love #572

Closed
eladb opened this issue Aug 15, 2018 · 5 comments
Closed

aws-cloudfront needs love #572

eladb opened this issue Aug 15, 2018 · 5 comments
Assignees
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront effort/large Large work item – several weeks of effort feature-request A feature should be added or improved.

Comments

@eladb
Copy link
Contributor

eladb commented Aug 15, 2018

We should redesign the API for the aws-cloudfront construct library. There's a lot we can do in order to make it friendlier, less error-prone and less flat and declarative. At the moment, it's basically "leaking" the low level API provided by CloudFormation.

Furthermore, it would be awesome if one could use Assets to point to a local directory that includes the web distribution content and have the toolkit do the work of uploading it to S3 and all that.

@mindstorms6
Copy link
Contributor

Internally, maybe take a look at BONESConstructs - it's got a mildly less bad wrapper around this. I can port some of that out if you find it useful

@PaulMaddox
Copy link
Contributor

For reference, this is what the current API looks like just to setup a simple SPA/PWA:

const bucket = new s3.Bucket(this, 'WebsiteBucket', {
    bucketName: 'pmaddox-website-bucket',
    versioned: true,
});

const originId = new cloudfront.cloudformation.CloudFrontOriginAccessIdentityResource(this, 'OriginAccessIdentity', {
    cloudFrontOriginAccessIdentityConfig: {
        comment: 'A comment to associate with this CloudFront origin access identity',
    }
});

const cert = acm.Certificate.import(this, 'ExistingCertificate', {
    certificateArn: new acm.CertificateArn('arn:aws:acm:us-east-1:123456789012:certificate/7d3591e4-de7f-45cf-f8f5-7cc3d1732930'),
});

const distribution = new cloudfront.CloudFrontWebDistribution(this, 'WebsiteDistribution', {
    aliasConfiguration: {
        names: ['my-website.com', 'www.my-website.com'],
        acmCertRef: cert.certificateArn,
    },
    originConfigs: [{
        behaviors: [{
            isDefaultBehavior: true,
        }],
        s3OriginSource: {
            originAccessIdentity: originId,
            s3BucketSource: bucket,
        },
    }]
})

The new API should improve the linking of S3 buckets to CloudFront distributions, and abstract away the need to explicitly create a CloudFrontOriginAccessIdentityResource.

It would be really nice if we could tie this in with #605 to handle certificate generation/validation too.

Furthermore, it would be awesome if one could use Assets to point to a local directory that includes the web distribution content and have the toolkit do the work of uploading it to S3 and all that.

👍

@debora-ito debora-ito added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Nov 7, 2018
@rix0rrr rix0rrr added the gap label Jan 4, 2019
@nathanpeck
Copy link
Member

At a minimum I would recommend making the Cloudfront construct aware of other constructs such as:

  • elb.LoadBalancer
  • elbv2.ApplicationLoadBalancer
  • elbv2.NetworkLoadBalancer
  • apiGateway.LambdaRestApi
  • ecs.LoadBalancedFargateService (add the LB created by this construct)
  • ecs.LoadBalancedEc2Service (add the LB created by this construct)
  • ecs.FargateService and ecs.Ec2Service (lookup the attached LB)
  • s3.Bucket

CloudFront construct should be aware of how to get the domain name / address and potentially even the port from each construct and auto configure the origin details

I imagine an interface like:

const dist = new cloudfront.CloudFrontWebDistribution(this, 'WebsiteDistribution');

const ecsOrigin = dist.addOrigin(this, {
   target: myEcsServiceConstruct,
   port: 3000
});

const apiOrigin = dist.addOrigin(this, {
   target: myApiGatewayConstruct
});

dist.addBehavior(this, {
   path: '*',
   origin: ecsOrigin
   default: true
});

dist.addBehavior(this, {
   path: 'api*',
   origin: apiOrigin
   default: true
});

@KnisterPeter
Copy link
Contributor

Hi @eladb, @PaulMaddox, @mindstorms6, @nathanpeck

I'm trying to add policy statements to my bucket, but it fails with the message Invalid principal in policy.
Any hint on how to reference the OID arn correctly?

    const originAccessIdentity = new cloudfront.CfnCloudFrontOriginAccessIdentity(
      this,
      'PublicSiteOAI',
      {
        cloudFrontOriginAccessIdentityConfig: {
          comment: 'OAI for adac-public-site-bucket'
        }
      }
    );

    const bucket = new s3.Bucket(this, 'Bucket', {
      blockPublicAccess: s3.BlockPublicAccess.BlockAll,
      bucketName: 'public-site-bucket'
    });

    const distribution = new cloudfront.CloudFrontWebDistribution(
      this,
      'Cloudfront',
      {
        comment: 'Public Site Cloudfront',
        originConfigs: [
          {
            behaviors: [{ isDefaultBehavior: true }],
            s3OriginSource: {
              originAccessIdentity,
              s3BucketSource: bucket
            }
          }
        ]
      }
    );

    const listBucketPolicy = new iam.PolicyStatement()
      .addAction('s3:ListBucket')
      .allow()
      .addResource(`arn:aws:s3:::${distribution.domainName}`)
      .addAwsPrincipal(originAccessIdentity.cloudFrontOriginAccessIdentityId);
    listBucketPolicy.sid = 'bucket_policy_site_root';
    bucket.addToResourcePolicy(listBucketPolicy);

    const getObjectPolicy = new iam.PolicyStatement()
      .addAction('s3:GetObject')
      .allow()
      .addResource(`arn:aws:s3:::${distribution.domainName}/*`)
      .addAwsPrincipal(originAccessIdentity.cloudFrontOriginAccessIdentityId);
    getObjectPolicy.sid = 'bucket_policy_site_all';
    bucket.addToResourcePolicy(getObjectPolicy);

@eladb eladb self-assigned this Aug 12, 2019
@SomayaB SomayaB added the feature-request A feature should be added or improved. label Nov 11, 2019
@eladb eladb assigned iliapolo and unassigned eladb Jan 23, 2020
@SomayaB SomayaB removed the gap label Feb 25, 2020
@iliapolo iliapolo added the effort/large Large work item – several weeks of effort label Mar 9, 2020
@iliapolo iliapolo assigned njlynch and unassigned iliapolo Aug 19, 2020
@njlynch
Copy link
Contributor

njlynch commented Aug 21, 2020

The "love" this module needs was designed in this RFC (aws/aws-cdk-rfcs#171) and implementation of the initial "Developer Preview" version is being tracked via this milestone: https://github.com/aws/aws-cdk/milestone/5.

There is also the overall tracking issue #6490 where high-level feature work can be tracked.

Closing this issue out in favor of the above.

@njlynch njlynch closed this as completed Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront effort/large Large work item – several weeks of effort feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

10 participants