From 237139644ddb93708e7c6bffbe714c1e5c65b9e8 Mon Sep 17 00:00:00 2001 From: Ian Liu <81595625+ianliuwk1019@users.noreply.github.com> Date: Wed, 16 Oct 2024 16:44:03 -0700 Subject: [PATCH] fix: #1628 Improve local testcontainer startup to avoid overriding running main docker db instance (#1632) --- .github/workflows/sonar.yml | 19 ++++------ docker-base-services.yml | 5 --- docker-compose-testcontainer.yml | 20 ++++++++--- docker-compose.yml | 5 +++ server/admin_management/tests/conftest.py | 2 +- server/auth_function/test/conftest.py | 2 +- server/backend/testspg/conftest.py | 2 +- .../router/test_router_permission_audit.py | 36 ++++++------------- 8 files changed, 40 insertions(+), 51 deletions(-) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 6e3c0b36a..e608f56ea 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -9,9 +9,6 @@ on: jobs: ci-auth: runs-on: ubuntu-latest - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres steps: - uses: actions/checkout@v4 with: @@ -24,6 +21,10 @@ jobs: - name: Tests and coverage env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }} + POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }} FC_API_TOKEN_TEST: ${{ secrets.FOREST_CLIENT_API_API_KEY_TEST }} run: | cd server/auth_function @@ -53,11 +54,6 @@ jobs: env: environment: dev organization: bcgov - POSTGRES_USER: fam_proxy_api - POSTGRES_PASSWORD: test - POSTGRES_HOST: localhost - POSTGRES_DB: fam - POSTGRES_PORT: 5432 USE_POSTGRES: false steps: - uses: actions/checkout@v4 @@ -80,6 +76,7 @@ jobs: POSTGRES_HOST: ${{ vars.LOCAL_POSTGRES_HOST }} POSTGRES_DB: ${{ vars.LOCAL_POSTGRES_DB }} POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }} + POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }} COGNITO_REGION: ${{ vars.LOCAL_COGNITO_REGION }} COGNITO_USER_POOL_ID: ${{ vars.LOCAL_COGNITO_USER_POOL_ID }} COGNITO_CLIENT_ID: ${{ vars.LOCAL_COGNITO_CLIENT_ID }} @@ -113,11 +110,6 @@ jobs: env: environment: dev organization: bcgov - POSTGRES_USER: fam_admin_management_api - POSTGRES_PASSWORD: test - POSTGRES_HOST: localhost - POSTGRES_DB: fam - POSTGRES_PORT: 5432 steps: - uses: actions/checkout@v4 with: @@ -138,6 +130,7 @@ jobs: POSTGRES_HOST: ${{ vars.LOCAL_POSTGRES_HOST }} POSTGRES_DB: ${{ vars.LOCAL_POSTGRES_DB }} POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }} + POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }} COGNITO_REGION: ${{ vars.LOCAL_COGNITO_REGION }} COGNITO_USER_POOL_ID: ${{ vars.LOCAL_COGNITO_USER_POOL_ID }} COGNITO_CLIENT_ID: ${{ vars.LOCAL_COGNITO_CLIENT_ID }} diff --git a/docker-base-services.yml b/docker-base-services.yml index 904c2bca4..392be6aab 100644 --- a/docker-base-services.yml +++ b/docker-base-services.yml @@ -8,21 +8,17 @@ services: fam-database: image: postgres:14.1-alpine - container_name: fam-postgres mem_limit: 128m environment: - POSTGRES_USER=postgres - POSTGRES_PASSWORD=postgres - POSTGRES_DB=fam - POSTGRES_HOST_AUTH_METHOD=password - - POSTGRES_PORT=5432 healthcheck: test: ["CMD-SHELL", "pg_isready -U postgres"] interval: 5s timeout: 5s retries: 6 - ports: - - "5432:5432" networks: - fam @@ -31,7 +27,6 @@ services: context: server/flyway dockerfile: Dockerfile environment: - - FLYWAY_URL=jdbc:postgresql://fam-postgres:5432/fam - FLYWAY_USER=postgres - FLYWAY_PASSWORD=postgres - FLYWAY_BASELINE_ON_MIGRATE=true diff --git a/docker-compose-testcontainer.yml b/docker-compose-testcontainer.yml index bc9db8cfa..c6452f66f 100644 --- a/docker-compose-testcontainer.yml +++ b/docker-compose-testcontainer.yml @@ -1,16 +1,28 @@ +# Note, the compose service name and container_name for "fam-database" and "fam-flyway" are named differently +# than the main application docker-compose so that when testcontainer is up it does not delete previous running main container. --- + services: - fam-database: + fam-database-testcontainer: extends: file: docker-base-services.yml service: fam-database + container_name: fam-postgres-testcontainer + environment: + - PGPORT=${POSTGRES_PORT_TESTCONTAINER} # the easiest way to change postgres db default port from '5432' to PGPORT. + ports: + - "${POSTGRES_PORT_TESTCONTAINER}:${POSTGRES_PORT_TESTCONTAINER}" # use host-port: POSTGRES_PORT_TESTCONTAINER ='5433' for testcontainer db instance than '5432' for main db. - fam-flyway: + fam-flyway-testcontainer: extends: file: docker-base-services.yml service: fam-flyway + environment: + # note: this environment is separated out from docker-base-services.yml for testcontainer to avoid + # colliding with main app db instance. + - FLYWAY_URL=jdbc:postgresql://fam-postgres-testcontainer:${POSTGRES_PORT_TESTCONTAINER}/fam depends_on: - fam-database: + fam-database-testcontainer: condition: service_healthy fam-test-nginx: @@ -19,7 +31,7 @@ services: ports: - "8181:80" depends_on: - fam-flyway: + fam-flyway-testcontainer: condition: service_completed_successfully networks: diff --git a/docker-compose.yml b/docker-compose.yml index 6abdf0494..3a26e1bd3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,11 +4,16 @@ services: extends: file: docker-base-services.yml service: fam-database + container_name: fam-postgres + ports: + - "${POSTGRES_PORT}:${POSTGRES_PORT}" fam-flyway: extends: file: docker-base-services.yml service: fam-flyway + environment: + - FLYWAY_URL=jdbc:postgresql://fam-postgres:${POSTGRES_PORT}/fam depends_on: fam-database: condition: service_healthy diff --git a/server/admin_management/tests/conftest.py b/server/admin_management/tests/conftest.py index 41aad5168..893fa4a15 100644 --- a/server/admin_management/tests/conftest.py +++ b/server/admin_management/tests/conftest.py @@ -77,7 +77,7 @@ def db_pg_connection(db_pg_container): + f"{os.environ.get('POSTGRES_USER')}:" + f"{os.environ.get('POSTGRES_PASSWORD')}@" + f"{os.environ.get('POSTGRES_HOST')}:" - f"{os.environ.get('POSTGRES_PORT')}/" + f"{os.environ.get('POSTGRES_PORT_TESTCONTAINER')}/" f"{os.environ.get('POSTGRES_DB')}" ) diff --git a/server/auth_function/test/conftest.py b/server/auth_function/test/conftest.py index 2df70ef7d..c363371af 100644 --- a/server/auth_function/test/conftest.py +++ b/server/auth_function/test/conftest.py @@ -43,7 +43,7 @@ def get_local_db_string(): password = os.getenv("POSTGRES_PASSWORD", "postgres") # test host = os.getenv("POSTGRES_HOST", "localhost") dbname = os.getenv("POSTGRES_DB", "fam") - port = os.getenv("POSTGRES_PORT", "5432") + port = os.getenv("POSTGRES_PORT_TESTCONTAINER", "5433") db_conn_string = ( f"user={username} password={password} host={host} " diff --git a/server/backend/testspg/conftest.py b/server/backend/testspg/conftest.py index 7c4ab3198..2d499805f 100644 --- a/server/backend/testspg/conftest.py +++ b/server/backend/testspg/conftest.py @@ -59,7 +59,7 @@ def db_pg_connection(db_pg_container): + f"{os.environ.get('POSTGRES_USER')}:" + f"{os.environ.get('POSTGRES_PASSWORD')}@" + f"{os.environ.get('POSTGRES_HOST')}:" - f"{os.environ.get('POSTGRES_PORT')}/" + f"{os.environ.get('POSTGRES_PORT_TESTCONTAINER')}/" f"{os.environ.get('POSTGRES_DB')}" ) diff --git a/server/backend/testspg/router/test_router_permission_audit.py b/server/backend/testspg/router/test_router_permission_audit.py index 887aab727..2d021d9c8 100644 --- a/server/backend/testspg/router/test_router_permission_audit.py +++ b/server/backend/testspg/router/test_router_permission_audit.py @@ -1,33 +1,17 @@ -import pytest -from fastapi.testclient import TestClient -from api.app.main import app, apiPrefix +from api.app.main import apiPrefix from testspg.fixture.permission_audit_fixture import ( - APPLICATION_ID_1, - USER_ID_1, - MOCKED_PERMISSION_HISTORY_RESPONSE, -) + APPLICATION_ID_1, MOCKED_PERMISSION_HISTORY_RESPONSE, USER_ID_1) -client = TestClient(app) ENDPOINT_ROOT = "permission-audit-history" - -@pytest.fixture(scope="function", autouse=True) -def mock_get_db(mocker, db_pg_session): - # This will mock the get_db dependency for all tests in this module - mocker.patch( - "api.app.routers.router_permission_audit.database.get_db", - return_value=db_pg_session, - ) - - # Test successful retrieval -def test_get_permission_audit_history_success(mocker, auth_headers): +def test_get_permission_audit_history_success(mocker, test_client_fixture, auth_headers): mocker.patch( "api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application", return_value=MOCKED_PERMISSION_HISTORY_RESPONSE, ) - response = client.get( + response = test_client_fixture.get( f"{apiPrefix}/{ENDPOINT_ROOT}?user_id={USER_ID_1}&application_id={APPLICATION_ID_1}", headers=auth_headers, ) @@ -40,13 +24,13 @@ def test_get_permission_audit_history_success(mocker, auth_headers): # Test retrieval with no records -def test_get_permission_audit_history_bad_request(mocker, auth_headers): +def test_get_permission_audit_history_bad_request(mocker, test_client_fixture, auth_headers): mocker.patch( "api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application", return_value=[], ) - response = client.get( + response = test_client_fixture.get( f"{apiPrefix}/{ENDPOINT_ROOT}?user_id=999&application_id=999", headers=auth_headers, ) @@ -55,8 +39,8 @@ def test_get_permission_audit_history_bad_request(mocker, auth_headers): # Test handling of invalid user_id -def test_get_permission_audit_history_invalid_user_id_type(auth_headers): - response = client.get( +def test_get_permission_audit_history_invalid_user_id_type(auth_headers, test_client_fixture): + response = test_client_fixture.get( f"{apiPrefix}/{ENDPOINT_ROOT}?user_id=invalid_user_id&application_id={APPLICATION_ID_1}", headers=auth_headers, ) @@ -65,13 +49,13 @@ def test_get_permission_audit_history_invalid_user_id_type(auth_headers): # Test unauthorized access -def test_get_permission_audit_history_unauthorized(mocker): +def test_get_permission_audit_history_unauthorized(mocker, test_client_fixture): mocker.patch( "api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application", side_effect=Exception("Unauthorized"), ) - response = client.get( + response = test_client_fixture.get( f"{apiPrefix}/{ENDPOINT_ROOT}?user_id={USER_ID_1}&application_id={APPLICATION_ID_1}" )