Financial-Times / n-gage

Shared developer and build tools for FT.com applications and components
9 stars 3 forks source link

replaces the existing nht deploy command with shared helpers 🐿 v2.12.5 #213

Closed magsallen closed 4 years ago

magsallen commented 5 years ago

Refactors the deploy assets command in n-gage to remove the dependency on n-heroku-tools for apps which are using Page Kit

PR to add the aws-cli helpers PR to add the upload assets to S3 helper

Related issue on Page Kit: https://github.com/Financial-Times/next-ci-shared-helpers/issues/21

magsallen commented 5 years ago

I've refactored this based on comments in https://github.com/Financial-Times/next-ci-shared-helpers/pull/22

Page Kit builds will now call the aws-cli install and configure steps via separate helpers and to pass the required aws keys as arguments to the configure step.

i-like-robots commented 4 years ago

I have tested this by copying the below into my app's Makefile:

deploy-test:
    @if [ -e public/manifest.json ]; then \
        if [ -e .circleci/shared-helpers ]; then \
            .circleci/shared-helpers/helper-configure-awscli $(aws_access_hashed_assets) $(aws_secret_hashed_assets) \
            && .circleci/shared-helpers/helper-upload-assets-to-s3 public hashed-assets/page-kit-test; \
        else \
            echo "Could not find the shared-helpers directory"; \
        fi \
    else \
        echo "Could not find a manifest.json"; \
    fi

Then running:

$ git clone --depth 1 git@github.com:Financial-Times/next-ci-shared-helpers.git .circleci/shared-helpers
$ make deploy-test

A few things I noticed:

  1. I had to skip the install aws cli script as this calls sudo which created an interactive prompt... do we know what the deal is with sudo on CI?
  2. I had to fix some missing ; and &&
  3. The upload bash script is being output which is super ugly.
  4. https://github.com/Financial-Times/next-ci-shared-helpers/pull/24
  5. https://github.com/Financial-Times/next-ci-shared-helpers/pull/25