Skip to content

Commit

Permalink
ci: migrate to github workflow ci (#6008)
Browse files Browse the repository at this point in the history 
Signed-off-by: Ye Sijun <junnplus@gmail.com>
  • Loading branch information
@junnplus
junnplus authored Jun 24, 2023
1 parent f45bd27 commit f3892e0
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 185 deletions.
0 .circleci/Dockerfile.cypress → .ci/Dockerfile.cypress
View file
File renamed without changes.
0 .circleci/docker-compose.circle.yml → .ci/docker-compose.ci.yml
View file
File renamed without changes.
2 changes: 1 addition & 1 deletion .circleci/docker-compose.cypress.yml → .ci/docker-compose.cypress.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ services:
ipc: host
build:
context: ../
dockerfile: .circleci/Dockerfile.cypress
dockerfile: .ci/Dockerfile.cypress
depends_on:
- server
- worker
Expand Down
0 .circleci/docker_build → .ci/docker_build
View file
File renamed without changes.
0 .circleci/pack → .ci/pack
View file
File renamed without changes.
0 .circleci/update_version → .ci/update_version
View file
File renamed without changes.
182 changes: 0 additions & 182 deletions .circleci/config.yml
View file

This file was deleted.

149 changes: 149 additions & 0 deletions .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
name: Tests
on:
push:
branches:
- master
pull_request:
env:
NODE_VERSION: 14.17
jobs:
backend-lint:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 1
- uses: actions/setup-python@v4
with:
python-version: '3.7'
- run: sudo pip install flake8
- run: ./bin/flake8_tests.sh

backend-unit-tests:
runs-on: ubuntu-22.04
needs: backend-lint
env:
COMPOSE_FILE: .ci/docker-compose.ci.yml
COMPOSE_PROJECT_NAME: redash
COMPOSE_DOCKER_CLI_BUILD: 1
DOCKER_BUILDKIT: 1
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 1
- name: Build Docker Images
run: |
set -x
docker-compose build --build-arg skip_ds_deps=true --build-arg skip_frontend_build=true
docker-compose up -d
sleep 10
- name: Create Test Database
run: docker-compose -p redash run --rm postgres psql -h postgres -U postgres -c "create database tests;"
- name: List Enabled Query Runners
run: docker-compose -p redash run --rm redash manage ds list_types
- name: Run Tests
run: docker-compose -p redash run --name tests redash tests --junitxml=junit.xml --cov-report xml --cov=redash --cov-config .coveragerc tests/
- name: Copy Test Results
run: |
mkdir -p /tmp/test-results/unit-tests
docker cp tests:/app/coverage.xml ./coverage.xml
docker cp tests:/app/junit.xml /tmp/test-results/unit-tests/results.xml
- name: Store Test Results
uses: actions/upload-artifact@v3
with:
name: test-results
path: /tmp/test-results
- name: Store Coverage Results
uses: actions/upload-artifact@v3
with:
name: coverage
path: coverage.xml

frontend-lint:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 1
- uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}
cache: 'yarn'
- name: Install Dependencies
run: |
npm install --global --force yarn@1.22.10
yarn cache clean && yarn --frozen-lockfile --network-concurrency 1
- name: Run Lint
run: yarn lint:ci
- name: Store Test Results
uses: actions/upload-artifact@v3
with:
name: test-results
path: /tmp/test-results

frontend-unit-tests:
runs-on: ubuntu-22.04
needs: frontend-lint
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 1
- uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}
cache: 'yarn'
- name: Install Dependencies
run: |
npm install --global --force yarn@1.22.10
yarn cache clean && yarn --frozen-lockfile --network-concurrency 1
- name: Run App Tests
run: yarn test
- name: Run Visualizations Tests
run: cd viz-lib && yarn test
- run: yarn lint

frontend-e2e-tests:
runs-on: ubuntu-22.04
needs: frontend-lint
env:
COMPOSE_FILE: .ci/docker-compose.cypress.yml
COMPOSE_PROJECT_NAME: cypress
PERCY_TOKEN_ENCODED: ZGRiY2ZmZDQ0OTdjMzM5ZWE0ZGQzNTZiOWNkMDRjOTk4Zjg0ZjMxMWRmMDZiM2RjOTYxNDZhOGExMjI4ZDE3MA==
CYPRESS_PROJECT_ID_ENCODED: OTI0Y2th
CYPRESS_RECORD_KEY_ENCODED: YzA1OTIxMTUtYTA1Yy00NzQ2LWEyMDMtZmZjMDgwZGI2ODgx
CYPRESS_INSTALL_BINARY: 0
PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: 1
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 1
- uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}
cache: 'yarn'
- name: Enable Code Coverage Report For Master Branch
if: endsWith(github.ref, '/master')
run: |
echo "CODE_COVERAGE=true" >> $GITHUB_ENV
- name: Install Dependencies
run: |
npm install --global --force yarn@1.22.10
yarn cache clean && yarn --frozen-lockfile --network-concurrency 1
- name: Setup Redash Server
run: |
set -x
yarn cypress build
yarn cypress start -- --skip-db-seed
docker-compose run cypress yarn cypress db-seed
- name: Execute Cypress Tests
run: yarn cypress run-ci
- name: "Failure: output container logs to console"
if: failure()
run: docker-compose logs
- name: Copy Code Coverage Results
run: docker cp cypress:/usr/src/app/coverage ./coverage || true
- name: Store Coverage Results
uses: actions/upload-artifact@v3
with:
name: coverage
path: coverage
4 changes: 2 additions & 2 deletions client/cypress/cypress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ function runCypressCI() {
PERCY_TOKEN_ENCODED,
CYPRESS_PROJECT_ID_ENCODED,
CYPRESS_RECORD_KEY_ENCODED,
CIRCLE_REPOSITORY_URL,
GITHUB_REPOSITORY,
} = process.env;

