nideveloper / CDK-SPA-Deploy

This is an AWS CDK Construct to make deploying a single page website (Angular/React/Vue) to AWS S3 behind SSL/Cloudfront easier
MIT License
235 stars 40 forks source link

handle subdomain #130

Open StErMi opened 4 years ago

StErMi commented 4 years ago

Hi,

I look up at the source code and it seems that this construct does not support custom subdomain. In this use-case I would like to deploy a website that will be available under ..com and it should create/use the same resources but add a CNAME with the CloudFront Distribution ID to the hosted zone.

Let me know if maybe I just missed some steps.

Best regards,

Emanuele.

nideveloper commented 4 years ago

Let me just make sure I fully understand the ask. So you already have a site deployed to say "cdk.com" but you want to add a new site on "subdomain.cdk.com" which should just add a CNAME record to Route53?

This construct has been evolving to meet people's needs rather than starting complete so happy to see how much effort this is

StErMi commented 4 years ago

Exactly. Now, I've just started learning CDK so I just assumed that the construct was not doing this looking at the source code. The "cdk.com" has been added manually from the AWS Console but I don't think that this will be a problem.

Usually, when at the moment we want to add a subdomain website we do these:

After that, for each build we

I've another question (that might be better if I ask it in a different issue). I don't know if it's just my lack of knowledge but how can I specify the S3 bucket region? CF and Route53 are global but I would like to specify where my S3 bucket should be created.

nideveloper commented 4 years ago

You're 100% correct in that you are the first person to ask for sub domains so it isn't built into the construct yet. I'll take a look into it over the next couple of days

StErMi commented 4 years ago

@nideveloper I just saw that there's already an example (not complete like yours) directly from @aws-samples for static site deployment.

I don't get why they don't allow you to specify to which AWS region they allow you to deploy your site / CF stack. Am I missing something?

nideveloper commented 4 years ago

You can define region at the stack level e.g. https://github.com/cdk-patterns/serverless/blob/f183222d260cb7e64d37f462acee69ea529351c6/the-rds-proxy/typescript/bin/the-rds-proxy.ts#L8

SnapPetal commented 4 years ago

I am confused since it takes an array of an alias. Would that not work?

new SPADeploy(this, 'valor-privacy').createSiteWithCloudfront({ indexDoc: 'index.html', websiteFolder: '../privacy', certificateARN: 'arn:aws:acm:us-east-1:664759038511:certificate/be6ac207-1fc3-4642-9729-7ca0eff2b5bc', cfAliases: ['privacy.valorskateandserve.org'], });

StErMi commented 4 years ago

As I said I'm still new to CDK, I mostly come from serverless-framework but I will give a try today to understand the implication.

