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

Gcs pipeline #1818

Merged
merged 30 commits into from
Apr 9, 2024
Merged

Gcs pipeline #1818

merged 30 commits into from
Apr 9, 2024

Conversation

pattersonbl2
Copy link
Contributor

Opening pr for stage gcs bucket deploy pipeline.

with:
credentials_json: ${{ secrets.GCP_SA_KEY }}

- name: set up gcloud
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: set up gcloud
- name: Set up gcloud

- name: set up gcloud
uses: google-github-actions/setup-gcloud@v2

- name: stage build with unpublsiehd contents
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: stage build with unpublsiehd contents
- name: Build the website

.github/workflows/gcp_deploy.yaml Outdated Show resolved Hide resolved
yarn build:production
- name: Deploying unpublsiehd content to stage.
run: |
sh ./bin/gcs-deploy.sh ${{vars.EXTENSIONWORKSHOP_BUCKET_STAGE}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sh ./bin/gcs-deploy.sh ${{vars.EXTENSIONWORKSHOP_BUCKET_STAGE}}
sh ./bin/gcs-deploy.sh ${{ vars.EXTENSIONWORKSHOP_BUCKET_STAGE }}

push:
tags:
- "20[0-9][0-9].[0-9][0-9].[0-9][0-9](-[0-9]+)?"
workflow_dispatch:
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not in the longer term, workflow dispatch just allows me to run the workflow manually without waiting on the pushed git tag. So i can build a prod website during testing without creating multiple tags that will trigger the existing pipeline.

run: |
yarn install
yarn build:production
- name: Deploying unpublsiehd content to stage.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Deploying unpublsiehd content to stage.
- name: Deploy the website to prod

yarn build:production
- name: Deploying unpublsiehd content to stage.
run: |
sh ./bin/gcs-deploy.sh ${{vars.EXTENSIONWORKSHOP_BUCKET_PROD}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sh ./bin/gcs-deploy.sh ${{vars.EXTENSIONWORKSHOP_BUCKET_PROD}}
sh ./bin/gcs-deploy.sh ${{ vars.EXTENSIONWORKSHOP_BUCKET_PROD }}

@@ -0,0 +1,14 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Why is this copied? Can't we reuse .utils/build-version-json.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it being used in the gcs_deploy.sh script. i wanted both grouped together to keep track of them. just so i can keep things circle ci related separate from my work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am probably going to change some things here. would you like if the GitHub run number include ? it's a

"unique number for each run of a particular workflow in a repository. This number begins at 1 for the workflow's first run, and increments with each new run. This number does not change if you re-run the workflow run."

Copy link
Member

Choose a reason for hiding this comment

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

sorry, include what? The unique number? Like we have, e.g. here for AMO frontend (build)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it would be similar to that.

Copy link
Member

Choose a reason for hiding this comment

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

Oh ok, I guess if this could be a link that'd be great. If it's just a numeric ID, I guess that's better than nothing. What matters the most for me is to have the git tag or git hash in this version.json file.

bin/gcs-deploy.sh Outdated Show resolved Hide resolved
set -ex
EXTENSION_WORKSHOP_BUCKET_GCS=$1

echo $EXTENSION_WORKSHOP_BUCKET_GCS
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for this script to be slightly indented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, reason could just a linting problem in my vs code set up.

I have added a test job before the deploy job , fix spelling errors and move op scripts
increase checkout depth with tags and fix error with version file not being copied.
- name: Setup Node js
uses: actions/setup-node@v3

- name: Set up Google Cloud SDK
Copy link

@dlactin dlactin Apr 5, 2024

Choose a reason for hiding this comment

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

@pattersonbl2 already mentioned this was on his radar, but the only change I'd make here is using workload identity instead of passing around a service account key. https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-google-cloud-platform

otherwise lgtm!

Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

Only nits but would be good to make the yaml consistent, and the shell script indented.

Comment on lines 19 to 21
- name: Install dependencies
run: yarn install --immutable
- name: Test current build
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Install dependencies
run: yarn install --immutable
- name: Test current build
- name: Install dependencies
run: yarn install --immutable
- name: Test current build

run: |
yarn install
yarn build:production
- name: Build version.json file
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Build version.json file
- name: Build version.json file

Comment on lines 23 to 25
- name: Install dependencies
run: yarn install --immutable
- name: Test current build
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Install dependencies
run: yarn install --immutable
- name: Test current build
- name: Install dependencies
run: yarn install --immutable
- name: Test current build

steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Setup Node js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Node js
- name: Setup Node.js

fetch-depth: 1
fetch-tags: true

- name: Setup Node js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Node js
- name: Setup Node.js

Comment on lines 55 to 58
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh

fetch-depth: 1
fetch-tags: true

- name: Setup Node js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Node js
- name: Setup Node.js

- name: Checkout code
uses: actions/checkout@v3

- name: Setup Node js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Node js
- name: Setup Node.js

Comment on lines 53 to 60
- name: Build version.json file
run: |
if [ -e version.json ]; then
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh
fi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Build version.json file
run: |
if [ -e version.json ]; then
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh
fi
- name: Build version.json file
run: |
if [ -e version.json ]; then
echo "version.json exists, skipping build"
else
echo "version.json does not exist, running build-version-json.sh"
bash ./.utils/build-version-json.sh
fi

.utils/gcs-deploy.sh Outdated Show resolved Hide resolved
@pattersonbl2
Copy link
Contributor Author

I can make these changes today

.github/workflows/gcp_deploy.yaml Outdated Show resolved Hide resolved
.github/workflows/gcp_deploy_prod.yaml Outdated Show resolved Hide resolved
@pattersonbl2 pattersonbl2 merged commit 50c329f into master Apr 9, 2024
1 check passed
@pattersonbl2 pattersonbl2 deleted the gcs-pipeline branch April 9, 2024 15:19
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.

3 participants