if (CIRCLE_REPOSITORY_URL && CIRCLE_REPOSITORY_URL.includes("getredash/redash")) {
if (GITHUB_REPOSITORY === "getredash/redash") {
if (PERCY_TOKEN_ENCODED) {
process.env.PERCY_TOKEN = atob(`${PERCY_TOKEN_ENCODED}`);
}
Expand Down

13 comments on commit f3892e0

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, it seems to still be trying to run CircleCI based CI for this commit.

I wonder if that's just some weirdness with GitHub, and it'll correctly use GitHub Actions for all commits after this one?

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Ahhh. I needed to disable/delete the CircleCI web hook in the repo settings. Done now.

@myonlylonely
Copy link
Contributor

@myonlylonely myonlylonely commented on f3892e0 Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ahhh. I needed to disable/delete the CircleCI web hook in the repo settings. Done now.

You should use secrets instead of writing Cypress tokens directly in ci.yml. These encoded tokens are just Base64 strings, can be easily decoded and used for other projects.
It would be something like this:

frontend-e2e-tests:
  runs-on: ubuntu-22.04
  needs: frontend-lint
  env:
    PERCY_TOKEN_ENCODED: ${{ secrets.PERCY_TOKEN_ENCODED }}
    CYPRESS_PROJECT_ID_ENCODED: ${{ secrets.CYPRESS_PROJECT_ID_ENCODED }}
    CYPRESS_RECORD_KEY_ENCODED: ${{ secrets.CYPRESS_RECORD_KEY_ENCODED }}

@justinclift
Copy link
Member

@justinclift justinclift commented on f3892e0 Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ouch. Looking at that file now, you're completely right.

@junnplus Any interest in looking at this? It's pretty important. 😄

We'll probably need to regenerate the token too, so these existing ones aren't useful any more either.

@gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented on f3892e0 Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@myonlylonely @justinclift this was intentional for CircleCI, because secrets don't work with PRs from Forked repositories . I don't know how Github actions does with it though.

No secrets are "safe" when used by forks anyway (e.g.: anyone can go, edit and put an echo $SECRET). For Cypress/Percy we still need them for triggering the PR checks 😕

@justinclift
Copy link
Member

@justinclift justinclift commented on f3892e0 Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh. Something along those lines was mentioned (maybe by @hazmeister or @gaecoli?) when we were getting GitHub Actions working for PRs in the Community repo too.

@gaecoli
Copy link
Member

@gaecoli gaecoli commented on f3892e0 Jul 3, 2023

Choose a reason for hiding this comment

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

Oh. Something along those lines was mentioned (maybe by @hazmeister or @gaecoli?) when we were getting GitHub Actions working for PRs in the Community repo too.

Yeah

@hazmeister
Copy link

Choose a reason for hiding this comment

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

No secrets are "safe" when used by forks anyway (e.g.: anyone can go, edit and put an echo $SECRET). For Cypress/Percy we still need them for triggering the PR checks 😕

The only defence here is to limit how can trigger a workflow - the docker secrets in the other repo don't get included when you fork, and could only be used on certain branches. We could make certain workflows manually triggered by members of the team after they've been given a review.

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, with there being no perfect solution... maybe we should just leave it as-is?

It's been like that for ages now, and the automatic running of submitted PRs is pretty useful.

@gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented on f3892e0 Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The only defence here is to limit how can trigger a workflow - the docker secrets in the other repo don't get included when you fork, and could only be used on certain branches. We could make certain workflows manually triggered by members of the team after they've been given a review.

This is true, but I still don't think it's worth it unless we face any issues. For Cypress/Percy it's still not that big of a deal with those tokens and we get more benefit from having their checks right away, than to forgetting to run them (e.g.: the visual regression testing from Percy).

But if we either have issues or start reaching rate limits, then yes, having the maintainers to manually trigger the jobs that require them is a nice way to fix it

@myonlylonely
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, with there being no perfect solution... maybe we should just leave it as-is?

It's been like that for ages now, and the automatic running of submitted PRs is pretty useful.

Redash used CircleCI in the past, and it also use secrets(contexts) provided by CircleCI.

@gabrieldutra
Copy link
Member

Choose a reason for hiding this comment

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

Redash used CircleCI in the past, and it also use secrets(contexts) provided by CircleCI

Those 2 were not secrets in CircleCI, because the secrets don't work on Forked repositories when someone opens a PR.
We went with this solution, so that all PRs would have both Cypress and percy running by default

@myonlylonely
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I understand. But it still might be a security risk, although it's not that big of a deal with CI.

Please sign in to comment.