Skip to content

Commit

Permalink
Merge pull request #60 from lalalilo/fix-aws-spa-list-cloudfront-perm…
Browse files Browse the repository at this point in the history
…ission

Do not list OAC when unnecessary
  • Loading branch information
GregdTd committed Jun 4, 2024
2 parents cbf7838 + 5c3f433 commit f378c76
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 51 deletions.
79 changes: 42 additions & 37 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,49 @@ version: 2
jobs:
build:
docker:
- image: cimg/node:16.17
- image: cimg/node:16.17

working_directory: ~/aws-spa

steps:
- checkout

- restore_cache:
name: Restore Yarn Package Cache
keys:
- yarn-packages-{{ checksum "yarn.lock" }}

- run:
name: Install Dependencies
command: yarn install --frozen-lockfile

- save_cache:
name: Save Yarn Package Cache
key: yarn-packages-{{ checksum "yarn.lock" }}
paths:
- ~/.cache/yarn

- run:
name: Check types
command: yarn check-types

- run:
name: Run tests
command: yarn test --coverage

- run:
name: Code coverage
command: npx codecov

- run:
name: Build
command: yarn build

- run:
name: Release
command: npx semantic-release
- checkout

- restore_cache:
name: Restore Yarn Package Cache
keys:
- yarn-packages-{{ checksum "yarn.lock" }}

- run:
name: Install Dependencies
command: yarn install --frozen-lockfile

- save_cache:
name: Save Yarn Package Cache
key: yarn-packages-{{ checksum "yarn.lock" }}
paths:
- ~/.cache/yarn

- run:
name: Check types
command: yarn check-types

- run:
name: Run tests
command: yarn test --coverage

- run:
name: Code coverage
command: npx codecov

- run:
name: Build
command: yarn build

- run:
name: Release
command: |
if [ "${CIRCLE_BRANCH}" == "main" ]; then
npx semantic-release
else
npx semantic-release --dry-run --no-ci --branches ${CIRCLE_BRANCH}
fi
22 changes: 20 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,26 @@ If a CloudFront distribution with this S3 bucket already exists, the script will
- cloudfront:TagResource
- cloudfront:GetDistributionConfig
- cloudfront:CreateInvalidation

**TODO**: complete missing policies
- cloudfront:UpdateDistribution
- cloudfront:ListOriginAccessControls
- cloudfront:GetOriginAccessControl
- cloudfront:CreateOriginAccessControl
- cloudfront:DeleteOriginAccessControl

- s3:PutBucketPolicy
- s3:GetBucketPolicy
- s3:PutBucketWebsite
- s3:GetBucketWebsite
- s3:DeleteBucketWebsite
- s3:PutBucketTagging
- s3:GetBucketTagging
- s3:DeleteBucketTagging
- s3:ListBucket
- s3:PutBucketPublicAccessBlock
- s3:CreateBucket
- s3:PutObject

-**TODO**: complete missing policies

### If using simple auth

Expand Down
2 changes: 1 addition & 1 deletion src/cloudfront/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ describe("cloudfront", () => {
distribution.Id,
domainName,
shouldBlockBucketPublicAccess,
undefined
null
);

expect(updateDistribution).not.toHaveBeenCalled();
Expand Down
2 changes: 1 addition & 1 deletion src/cloudfront/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ export const updateCloudFrontDistribution = async (
distributionId: string,
domainName: string,
shouldBlockBucketPublicAccess: boolean,
oac: OAC | undefined,
oac: OAC | null,
) => {
try {
const { DistributionConfig, ETag } = await cloudfront
Expand Down
9 changes: 2 additions & 7 deletions src/cloudfront/origin-access.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,12 @@ describe("upsertOriginAccessControl", () => {
const domainName = "my-domain";
const shouldBlockBucketPublicAccess = false;

listOriginAccessControlsMock.mockReturnValue(
awsResolve({
OriginAccessControlList: {},
})
);

await upsertOriginAccessControl(
domainName,
"my-distribution-id",
shouldBlockBucketPublicAccess
);

expect(listOriginAccessControlsMock).not.toHaveBeenCalled();
expect(createOriginAccessControlMock).not.toHaveBeenCalled();
});

Expand All @@ -95,6 +89,7 @@ describe("upsertOriginAccessControl", () => {
shouldBlockBucketPublicAccess
);

expect(listOriginAccessControlsMock).toHaveBeenCalled();
expect(createOriginAccessControlMock).toHaveBeenCalledTimes(1);
expect(createOriginAccessControlMock).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down
7 changes: 4 additions & 3 deletions src/cloudfront/origin-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export const upsertOriginAccessControl = async (
distributionId: string,
shouldBlockBucketPublicAccess: boolean,
) => {
if (!shouldBlockBucketPublicAccess) {
return null;
}
const originAccessControlName = getOriginAccessControlName(
domainName,
distributionId,
Expand All @@ -74,9 +77,7 @@ export const upsertOriginAccessControl = async (
return existingOAC;
}

if (shouldBlockBucketPublicAccess) {
return await createOAC(originAccessControlName, domainName, distributionId);
}
return await createOAC(originAccessControlName, domainName, distributionId);
};

export const deleteOriginAccessControl = async (oac: OAC) => {
Expand Down

0 comments on commit f378c76

Please sign in to comment.