Skip to content

Commit

Permalink
Uplift code quality in metamist repo (#957)
Browse files Browse the repository at this point in the history
* Add tests directory to sonarqube config file

* Hide db backup folder results

* Fix test exclusion for code coverage settings

* Address all current metamist bugs

* Fix security issues, including exclusive use of formatMoney with resistent regex

* formatMoney bug fix

* Fix the ValueFilter so that the useState is called in the same order in every render

* Change so that the nonroot user is used in the db dockerfile

* Change so that the nonroot user is used in the api dockerfile

* Fix default logging level to warning to reduce the leak of secure information

* Add merge_group to the test workflow

* Fix web pages format using prettier

* Fix web pages format using prettier - npm ci ran first

* Add referenceBranch to sonar-project.properties

* Update test workflow to include metamist dev branch version in analysis

* Add Sonarqube scan to deploy check, update db Dockerfile based on comments

* Fix run fastapi command in api Dockerfile
  • Loading branch information
violetbrina authored Oct 4, 2024
1 parent f1eba97 commit a4450ee
Show file tree
Hide file tree
Showing 31 changed files with 228 additions and 76 deletions.
147 changes: 147 additions & 0 deletions .github/workflows/deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,155 @@ permissions:
contents: read

jobs:
unittests:
name: Run unit tests
# Run on merge to main, where the commit name starts with "Bump version:" (for bump2version)
# if: "startsWith(github.event.head_commit.message, 'Bump version:')"
runs-on: ubuntu-latest
env:
DOCKER_BUILDKIT: 1
BUILDKIT_PROGRESS: plain
CLOUDSDK_CORE_DISABLE_PROMPTS: 1
# used for generating API
SM_DOCKER: samplemetadata:dev
defaults:
run:
shell: bash -eo pipefail -l {0}
steps:
- uses: actions/checkout@v4

- uses: actions/setup-python@v5
with:
python-version: '3.11'

- uses: actions/setup-java@v4
with:
distribution: 'temurin' # See 'Supported distributions' for available options
java-version: '17'

- name: Setup build env
run: |
set -euxo pipefail
pip install --no-deps -r requirements-dev.txt
# openapi-generator
wget https://repo1.maven.org/maven2/org/openapitools/openapi-generator-cli/5.3.0/openapi-generator-cli-5.3.0.jar -O openapi-generator-cli.jar
# liquibase connector
pushd db/
wget https://repo1.maven.org/maven2/org/mariadb/jdbc/mariadb-java-client/3.0.3/mariadb-java-client-3.0.3.jar
popd
# liquibase
VERSION=4.28.0
curl -L https://github.com/liquibase/liquibase/releases/download/v${VERSION}/liquibase-${VERSION}.zip --output liquibase-${VERSION}.zip
unzip -o -d liquibase liquibase-${VERSION}.zip
echo "$(pwd)/liquibase" >> $GITHUB_PATH
- name: 'build image'
run: |
docker build \
--build-arg SM_ENVIRONMENT=local \
--tag $SM_DOCKER \
-f deploy/api/Dockerfile \
.
- name: 'build deployable API'
run: |
export OPENAPI_COMMAND="java -jar openapi-generator-cli.jar"
python regenerate_api.py
pip install .
- name: 'Run unit tests'
id: runtests
run: |
coverage run -m pytest --doctest-modules --doctest-continue-on-failure test/ --junitxml=test-execution.xml
rc=$?
coverage xml
echo "rc=$rc" >> $GITHUB_OUTPUT
- name: 'Upload coverage report'
uses: codecov/codecov-action@v4
with:
files: ./coverage.xml
token: ${{ secrets.CODECOV_TOKEN }}

- name: 'Save coverage report as an Artifact'
uses: actions/upload-artifact@v4
with:
name: coverage-report
path: ./coverage.xml

- name: 'Save execution report as an Artifact'
uses: actions/upload-artifact@v4
with:
name: execution-report
path: ./test-execution.xml

- name: 'build web front-end'
run: |
set -eo pipefail
pushd web
# installs package-lock, not what it thinks it should be
npm ci
npm run build
rc=$?
echo "web_rc=$rc" >> $GITHUB_OUTPUT
# eventually run web front-end tests
popd
- name: Fail if unit tests are not passing
if: ${{ steps.runtests.outputs.rc != 0}}
uses: actions/github-script@v6
with:
script: |
core.setFailed('Unittests failed with rc = ${{ steps.runtests.outputs.rc }}')
- name: Fail if web build fails
if: ${{ steps.runtests.outputs.rc != 0}}
uses: actions/github-script@v6
with:
script: |
core.setFailed('Web failed to build with rc = ${{ steps.runtests.outputs.web_rc }}')
sonarqube:
name: SonarQube scan
runs-on: ubuntu-latest
needs: unittests
environment: production
if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/dev'
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis

# Download the coverage report artifact
- name: 'Download coverage and execution report'
uses: actions/download-artifact@v4
with:
pattern: '*-report'

# Perform the SonarQube scan
- uses: sonarsource/sonarqube-scan-action@master
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}