CDK as far as I can see is missing a lot of documentation that explain to newcomers what is doing (at least that's my impression) and that's something not cool when you could harm your production env :D

Tannheuser commented 4 years ago

I am confused since it takes an array of an alias. Would that not work?

new SPADeploy(this, 'valor-privacy').createSiteWithCloudfront({ indexDoc: 'index.html', websiteFolder: '../privacy', certificateARN: 'arn:aws:acm:us-east-1:664759038511:certificate/be6ac207-1fc3-4642-9729-7ca0eff2b5bc', cfAliases: ['privacy.valorskateandserve.org'], });

This just populates CNAMEs property in CF distribution configuration, but does not add a new record to Route53 hosted zone.

FavorWedge commented 4 years ago

@Tannheuser This is what we use to set the Route 53 record. Not sure if it helps

const deployment = new SPADeploy(this, this.stackName, { encryptBucket: true, }).createSiteWithCloudfront({ indexDoc: 'index.html', websiteFolder: '../admin/build', certificateARN: props.certArn, cfAliases: [props.siteName], });

new r53.ARecord(this, 'cdn-a-record', { zone: props.hostedZone, recordName: props.siteName, target: r53.RecordTarget.fromAlias(new CloudFrontTarget(deployment.distribution)), });

Tannheuser commented 4 years ago

@Tannheuser This is what we use to set the Route 53 record. Not sure if it helps

const deployment = new SPADeploy(this, this.stackName, { encryptBucket: true, }).createSiteWithCloudfront({ indexDoc: 'index.html', websiteFolder: '../admin/build', certificateARN: props.certArn, cfAliases: [props.siteName], });

new r53.ARecord(this, 'cdn-a-record', { zone: props.hostedZone, recordName: props.siteName, target: r53.RecordTarget.fromAlias(new CloudFrontTarget(deployment.distribution)), });

Yes, it's helpful. We have the same approach by using SPADeploy output to create a new R53 record.

StErMi commented 4 years ago

I'm going to release my SPA version today or at max tomorrow. Something that I would like to do before that is to find a good way to handle different env with different configs.

For example, I will have a staging and production config, and based on that I would like to be able to specify a different region and a different domain.

Tannheuser commented 4 years ago

I'm going to release my SPA version today or at max tomorrow. Something that I would like to do before that is to find a good way to handle different env with different configs.

For example, I will have a staging and production config, and based on that I would like to be able to specify a different region and a different domain.

@StErMi This is how do we handle this in our project: we use yaml files with configurations for each environment. Also you could use default environment variables like CDK_DEFAULT_ACCOUNT and CDK_DEFAULT_REGION.

import { App, StackProps } from '@aws-cdk/core';
import * as fs from 'fs';
import * as yaml from 'js-yaml';

import { CdkStack } from './cdk-stack';
import { EnvironmentProps, HostedZoneProps, SecurityProps } from './models';

const fileContents = fs.readFileSync('./config/variables.yml', 'utf8');
const yamlConfig: EnvironmentProps & HostedZoneProps & SecurityProps = yaml.safeLoad(fileContents);
const { account, region, stage, ...config } = yamlConfig;

const app = new App();
const stackProps: StackProps & HostedZoneProps & SecurityProps = {
  env: {
    account: account || process.env.CDK_DEFAULT_ACCOUNT,
    region: region || process.env.CDK_DEFAULT_REGION
  },
  ...config
};

new CdkStack(app, `my-stack-name-${stage}`, stackProps);

and variables.yml content

stage: dev
region: eu-central-1
zoneName: mywebsite.com
subdomain: subsite
certificateARN: arn:aws:acm:...........
FavorWedge commented 4 years ago

In most cases using the default account is not sustainable this is a good blog post about that topic.

https://taimos.de/blog/create-a-cicd-pipeline-for-your-cdk-app

StErMi commented 4 years ago

I'm going to release my SPA version today or at max tomorrow. Something that I would like to do before that is to find a good way to handle different env with different configs. For example, I will have a staging and production config, and based on that I would like to be able to specify a different region and a different domain.

@StErMi This is how do we handle this in our project: we use yaml files with configurations for each environment. Also you could use default environment variables like CDK_DEFAULT_ACCOUNT and CDK_DEFAULT_REGION.

import { App, StackProps } from '@aws-cdk/core';
import * as fs from 'fs';
import * as yaml from 'js-yaml';

import { CdkStack } from './cdk-stack';
import { EnvironmentProps, HostedZoneProps, SecurityProps } from './models';

const fileContents = fs.readFileSync('./config/variables.yml', 'utf8');
const yamlConfig: EnvironmentProps & HostedZoneProps & SecurityProps = yaml.safeLoad(fileContents);
const { account, region, stage, ...config } = yamlConfig;

const app = new App();
const stackProps: StackProps & HostedZoneProps & SecurityProps = {
  env: {
    account: account || process.env.CDK_DEFAULT_ACCOUNT,
    region: region || process.env.CDK_DEFAULT_REGION
  },
  ...config
};

new CdkStack(app, `my-stack-name-${stage}`, stackProps);

and variables.yml content

stage: dev
region: eu-central-1
zoneName: mywebsite.com
subdomain: subsite
certificateARN: arn:aws:acm:...........

How do you specify which file should be loaded when you deploy it?

StErMi commented 4 years ago

In most cases using the default account is not sustainable this is a good blog post about that topic.

https://taimos.de/blog/create-a-cicd-pipeline-for-your-cdk-app

Is it ok to always deploy everything even if I'm making changes at my infrastructure in the staging branch? What I would like to do is to be able to deploy to staging envs only when I deploy on the staging branch and on production envs only when I deploy on the master branch.

FavorWedge commented 4 years ago

@StErMi I do not mind keep discussing this but I feel like this is not a real issue for cdk-spa-deploy and maybe we could use some other form of communication.

StErMi commented 4 years ago

@StErMi I do not mind keep discussing this but I feel like this is not a real issue for cdk-spa-deploy and maybe we could use some other form of communication.

you are totally right, sorry! Where could we continue the discussion? I'm also available on the slack channel

SnapPetal commented 3 years ago

@StErMi I do not mind keep discussing this but I feel like this is not a real issue for cdk-spa-deploy and maybe we could use some other form of communication.

you are totally right, sorry! Where could we continue the discussion? I'm also available on the slack channel

What slack channel do you use?

mattvb91 commented 3 years ago

@FavorWedge currently experiencing the same issue as this and tried your code but receiving:

Argument of type 'CloudFrontTarget' is not assignable to parameter of type 'IAliasRecordTarget'.
  Types of property 'bind' are incompatible.
    Type '(_record: IRecordSet) => AliasRecordTargetConfig' is not assignable to type '(record: IRecordSet) => AliasRecordTargetConfig'.
      Types of parameters '_record' and 'record' are incompatible.
        Type 'import("node_modules/@aws-cdk/aws-route53/lib/record-set").IRecordSet' is not assignable to type 'import("node_modules/@aws-cdk/aws-apigateway/node_modules/@aws-cdk/aws-route53/lib/record-set").IRecordSet'.
          Types of property 'stack' are incompatible.
            Type 'import("node_modules/@aws-cdk/aws-iam/node_modules/@aws-cdk/core/lib/stack").Stack' is not assignable to type 'import("node_modules/@aws-cdk/core/lib/stack").Stack'.ts(2345)
Argument of type 'CloudFrontWebDistribution' is not assignable to parameter of type 'IDistribution'.
  The types of 'stack.tags' are incompatible between these types.
    Type 'import("node_modules/@aws-cdk/aws-iam/node_modules/@aws-cdk/core/lib/tag-manager").TagManager' is not assignable to type 'import("/node_modules/@aws-cdk/core/lib/tag-manager").TagManager'.
      Types have separate declarations of a private property 'tags'.ts(2345)

This is the code

 const deployment = new SPADeploy(this, "websiteDeploy").createSiteWithCloudfront({
      indexDoc: "index.html",
      websiteFolder: "./app/www",
      cfAliases: ["test.mydomain.com"],
    })

    new route53.ARecord(this, 'test.my.domain', {
      zone: new route53.HostedZone(this, "my_zone_id", {
        zoneName: "my_zone_name
      }),
      recordName: "my.record.name",
      target: route53.RecordTarget.fromAlias(new CloudFrontTarget(deployment.distribution)),
      });

Any chance you can spot the issue?

SnapPetal commented 3 years ago

@mattvb91 I created a new version of this project using CDK Cloudfront distributions. Not sure if will solve this problem.

https://github.com/SnapPetal/cdk-simplewebsite-deploy

nideveloper commented 3 years ago

Feature merged and released in 1.86.0-dev.1 which has been pushed to NPM, Pypi, Maven and Nuget

ppena-LiveData commented 3 years ago

Instead of having to call createSiteWithCloudfront() with a certificateARN and then having to call r53.ARecord(), it would be nice if createSiteFromHostedZone() first checked if config.certificateARN existed before it tried to create a new certificate, so that way it would handle the r53.ARecord() call for us.