From 074e8c844ec20317827cb4aa886c40de212732dc Mon Sep 17 00:00:00 2001 From: pgaetani Date: Fri, 13 Dec 2024 10:58:06 +0100 Subject: [PATCH 1/6] new pr workflow, pytest fixes --- .github/workflows/docker.yml | 62 --------------- .github/workflows/pr.yaml | 148 +++++++++++++++++++++++++++++++++++ pytest.ini | 2 +- 3 files changed, 149 insertions(+), 63 deletions(-) delete mode 100644 .github/workflows/docker.yml create mode 100644 .github/workflows/pr.yaml diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml deleted file mode 100644 index 5797c453d6..0000000000 --- a/.github/workflows/docker.yml +++ /dev/null @@ -1,62 +0,0 @@ -name: Docker - -on: - push: - branches: ["*"] - tags: ["*"] - -env: - REGISTRY: ghcr.io - REPO: ${{ github.repository }} - -jobs: - build: - runs-on: ubuntu-latest - - permissions: - contents: read - packages: write - - steps: - - name: Check out repository - uses: actions/checkout@v3 - - - name: Generate version.py before building image - run: | - echo -e "__git_commit__ = \"${{ github.sha }}\"\n__time__ = \"$(date)\"" > ./app/version.py - - - name: Set up QEMU - uses: docker/setup-qemu-action@v2 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 - - - name: Log in to container registry - uses: docker/login-action@v2 - with: - registry: ${{ env.REGISTRY }} - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Keep Docker happy - run: | - echo "REPO=${GITHUB_REPOSITORY,,}" >>${GITHUB_ENV} - # This is only here because docker freaks out if the repo/tag that you're pushing to is not all lowercase - - - name: Extract metadata from Git - id: meta - uses: docker/metadata-action@v4 - with: - images: ${{ env.REGISTRY }}/${{ env.REPO }} - - - name: Build and push API image - id: build-admin-image - uses: docker/build-push-action@v4 - with: - platforms: linux/amd64 - context: . - file: ./docker/Dockerfile - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - push: false - # TODO: add a git tag and associated docker tag for it on merge diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml new file mode 100644 index 0000000000..d715870c35 --- /dev/null +++ b/.github/workflows/pr.yaml @@ -0,0 +1,148 @@ +name: PR Validation + +on: + pull_request: + branches: ["main"] + types: [opened, synchronize] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + # Don't want the tests running in parallel + cancel-in-progress: true + +jobs: + app-build-and-test: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.11"] + node-version: ["20.10.0"] + + steps: + - uses: szenius/set-timezone@v2.0 + with: + timezoneLinux: "Europe/Amsterdam" + name: Set Timezone to Europe/Amsterdam + + - uses: actions/checkout@v4 + name: Checkout repository + + - uses: gerlero/apt-install@v1 + with: + packages: build-essential git libcurl4-openssl-dev curl libssl-dev + install-recommends: false + name: Install OS dependencies (apt) + + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: pip + cache-dependency-path: pyproject.toml + name: Install Python [${{ matrix.python-version }}] and setup cache + + - uses: BSFishy/pip-action@v1 + with: + requirements: requirements_for_test.txt + name: Install application requirements (pip) + + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + cache: npm + cache-dependency-path: package-lock.json + name: Install NodeJS [${{ matrix.node-version }}] and setup cache + + - uses: anna-money/github-actions-npm@v2 + with: + target: ci + name: Install application requirements (npm) + + - uses: astral-sh/ruff-action@v2 + name: Linting (ruff) + + - uses: jpetrucciani/black-check@master + name: Formatting (black) + + - uses: BSFishy/pip-action@v1 + with: + packages: | + pytest-md + pytest-emoji + name: Install test dependencies + + # TODO: fix automated tests + # Connection from runner to test database is needed for tests to run + # or somehow sending a custom job to k8s that would run the tests and report back + - uses: pavelzw/pytest-action@v2 + with: + verbose: false + job-summary: true + continue-on-error: true + name: Run tests (pytest) + + - uses: anna-money/github-actions-npm@v2 + with: + target: test + name: Run tests (node) + + docker-build-and-push: + runs-on: ubuntu-latest + needs: app-build-and-test + + steps: + - uses: actions/checkout@v4 + + - run: | + TAG=$(date +%Y%m%d).${{ github.run_number }}.dev + echo "TAG=$TAG" >> $GITHUB_ENV + echo "tag=$TAG" >> $GITHUB_OUTPUT + name: Set tag + id: set-tag + + - run: | + echo -e "__git_commit__ = \"${{ github.sha }}\"\n__time__ = \"$(date)\"\n__version__ = \"${{ env.TAG }}\"" > ./app/version.py + name: Generate version.py before building image + + - uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + name: Login at dockerhub + + - uses: docker/setup-qemu-action@v3 + name: Setup QEMU + + - uses: docker/setup-buildx-action@v3 + name: Setup buildx + + - uses: docker/build-push-action@v6 + with: + file: docker/Dockerfile + push: true + tags: worthnl/notifynl-admin:${{ env.TAG }} + name: Docker build and push ${{ env.TAG }} + + outputs: + tag: ${{ steps.set-tag.outputs.tag }} + + helm-release: + runs-on: ubuntu-latest + needs: docker-build-and-push + environment: Test + + steps: + - uses: actions/checkout@v4 + with: + repository: Worth-NL/notifynl-charts-private + ref: main + token: ${{ secrets.WORTHNL_PAT }} + name: Checkout Worth-NL/notifynl-charts-private + + - uses: bwvolleyball/k8s-toolkit@v1.0.0 + with: + config: ${{ secrets.K8S_CONFIG }} + + - run: | + helm version + helm upgrade --install notifynl-admin notifynl-admin/ -n ${{ secrets.K8S_NAMESPACE }} --reuse-values --set dockerTagOverride=${{ needs.docker-build-and-push.outputs.tag }} --wait + name: Deploy chart diff --git a/pytest.ini b/pytest.ini index 5e9b75af86..d08a71490c 100644 --- a/pytest.ini +++ b/pytest.ini @@ -13,4 +13,4 @@ env = filterwarnings = error:Applying marks directly:pytest.RemovedInPytest4Warning -addopts = -p no:warnings +addopts = -p no:warnings --maxfail=10 From f2bf9cdab1292537e1c890499d071e9fd5d2ae59 Mon Sep 17 00:00:00 2001 From: pgaetani Date: Fri, 13 Dec 2024 11:04:45 +0100 Subject: [PATCH 2/6] npm troubleshooting --- .github/workflows/pr.yaml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index d715870c35..f9fd2d66c4 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -52,9 +52,7 @@ jobs: cache-dependency-path: package-lock.json name: Install NodeJS [${{ matrix.node-version }}] and setup cache - - uses: anna-money/github-actions-npm@v2 - with: - target: ci + - run: npm ci name: Install application requirements (npm) - uses: astral-sh/ruff-action@v2 @@ -80,9 +78,7 @@ jobs: continue-on-error: true name: Run tests (pytest) - - uses: anna-money/github-actions-npm@v2 - with: - target: test + - run: npm test name: Run tests (node) docker-build-and-push: From d15ea6525b243db0ed575bfb290d0ea974eecd4b Mon Sep 17 00:00:00 2001 From: pgaetani Date: Fri, 13 Dec 2024 11:13:33 +0100 Subject: [PATCH 3/6] ruff corrections and update --- .pre-commit-config.yaml | 4 ++-- app/__init__.py | 4 +--- app/main/forms.py | 2 +- app/main/validators.py | 1 - app/main/views/agreement.py | 2 +- app/main/views/email_branding.py | 4 +--- app/main/views/index.py | 3 +-- app/main/views/pricing.py | 6 +----- app/main/views/security_policy.py | 1 + app/main/views/service_settings/index.py | 1 - app/main/views/sign_in.py | 1 - requirements_for_test.txt | 4 ++-- 12 files changed, 11 insertions(+), 22 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 995b890468..3eddef6625 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,12 +7,12 @@ repos: - id: check-yaml - id: debug-statements - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: 'v0.0.275' + rev: 'v0.8.3' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] - repo: https://github.com/psf/black - rev: 23.10.1 + rev: 24.10.0 hooks: - id: black name: black (python) diff --git a/app/__init__.py b/app/__init__.py index ed886ee466..b76da318e8 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -16,8 +16,6 @@ session, url_for, ) - -from app.limiters import init_limiters from flask_login import LoginManager, current_user from flask_wtf import CSRFProtect from flask_wtf.csrf import CSRFError @@ -80,6 +78,7 @@ redact_mobile_number, valid_phone_number, ) +from app.limiters import init_limiters from app.models.organisation import Organisation from app.models.service import Service from app.models.user import AnonymousUser, User @@ -130,7 +129,6 @@ from app.utils import format_provider from app.utils.user_id import get_user_id_from_flask_login_session - login_manager = LoginManager() csrf = CSRFProtect() metrics = GDSMetrics() diff --git a/app/main/forms.py b/app/main/forms.py index 3c9ce8627b..86d8d239e7 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,8 +1,8 @@ -from html import escape import weakref from contextlib import suppress from datetime import datetime, timedelta from functools import partial +from html import escape from itertools import chain from numbers import Number diff --git a/app/main/validators.py b/app/main/validators.py index 2683f34b73..8912552a12 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -43,7 +43,6 @@ def __call__(self, form, field): if field.data == "": return - from flask import url_for # message = """ # Enter a public sector email address or diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index 260e4c892a..5aebe0845f 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -4,7 +4,7 @@ from flask_login import current_user from app import current_service -from app.limiters import limiter, RateLimit +from app.limiters import RateLimit, limiter from app.main import main from app.main.forms import AcceptAgreementForm from app.models.organisation import Organisation diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py index 71d140e5c5..14f8154645 100644 --- a/app/main/views/email_branding.py +++ b/app/main/views/email_branding.py @@ -1,6 +1,6 @@ from io import BytesIO -from flask import abort, current_app, flash, redirect, render_template, request, url_for +from flask import current_app, flash, redirect, render_template, request, url_for from flask_login import current_user from notifications_python_client.errors import HTTPError from werkzeug.datastructures import FileStorage @@ -12,13 +12,11 @@ from app.main.forms import ( AdminEditEmailBrandingForm, GovernmentIdentityCoatOfArmsOrInsignia, - GovernmentIdentityColour, SearchByNameForm, ) from app.models.branding import ( AllEmailBranding, EmailBranding, - get_government_identity_system_crests_or_insignia, get_insignia_asset_path, ) from app.s3_client.logo_client import logo_client diff --git a/app/main/views/index.py b/app/main/views/index.py index a2636df022..762cf77996 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -18,9 +18,8 @@ from app.main import main from app.main.forms import FieldWithNoneOption from app.main.views.pricing import CURRENT_SMS_RATE -from app.main.views.sub_navigation_dictionaries import features_nav, using_notify_nav +from app.main.views.sub_navigation_dictionaries import using_notify_nav from app.models.branding import EmailBranding -from app.models.letter_rates import LetterRates from app.utils.templates import TemplatedLetterImageTemplate redirects = Blueprint("redirects", __name__) diff --git a/app/main/views/pricing.py b/app/main/views/pricing.py index 8c9133aa69..8f06ca9afd 100644 --- a/app/main/views/pricing.py +++ b/app/main/views/pricing.py @@ -1,12 +1,8 @@ -from flask import current_app, render_template -from flask_login import current_user -from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES +from flask import render_template from app.limiters import RateLimit from app.main import main -from app.main.forms import SearchByNameForm from app.main.views.sub_navigation_dictionaries import pricing_nav -from app.models.letter_rates import LetterRates CURRENT_SMS_RATE = "1.97" diff --git a/app/main/views/security_policy.py b/app/main/views/security_policy.py index 31394e1c14..b5ac8cc43a 100644 --- a/app/main/views/security_policy.py +++ b/app/main/views/security_policy.py @@ -1,4 +1,5 @@ from flask import redirect + from app.limiters import RateLimit from app.main import main diff --git a/app/main/views/service_settings/index.py b/app/main/views/service_settings/index.py index da8adc1fdb..64df92e15f 100644 --- a/app/main/views/service_settings/index.py +++ b/app/main/views/service_settings/index.py @@ -68,7 +68,6 @@ EmailBranding, LetterBranding, ) -from app.models.letter_rates import LetterRates from app.utils import DELIVERED_STATUSES, FAILURE_STATUSES, SENDING_STATUSES from app.utils.constants import SIGN_IN_METHOD_TEXT_OR_EMAIL from app.utils.services import service_has_or_is_expected_to_send_x_or_more_notifications diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index feb71c483c..c9f076d8ac 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -14,7 +14,6 @@ from markupsafe import Markup from app import login_manager -from app.limiters import RateLimit from app.main import main from app.main.forms import LoginForm from app.models.user import InvitedUser, User diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 9c2fcb2ecd..70b710f3a8 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,6 +1,6 @@ -r requirements.txt -black==23.10.1 # Also update `.pre-commit-config.yaml` if this changes -ruff==0.0.275 # Also update `.pre-commit-config.yaml` if this changes +black==24.10.0 # Also update `.pre-commit-config.yaml` if this changes +ruff==0.8.3 # Also update `.pre-commit-config.yaml` if this changes pytest==7.2.0 pytest-env==0.8.1 pytest-mock==3.10.0 From 500b549c28c63f856034f2e3ab7cf542095e8b6e Mon Sep 17 00:00:00 2001 From: pgaetani Date: Fri, 13 Dec 2024 11:20:42 +0100 Subject: [PATCH 4/6] ruff corrections --- app/main/views/pricing.py | 9 +++++---- tests/app/models/test_webauthn_credential.py | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/main/views/pricing.py b/app/main/views/pricing.py index 8f06ca9afd..5ad70428d4 100644 --- a/app/main/views/pricing.py +++ b/app/main/views/pricing.py @@ -18,13 +18,14 @@ def guidance_pricing(): # @main.route("/pricing/text-messages") # def guidance_pricing_text_messages(): +# sorted_international_rates = sorted( +# [(cc, country["names"], country["billable_units"]) for cc, country in INTERNATIONAL_BILLING_RATES.items()], +# key=lambda x: x[0], +# ) # return render_template( # "views/guidance/pricing/text-message-pricing.html", # sms_rate=CURRENT_SMS_RATE, -# international_sms_rates=sorted( -# [(cc, country["names"], country["billable_units"]) for cc, country in INTERNATIONAL_BILLING_RATES.items()], -# key=lambda x: x[0], -# ), +# international_sms_rates=sorted_international_rates, # _search_form=SearchByNameForm(), # navigation_links=pricing_nav(), # ) diff --git a/tests/app/models/test_webauthn_credential.py b/tests/app/models/test_webauthn_credential.py index 410e68c8f1..cbd12477aa 100644 --- a/tests/app/models/test_webauthn_credential.py +++ b/tests/app/models/test_webauthn_credential.py @@ -49,8 +49,8 @@ def test_from_registration_encodes_as_unicode(webauthn_dev_server): serialized_credential = credential.serialize() - assert type(serialized_credential["credential_data"]) == str - assert type(serialized_credential["registration_response"]) == str + assert type(serialized_credential["credential_data"]) is str + assert type(serialized_credential["registration_response"]) is str def test_from_registration_handles_library_errors(notify_admin): From c4029df301df5a6e874aba3d6b76557c2e6f4540 Mon Sep 17 00:00:00 2001 From: pgaetani Date: Fri, 13 Dec 2024 11:23:56 +0100 Subject: [PATCH 5/6] black formatting --- app/main/validators.py | 1 - get_zendesk_tickets.py | 1 + tests/app/utils/test_templates.py | 16 +++++++++------- tests/conftest.py | 8 +++++--- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/main/validators.py b/app/main/validators.py index 8912552a12..13d7428150 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -43,7 +43,6 @@ def __call__(self, form, field): if field.data == "": return - # message = """ # Enter a public sector email address or # find out who can use Notify diff --git a/get_zendesk_tickets.py b/get_zendesk_tickets.py index b33e24a1b7..daed43436e 100644 --- a/get_zendesk_tickets.py +++ b/get_zendesk_tickets.py @@ -2,6 +2,7 @@ This script can be used to retrieve Zendesk tickets. This can be run locally if you set the ZENDESK_API_KEY. Or the script can be run from a flask shell from a ssh session. """ + # flake8: noqa: T001 (print) import csv diff --git a/tests/app/utils/test_templates.py b/tests/app/utils/test_templates.py index 0ff284ff06..fd5f2c8a76 100644 --- a/tests/app/utils/test_templates.py +++ b/tests/app/utils/test_templates.py @@ -675,13 +675,15 @@ def test_first_attachment_page(self, mocker, fake_uuid, mocker_kwargs, expected_ service_id=SERVICE_ONE_ID, id_=fake_uuid, type_="letter", - letter_attachment={ - "id": "abc", - "original_filename": "blah.pdf", - "page_count": mocker_kwargs["attachment_page_count"], - } - if expected_value - else {}, + letter_attachment=( + { + "id": "abc", + "original_filename": "blah.pdf", + "page_count": mocker_kwargs["attachment_page_count"], + } + if expected_value + else {} + ), ), ) diff --git a/tests/conftest.py b/tests/conftest.py index 13a04b136a..b1e4db5afb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1061,9 +1061,11 @@ def create_service_templates(service_id, number_of_templates=6): name=f"{template_type}_template_{template_number}", type_=template_type, content=f"{template_type} template {template_number} content", - subject=f"{template_type} template {template_number} subject" - if template_type in ["email", "letter"] - else None, + subject=( + f"{template_type} template {template_number} subject" + if template_type in ["email", "letter"] + else None + ), ) ) From 2e430764a702c0ff9a1e97b1dd0de1da30ad1d77 Mon Sep 17 00:00:00 2001 From: pgaetani Date: Mon, 16 Dec 2024 10:27:18 +0100 Subject: [PATCH 6/6] merge action added --- .github/workflows/merge.yaml | 121 +++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 .github/workflows/merge.yaml diff --git a/.github/workflows/merge.yaml b/.github/workflows/merge.yaml new file mode 100644 index 0000000000..b460e92f43 --- /dev/null +++ b/.github/workflows/merge.yaml @@ -0,0 +1,121 @@ +name: Merge tag and release + +on: + push: + branches: + - master + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + # Don't want multiple builds running in parallel + cancel-in-progress: true + +jobs: + docker-build-and-push: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - run: echo "TAG=$(date +%Y%m%d).${{ github.run_number }}" >> $GITHUB_ENV + name: Set tag + id: set-tag + + - run: | + echo -e "__git_commit__ = \"${{ github.sha }}\"\n__time__ = \"$(date)\"\n__version__ = \"${{ env.TAG }}\"" > ./app/version.py + name: Generate version.py before building image + + - uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + name: Login at dockerhub + + - uses: docker/setup-qemu-action@v3 + name: Setup QEMU + + - uses: docker/setup-buildx-action@v3 + name: Setup buildx + + - uses: docker/build-push-action@v6 + with: + file: docker/Dockerfile + push: true + tags: worthnl/notifynl-admin:latest,worthnl/notifynl-admin:${{ env.TAG }} + name: Docker build and push ${{ env.TAG }} + + - uses: mathieudutour/github-tag-action@v6.2 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + custom_tag: ${{ env.TAG }} + name: Create git tag + + - uses: softprops/action-gh-release@v2 + with: + tag_name: ${{ env.TAG }} + make_latest: true + name: Create Github release + + outputs: + tag: ${{ steps.set-tag.outputs.tag }} + + helm-chart-bump: + runs-on: ubuntu-latest + needs: docker-build-and-push + + steps: + - uses: actions/checkout@v4 + with: + repository: Worth-NL/notifynl-charts-private + ref: main + token: ${{ secrets.WORTHNL_PAT }} + name: Checkout Worth-NL/notifynl-charts-private + + - uses: pietrobolcato/action-read-yaml@1.0.0 + with: + config: notifynl-admin/Chart.yaml + name: Read Chart.yaml + id: yaml-read + + - uses: olegsu/semver-action@v1 + with: + version: ${{ steps.yaml-read.outputs['version'] }} + name: Chart version bump + id: version-bump + + - uses: rmeneely/update-yaml@v1 + with: + infile: notifynl-api/Chart.yaml + varlist: version=${{ steps.version-bump.outputs.version }},appVersion=${{ needs.docker-build-and-push.outputs.tag }} + name: Update Chart.yaml + + - uses: offensive-vk/auto-commit-push@v7 + with: + message: 🤖 notifynl-admin chart bump + github-token: ${{ secrets.WORTHNL_PAT }} + + helm-release: + runs-on: ubuntu-latest + needs: helm-chart-bump + environment: Test + + steps: + - uses: actions/checkout@v4 + with: + repository: Worth-NL/notifynl-charts-private + ref: main + token: ${{ secrets.WORTHNL_PAT }} + name: Checkout Worth-NL/notifynl-charts-private + + - run: | + mkdir -p $HOME/.kube + echo "${{ secrets.K8S_CONFIG }}" > $HOME/.kube/config + name: Setup kubernetes config + + - uses: azure/setup-helm@v4 + name: Install helm + + - run: | + helm version + helm upgrade --install notifynl-admin notifynl-admin/ -n ${{ secrets.K8S_NAMESPACE }} --reuse-values --set dockerTagOverride=${{ needs.docker-build-and-push.outputs.tag }} --wait + name: Deploy chart