# Optional: Fail the job if Quality Gate is red
# If you wish to fail your job when the Quality Gate is red, uncomment the
# following lines. This would typically be used to fail a deployment.
# - uses: sonarsource/sonarqube-quality-gate-action@master
# timeout-minutes: 5
# env:
# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
# SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}

deploy:
name: Deploy
runs-on: ubuntu-latest
needs: sonarqube
environment: production
env:
DOCKER_BUILDKIT: 1
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
name: Test
on: push
on:
push:

jobs:
unittests:
name: Run unit tests
# Run on merge to main, where the commit name starts with "Bump version:" (for bump2version)
# if: "startsWith(github.event.head_commit.message, 'Bump version:')"
runs-on: ubuntu-latest
env:
DOCKER_BUILDKIT: 1
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,5 @@ profiles

# sonarqube
.scannerwork/

latest_backup/
18 changes: 16 additions & 2 deletions db/deploy/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,31 @@ ENV LIQUIBASE_VERSION=4.26.0
ENV MARIADB_JDBC_VERSION=3.0.3

# Install system dependencies
RUN apt-get update && apt-get install -y --no-install-recommends gcc git ssh default-jdk wget unzip
RUN apt-get update && apt-get install -y --no-install-recommends gcc git ssh default-jdk wget unzip \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

# Create a non-root user and group
RUN groupadd -r appuser && useradd -r -g appuser -d /home/appuser -m -s /bin/bash appuser

# Download and install Liquibase
RUN wget https://github.com/liquibase/liquibase/releases/download/v${LIQUIBASE_VERSION}/liquibase-${LIQUIBASE_VERSION}.zip \
&& unzip liquibase-${LIQUIBASE_VERSION}.zip -d /opt/liquibase \
&& chmod +x /opt/liquibase/liquibase \
# Clean up to reduce layer size
&& rm liquibase-${LIQUIBASE_VERSION}.zip \
# Clean up to reduce layer size
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

# Change ownership of the liquibase directory to the non-root user
RUN chown -R appuser:appuser /opt/liquibase

# Switch to the non-root user
USER appuser

# Set the working directory
WORKDIR /home/appuser

# Download the MariaDB JDBC driver
RUN wget https://downloads.mariadb.com/Connectors/java/connector-java-${MARIADB_JDBC_VERSION}/mariadb-java-client-${MARIADB_JDBC_VERSION}.jar
RUN mv mariadb-java-client-${MARIADB_JDBC_VERSION}.jar /opt/
Expand Down
2 changes: 1 addition & 1 deletion db/python/connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
)
from models.models.project import Project, ProjectId, ProjectMemberRole

logging.basicConfig(level=logging.DEBUG)
logging.basicConfig(level=logging.WARNING)
logger = logging.getLogger(__name__)

TABLES_ORDERED_BY_FK_DEPS = [
Expand Down
2 changes: 1 addition & 1 deletion db/python/gcp_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from db.python.utils import InternalError

logging.basicConfig(level=logging.DEBUG)
logging.basicConfig(level=logging.WARNING)
logger = logging.getLogger(__name__)


Expand Down
21 changes: 16 additions & 5 deletions deploy/api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,26 @@ ENV SM_ENVIRONMENT ${SM_ENVIRONMENT}
# Allow statements and log messages to immediately appear in the Knative logs.
ENV PYTHONUNBUFFERED 1

EXPOSE $PORT
# Create a non-root user and group
RUN groupadd -r appuser && useradd -r -g appuser -d /home/appuser -m -s /bin/bash appuser

# Set the working directory
WORKDIR /app/sample_metadata/
COPY requirements.txt requirements.txt

# Copy requirements file and install dependencies
COPY requirements.txt requirements.txt
RUN pip install --no-cache-dir --no-deps -r requirements.txt

# Copy the application code
COPY api api
COPY db db/
COPY models models/

CMD uvicorn --port ${PORT} --host 0.0.0.0 api.server:app
# Change ownership of the application directory to the non-root user
RUN chown -R appuser:appuser /app/sample_metadata/

# Switch to the non-root user
USER appuser

EXPOSE $PORT

# Command to run the FastAPI app
CMD ["uvicorn", "api.main:app", "--host", "0.0.0.0", "--port", "$PORT"]
2 changes: 1 addition & 1 deletion metamist/audit/audit_upload_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ def main(

if __name__ == '__main__':
logging.basicConfig(
level=logging.INFO,
level=logging.WARNING,
format='%(asctime)s %(levelname)s %(module)s:%(lineno)d - %(message)s',
datefmt='%Y-%m-%d %H:%M:%S',
stream=sys.stderr,
Expand Down
2 changes: 1 addition & 1 deletion metamist/audit/delete_assay_files_from_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def main(delete_field_name, delete_file_path):

if __name__ == '__main__':
logging.basicConfig(
level=logging.INFO,
level=logging.WARNING,
format='%(asctime)s %(levelname)s %(module)s:%(lineno)d - %(message)s',
datefmt='%Y-%M-%d %H:%M:%S',
stream=sys.stderr,
Expand Down
3 changes: 1 addition & 2 deletions metamist/parser/generic_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@
else:
from typing_extensions import Literal

logging.basicConfig()
logging.basicConfig(level=logging.WARNING)
logger = logging.getLogger(__file__)
logger.setLevel(logging.INFO)

PRIMARY_EXTERNAL_ORG = ''

Expand Down
4 changes: 4 additions & 0 deletions sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@ sonar.projectKey=populationgenomics_metamist
sonar.python.version=3.11
sonar.python.coverage.reportPaths=coverage.xml
sonar.python.xunit.reportPath=test-execution.xml
sonar.sources=.
sonar.exclusions=test/**,*.xml
sonar.tests=test/
sonar.cpd.cache.enabled=true
2 changes: 1 addition & 1 deletion web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react'
import { BrowserRouter as Router } from 'react-router-dom'
import Routes from './Routes'
import NavBar from './shared/components/Header/NavBar'
import { ViewerContext, useViewer } from './viewer'
import { useViewer, ViewerContext } from './viewer'

const App: React.FunctionComponent = () => {
const { viewer, error } = useViewer()
Expand Down
2 changes: 1 addition & 1 deletion web/src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ body {

background-color: var(--color-bg);
color: var(--color-text-primary);
background: var(--color-bg);
}

code {
Expand Down Expand Up @@ -203,6 +202,7 @@ blockquote {
.ui .modal > .content {
background: var(--color-bg-card);
}

.ui .modal > .header {
background: var(--color-bg-card);
color: var(--color-text-primary);
Expand Down
6 changes: 1 addition & 5 deletions web/src/pages/billing/BillingCostByAnalysis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { Button, Card, Dropdown, Grid, Input, Message } from 'semantic-ui-react'
import { PaddedPage } from '../../shared/components/Layout/PaddedPage'
import LoadingDucks from '../../shared/components/LoadingDucks/LoadingDucks'
import generateUrl from '../../shared/utilities/generateUrl'
import { getMonthStartDate } from '../../shared/utilities/monthStartEndDate'
import { AnalysisCostRecord, BillingApi } from '../../sm-api'
import BatchGrid from './components/BatchGrid'

Expand All @@ -22,9 +21,6 @@ const BillingCostByAnalysis: React.FunctionComponent = () => {
const [isLoading, setIsLoading] = React.useState<boolean>(true)
const [error, setError] = React.useState<string | undefined>()
const [data, setData] = React.useState<AnalysisCostRecord[] | undefined>()
const [start, setStart] = React.useState<string>(
searchParams.get('start') ?? getMonthStartDate()
)

const setBillingRecord = (records: AnalysisCostRecord[]) => {
setIsLoading(false)
Expand Down Expand Up @@ -138,7 +134,7 @@ const BillingCostByAnalysis: React.FunctionComponent = () => {
<Message negative onDismiss={() => setError(undefined)}>
{error}
<br />
<Button negative onClick={() => setStart(start)}>
<Button negative onClick={() => window.location.reload()}>
Retry
</Button>
</Message>
Expand Down
2 changes: 1 addition & 1 deletion web/src/pages/billing/BillingCostByCategory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ const BillingCostByCategory: React.FunctionComponent = () => {
<Message negative onDismiss={() => setError(undefined)}>
{error}
<br />
<Button negative onClick={() => setStart(start)}>
<Button negative onClick={() => window.location.reload()}>
Retry
</Button>
</Message>
Expand Down
2 changes: 1 addition & 1 deletion web/src/pages/billing/BillingCostByMonth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ const BillingCostByTime: React.FunctionComponent = () => {
<Message negative onDismiss={() => setError(undefined)}>
{error}
<br />
<Button negative onClick={() => setStart(start)}>
<Button negative onClick={() => window.location.reload()}>
Retry
</Button>
</Message>
Expand Down
2 changes: 1 addition & 1 deletion web/src/pages/billing/BillingCostByTime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ const BillingCostByTime: React.FunctionComponent = () => {
<Message negative onDismiss={() => setError(undefined)}>
{error}
<br />
<Button negative onClick={() => setStart(start)}>
<Button negative onClick={() => window.location.reload()}>
Retry
</Button>
</Message>
Expand Down
2 changes: 1 addition & 1 deletion web/src/pages/billing/BillingHome.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ReactGoogleSlides from 'react-google-slides'
import { Button, Menu, MenuItem, Segment, SemanticWIDTHS } from 'semantic-ui-react'
import { PaddedPage } from '../../shared/components/Layout/PaddedPage'
import { ThemeContext } from '../../shared/components/ThemeProvider'
import { IBillingPage, billingPages } from './BillingPages'
import { billingPages, IBillingPage } from './BillingPages'

// Google Slides
const Slides = React.memo(function ReactGoogleSlidesWrapper({ link }: { link: string }) {
Expand Down
Loading

0 comments on commit a4450ee

Please sign in to comment.