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

Finished task #97

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

SaimonWELL
Copy link

TASK-2.1 Manual Deployment

S3 URL(Blocked) - https://task-1-rs.s3.eu-central-1.amazonaws.com/index.html
CloudFront URL - https://dbk4hrf7n35us.cloudfront.net/

TASK-2.2 Automated Deployment

S3 URL(Blocked) - https://my-rs-bucket-ttt.s3.eu-central-1.amazonaws.com/index.html
CloudFront URL - https://d2rv8rvax2fpqy.cloudfront.net/

npm run deploy - for deploying

Copy link

@naXa777 naXa777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Score: 100 / 100.
I tested that your bootstrap, build & deploy scripts work. It's a good work!!


Found problems that don't affect your score:

  1. wrong target repository. you should create pull requests in your own fork of the repository.

image

  1. bug: React routes don't work with AWS S3 + CloudFront

  2. Other minor problems are highlighted in comments.

"prettier": "prettier src --write",
"prepare": "cd aws-deployment && npm i",
"bootstrap": "npm run prepare && npm run build && cd aws-deployment && cdk bootstrap",
"deploy": "npm run bootstrap && cd aws-deployment && cdk deploy "
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"deploy": "npm run bootstrap && cd aws-deployment && cdk deploy "
"deploy": "cd aws-deployment && cdk deploy "

cdk bootstrap should only be run once. While npm run deploy command is used every time when re-deploy is needed. so it's not correct to include npm run bootstrap call in the deploy command.

Comment on lines +1 to +8
import * as route53 from 'aws-cdk-lib/aws-route53';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as acm from 'aws-cdk-lib/aws-certificatemanager';
import * as cloudfront from 'aws-cdk-lib/aws-cloudfront';
import * as s3deploy from 'aws-cdk-lib/aws-s3-deployment';
import * as targets from 'aws-cdk-lib/aws-route53-targets';
import * as cloudfront_origins from 'aws-cdk-lib/aws-cloudfront-origins';
import {CfnOutput, Duration, RemovalPolicy, Stack, StackProps} from 'aws-cdk-lib';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete unused imports

Suggested change
import * as route53 from 'aws-cdk-lib/aws-route53';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as acm from 'aws-cdk-lib/aws-certificatemanager';
import * as cloudfront from 'aws-cdk-lib/aws-cloudfront';
import * as s3deploy from 'aws-cdk-lib/aws-s3-deployment';
import * as targets from 'aws-cdk-lib/aws-route53-targets';
import * as cloudfront_origins from 'aws-cdk-lib/aws-cloudfront-origins';
import {CfnOutput, Duration, RemovalPolicy, Stack, StackProps} from 'aws-cdk-lib';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as cloudfront from 'aws-cdk-lib/aws-cloudfront';
import * as s3deploy from 'aws-cdk-lib/aws-s3-deployment';
import * as cloudfront_origins from 'aws-cdk-lib/aws-cloudfront-origins';
import {Duration, RemovalPolicy, Stack, StackProps} from 'aws-cdk-lib';

{
httpStatus: 403,
responseHttpStatus: 403,
responsePagePath: '/error.html',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where's error.html file? It looks like you are referencing a non-existent file here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants