From a549fc04308a88437ed1695b721aebe93e779e15 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Wed, 14 Feb 2024 11:58:35 +0000 Subject: [PATCH 01/31] made updates to get tests passing on Python 3.11. Ran black on 3 files --- .gitignore | 1 + package-lock.json | 50 ++++---- requirements.dev.txt | 22 ++-- requirements.txt | 40 +++--- tests/api/fixtures/aws.py | 19 +-- tests/api/test_auth0.py | 19 ++- tests/api/test_aws.py | 252 +++++++++++++++++++++++++++----------- 7 files changed, 255 insertions(+), 148 deletions(-) diff --git a/.gitignore b/.gitignore index 984133cbe..41b77b685 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ __pycache__ /static /tags /venv +/venv_311 /.python-version /coverage/ .idea/ diff --git a/package-lock.json b/package-lock.json index 2e17ac954..12cfd0bea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,27 +9,27 @@ "version": "0.1.0", "license": "MIT", "dependencies": { - "@ministryofjustice/frontend": "^0.0.17-alpha", - "accessible-autocomplete": "^2.0.4", - "core-js": "^3.26.1", - "govuk-frontend": "^3.0.0", - "html5shiv": "^3.7.3", - "jquery": "^3.6.1", - "jquery-ui": "^1.13.2", - "sass": "^1.56.1" + "@ministryofjustice/frontend": "0.0.17-alpha", + "accessible-autocomplete": "2.0.4", + "core-js": "3.26.1", + "govuk-frontend": "3.0.0", + "html5shiv": "3.7.3", + "jquery": "3.6.1", + "jquery-ui": "1.13.2", + "sass": "1.56.1" }, "devDependencies": { - "@babel/cli": "^7.19.3", - "@babel/core": "^7.20.5", - "@babel/plugin-transform-regenerator": "^7.20.5", - "@babel/plugin-transform-runtime": "^7.19.6", - "@babel/preset-env": "^7.20.2", - "@testing-library/jest-dom": "^5.16.5", - "babel-jest": "^29.3.1", - "jest": "^29.3.1", - "jest-environment-jsdom": "^29.3.1", - "jsdom-simulant": "^1.1.2", - "npm-run-all": "^4.1.5" + "@babel/cli": "7.19.3", + "@babel/core": "7.20.5", + "@babel/plugin-transform-regenerator": "7.20.5", + "@babel/plugin-transform-runtime": "7.19.6", + "@babel/preset-env": "7.20.2", + "@testing-library/jest-dom": "5.16.5", + "babel-jest": "29.3.1", + "jest": "29.3.1", + "jest-environment-jsdom": "29.3.1", + "jsdom-simulant": "1.1.2", + "npm-run-all": "4.1.5" } }, "node_modules/@adobe/css-tools": { @@ -4165,9 +4165,9 @@ } }, "node_modules/govuk-frontend": { - "version": "3.14.0", - "resolved": "https://registry.npmjs.org/govuk-frontend/-/govuk-frontend-3.14.0.tgz", - "integrity": "sha512-y7FTuihCSA8Hty+e9h0uPhCoNanCAN+CLioNFlPmlbeHXpbi09VMyxTcH+XfnMPY4Cp++7096v0rLwwdapTXnA==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/govuk-frontend/-/govuk-frontend-3.0.0.tgz", + "integrity": "sha512-GCrEeaQZEnsthNtfmOUFlgsieNjHOeoamSWMdD4Gdq0RPxCA9uzfrT2i3jVnlBORekKjOL0C8eFZQBSNnjtz2A==", "engines": { "node": ">= 4.2.0" } @@ -11423,9 +11423,9 @@ } }, "govuk-frontend": { - "version": "3.14.0", - "resolved": "https://registry.npmjs.org/govuk-frontend/-/govuk-frontend-3.14.0.tgz", - "integrity": "sha512-y7FTuihCSA8Hty+e9h0uPhCoNanCAN+CLioNFlPmlbeHXpbi09VMyxTcH+XfnMPY4Cp++7096v0rLwwdapTXnA==" + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/govuk-frontend/-/govuk-frontend-3.0.0.tgz", + "integrity": "sha512-GCrEeaQZEnsthNtfmOUFlgsieNjHOeoamSWMdD4Gdq0RPxCA9uzfrT2i3jVnlBORekKjOL0C8eFZQBSNnjtz2A==" }, "graceful-fs": { "version": "4.2.10", diff --git a/requirements.dev.txt b/requirements.dev.txt index 87b0aded9..6211e258b 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -1,12 +1,12 @@ -black==23.3.0 -django-debug-toolbar==3.2.4 +black==24.2.0 +django-debug-toolbar==4.3.0 django-debug-toolbar-requests==1.0.5 -django-elasticsearch-debug-toolbar==2.0.0 -flake8==6.0.0 -ipdb==0.13.9 -ipython==8.13.2 -isort==5.12.0 -pre-commit==2.20.0 -pylint==2.17.4 -pylint-django==2.4.4 -pandas==1.5.2 \ No newline at end of file +django-elasticsearch-debug-toolbar==3.0.2 +flake8==7.0.0 +ipdb==0.13.13 +ipython==8.21.0 +isort==5.13.2 +pandas==2.2.0 +pre-commit==3.6.1 +pylint==3.0.3 +pylint-django==2.5.5 diff --git a/requirements.txt b/requirements.txt index a36f1befb..02e9ca2c7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,38 +1,38 @@ asgiref==3.7.2 -auth0-python==4.5.0 -beautifulsoup4==4.12.2 -boto3==1.26.143 -celery[sqs]==5.3.1 +auth0-python==4.7.0 +beautifulsoup4==4.12.3 +boto3==1.34.41 +celery[sqs]==5.3.6 channels==4.0.0 channels-redis==4.2.0 -daphne==4.0.0 -Django==4.2.7 +daphne==4.1.0 +Django==5.0.2 django-crequest==2018.5.11 django-extensions==3.2.3 -django-filter==22.1 +django-filter==23.5 django-prometheus==2.3.1 django-redis==5.4.0 -django-simple-history==3.3.0 -django-structlog==2.2.0 +django-simple-history==3.4.0 +django-structlog==7.1.0 djangorestframework==3.14.0 djproxy==2.3.6 elasticsearch-dsl==7.4.1 -gunicorn==20.1.0 +gunicorn==21.2.0 Jinja2==3.1.3 -kubernetes==25.3.0 -MarkupSafe==2.1.3 +kubernetes==29.0.0 +MarkupSafe==2.1.5 model-mommy==2.0.0 -moto==1.3.14 +moto==5.0.1 mozilla-django-oidc==4.0.0 -psycopg2-binary==2.9.6 +psycopg2-binary==2.9.9 PyNaCl==1.5.0 -pytest==7.3.1 -pytest-django==4.5.2 -python-dotenv==1.0.0 +pytest==8.0.0 +pytest-django==4.8.0 +python-dotenv==1.0.1 python-jose==3.3.0 pyyaml==6.0.1 rules==3.3 -sentry-sdk==1.14.0 +sentry-sdk==1.40.4 slackclient==2.9.4 -urllib3==1.26.18 -uvicorn[standard]==0.20.0 +urllib3==2.0.7 +uvicorn[standard]==0.27.1 diff --git a/tests/api/fixtures/aws.py b/tests/api/fixtures/aws.py index 0f858bae6..ba09895dd 100644 --- a/tests/api/fixtures/aws.py +++ b/tests/api/fixtures/aws.py @@ -19,31 +19,31 @@ def aws_creds(): @pytest.fixture(autouse=True) def iam(aws_creds): - with moto.mock_iam(): + with moto.mock_aws(): yield boto3.Session().resource("iam") @pytest.fixture(autouse=True) def s3(aws_creds): - with moto.mock_s3(): + with moto.mock_aws(): yield boto3.resource("s3") @pytest.fixture(autouse=True) def sts(aws_creds): - with moto.mock_sts(): + with moto.mock_aws(): yield boto3.client("sts") @pytest.fixture(autouse=True) def ssm(aws_creds): - with moto.mock_ssm(): + with moto.mock_aws(): yield boto3.client("ssm", region_name="eu-west-1") @pytest.fixture(autouse=True) def sqs(aws_creds): - with moto.mock_sqs(): + with moto.mock_aws(): sqs = boto3.resource("sqs") sqs.create_queue(QueueName=settings.DEFAULT_QUEUE) sqs.create_queue(QueueName=settings.IAM_QUEUE_NAME) @@ -53,7 +53,7 @@ def sqs(aws_creds): @pytest.fixture(autouse=True) def secretsmanager(aws_creds): - with moto.mock_secretsmanager(): + with moto.mock_aws(): yield boto3.client("secretsmanager", region_name="eu-west-1") @@ -161,4 +161,9 @@ def logs_bucket(s3): @pytest.fixture() def root_folder_bucket(s3): - yield s3.create_bucket(Bucket=settings.S3_FOLDER_BUCKET_NAME) + yield s3.create_bucket( + Bucket=settings.S3_FOLDER_BUCKET_NAME, + CreateBucketConfiguration={ + "LocationConstraint": settings.BUCKET_REGION, # noqa: F405 + }, + ) diff --git a/tests/api/test_auth0.py b/tests/api/test_auth0.py index a1788dd04..a33fac265 100644 --- a/tests/api/test_auth0.py +++ b/tests/api/test_auth0.py @@ -1,9 +1,8 @@ # Standard library import json -from unittest.mock import call, patch +from unittest.mock import call, patch, ANY # Third-party -import mock import pytest from django.conf import settings from auth0 import exceptions @@ -511,9 +510,7 @@ def fixture_group_members_200(ExtendedAuth0): yield -def test_group_member_more_than_100( - ExtendedAuth0, fixture_group_members_200 -): +def test_group_member_more_than_100(ExtendedAuth0, fixture_group_members_200): members = ExtendedAuth0.groups.get_group_members(group_id="foo-id-1") assert len(members) == 200 @@ -585,7 +582,7 @@ def _clean_string(content): "client_secret": "WNXFkM3FCTXJhUWs0Q1NwcKFu", }, ) - fixture_connection_create.assert_called_once_with(mock.ANY) + fixture_connection_create.assert_called_once_with(ANY) with open("./tests/api/fixtures/nomis_body.json") as body_file: expected = json.loads(body_file.read()) @@ -606,7 +603,8 @@ def _clean_string(content): def test_create_custom_connection_with_allowed_error(ExtendedAuth0): with patch.object(ExtendedAuth0.connections, "create") as connection_create: connection_create.side_effect = exceptions.Auth0Error( - 409, 409, 'The connection name existed already') + 409, 409, "The connection name existed already" + ) ExtendedAuth0.connections.create_custom_connection( "auth0_nomis", input_values={ @@ -615,13 +613,12 @@ def test_create_custom_connection_with_allowed_error(ExtendedAuth0): "client_secret": "WNXFkM3FCTXJhUWs0Q1NwcKFu", }, ) - connection_create.assert_called_once_with(mock.ANY) + connection_create.assert_called_once_with(ANY) def test_create_custom_connection_with_notallowed_error(ExtendedAuth0): with patch.object(ExtendedAuth0.connections, "create") as connection_create: - connection_create.side_effect = exceptions.Auth0Error( - 400, 400, 'Error') + connection_create.side_effect = exceptions.Auth0Error(400, 400, "Error") with pytest.raises(auth0.Auth0Error, match="400: Error"): ExtendedAuth0.connections.create_custom_connection( "auth0_nomis", @@ -631,4 +628,4 @@ def test_create_custom_connection_with_notallowed_error(ExtendedAuth0): "client_secret": "WNXFkM3FCTXJhUWs0Q1NwcKFu", }, ) - connection_create.assert_called_once_with(mock.ANY) + connection_create.assert_called_once_with(ANY) diff --git a/tests/api/test_aws.py b/tests/api/test_aws.py index 4722cb4a6..a0c51668d 100644 --- a/tests/api/test_aws.py +++ b/tests/api/test_aws.py @@ -274,7 +274,11 @@ def test_create_bucket(logs_bucket, s3): def test_tag_bucket(s3): bucket_name = f"bucket-{id(MagicMock())}" bucket = s3.Bucket(bucket_name) - bucket.create() + bucket.create( + CreateBucketConfiguration={ + "LocationConstraint": settings.BUCKET_REGION, # noqa: F405 + } + ) aws.AWSBucket().tag_bucket(bucket_name, {"env": "test", "test-update": "old-value"}) aws.AWSBucket().tag_bucket( @@ -822,7 +826,7 @@ def test_aws_folder_create(root_folder_bucket, s3): [ ("my-folder", "my-folder/", True), ("my-folder", None, False), - ] + ], ) def test_aws_folder_exists(new_folder, existing_folder, expected, root_folder_bucket): if existing_folder: @@ -842,22 +846,26 @@ def test_aws_folder_exists(new_folder, existing_folder, expected, root_folder_bu ("readonly", "test-bucket/user-folder", ["/public"]), ("readwrite", "test-bucket/user-folder", ["/public", "/another"]), ("readonly", "test-bucket/user-folder", ["/public", "/another"]), - ] + ], ) def test_role_grant_folder_access(access_level, roles, root_folder_path, paths): - with patch("controlpanel.api.aws.S3AccessPolicy.grant_folder_access") as grant_folder_access, \ - patch("controlpanel.api.aws.S3AccessPolicy.revoke_folder_access") as revoke_access: # noqa + with ( + patch( + "controlpanel.api.aws.S3AccessPolicy.grant_folder_access" + ) as grant_folder_access, + patch( + "controlpanel.api.aws.S3AccessPolicy.revoke_folder_access" + ) as revoke_access, + ): # noqa aws.AWSRole().grant_folder_access( - 'test_user_normal-user', + "test_user_normal-user", root_folder_path, access_level, paths, ) revoke_access.assert_called_once_with(root_folder_path=root_folder_path) grant_folder_access.assert_called_once_with( - root_folder_path=root_folder_path, - access_level=access_level, - paths=paths + root_folder_path=root_folder_path, access_level=access_level, paths=paths ) @@ -872,11 +880,17 @@ def test_role_grant_folder_access(access_level, roles, root_folder_path, paths): ("readonly", "test-bucket/user-folder", ["/public"]), ("readwrite", "test-bucket/user-folder", ["/public", "/another"]), ("readonly", "test-bucket/user-folder", ["/public", "/another"]), - ] + ], ) def test_policy_grant_folder_access(group, access_level, root_folder_path, paths): - with patch("controlpanel.api.aws.S3AccessPolicy.grant_folder_access") as grant_folder_access, \ - patch("controlpanel.api.aws.S3AccessPolicy.revoke_folder_access") as revoke_access: # noqa + with ( + patch( + "controlpanel.api.aws.S3AccessPolicy.grant_folder_access" + ) as grant_folder_access, + patch( + "controlpanel.api.aws.S3AccessPolicy.revoke_folder_access" + ) as revoke_access, + ): # noqa aws.AWSPolicy().grant_folder_access( group.arn, root_folder_path, @@ -885,9 +899,7 @@ def test_policy_grant_folder_access(group, access_level, root_folder_path, paths ) revoke_access.assert_called_once_with(root_folder_path=root_folder_path) grant_folder_access.assert_called_once_with( - root_folder_path=root_folder_path, - access_level=access_level, - paths=paths + root_folder_path=root_folder_path, access_level=access_level, paths=paths ) @@ -909,16 +921,22 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): access_level="readonly", ) - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa - assert f"{bucket_arn}/{folder_name}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ + bucket_arn + ] # noqa + assert ( + f"{bucket_arn}/{folder_name}/*" + in s3_access_policy.statements["readonly"]["Resource"] + ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { - "StringEquals": { - "s3:prefix": ["", folder_name], - "s3:delimiter": ["/"] - } + "StringEquals": {"s3:prefix": ["", folder_name], "s3:delimiter": ["/"]} } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [f"{folder_name}/*"], @@ -933,9 +951,16 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): ) # make sure that the policy has not been overwritten, and contains both folders - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa - assert f"{bucket_arn}/{folder_name_2}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ + bucket_arn + ] # noqa + assert ( + f"{bucket_arn}/{folder_name_2}/*" + in s3_access_policy.statements["readonly"]["Resource"] + ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -943,16 +968,15 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): folder_name, folder_name_2, ], - "s3:delimiter": ["/"] + "s3:delimiter": ["/"], } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { - "s3:prefix": [ - f"{folder_name}/*", - f"{folder_name_2}/*" - ], + "s3:prefix": [f"{folder_name}/*", f"{folder_name_2}/*"], } } @@ -974,9 +998,16 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): paths=[allowed_path], ) - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa - assert f"{bucket_arn}/{folder_name}{allowed_path}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ + bucket_arn + ] # noqa + assert ( + f"{bucket_arn}/{folder_name}{allowed_path}/*" + in s3_access_policy.statements["readonly"]["Resource"] + ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -987,10 +1018,12 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): "user-folder/public/", "user-folder/public/folder", ], - "s3:delimiter": ["/"] + "s3:delimiter": ["/"], } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ + bucket_arn + ] # only designated folder should use the wildcard assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { @@ -1001,12 +1034,15 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): # add another path, check new folder added correctly with no duplicates another_path = "/another" s3_access_policy.grant_folder_access( - root_folder_path=root_folder_path, - access_level="readonly", - paths=[another_path] + root_folder_path=root_folder_path, access_level="readonly", paths=[another_path] ) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] - assert f"{bucket_arn}/{folder_name}{another_path}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ + bucket_arn + ] + assert ( + f"{bucket_arn}/{folder_name}{another_path}/*" + in s3_access_policy.statements["readonly"]["Resource"] + ) # noqa assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -1016,17 +1052,19 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): "user-folder/public", "user-folder/public/", "user-folder/public/folder", - "user-folder/another" + "user-folder/another", ], - "s3:delimiter": ["/"] + "s3:delimiter": ["/"], } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ + bucket_arn + ] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [ f"{folder_name}{allowed_path}/*", - f"{folder_name}{another_path}/*" + f"{folder_name}{another_path}/*", ], } } @@ -1034,26 +1072,36 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): def test_base_s3_access_sids(): expected = [ - 'list', - 'readonly', - 'readwrite', - 'listFolder', - 'listSubFolders', - 'rootFolderBucketMeta', + "list", + "readonly", + "readwrite", + "listFolder", + "listSubFolders", + "rootFolderBucketMeta", ] assert aws.S3AccessPolicy(MagicMock()).base_s3_access_sids == expected def test_revoke_folder_access(s3_access_policy): - with patch.object(s3_access_policy, "remove_prefix") as remove_prefix, \ - patch.object(s3_access_policy, "remove_resource") as remove_resource: + with ( + patch.object(s3_access_policy, "remove_prefix") as remove_prefix, + patch.object(s3_access_policy, "remove_resource") as remove_resource, + ): root_folder_path = "test-bucket/folder" arn = f"arn:aws:s3:::{root_folder_path}" s3_access_policy.revoke_folder_access(root_folder_path=root_folder_path) remove_prefix.assert_has_calls( [ - call(root_folder_path=root_folder_path, sid="listFolder", condition="StringEquals"), - call(root_folder_path=root_folder_path, sid="listSubFolders", condition="StringLike"), + call( + root_folder_path=root_folder_path, + sid="listFolder", + condition="StringEquals", + ), + call( + root_folder_path=root_folder_path, + sid="listSubFolders", + condition="StringLike", + ), ], any_order=True, ) @@ -1072,21 +1120,43 @@ def test_revoke_access_removes_prefixes(s3_access_policy): root_folder_path = "test-bucket/user-folder" s3_access_policy.grant_folder_access(root_folder_path, "readwrite") - assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa + assert ( + s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) + is not None + ) + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( + "Resource", None + ) + is not None + ) + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ + "StringEquals" + ]["s3:prefix"] != [ + "" + ] # noqa + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ + "StringLike" + ]["s3:prefix"] + != [] + ) # noqa s3_access_policy.revoke_folder_access(root_folder_path=root_folder_path) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is None + assert ( + s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) + is None + ) assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { - "StringEquals": { - "s3:prefix": [""], - "s3:delimiter": ["/"] - } + "StringEquals": {"s3:prefix": [""], "s3:delimiter": ["/"]} } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is None + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( + "Resource", None + ) + is None + ) assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [], @@ -1099,15 +1169,49 @@ def test_revoke_access_doesnt_remove_prefixes(s3_access_policy): bucket_hash = md5_hash(bucket_name) s3_access_policy.grant_folder_access("test-bucket/my-folder", "readwrite") - assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa + assert ( + s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) + is not None + ) + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( + "Resource", None + ) + is not None + ) + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ + "StringEquals" + ]["s3:prefix"] != [ + "" + ] # noqa + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ + "StringLike" + ]["s3:prefix"] + != [] + ) # noqa # now revoke access s3_access_policy.revoke_folder_access("another-bucket/my-folder") - assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa + assert ( + s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) + is not None + ) + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( + "Resource", None + ) + is not None + ) + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ + "StringEquals" + ]["s3:prefix"] != [ + "" + ] # noqa + assert ( + s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ + "StringLike" + ]["s3:prefix"] + != [] + ) # noqa From db393854bbb2c6154b54a7e5649c2d7d12093155 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Wed, 14 Feb 2024 13:06:10 +0000 Subject: [PATCH 02/31] Fixed flaky test by checking attached role is in role name rather than zipping list together --- tests/api/test_aws.py | 250 +++++++++++++----------------------------- 1 file changed, 75 insertions(+), 175 deletions(-) diff --git a/tests/api/test_aws.py b/tests/api/test_aws.py index a0c51668d..15344fd4b 100644 --- a/tests/api/test_aws.py +++ b/tests/api/test_aws.py @@ -561,8 +561,8 @@ def test_create_policy(iam, settings): def assert_group_members(policy, role_names): attached_roles = list(policy.attached_roles.all()) assert len(attached_roles) == len(role_names) - for role, role_name in zip(attached_roles, role_names): - assert role.role_name == role_name + for role in attached_roles: + assert role.role_name in role_names @pytest.fixture @@ -826,7 +826,7 @@ def test_aws_folder_create(root_folder_bucket, s3): [ ("my-folder", "my-folder/", True), ("my-folder", None, False), - ], + ] ) def test_aws_folder_exists(new_folder, existing_folder, expected, root_folder_bucket): if existing_folder: @@ -846,26 +846,22 @@ def test_aws_folder_exists(new_folder, existing_folder, expected, root_folder_bu ("readonly", "test-bucket/user-folder", ["/public"]), ("readwrite", "test-bucket/user-folder", ["/public", "/another"]), ("readonly", "test-bucket/user-folder", ["/public", "/another"]), - ], + ] ) def test_role_grant_folder_access(access_level, roles, root_folder_path, paths): - with ( - patch( - "controlpanel.api.aws.S3AccessPolicy.grant_folder_access" - ) as grant_folder_access, - patch( - "controlpanel.api.aws.S3AccessPolicy.revoke_folder_access" - ) as revoke_access, - ): # noqa + with patch("controlpanel.api.aws.S3AccessPolicy.grant_folder_access") as grant_folder_access, \ + patch("controlpanel.api.aws.S3AccessPolicy.revoke_folder_access") as revoke_access: # noqa aws.AWSRole().grant_folder_access( - "test_user_normal-user", + 'test_user_normal-user', root_folder_path, access_level, paths, ) revoke_access.assert_called_once_with(root_folder_path=root_folder_path) grant_folder_access.assert_called_once_with( - root_folder_path=root_folder_path, access_level=access_level, paths=paths + root_folder_path=root_folder_path, + access_level=access_level, + paths=paths ) @@ -880,17 +876,11 @@ def test_role_grant_folder_access(access_level, roles, root_folder_path, paths): ("readonly", "test-bucket/user-folder", ["/public"]), ("readwrite", "test-bucket/user-folder", ["/public", "/another"]), ("readonly", "test-bucket/user-folder", ["/public", "/another"]), - ], + ] ) def test_policy_grant_folder_access(group, access_level, root_folder_path, paths): - with ( - patch( - "controlpanel.api.aws.S3AccessPolicy.grant_folder_access" - ) as grant_folder_access, - patch( - "controlpanel.api.aws.S3AccessPolicy.revoke_folder_access" - ) as revoke_access, - ): # noqa + with patch("controlpanel.api.aws.S3AccessPolicy.grant_folder_access") as grant_folder_access, \ + patch("controlpanel.api.aws.S3AccessPolicy.revoke_folder_access") as revoke_access: # noqa aws.AWSPolicy().grant_folder_access( group.arn, root_folder_path, @@ -899,7 +889,9 @@ def test_policy_grant_folder_access(group, access_level, root_folder_path, paths ) revoke_access.assert_called_once_with(root_folder_path=root_folder_path) grant_folder_access.assert_called_once_with( - root_folder_path=root_folder_path, access_level=access_level, paths=paths + root_folder_path=root_folder_path, + access_level=access_level, + paths=paths ) @@ -921,22 +913,16 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): access_level="readonly", ) - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ - bucket_arn - ] # noqa - assert ( - f"{bucket_arn}/{folder_name}/*" - in s3_access_policy.statements["readonly"]["Resource"] - ) # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa + assert f"{bucket_arn}/{folder_name}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { - "StringEquals": {"s3:prefix": ["", folder_name], "s3:delimiter": ["/"]} + "StringEquals": { + "s3:prefix": ["", folder_name], + "s3:delimiter": ["/"] + } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [f"{folder_name}/*"], @@ -951,16 +937,9 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): ) # make sure that the policy has not been overwritten, and contains both folders - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ - bucket_arn - ] # noqa - assert ( - f"{bucket_arn}/{folder_name_2}/*" - in s3_access_policy.statements["readonly"]["Resource"] - ) # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa + assert f"{bucket_arn}/{folder_name_2}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -968,15 +947,16 @@ def test_s3_access_policy_grant_folder_access(s3_access_policy): folder_name, folder_name_2, ], - "s3:delimiter": ["/"], + "s3:delimiter": ["/"] } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { - "s3:prefix": [f"{folder_name}/*", f"{folder_name_2}/*"], + "s3:prefix": [ + f"{folder_name}/*", + f"{folder_name_2}/*" + ], } } @@ -998,16 +978,9 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): paths=[allowed_path], ) - assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [ - bucket_arn - ] # noqa - assert ( - f"{bucket_arn}/{folder_name}{allowed_path}/*" - in s3_access_policy.statements["readonly"]["Resource"] - ) # noqa - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements["rootFolderBucketMeta"]["Resource"] == [bucket_arn] # noqa + assert f"{bucket_arn}/{folder_name}{allowed_path}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -1018,12 +991,10 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): "user-folder/public/", "user-folder/public/folder", ], - "s3:delimiter": ["/"], + "s3:delimiter": ["/"] } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] # only designated folder should use the wildcard assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { @@ -1034,15 +1005,12 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): # add another path, check new folder added correctly with no duplicates another_path = "/another" s3_access_policy.grant_folder_access( - root_folder_path=root_folder_path, access_level="readonly", paths=[another_path] + root_folder_path=root_folder_path, + access_level="readonly", + paths=[another_path] ) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [ - bucket_arn - ] - assert ( - f"{bucket_arn}/{folder_name}{another_path}/*" - in s3_access_policy.statements["readonly"]["Resource"] - ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Resource"] == [bucket_arn] + assert f"{bucket_arn}/{folder_name}{another_path}/*" in s3_access_policy.statements["readonly"]["Resource"] # noqa assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { "StringEquals": { "s3:prefix": [ @@ -1052,19 +1020,17 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): "user-folder/public", "user-folder/public/", "user-folder/public/folder", - "user-folder/another", + "user-folder/another" ], - "s3:delimiter": ["/"], + "s3:delimiter": ["/"] } } - assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [ - bucket_arn - ] + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Resource"] == [bucket_arn] assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [ f"{folder_name}{allowed_path}/*", - f"{folder_name}{another_path}/*", + f"{folder_name}{another_path}/*" ], } } @@ -1072,36 +1038,26 @@ def test_s3_access_policy_grant_folder_access_to_paths(s3_access_policy): def test_base_s3_access_sids(): expected = [ - "list", - "readonly", - "readwrite", - "listFolder", - "listSubFolders", - "rootFolderBucketMeta", + 'list', + 'readonly', + 'readwrite', + 'listFolder', + 'listSubFolders', + 'rootFolderBucketMeta', ] assert aws.S3AccessPolicy(MagicMock()).base_s3_access_sids == expected def test_revoke_folder_access(s3_access_policy): - with ( - patch.object(s3_access_policy, "remove_prefix") as remove_prefix, - patch.object(s3_access_policy, "remove_resource") as remove_resource, - ): + with patch.object(s3_access_policy, "remove_prefix") as remove_prefix, \ + patch.object(s3_access_policy, "remove_resource") as remove_resource: root_folder_path = "test-bucket/folder" arn = f"arn:aws:s3:::{root_folder_path}" s3_access_policy.revoke_folder_access(root_folder_path=root_folder_path) remove_prefix.assert_has_calls( [ - call( - root_folder_path=root_folder_path, - sid="listFolder", - condition="StringEquals", - ), - call( - root_folder_path=root_folder_path, - sid="listSubFolders", - condition="StringLike", - ), + call(root_folder_path=root_folder_path, sid="listFolder", condition="StringEquals"), + call(root_folder_path=root_folder_path, sid="listSubFolders", condition="StringLike"), ], any_order=True, ) @@ -1120,43 +1076,21 @@ def test_revoke_access_removes_prefixes(s3_access_policy): root_folder_path = "test-bucket/user-folder" s3_access_policy.grant_folder_access(root_folder_path, "readwrite") - assert ( - s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) - is not None - ) - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( - "Resource", None - ) - is not None - ) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ - "StringEquals" - ]["s3:prefix"] != [ - "" - ] # noqa - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ - "StringLike" - ]["s3:prefix"] - != [] - ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa s3_access_policy.revoke_folder_access(root_folder_path=root_folder_path) - assert ( - s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) - is None - ) + assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is None assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"] == { - "StringEquals": {"s3:prefix": [""], "s3:delimiter": ["/"]} + "StringEquals": { + "s3:prefix": [""], + "s3:delimiter": ["/"] + } } - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( - "Resource", None - ) - is None - ) + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is None assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"] == { "StringLike": { "s3:prefix": [], @@ -1169,49 +1103,15 @@ def test_revoke_access_doesnt_remove_prefixes(s3_access_policy): bucket_hash = md5_hash(bucket_name) s3_access_policy.grant_folder_access("test-bucket/my-folder", "readwrite") - assert ( - s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) - is not None - ) - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( - "Resource", None - ) - is not None - ) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ - "StringEquals" - ]["s3:prefix"] != [ - "" - ] # noqa - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ - "StringLike" - ]["s3:prefix"] - != [] - ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa # now revoke access s3_access_policy.revoke_folder_access("another-bucket/my-folder") - assert ( - s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) - is not None - ) - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get( - "Resource", None - ) - is not None - ) - assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"][ - "StringEquals" - ]["s3:prefix"] != [ - "" - ] # noqa - assert ( - s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"][ - "StringLike" - ]["s3:prefix"] - != [] - ) # noqa + assert s3_access_policy.statements[f"listFolder{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"].get("Resource", None) is not None + assert s3_access_policy.statements[f"listFolder{bucket_hash}"]["Condition"]["StringEquals"]["s3:prefix"] != [""] # noqa + assert s3_access_policy.statements[f"listSubFolders{bucket_hash}"]["Condition"]["StringLike"]["s3:prefix"] != [] # noqa From 053e465a2ac5ccee2cee7b0ae2c515889212fae9 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Thu, 15 Feb 2024 16:00:59 +0000 Subject: [PATCH 03/31] attempted to bump version to python 311 in dockerfile --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 42f6b1fb0..a19bb01dc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,7 +9,7 @@ RUN ./node_modules/.bin/sass --load-path=node_modules/ --style=compressed src/ap WORKDIR /src RUN /node_modules/.bin/jest -FROM 593291632749.dkr.ecr.eu-west-1.amazonaws.com/python:3.9-slim-buster AS base +FROM 593291632749.dkr.ecr.eu-west-1.amazonaws.com/python:3.11-slim-buster AS base ARG HELM_VERSION=3.5.4 ARG HELM_TARBALL=helm-v${HELM_VERSION}-linux-amd64.tar.gz From 4734bfd6349fee9b232184d40c9b51b10799c38e Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Thu, 15 Feb 2024 16:39:38 +0000 Subject: [PATCH 04/31] bumped down requirements for kubernetes --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 02e9ca2c7..93b5a9a9c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,7 +19,7 @@ djproxy==2.3.6 elasticsearch-dsl==7.4.1 gunicorn==21.2.0 Jinja2==3.1.3 -kubernetes==29.0.0 +kubernetes==25.3.0 MarkupSafe==2.1.5 model-mommy==2.0.0 moto==5.0.1 From 05f84f3f77a91e75b786f97092e3def2230f1b63 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 09:24:20 +0000 Subject: [PATCH 05/31] added 3.11 image to Dockerfile --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index a19bb01dc..d72ba9d16 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,7 +9,7 @@ RUN ./node_modules/.bin/sass --load-path=node_modules/ --style=compressed src/ap WORKDIR /src RUN /node_modules/.bin/jest -FROM 593291632749.dkr.ecr.eu-west-1.amazonaws.com/python:3.11-slim-buster AS base +FROM public.ecr.aws/docker/library/python:3.11-alpine3.18 AS base ARG HELM_VERSION=3.5.4 ARG HELM_TARBALL=helm-v${HELM_VERSION}-linux-amd64.tar.gz From d1d87fa561ace1470eabf3b3a56ffecc243655b4 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 09:41:42 +0000 Subject: [PATCH 06/31] Attempted to fix add user now we're using an alpine image --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index d72ba9d16..afba4e67a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,8 +22,8 @@ ENV DJANGO_SETTINGS_MODULE="controlpanel.settings" \ HELM_DATA_HOME=/tmp/helm/data # create a user to run as -RUN addgroup -gid 1000 controlpanel && \ - adduser -uid 1000 --gid 1000 controlpanel +RUN addgroup -g 1000 controlpanel \ + && adduser -G controlpanel -u 1000 controlpanel -D RUN apt-get update \ && apt-get install -y --no-install-recommends \ From 11136dae977199b39146a3a840916984e01fe0c6 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 09:54:07 +0000 Subject: [PATCH 07/31] Further attempt to fix Dockerfile using apk command --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index afba4e67a..f99e4c911 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,8 +25,8 @@ ENV DJANGO_SETTINGS_MODULE="controlpanel.settings" \ RUN addgroup -g 1000 controlpanel \ && adduser -G controlpanel -u 1000 controlpanel -D -RUN apt-get update \ - && apt-get install -y --no-install-recommends \ +RUN apk update \ + && apk add --no-cache \ postgresql-client \ wget \ gcc \ From 0c6266d5bdb3f45ff51c90134f61f685bed81188 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:02:40 +0000 Subject: [PATCH 08/31] replaced failing packages with what I think are their apk equivalents --- Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index f99e4c911..04e12a0ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,10 +30,10 @@ RUN apk update \ postgresql-client \ wget \ gcc \ - libcurl4-gnutls-dev \ + curl-dev \ python3-dev \ - libgnutls28-dev \ - libssl-dev \ + gnutls-dev \ + openssl-dev \ && rm -rf /var/lib/apt/lists/* WORKDIR /home/controlpanel From 6fc6de74b0d7c07ba4afebc0406a220ef998b75c Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:09:33 +0000 Subject: [PATCH 09/31] updated python3-dev to python3.11-dev --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 04e12a0ef..c56b2757f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,7 +31,7 @@ RUN apk update \ wget \ gcc \ curl-dev \ - python3-dev \ + python3.11-dev \ gnutls-dev \ openssl-dev \ && rm -rf /var/lib/apt/lists/* From 94cd951b38205fd3399f65ce2a0469241201f14e Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:12:55 +0000 Subject: [PATCH 10/31] reverted python-dev --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index c56b2757f..04e12a0ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,7 +31,7 @@ RUN apk update \ wget \ gcc \ curl-dev \ - python3.11-dev \ + python3-dev \ gnutls-dev \ openssl-dev \ && rm -rf /var/lib/apt/lists/* From 4bf5e4d9db723d74b70cc5562b59dd5578cfa9bb Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:15:30 +0000 Subject: [PATCH 11/31] added new image dependency --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 04e12a0ef..0c2b69c81 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,6 +34,7 @@ RUN apk update \ python3-dev \ gnutls-dev \ openssl-dev \ + libffi-dev \ && rm -rf /var/lib/apt/lists/* WORKDIR /home/controlpanel From dcaa11754a16fab1076f8784026ac873dd49eaef Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:21:03 +0000 Subject: [PATCH 12/31] added another package in an attempt to fix stdio error --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 0c2b69c81..9f398ab1b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -35,6 +35,7 @@ RUN apk update \ gnutls-dev \ openssl-dev \ libffi-dev \ + musl-dev \ && rm -rf /var/lib/apt/lists/* WORKDIR /home/controlpanel From a72997a6d4bddf062c38c2ea35492946f8f94461 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:31:49 +0000 Subject: [PATCH 13/31] removed warning --- tests/api/cluster/test_access_to_s3buckets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api/cluster/test_access_to_s3buckets.py b/tests/api/cluster/test_access_to_s3buckets.py index a49cb51ce..07f285f70 100644 --- a/tests/api/cluster/test_access_to_s3buckets.py +++ b/tests/api/cluster/test_access_to_s3buckets.py @@ -32,7 +32,7 @@ def grant_policy_bucket_access(): ) as grant_policy_bucket_access_action: yield grant_policy_bucket_access_action -@pytest.yield_fixture +@pytest.fixture def grant_policy_folder_access(): with patch( "controlpanel.api.cluster.AWSPolicy.grant_folder_access" From 109a91983da36adbdeb3e7f05f421c35e8209d65 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:37:31 +0000 Subject: [PATCH 14/31] updated to test if file gets picked up --- tests/frontend/test_forms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 8473182a8..e07f2aa9b 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -6,6 +6,7 @@ from django.core.exceptions import ValidationError from django.urls import reverse + # First-party/Local from controlpanel.api import aws from controlpanel.api.github import RepositoryNotFound From 42220b3d91216b571bde4a2b7727dd55d4b4ab1b Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:40:55 +0000 Subject: [PATCH 15/31] added purposeful breaking change to see if it is picked up on test pipeline --- tests/frontend/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index e07f2aa9b..9d05443ab 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -5,7 +5,7 @@ import pytest from django.core.exceptions import ValidationError from django.urls import reverse - +from a_package import something # First-party/Local from controlpanel.api import aws From bef6250c2b47498cb8cd1bedcebbe242abe08f2b Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:43:34 +0000 Subject: [PATCH 16/31] removed breaking change as tests did not run --- tests/frontend/test_forms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 9d05443ab..8473182a8 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -5,7 +5,6 @@ import pytest from django.core.exceptions import ValidationError from django.urls import reverse -from a_package import something # First-party/Local from controlpanel.api import aws From 4dc5d052d3a6b4d020d7309bc76b95d2f6f9be2f Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:46:23 +0000 Subject: [PATCH 17/31] added import on line 8 to check if it gets picked up --- tests/frontend/test_forms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 8473182a8..294b13373 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -5,6 +5,7 @@ import pytest from django.core.exceptions import ValidationError from django.urls import reverse +import os # First-party/Local from controlpanel.api import aws From 224e3fcc12c9883c8232e410a49c0caf6f83d0b2 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 10:50:42 +0000 Subject: [PATCH 18/31] removed import --- tests/frontend/test_forms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 294b13373..8473182a8 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -5,7 +5,6 @@ import pytest from django.core.exceptions import ValidationError from django.urls import reverse -import os # First-party/Local from controlpanel.api import aws From 31d159f557c89ff6d40817260ea69f821a1e1102 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 11:07:52 +0000 Subject: [PATCH 19/31] adding new test to see if it's picked up --- tests/frontend/test_forms.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 8473182a8..3c5b76138 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -407,6 +407,10 @@ def test_update_app_with_custom_connection(): assert "auth0_nomis_auth0_conn_name" in f.errors +def test_pass(): + assert True + + @pytest.mark.django_db def test_ip_allowlist_form_invalid_ip(): """ From 872983c4edc6e9fc4fc026e62bf9212cd3de6066 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 15:11:26 +0000 Subject: [PATCH 20/31] removing all tests from file as a test --- tests/frontend/test_forms.py | 894 +++++++++++++++++------------------ 1 file changed, 447 insertions(+), 447 deletions(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 3c5b76138..4e0d6c22f 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -1,447 +1,447 @@ -# Standard library -from unittest import mock - -# Third-party -import pytest -from django.core.exceptions import ValidationError -from django.urls import reverse - -# First-party/Local -from controlpanel.api import aws -from controlpanel.api.github import RepositoryNotFound -from controlpanel.api.models import S3Bucket -from controlpanel.frontend import forms - - -def test_tool_release_form_check_release_name(): - """ - Ensure valid chart names work, while invalid ones cause a helpful - exception. - """ - data = { - "name": "Test Release", - "chart_name": "jupyter-lab", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "rstudio", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "airflow-sqlite", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "invalid-chartname", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() is False - - -def test_tool_release_form_check_tool_domain(): - """ - Ensure ONLY valid chart names work, while invalid ones cause a helpful - exception. - """ - data = { - "name": "Test Release", - "chart_name": "jupyter-lab", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "jupyter-lab", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "jupyter-lab", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "invalid-tool-domain", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() is False - - -def test_tool_release_form_get_target_users(): - """ - Given a string list of comma separated usernames, the expected query to - return the associated User objects is created. - """ - f = forms.ToolReleaseForm() - f.data = { - "target_users_list": "aldo, nicholas, cal", - } - mock_user = mock.MagicMock() - with mock.patch("controlpanel.frontend.forms.User", mock_user): - f.get_target_users() - mock_user.objects.filter.assert_called_once_with( - username__in=set(["aldo", "nicholas", "cal"]) - ) - - -@pytest.fixture -def create_app_request_superuser(rf, users): - request = rf.post(reverse("create-app")) - request.user = users["superuser"] - return request - - -def test_create_app_form_clean_new_datasource(create_app_request_superuser): - """ - The CreateAppForm class has a bespoke "clean" method. We should ensure it - checks the expected things in the correct way. - """ - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/ministryofjustice/my_repo", - "connect_bucket": "new", - "new_datasource_name": "test-bucketname", - }, - request=create_app_request_superuser, - ) - f.clean_repo_url = mock.MagicMock() - mock_s3 = mock.MagicMock() - mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") - # A valid form returns True. - with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): - assert f.is_valid() is True - # A new datasource name is required if the connection is new. - f = forms.CreateAppForm( - data={ - "deployment_envs": ["test"], - "repo_url": "https://github.com/ministryofjustice/my_repo", - "connect_bucket": "new", - }, - request=create_app_request_superuser, - ) - f.clean_repo_url = mock.MagicMock() - assert f.is_valid() is False - assert "new_datasource_name" in f.errors - assert "This field is required" in f.errors["new_datasource_name"][0] - # If a datasource already exists, report the duplication. - f = forms.CreateAppForm( - data={ - "deployment_envs": ["test"], - "repo_url": "https://github.com/ministryofjustice/my_repo", - "connect_bucket": "new", - "new_datasource_name": "test-bucketname", - }, - request=create_app_request_superuser, - ) - f.clean_repo_url = mock.MagicMock() - mock_s3 = mock.MagicMock() - with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): - assert f.is_valid() is False - assert "Datasource named test-bucketname already exists" in f.errors["new_datasource_name"][0] - - -def test_create_app_form_clean_existing_datasource(create_app_request_superuser): - """ - An existing datasource name is required if the datasource is marked as - already existing. - """ - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/moj-analytical-services/my_repo", - "connect_bucket": "existing", - "connections": ["email"], - }, - request=create_app_request_superuser, - ) - f.clean_repo_url = mock.MagicMock() - # A valid form returns True. - assert f.is_valid() is False - assert "existing_datasource_id" in f.errors - - -def test_create_app_form_new_datasource_but_bucket_existed(create_app_request_superuser): - bucket_name = "test-bucketname" - aws.AWSBucket().create(bucket_name, is_data_warehouse=True) - - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/moj-analytical-services/my_repo", - "connect_bucket": "new", - "new_datasource_name": bucket_name, - "connections": ["email"], - }, - request=create_app_request_superuser, - ) - f.clean_repo_url = mock.MagicMock() - assert f.is_valid() is False - assert "already exists" in ".".join(f.errors["new_datasource_name"]) - - -def test_create_new_datasource_but_bucket_existed(): - bucket_name = "test-bucketname" - aws.AWSBucket().create(bucket_name, is_data_warehouse=True) - - f = forms.CreateDatasourceForm( - data={ - "name": bucket_name, - } - ) - assert f.is_valid() is False - assert "already exists" in ".".join(f.errors["name"]) - - -def test_create_new_datasource_folder_exists(root_folder_bucket): - root_folder_bucket.put_object(Key='test-folder/') - form = forms.CreateDatasourceFolderForm( - data={"name": "test-folder"} - ) - - assert form.is_valid() is False - assert "Folder 'test-folder' already exists" in form.errors["name"] - - -@pytest.mark.parametrize( - "path, expected_error", - [ - ("noslash", "Enter paths prefixed with a forward slash"), - ("/trailingslash/", "Enter paths without a trailing forward slash"), - ("/valid", None), - ] -) -def test_grant_access_form_clean_paths(path, expected_error): - data = { - "paths": [path] - } - form = forms.GrantAccessForm() - form.cleaned_data = data - - if expected_error: - with pytest.raises(ValidationError, match=expected_error): - form.clean_paths() - else: - assert form.clean_paths() == [path] - - -def test_create_app_form_clean_repo_url(create_app_request_superuser): - """ - Ensure the various states of a GitHub repository result in a valid form or - errors. - """ - # The good case. - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/ministryofjustice/my_repo", - "connect_bucket": "new", - "new_datasource_name": "test-bucketname", - }, - request=create_app_request_superuser, - ) - f.request = mock.MagicMock() - mock_get_repo = mock.MagicMock(return_value=True) - mock_app = mock.MagicMock() - mock_app.objects.filter().exists.return_value = False - mock_s3 = mock.MagicMock() - mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") - with mock.patch( - "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo - ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( - "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 - ): - assert f.is_valid() is True - - # App already exists. - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/ministryofjustice/my_repo", - "connect_bucket": "new", - "new_datasource_name": "test-bucketname", - }, - request=create_app_request_superuser - ) - f.request = mock.MagicMock() - mock_app = mock.MagicMock() - mock_app.objects.filter().exists.return_value = True - mock_s3 = mock.MagicMock() - mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") - with mock.patch( - "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo - ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( - "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 - ): - assert f.is_valid() is False - assert f.errors["repo_url"][0] == "App already exists for this repository URL" - - # Repo in correct org but not found - f = forms.CreateAppForm( - data={ - "repo_url": "https://github.com/ministryofjustice/doesnt-exist", - "connect_bucket": "new", - "new_datasource_name": "test-bucketname", - }, - request=create_app_request_superuser, - ) - f.request = mock.MagicMock() - mock_app = mock.MagicMock() - mock_app.objects.filter().exists.return_value = False - with mock.patch( - "controlpanel.frontend.forms.GithubAPI.get_repository", - side_effect=RepositoryNotFound - ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( - "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 - ): - assert f.is_valid() is False - error = f.errors["repo_url"][0] - assert "Github repository not found - it may be private" in error - - -@pytest.mark.parametrize("user", ["superuser", "normal_user", "other_user"]) -def test_create_app_form_get_datasource_queryset(users, rf, user): - """ - Assert that for each user, - """ - superuser_bucket = S3Bucket.objects.create( - name="superuser_bucket", - created_by=users["superuser"], - is_data_warehouse=False, - ) - user_bucket = S3Bucket.objects.create( - name="user_bucket", - created_by=users["normal_user"], - is_data_warehouse=False, - ) - warehouse_bucket = S3Bucket.objects.create( - name="warehouse_bucket", - created_by=users["superuser"], - is_data_warehouse=True, - ) - expected_buckets = { - "superuser": [superuser_bucket, user_bucket], - "normal_user": [user_bucket], - "other_user": [] - } - - request = rf.post(reverse("create-app")) - request.user = users[user] - form = forms.CreateAppForm(request=request) - - queryset = form.get_datasource_queryset() - - assert list(queryset) == expected_buckets[user] - assert warehouse_bucket not in expected_buckets[user] - - -def test_update_app_with_custom_connection(): - # Good case. - f = forms.UpdateAppAuth0ConnectionsForm( - data={ - "env_name": "test", - "connections": ["email", "auth0_nomis"], - "auth0_nomis_auth0_client_id": "nomis-client-id", - "auth0_nomis_auth0_client_secret": "nomis-client-secret", - "auth0_nomis_auth0_conn_name": "nomis-conn-name", - }, - all_connections_names=["github", "email", "auth0_nomis"], - custom_connections=["auth0_nomis"], - auth0_connections=["github"], - ) - f.request = mock.MagicMock() - mock_app = mock.MagicMock() - mock_app.objects.filter().exists.return_value = False - with mock.patch("controlpanel.frontend.forms.App", mock_app): - assert f.is_valid() is True - - # Bad case: missing client credential for nomis login + not valid connection name - f = forms.UpdateAppAuth0ConnectionsForm( - data={ - "env_name": "test", - "connections": ["email", "auth0_nomis"], - "auth0_nomis_auth0_client_id": "nomis-client-id", - "auth0_nomis_auth0_client_secret": "", - "auth0_nomis_auth0_conn_name": "nomis_conn_name", - }, - all_connections_names=["github", "email", "auth0_nomis"], - custom_connections=["auth0_nomis"], - auth0_connections=["github"], - ) - f.request = mock.MagicMock() - mock_app = mock.MagicMock() - mock_app.objects.filter().exists.return_value = False - with mock.patch("controlpanel.frontend.forms.App", mock_app): - assert f.is_valid() is False - assert "auth0_nomis_auth0_client_secret" in f.errors - assert "auth0_nomis_auth0_conn_name" in f.errors - - -def test_pass(): - assert True - - -@pytest.mark.django_db -def test_ip_allowlist_form_invalid_ip(): - """ - Make sure invalid IP allowlist configurations throw errors as expected - (See also validation tests in ../test_validators.py) - """ - data = { - "name": "An IP allowlist", - "allowed_ip_ranges": "123, 456", - } - f = forms.IPAllowlistForm(data) - assert f.errors["allowed_ip_ranges"] == [ - "123 should be an IPv4 or IPv6 address (in a comma-separated list if several IP addresses are provided)." # noqa: E501 - ] - - -@pytest.mark.django_db -def test_ip_allowlist_form_missing_ip(): - data = { - "name": "An IP allowlist", - "allowed_ip_ranges": "", - } - f = forms.IPAllowlistForm(data) - assert f.errors["allowed_ip_ranges"] == ["This field is required."] - - -@pytest.mark.django_db -def test_ip_allowlist_form_missing_name(): - data = { - "name": "", - "allowed_ip_ranges": "192.168.0.0/28", - } - f = forms.IPAllowlistForm(data) - assert f.errors["name"] == ["This field is required."] +# # Standard library +# from unittest import mock + +# # Third-party +# import pytest +# from django.core.exceptions import ValidationError +# from django.urls import reverse + +# # First-party/Local +# from controlpanel.api import aws +# from controlpanel.api.github import RepositoryNotFound +# from controlpanel.api.models import S3Bucket +# from controlpanel.frontend import forms + + +# def test_tool_release_form_check_release_name(): +# """ +# Ensure valid chart names work, while invalid ones cause a helpful +# exception. +# """ +# data = { +# "name": "Test Release", +# "chart_name": "jupyter-lab", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "jupyter-lab-all-spark", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "rstudio", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "airflow-sqlite", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "invalid-chartname", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() is False + + +# def test_tool_release_form_check_tool_domain(): +# """ +# Ensure ONLY valid chart names work, while invalid ones cause a helpful +# exception. +# """ +# data = { +# "name": "Test Release", +# "chart_name": "jupyter-lab", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# "tool_domain": "jupyter-lab", +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "jupyter-lab-all-spark", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# "tool_domain": "jupyter-lab", +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() +# data = { +# "name": "Test Release", +# "chart_name": "jupyter-lab-all-spark", +# "version": "1.2.3", +# "values": {"foo": "bar"}, +# "is_restricted": False, +# "tool_domain": "invalid-tool-domain", +# } +# f = forms.ToolReleaseForm(data) +# assert f.is_valid() is False + + +# def test_tool_release_form_get_target_users(): +# """ +# Given a string list of comma separated usernames, the expected query to +# return the associated User objects is created. +# """ +# f = forms.ToolReleaseForm() +# f.data = { +# "target_users_list": "aldo, nicholas, cal", +# } +# mock_user = mock.MagicMock() +# with mock.patch("controlpanel.frontend.forms.User", mock_user): +# f.get_target_users() +# mock_user.objects.filter.assert_called_once_with( +# username__in=set(["aldo", "nicholas", "cal"]) +# ) + + +# @pytest.fixture +# def create_app_request_superuser(rf, users): +# request = rf.post(reverse("create-app")) +# request.user = users["superuser"] +# return request + + +# def test_create_app_form_clean_new_datasource(create_app_request_superuser): +# """ +# The CreateAppForm class has a bespoke "clean" method. We should ensure it +# checks the expected things in the correct way. +# """ +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/ministryofjustice/my_repo", +# "connect_bucket": "new", +# "new_datasource_name": "test-bucketname", +# }, +# request=create_app_request_superuser, +# ) +# f.clean_repo_url = mock.MagicMock() +# mock_s3 = mock.MagicMock() +# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") +# # A valid form returns True. +# with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): +# assert f.is_valid() is True +# # A new datasource name is required if the connection is new. +# f = forms.CreateAppForm( +# data={ +# "deployment_envs": ["test"], +# "repo_url": "https://github.com/ministryofjustice/my_repo", +# "connect_bucket": "new", +# }, +# request=create_app_request_superuser, +# ) +# f.clean_repo_url = mock.MagicMock() +# assert f.is_valid() is False +# assert "new_datasource_name" in f.errors +# assert "This field is required" in f.errors["new_datasource_name"][0] +# # If a datasource already exists, report the duplication. +# f = forms.CreateAppForm( +# data={ +# "deployment_envs": ["test"], +# "repo_url": "https://github.com/ministryofjustice/my_repo", +# "connect_bucket": "new", +# "new_datasource_name": "test-bucketname", +# }, +# request=create_app_request_superuser, +# ) +# f.clean_repo_url = mock.MagicMock() +# mock_s3 = mock.MagicMock() +# with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): +# assert f.is_valid() is False +# assert "Datasource named test-bucketname already exists" in f.errors["new_datasource_name"][0] + + +# def test_create_app_form_clean_existing_datasource(create_app_request_superuser): +# """ +# An existing datasource name is required if the datasource is marked as +# already existing. +# """ +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/moj-analytical-services/my_repo", +# "connect_bucket": "existing", +# "connections": ["email"], +# }, +# request=create_app_request_superuser, +# ) +# f.clean_repo_url = mock.MagicMock() +# # A valid form returns True. +# assert f.is_valid() is False +# assert "existing_datasource_id" in f.errors + + +# def test_create_app_form_new_datasource_but_bucket_existed(create_app_request_superuser): +# bucket_name = "test-bucketname" +# aws.AWSBucket().create(bucket_name, is_data_warehouse=True) + +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/moj-analytical-services/my_repo", +# "connect_bucket": "new", +# "new_datasource_name": bucket_name, +# "connections": ["email"], +# }, +# request=create_app_request_superuser, +# ) +# f.clean_repo_url = mock.MagicMock() +# assert f.is_valid() is False +# assert "already exists" in ".".join(f.errors["new_datasource_name"]) + + +# def test_create_new_datasource_but_bucket_existed(): +# bucket_name = "test-bucketname" +# aws.AWSBucket().create(bucket_name, is_data_warehouse=True) + +# f = forms.CreateDatasourceForm( +# data={ +# "name": bucket_name, +# } +# ) +# assert f.is_valid() is False +# assert "already exists" in ".".join(f.errors["name"]) + + +# def test_create_new_datasource_folder_exists(root_folder_bucket): +# root_folder_bucket.put_object(Key='test-folder/') +# form = forms.CreateDatasourceFolderForm( +# data={"name": "test-folder"} +# ) + +# assert form.is_valid() is False +# assert "Folder 'test-folder' already exists" in form.errors["name"] + + +# @pytest.mark.parametrize( +# "path, expected_error", +# [ +# ("noslash", "Enter paths prefixed with a forward slash"), +# ("/trailingslash/", "Enter paths without a trailing forward slash"), +# ("/valid", None), +# ] +# ) +# def test_grant_access_form_clean_paths(path, expected_error): +# data = { +# "paths": [path] +# } +# form = forms.GrantAccessForm() +# form.cleaned_data = data + +# if expected_error: +# with pytest.raises(ValidationError, match=expected_error): +# form.clean_paths() +# else: +# assert form.clean_paths() == [path] + + +# def test_create_app_form_clean_repo_url(create_app_request_superuser): +# """ +# Ensure the various states of a GitHub repository result in a valid form or +# errors. +# """ +# # The good case. +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/ministryofjustice/my_repo", +# "connect_bucket": "new", +# "new_datasource_name": "test-bucketname", +# }, +# request=create_app_request_superuser, +# ) +# f.request = mock.MagicMock() +# mock_get_repo = mock.MagicMock(return_value=True) +# mock_app = mock.MagicMock() +# mock_app.objects.filter().exists.return_value = False +# mock_s3 = mock.MagicMock() +# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") +# with mock.patch( +# "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo +# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( +# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 +# ): +# assert f.is_valid() is True + +# # App already exists. +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/ministryofjustice/my_repo", +# "connect_bucket": "new", +# "new_datasource_name": "test-bucketname", +# }, +# request=create_app_request_superuser +# ) +# f.request = mock.MagicMock() +# mock_app = mock.MagicMock() +# mock_app.objects.filter().exists.return_value = True +# mock_s3 = mock.MagicMock() +# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") +# with mock.patch( +# "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo +# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( +# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 +# ): +# assert f.is_valid() is False +# assert f.errors["repo_url"][0] == "App already exists for this repository URL" + +# # Repo in correct org but not found +# f = forms.CreateAppForm( +# data={ +# "repo_url": "https://github.com/ministryofjustice/doesnt-exist", +# "connect_bucket": "new", +# "new_datasource_name": "test-bucketname", +# }, +# request=create_app_request_superuser, +# ) +# f.request = mock.MagicMock() +# mock_app = mock.MagicMock() +# mock_app.objects.filter().exists.return_value = False +# with mock.patch( +# "controlpanel.frontend.forms.GithubAPI.get_repository", +# side_effect=RepositoryNotFound +# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( +# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 +# ): +# assert f.is_valid() is False +# error = f.errors["repo_url"][0] +# assert "Github repository not found - it may be private" in error + + +# @pytest.mark.parametrize("user", ["superuser", "normal_user", "other_user"]) +# def test_create_app_form_get_datasource_queryset(users, rf, user): +# """ +# Assert that for each user, +# """ +# superuser_bucket = S3Bucket.objects.create( +# name="superuser_bucket", +# created_by=users["superuser"], +# is_data_warehouse=False, +# ) +# user_bucket = S3Bucket.objects.create( +# name="user_bucket", +# created_by=users["normal_user"], +# is_data_warehouse=False, +# ) +# warehouse_bucket = S3Bucket.objects.create( +# name="warehouse_bucket", +# created_by=users["superuser"], +# is_data_warehouse=True, +# ) +# expected_buckets = { +# "superuser": [superuser_bucket, user_bucket], +# "normal_user": [user_bucket], +# "other_user": [] +# } + +# request = rf.post(reverse("create-app")) +# request.user = users[user] +# form = forms.CreateAppForm(request=request) + +# queryset = form.get_datasource_queryset() + +# assert list(queryset) == expected_buckets[user] +# assert warehouse_bucket not in expected_buckets[user] + + +# def test_update_app_with_custom_connection(): +# # Good case. +# f = forms.UpdateAppAuth0ConnectionsForm( +# data={ +# "env_name": "test", +# "connections": ["email", "auth0_nomis"], +# "auth0_nomis_auth0_client_id": "nomis-client-id", +# "auth0_nomis_auth0_client_secret": "nomis-client-secret", +# "auth0_nomis_auth0_conn_name": "nomis-conn-name", +# }, +# all_connections_names=["github", "email", "auth0_nomis"], +# custom_connections=["auth0_nomis"], +# auth0_connections=["github"], +# ) +# f.request = mock.MagicMock() +# mock_app = mock.MagicMock() +# mock_app.objects.filter().exists.return_value = False +# with mock.patch("controlpanel.frontend.forms.App", mock_app): +# assert f.is_valid() is True + +# # Bad case: missing client credential for nomis login + not valid connection name +# f = forms.UpdateAppAuth0ConnectionsForm( +# data={ +# "env_name": "test", +# "connections": ["email", "auth0_nomis"], +# "auth0_nomis_auth0_client_id": "nomis-client-id", +# "auth0_nomis_auth0_client_secret": "", +# "auth0_nomis_auth0_conn_name": "nomis_conn_name", +# }, +# all_connections_names=["github", "email", "auth0_nomis"], +# custom_connections=["auth0_nomis"], +# auth0_connections=["github"], +# ) +# f.request = mock.MagicMock() +# mock_app = mock.MagicMock() +# mock_app.objects.filter().exists.return_value = False +# with mock.patch("controlpanel.frontend.forms.App", mock_app): +# assert f.is_valid() is False +# assert "auth0_nomis_auth0_client_secret" in f.errors +# assert "auth0_nomis_auth0_conn_name" in f.errors + + +# def test_pass(): +# assert True + + +# @pytest.mark.django_db +# def test_ip_allowlist_form_invalid_ip(): +# """ +# Make sure invalid IP allowlist configurations throw errors as expected +# (See also validation tests in ../test_validators.py) +# """ +# data = { +# "name": "An IP allowlist", +# "allowed_ip_ranges": "123, 456", +# } +# f = forms.IPAllowlistForm(data) +# assert f.errors["allowed_ip_ranges"] == [ +# "123 should be an IPv4 or IPv6 address (in a comma-separated list if several IP addresses are provided)." # noqa: E501 +# ] + + +# @pytest.mark.django_db +# def test_ip_allowlist_form_missing_ip(): +# data = { +# "name": "An IP allowlist", +# "allowed_ip_ranges": "", +# } +# f = forms.IPAllowlistForm(data) +# assert f.errors["allowed_ip_ranges"] == ["This field is required."] + + +# @pytest.mark.django_db +# def test_ip_allowlist_form_missing_name(): +# data = { +# "name": "", +# "allowed_ip_ranges": "192.168.0.0/28", +# } +# f = forms.IPAllowlistForm(data) +# assert f.errors["name"] == ["This field is required."] From a985206fc88a1d06446c987c883390c53fa6829a Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 15:15:51 +0000 Subject: [PATCH 21/31] adding tests back --- tests/frontend/test_forms.py | 894 +++++++++++++++++------------------ 1 file changed, 447 insertions(+), 447 deletions(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 4e0d6c22f..3c5b76138 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -1,447 +1,447 @@ -# # Standard library -# from unittest import mock - -# # Third-party -# import pytest -# from django.core.exceptions import ValidationError -# from django.urls import reverse - -# # First-party/Local -# from controlpanel.api import aws -# from controlpanel.api.github import RepositoryNotFound -# from controlpanel.api.models import S3Bucket -# from controlpanel.frontend import forms - - -# def test_tool_release_form_check_release_name(): -# """ -# Ensure valid chart names work, while invalid ones cause a helpful -# exception. -# """ -# data = { -# "name": "Test Release", -# "chart_name": "jupyter-lab", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "jupyter-lab-all-spark", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "rstudio", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "airflow-sqlite", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "invalid-chartname", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() is False - - -# def test_tool_release_form_check_tool_domain(): -# """ -# Ensure ONLY valid chart names work, while invalid ones cause a helpful -# exception. -# """ -# data = { -# "name": "Test Release", -# "chart_name": "jupyter-lab", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# "tool_domain": "jupyter-lab", -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "jupyter-lab-all-spark", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# "tool_domain": "jupyter-lab", -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() -# data = { -# "name": "Test Release", -# "chart_name": "jupyter-lab-all-spark", -# "version": "1.2.3", -# "values": {"foo": "bar"}, -# "is_restricted": False, -# "tool_domain": "invalid-tool-domain", -# } -# f = forms.ToolReleaseForm(data) -# assert f.is_valid() is False - - -# def test_tool_release_form_get_target_users(): -# """ -# Given a string list of comma separated usernames, the expected query to -# return the associated User objects is created. -# """ -# f = forms.ToolReleaseForm() -# f.data = { -# "target_users_list": "aldo, nicholas, cal", -# } -# mock_user = mock.MagicMock() -# with mock.patch("controlpanel.frontend.forms.User", mock_user): -# f.get_target_users() -# mock_user.objects.filter.assert_called_once_with( -# username__in=set(["aldo", "nicholas", "cal"]) -# ) - - -# @pytest.fixture -# def create_app_request_superuser(rf, users): -# request = rf.post(reverse("create-app")) -# request.user = users["superuser"] -# return request - - -# def test_create_app_form_clean_new_datasource(create_app_request_superuser): -# """ -# The CreateAppForm class has a bespoke "clean" method. We should ensure it -# checks the expected things in the correct way. -# """ -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/ministryofjustice/my_repo", -# "connect_bucket": "new", -# "new_datasource_name": "test-bucketname", -# }, -# request=create_app_request_superuser, -# ) -# f.clean_repo_url = mock.MagicMock() -# mock_s3 = mock.MagicMock() -# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") -# # A valid form returns True. -# with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): -# assert f.is_valid() is True -# # A new datasource name is required if the connection is new. -# f = forms.CreateAppForm( -# data={ -# "deployment_envs": ["test"], -# "repo_url": "https://github.com/ministryofjustice/my_repo", -# "connect_bucket": "new", -# }, -# request=create_app_request_superuser, -# ) -# f.clean_repo_url = mock.MagicMock() -# assert f.is_valid() is False -# assert "new_datasource_name" in f.errors -# assert "This field is required" in f.errors["new_datasource_name"][0] -# # If a datasource already exists, report the duplication. -# f = forms.CreateAppForm( -# data={ -# "deployment_envs": ["test"], -# "repo_url": "https://github.com/ministryofjustice/my_repo", -# "connect_bucket": "new", -# "new_datasource_name": "test-bucketname", -# }, -# request=create_app_request_superuser, -# ) -# f.clean_repo_url = mock.MagicMock() -# mock_s3 = mock.MagicMock() -# with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): -# assert f.is_valid() is False -# assert "Datasource named test-bucketname already exists" in f.errors["new_datasource_name"][0] - - -# def test_create_app_form_clean_existing_datasource(create_app_request_superuser): -# """ -# An existing datasource name is required if the datasource is marked as -# already existing. -# """ -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/moj-analytical-services/my_repo", -# "connect_bucket": "existing", -# "connections": ["email"], -# }, -# request=create_app_request_superuser, -# ) -# f.clean_repo_url = mock.MagicMock() -# # A valid form returns True. -# assert f.is_valid() is False -# assert "existing_datasource_id" in f.errors - - -# def test_create_app_form_new_datasource_but_bucket_existed(create_app_request_superuser): -# bucket_name = "test-bucketname" -# aws.AWSBucket().create(bucket_name, is_data_warehouse=True) - -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/moj-analytical-services/my_repo", -# "connect_bucket": "new", -# "new_datasource_name": bucket_name, -# "connections": ["email"], -# }, -# request=create_app_request_superuser, -# ) -# f.clean_repo_url = mock.MagicMock() -# assert f.is_valid() is False -# assert "already exists" in ".".join(f.errors["new_datasource_name"]) - - -# def test_create_new_datasource_but_bucket_existed(): -# bucket_name = "test-bucketname" -# aws.AWSBucket().create(bucket_name, is_data_warehouse=True) - -# f = forms.CreateDatasourceForm( -# data={ -# "name": bucket_name, -# } -# ) -# assert f.is_valid() is False -# assert "already exists" in ".".join(f.errors["name"]) - - -# def test_create_new_datasource_folder_exists(root_folder_bucket): -# root_folder_bucket.put_object(Key='test-folder/') -# form = forms.CreateDatasourceFolderForm( -# data={"name": "test-folder"} -# ) - -# assert form.is_valid() is False -# assert "Folder 'test-folder' already exists" in form.errors["name"] - - -# @pytest.mark.parametrize( -# "path, expected_error", -# [ -# ("noslash", "Enter paths prefixed with a forward slash"), -# ("/trailingslash/", "Enter paths without a trailing forward slash"), -# ("/valid", None), -# ] -# ) -# def test_grant_access_form_clean_paths(path, expected_error): -# data = { -# "paths": [path] -# } -# form = forms.GrantAccessForm() -# form.cleaned_data = data - -# if expected_error: -# with pytest.raises(ValidationError, match=expected_error): -# form.clean_paths() -# else: -# assert form.clean_paths() == [path] - - -# def test_create_app_form_clean_repo_url(create_app_request_superuser): -# """ -# Ensure the various states of a GitHub repository result in a valid form or -# errors. -# """ -# # The good case. -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/ministryofjustice/my_repo", -# "connect_bucket": "new", -# "new_datasource_name": "test-bucketname", -# }, -# request=create_app_request_superuser, -# ) -# f.request = mock.MagicMock() -# mock_get_repo = mock.MagicMock(return_value=True) -# mock_app = mock.MagicMock() -# mock_app.objects.filter().exists.return_value = False -# mock_s3 = mock.MagicMock() -# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") -# with mock.patch( -# "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo -# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( -# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 -# ): -# assert f.is_valid() is True - -# # App already exists. -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/ministryofjustice/my_repo", -# "connect_bucket": "new", -# "new_datasource_name": "test-bucketname", -# }, -# request=create_app_request_superuser -# ) -# f.request = mock.MagicMock() -# mock_app = mock.MagicMock() -# mock_app.objects.filter().exists.return_value = True -# mock_s3 = mock.MagicMock() -# mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") -# with mock.patch( -# "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo -# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( -# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 -# ): -# assert f.is_valid() is False -# assert f.errors["repo_url"][0] == "App already exists for this repository URL" - -# # Repo in correct org but not found -# f = forms.CreateAppForm( -# data={ -# "repo_url": "https://github.com/ministryofjustice/doesnt-exist", -# "connect_bucket": "new", -# "new_datasource_name": "test-bucketname", -# }, -# request=create_app_request_superuser, -# ) -# f.request = mock.MagicMock() -# mock_app = mock.MagicMock() -# mock_app.objects.filter().exists.return_value = False -# with mock.patch( -# "controlpanel.frontend.forms.GithubAPI.get_repository", -# side_effect=RepositoryNotFound -# ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( -# "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 -# ): -# assert f.is_valid() is False -# error = f.errors["repo_url"][0] -# assert "Github repository not found - it may be private" in error - - -# @pytest.mark.parametrize("user", ["superuser", "normal_user", "other_user"]) -# def test_create_app_form_get_datasource_queryset(users, rf, user): -# """ -# Assert that for each user, -# """ -# superuser_bucket = S3Bucket.objects.create( -# name="superuser_bucket", -# created_by=users["superuser"], -# is_data_warehouse=False, -# ) -# user_bucket = S3Bucket.objects.create( -# name="user_bucket", -# created_by=users["normal_user"], -# is_data_warehouse=False, -# ) -# warehouse_bucket = S3Bucket.objects.create( -# name="warehouse_bucket", -# created_by=users["superuser"], -# is_data_warehouse=True, -# ) -# expected_buckets = { -# "superuser": [superuser_bucket, user_bucket], -# "normal_user": [user_bucket], -# "other_user": [] -# } - -# request = rf.post(reverse("create-app")) -# request.user = users[user] -# form = forms.CreateAppForm(request=request) - -# queryset = form.get_datasource_queryset() - -# assert list(queryset) == expected_buckets[user] -# assert warehouse_bucket not in expected_buckets[user] - - -# def test_update_app_with_custom_connection(): -# # Good case. -# f = forms.UpdateAppAuth0ConnectionsForm( -# data={ -# "env_name": "test", -# "connections": ["email", "auth0_nomis"], -# "auth0_nomis_auth0_client_id": "nomis-client-id", -# "auth0_nomis_auth0_client_secret": "nomis-client-secret", -# "auth0_nomis_auth0_conn_name": "nomis-conn-name", -# }, -# all_connections_names=["github", "email", "auth0_nomis"], -# custom_connections=["auth0_nomis"], -# auth0_connections=["github"], -# ) -# f.request = mock.MagicMock() -# mock_app = mock.MagicMock() -# mock_app.objects.filter().exists.return_value = False -# with mock.patch("controlpanel.frontend.forms.App", mock_app): -# assert f.is_valid() is True - -# # Bad case: missing client credential for nomis login + not valid connection name -# f = forms.UpdateAppAuth0ConnectionsForm( -# data={ -# "env_name": "test", -# "connections": ["email", "auth0_nomis"], -# "auth0_nomis_auth0_client_id": "nomis-client-id", -# "auth0_nomis_auth0_client_secret": "", -# "auth0_nomis_auth0_conn_name": "nomis_conn_name", -# }, -# all_connections_names=["github", "email", "auth0_nomis"], -# custom_connections=["auth0_nomis"], -# auth0_connections=["github"], -# ) -# f.request = mock.MagicMock() -# mock_app = mock.MagicMock() -# mock_app.objects.filter().exists.return_value = False -# with mock.patch("controlpanel.frontend.forms.App", mock_app): -# assert f.is_valid() is False -# assert "auth0_nomis_auth0_client_secret" in f.errors -# assert "auth0_nomis_auth0_conn_name" in f.errors - - -# def test_pass(): -# assert True - - -# @pytest.mark.django_db -# def test_ip_allowlist_form_invalid_ip(): -# """ -# Make sure invalid IP allowlist configurations throw errors as expected -# (See also validation tests in ../test_validators.py) -# """ -# data = { -# "name": "An IP allowlist", -# "allowed_ip_ranges": "123, 456", -# } -# f = forms.IPAllowlistForm(data) -# assert f.errors["allowed_ip_ranges"] == [ -# "123 should be an IPv4 or IPv6 address (in a comma-separated list if several IP addresses are provided)." # noqa: E501 -# ] - - -# @pytest.mark.django_db -# def test_ip_allowlist_form_missing_ip(): -# data = { -# "name": "An IP allowlist", -# "allowed_ip_ranges": "", -# } -# f = forms.IPAllowlistForm(data) -# assert f.errors["allowed_ip_ranges"] == ["This field is required."] - - -# @pytest.mark.django_db -# def test_ip_allowlist_form_missing_name(): -# data = { -# "name": "", -# "allowed_ip_ranges": "192.168.0.0/28", -# } -# f = forms.IPAllowlistForm(data) -# assert f.errors["name"] == ["This field is required."] +# Standard library +from unittest import mock + +# Third-party +import pytest +from django.core.exceptions import ValidationError +from django.urls import reverse + +# First-party/Local +from controlpanel.api import aws +from controlpanel.api.github import RepositoryNotFound +from controlpanel.api.models import S3Bucket +from controlpanel.frontend import forms + + +def test_tool_release_form_check_release_name(): + """ + Ensure valid chart names work, while invalid ones cause a helpful + exception. + """ + data = { + "name": "Test Release", + "chart_name": "jupyter-lab", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "jupyter-lab-all-spark", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "rstudio", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "airflow-sqlite", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "invalid-chartname", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() is False + + +def test_tool_release_form_check_tool_domain(): + """ + Ensure ONLY valid chart names work, while invalid ones cause a helpful + exception. + """ + data = { + "name": "Test Release", + "chart_name": "jupyter-lab", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + "tool_domain": "jupyter-lab", + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "jupyter-lab-all-spark", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + "tool_domain": "jupyter-lab", + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() + data = { + "name": "Test Release", + "chart_name": "jupyter-lab-all-spark", + "version": "1.2.3", + "values": {"foo": "bar"}, + "is_restricted": False, + "tool_domain": "invalid-tool-domain", + } + f = forms.ToolReleaseForm(data) + assert f.is_valid() is False + + +def test_tool_release_form_get_target_users(): + """ + Given a string list of comma separated usernames, the expected query to + return the associated User objects is created. + """ + f = forms.ToolReleaseForm() + f.data = { + "target_users_list": "aldo, nicholas, cal", + } + mock_user = mock.MagicMock() + with mock.patch("controlpanel.frontend.forms.User", mock_user): + f.get_target_users() + mock_user.objects.filter.assert_called_once_with( + username__in=set(["aldo", "nicholas", "cal"]) + ) + + +@pytest.fixture +def create_app_request_superuser(rf, users): + request = rf.post(reverse("create-app")) + request.user = users["superuser"] + return request + + +def test_create_app_form_clean_new_datasource(create_app_request_superuser): + """ + The CreateAppForm class has a bespoke "clean" method. We should ensure it + checks the expected things in the correct way. + """ + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/ministryofjustice/my_repo", + "connect_bucket": "new", + "new_datasource_name": "test-bucketname", + }, + request=create_app_request_superuser, + ) + f.clean_repo_url = mock.MagicMock() + mock_s3 = mock.MagicMock() + mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") + # A valid form returns True. + with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): + assert f.is_valid() is True + # A new datasource name is required if the connection is new. + f = forms.CreateAppForm( + data={ + "deployment_envs": ["test"], + "repo_url": "https://github.com/ministryofjustice/my_repo", + "connect_bucket": "new", + }, + request=create_app_request_superuser, + ) + f.clean_repo_url = mock.MagicMock() + assert f.is_valid() is False + assert "new_datasource_name" in f.errors + assert "This field is required" in f.errors["new_datasource_name"][0] + # If a datasource already exists, report the duplication. + f = forms.CreateAppForm( + data={ + "deployment_envs": ["test"], + "repo_url": "https://github.com/ministryofjustice/my_repo", + "connect_bucket": "new", + "new_datasource_name": "test-bucketname", + }, + request=create_app_request_superuser, + ) + f.clean_repo_url = mock.MagicMock() + mock_s3 = mock.MagicMock() + with mock.patch("controlpanel.frontend.forms.S3Bucket.objects", mock_s3): + assert f.is_valid() is False + assert "Datasource named test-bucketname already exists" in f.errors["new_datasource_name"][0] + + +def test_create_app_form_clean_existing_datasource(create_app_request_superuser): + """ + An existing datasource name is required if the datasource is marked as + already existing. + """ + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/moj-analytical-services/my_repo", + "connect_bucket": "existing", + "connections": ["email"], + }, + request=create_app_request_superuser, + ) + f.clean_repo_url = mock.MagicMock() + # A valid form returns True. + assert f.is_valid() is False + assert "existing_datasource_id" in f.errors + + +def test_create_app_form_new_datasource_but_bucket_existed(create_app_request_superuser): + bucket_name = "test-bucketname" + aws.AWSBucket().create(bucket_name, is_data_warehouse=True) + + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/moj-analytical-services/my_repo", + "connect_bucket": "new", + "new_datasource_name": bucket_name, + "connections": ["email"], + }, + request=create_app_request_superuser, + ) + f.clean_repo_url = mock.MagicMock() + assert f.is_valid() is False + assert "already exists" in ".".join(f.errors["new_datasource_name"]) + + +def test_create_new_datasource_but_bucket_existed(): + bucket_name = "test-bucketname" + aws.AWSBucket().create(bucket_name, is_data_warehouse=True) + + f = forms.CreateDatasourceForm( + data={ + "name": bucket_name, + } + ) + assert f.is_valid() is False + assert "already exists" in ".".join(f.errors["name"]) + + +def test_create_new_datasource_folder_exists(root_folder_bucket): + root_folder_bucket.put_object(Key='test-folder/') + form = forms.CreateDatasourceFolderForm( + data={"name": "test-folder"} + ) + + assert form.is_valid() is False + assert "Folder 'test-folder' already exists" in form.errors["name"] + + +@pytest.mark.parametrize( + "path, expected_error", + [ + ("noslash", "Enter paths prefixed with a forward slash"), + ("/trailingslash/", "Enter paths without a trailing forward slash"), + ("/valid", None), + ] +) +def test_grant_access_form_clean_paths(path, expected_error): + data = { + "paths": [path] + } + form = forms.GrantAccessForm() + form.cleaned_data = data + + if expected_error: + with pytest.raises(ValidationError, match=expected_error): + form.clean_paths() + else: + assert form.clean_paths() == [path] + + +def test_create_app_form_clean_repo_url(create_app_request_superuser): + """ + Ensure the various states of a GitHub repository result in a valid form or + errors. + """ + # The good case. + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/ministryofjustice/my_repo", + "connect_bucket": "new", + "new_datasource_name": "test-bucketname", + }, + request=create_app_request_superuser, + ) + f.request = mock.MagicMock() + mock_get_repo = mock.MagicMock(return_value=True) + mock_app = mock.MagicMock() + mock_app.objects.filter().exists.return_value = False + mock_s3 = mock.MagicMock() + mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") + with mock.patch( + "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo + ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( + "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 + ): + assert f.is_valid() is True + + # App already exists. + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/ministryofjustice/my_repo", + "connect_bucket": "new", + "new_datasource_name": "test-bucketname", + }, + request=create_app_request_superuser + ) + f.request = mock.MagicMock() + mock_app = mock.MagicMock() + mock_app.objects.filter().exists.return_value = True + mock_s3 = mock.MagicMock() + mock_s3.get.side_effect = S3Bucket.DoesNotExist("Boom") + with mock.patch( + "controlpanel.frontend.forms.GithubAPI.get_repository", mock_get_repo + ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( + "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 + ): + assert f.is_valid() is False + assert f.errors["repo_url"][0] == "App already exists for this repository URL" + + # Repo in correct org but not found + f = forms.CreateAppForm( + data={ + "repo_url": "https://github.com/ministryofjustice/doesnt-exist", + "connect_bucket": "new", + "new_datasource_name": "test-bucketname", + }, + request=create_app_request_superuser, + ) + f.request = mock.MagicMock() + mock_app = mock.MagicMock() + mock_app.objects.filter().exists.return_value = False + with mock.patch( + "controlpanel.frontend.forms.GithubAPI.get_repository", + side_effect=RepositoryNotFound + ), mock.patch("controlpanel.frontend.forms.App", mock_app), mock.patch( + "controlpanel.frontend.forms.S3Bucket.objects", mock_s3 + ): + assert f.is_valid() is False + error = f.errors["repo_url"][0] + assert "Github repository not found - it may be private" in error + + +@pytest.mark.parametrize("user", ["superuser", "normal_user", "other_user"]) +def test_create_app_form_get_datasource_queryset(users, rf, user): + """ + Assert that for each user, + """ + superuser_bucket = S3Bucket.objects.create( + name="superuser_bucket", + created_by=users["superuser"], + is_data_warehouse=False, + ) + user_bucket = S3Bucket.objects.create( + name="user_bucket", + created_by=users["normal_user"], + is_data_warehouse=False, + ) + warehouse_bucket = S3Bucket.objects.create( + name="warehouse_bucket", + created_by=users["superuser"], + is_data_warehouse=True, + ) + expected_buckets = { + "superuser": [superuser_bucket, user_bucket], + "normal_user": [user_bucket], + "other_user": [] + } + + request = rf.post(reverse("create-app")) + request.user = users[user] + form = forms.CreateAppForm(request=request) + + queryset = form.get_datasource_queryset() + + assert list(queryset) == expected_buckets[user] + assert warehouse_bucket not in expected_buckets[user] + + +def test_update_app_with_custom_connection(): + # Good case. + f = forms.UpdateAppAuth0ConnectionsForm( + data={ + "env_name": "test", + "connections": ["email", "auth0_nomis"], + "auth0_nomis_auth0_client_id": "nomis-client-id", + "auth0_nomis_auth0_client_secret": "nomis-client-secret", + "auth0_nomis_auth0_conn_name": "nomis-conn-name", + }, + all_connections_names=["github", "email", "auth0_nomis"], + custom_connections=["auth0_nomis"], + auth0_connections=["github"], + ) + f.request = mock.MagicMock() + mock_app = mock.MagicMock() + mock_app.objects.filter().exists.return_value = False + with mock.patch("controlpanel.frontend.forms.App", mock_app): + assert f.is_valid() is True + + # Bad case: missing client credential for nomis login + not valid connection name + f = forms.UpdateAppAuth0ConnectionsForm( + data={ + "env_name": "test", + "connections": ["email", "auth0_nomis"], + "auth0_nomis_auth0_client_id": "nomis-client-id", + "auth0_nomis_auth0_client_secret": "", + "auth0_nomis_auth0_conn_name": "nomis_conn_name", + }, + all_connections_names=["github", "email", "auth0_nomis"], + custom_connections=["auth0_nomis"], + auth0_connections=["github"], + ) + f.request = mock.MagicMock() + mock_app = mock.MagicMock() + mock_app.objects.filter().exists.return_value = False + with mock.patch("controlpanel.frontend.forms.App", mock_app): + assert f.is_valid() is False + assert "auth0_nomis_auth0_client_secret" in f.errors + assert "auth0_nomis_auth0_conn_name" in f.errors + + +def test_pass(): + assert True + + +@pytest.mark.django_db +def test_ip_allowlist_form_invalid_ip(): + """ + Make sure invalid IP allowlist configurations throw errors as expected + (See also validation tests in ../test_validators.py) + """ + data = { + "name": "An IP allowlist", + "allowed_ip_ranges": "123, 456", + } + f = forms.IPAllowlistForm(data) + assert f.errors["allowed_ip_ranges"] == [ + "123 should be an IPv4 or IPv6 address (in a comma-separated list if several IP addresses are provided)." # noqa: E501 + ] + + +@pytest.mark.django_db +def test_ip_allowlist_form_missing_ip(): + data = { + "name": "An IP allowlist", + "allowed_ip_ranges": "", + } + f = forms.IPAllowlistForm(data) + assert f.errors["allowed_ip_ranges"] == ["This field is required."] + + +@pytest.mark.django_db +def test_ip_allowlist_form_missing_name(): + data = { + "name": "", + "allowed_ip_ranges": "192.168.0.0/28", + } + f = forms.IPAllowlistForm(data) + assert f.errors["name"] == ["This field is required."] From 33288d98b1945d850ec18c67c78572eeea84ee55 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 15:31:30 +0000 Subject: [PATCH 22/31] fixed merge conflicts --- tests/frontend/test_forms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 071994469..9eb1fa862 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -5,7 +5,6 @@ import pytest from django.core.exceptions import ValidationError from django.urls import reverse -from mock import MagicMock # First-party/Local from controlpanel.api import aws @@ -454,7 +453,7 @@ def test_ip_allowlist_form_missing_name(): @pytest.mark.parametrize("env", ["dev", "prod"]) @mock.patch( "controlpanel.frontend.forms.CreateAppForm.get_datasource_queryset", - new=MagicMock, + new=mock.MagicMock, ) def test_clean_namespace(env): form = forms.CreateAppForm() From f6b252115436980351788cf863bfb2e5db36da2d Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 26 Feb 2024 16:02:18 +0000 Subject: [PATCH 23/31] removed test --- tests/frontend/test_forms.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index 9eb1fa862..94fdb928e 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -410,10 +410,6 @@ def test_update_app_with_custom_connection(): assert "auth0_nomis_auth0_conn_name" in f.errors -def test_pass(): - assert True - - @pytest.mark.django_db def test_ip_allowlist_form_invalid_ip(): """ From 880628fb3a80e914be275a4ac624a4c8cf471305 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Thu, 4 Apr 2024 10:17:19 +0100 Subject: [PATCH 24/31] bumped python version to 3.12. Bumped some dependencies to latest version --- Dockerfile | 2 +- requirements.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5ea5de0b3..b10555791 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,7 +9,7 @@ RUN ./node_modules/.bin/sass --load-path=node_modules/ --style=compressed src/ap WORKDIR /src RUN /node_modules/.bin/jest -FROM public.ecr.aws/docker/library/python:3.11-alpine3.18 AS base +FROM public.ecr.aws/docker/library/python:3.12-alpine3.18 AS base ARG HELM_VERSION=3.14.1 ARG HELM_TARBALL=helm-v${HELM_VERSION}-linux-amd64.tar.gz diff --git a/requirements.txt b/requirements.txt index 52982f5bc..c7cf995b1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -asgiref==3.7.2 +asgiref==3.8.1 auth0-python==4.7.0 authlib==1.3.0 beautifulsoup4==4.12.3 @@ -7,7 +7,7 @@ celery[sqs]==5.3.6 channels==4.0.0 channels-redis==4.2.0 daphne==4.1.0 -Django==5.0.2 +Django==5.0.4 django-crequest==2018.5.11 django-extensions==3.2.3 django-filter==24.1 From 4b3161075fbcc1f83a5121d8adb2b5d1b69044b5 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Thu, 4 Apr 2024 11:11:26 +0100 Subject: [PATCH 25/31] attempt to fix tests --- tests/api/cluster/test_user.py | 11 ++++++++--- tests/api/models/test_user.py | 2 +- tests/frontend/views/test_app_variables.py | 4 ++-- tests/frontend/views/test_auth.py | 2 +- tests/frontend/views/test_index.py | 2 +- tests/frontend/views/test_secrets.py | 2 +- 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/api/cluster/test_user.py b/tests/api/cluster/test_user.py index 3c42619bf..f59321c33 100644 --- a/tests/api/cluster/test_user.py +++ b/tests/api/cluster/test_user.py @@ -1,5 +1,5 @@ # Standard library -from unittest.mock import MagicMock, call, patch +from unittest.mock import call, patch # Third-party import pytest @@ -16,6 +16,7 @@ def test_iam_role_name(users): def test_create(helm, settings, users): with patch("controlpanel.api.cluster.AWSRole.create_role") as aws_create_role: user = users["normal_user"] + print(user) cluster.User(user).create() aws_create_role.assert_called_with( user.iam_role_name, @@ -37,7 +38,8 @@ def test_create(helm, settings, users): f"--set=Username={user.slug}", ), ] - helm.upgrade_release.has_calls(expected_calls) + print(helm.upgrade_release.call_args_list) + helm.upgrade_release.assert_has_calls(expected_calls) def test_reset_home(helm, users): @@ -52,6 +54,7 @@ def test_reset_home(helm, users): f"--set=Username={user.slug}", ), ] + print(helm.upgrade_release.call_args_list) helm.upgrade_release.assert_has_calls(expected_calls) @@ -68,6 +71,7 @@ def test_delete(aws_delete_role, helm, users): Delete with Helm 3. """ user = users["normal_user"] + print(user) helm.list_releases.return_value = [ "chart-release", ] @@ -77,7 +81,8 @@ def test_delete(aws_delete_role, helm, users): call(f"user-{user.slug}", "chart-release"), call("cpanel", "chart-release"), ] - helm.delete.has_calls(expected_calls) + print(helm.upgrade_release.call_args_list) + helm.delete.assert_has_calls(expected_calls) def test_delete_eks_with_no_releases(aws_delete_role, helm, users): diff --git a/tests/api/models/test_user.py b/tests/api/models/test_user.py index aa173cbc0..c1dd9497e 100644 --- a/tests/api/models/test_user.py +++ b/tests/api/models/test_user.py @@ -45,7 +45,7 @@ def test_helm_create_user(helm): f"--set=Username={user.slug}", ), ] - helm.upgrade_release.has_calls(expected_calls) + helm.upgrade_release.assert_has_calls(expected_calls) def test_helm_delete_user(helm, auth0): diff --git a/tests/frontend/views/test_app_variables.py b/tests/frontend/views/test_app_variables.py index b1f7ac339..d82085039 100644 --- a/tests/frontend/views/test_app_variables.py +++ b/tests/frontend/views/test_app_variables.py @@ -99,7 +99,7 @@ def test_permission( client.force_login(users[user]) response = view(client, app, data_input) assert response.status_code == expected_status - assert fixture_create_update_var.called_once() - assert fixture_delete_var.called_once() + assert fixture_create_update_var.assert_called_once() + assert fixture_delete_var.assert_called_once() if data_input: assert fixture_create_update_var.called_once_with(data_input) diff --git a/tests/frontend/views/test_auth.py b/tests/frontend/views/test_auth.py index ce7e60cac..4d598d5d8 100644 --- a/tests/frontend/views/test_auth.py +++ b/tests/frontend/views/test_auth.py @@ -2,7 +2,7 @@ import pytest from authlib.integrations.base_client import OAuthError from django.urls import reverse, reverse_lazy -from mock import patch +from unittest.mock import patch from pytest_django.asserts import assertContains diff --git a/tests/frontend/views/test_index.py b/tests/frontend/views/test_index.py index eae802835..382c4be0b 100644 --- a/tests/frontend/views/test_index.py +++ b/tests/frontend/views/test_index.py @@ -2,7 +2,7 @@ import pytest from django.http import HttpResponse from django.urls import reverse -from mock import MagicMock, patch +from unittest.mock import MagicMock, patch class TestAccess: diff --git a/tests/frontend/views/test_secrets.py b/tests/frontend/views/test_secrets.py index baa970018..0acb94beb 100644 --- a/tests/frontend/views/test_secrets.py +++ b/tests/frontend/views/test_secrets.py @@ -139,7 +139,7 @@ def test_delete_secret( client.force_login(users[user]) response = delete_secret_post(client, app, key, data) assert response.status_code == expected_status - assert fixture_delete_secret.called_once() + assert fixture_delete_secret.assert_called_once() def test_add_secret(fixture_create_update_secret, From a3a6f5a6bae1075e715386baf2795d968a0776ad Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Thu, 4 Apr 2024 14:41:39 +0100 Subject: [PATCH 26/31] fixed tests that failed as a result of bumping to 3.12 --- tests/api/cluster/test_user.py | 22 +++++++---------- tests/api/models/test_user.py | 28 ---------------------- tests/frontend/views/test_app_variables.py | 26 ++++++++++++-------- tests/frontend/views/test_secrets.py | 13 +++++----- 4 files changed, 32 insertions(+), 57 deletions(-) diff --git a/tests/api/cluster/test_user.py b/tests/api/cluster/test_user.py index f59321c33..6abed4c02 100644 --- a/tests/api/cluster/test_user.py +++ b/tests/api/cluster/test_user.py @@ -16,7 +16,6 @@ def test_iam_role_name(users): def test_create(helm, settings, users): with patch("controlpanel.api.cluster.AWSRole.create_role") as aws_create_role: user = users["normal_user"] - print(user) cluster.User(user).create() aws_create_role.assert_called_with( user.iam_role_name, @@ -27,18 +26,18 @@ def test_create(helm, settings, users): call( f"bootstrap-user-{user.slug}", "mojanalytics/bootstrap-user", - f"--namespace=user-{user.slug}", - f"--set=Username={user.slug},", - f"Efsvolume={settings.EFS_VOLUME}", + f"--namespace=cpanel", + f"--set=Username={user.slug}", ), call( - f"config-user-{user.slug}", - "mojanalytics/config-user", + f"provision-user-{user.slug}", + "mojanalytics/provision-user", f"--namespace=user-{user.slug}", - f"--set=Username={user.slug}", + (f"--set=Username={user.slug},Efsvolume={settings.EFS_VOLUME}," + "OidcDomain=oidc.idp.example.com,Email=,Fullname="), ), ] - print(helm.upgrade_release.call_args_list) + helm.upgrade_release.assert_has_calls(expected_calls) @@ -54,7 +53,7 @@ def test_reset_home(helm, users): f"--set=Username={user.slug}", ), ] - print(helm.upgrade_release.call_args_list) + helm.upgrade_release.assert_has_calls(expected_calls) @@ -71,17 +70,14 @@ def test_delete(aws_delete_role, helm, users): Delete with Helm 3. """ user = users["normal_user"] - print(user) helm.list_releases.return_value = [ "chart-release", ] cluster.User(user).delete() aws_delete_role.assert_called_with(user.iam_role_name) expected_calls = [ - call(f"user-{user.slug}", "chart-release"), - call("cpanel", "chart-release"), + call('user-bob', 'chart-release', dry_run=False), ] - print(helm.upgrade_release.call_args_list) helm.delete.assert_has_calls(expected_calls) diff --git a/tests/api/models/test_user.py b/tests/api/models/test_user.py index c1dd9497e..2a629fe5f 100644 --- a/tests/api/models/test_user.py +++ b/tests/api/models/test_user.py @@ -3,8 +3,6 @@ # Third-party import pytest -from django.conf import settings -from model_mommy import mommy # First-party/Local from controlpanel.api import cluster @@ -22,32 +20,6 @@ def auth0(): yield auth0 -def test_helm_create_user(helm): - user = mommy.prepare("api.User") - - expected_calls = [ - call( - f"bootstrap-user-{user.slug}", - "mojanalytics/bootstrap-user", - f"--set=Username={user.slug}", - ), - call( - f"provision-user-{user.slug}", - "mojanalytics/provision-user", - f"--namespace=user-{user.slug}", - f"--set=Username={user.slug},", - f"Efsvolume={settings.EFS_VOLUME}", - ), - call( - f"config-user-{user.slug}", - "mojanalytics/config-user", - f"--namespace=user-{user.slug}", - f"--set=Username={user.slug}", - ), - ] - helm.upgrade_release.assert_has_calls(expected_calls) - - def test_helm_delete_user(helm, auth0): user = User.objects.create(username="bob", auth0_id="github|user_2") authz = auth0.ExtendedAuth0.return_value diff --git a/tests/frontend/views/test_app_variables.py b/tests/frontend/views/test_app_variables.py index d82085039..1e5f5fd96 100644 --- a/tests/frontend/views/test_app_variables.py +++ b/tests/frontend/views/test_app_variables.py @@ -70,19 +70,22 @@ def fixture_delete_var(): @pytest.mark.parametrize( - "view,user,expected_status,data_input", + "view,user,expected_status,data_input,create_call_count,delete_call_count,expected_data", [ ( create, "superuser", status.HTTP_302_FOUND, dict(key="environment", value="prod", env_name="test_env"), + 1, + 0, + dict(env_name="test_env", key_name="XXX_environment", key_value="prod"), ), - (create, "owner", status.HTTP_200_OK, {}), - (create, "normal_user", status.HTTP_200_OK, {}), - (delete, "superuser", status.HTTP_302_FOUND, dict(key="hello")), - (delete, "owner", status.HTTP_302_FOUND, dict(key="hello")), - (delete, "normal_user", status.HTTP_403_FORBIDDEN, dict(key="hello")), + (create, "owner", status.HTTP_200_OK, {}, 0, 0, None), + (create, "normal_user", status.HTTP_200_OK, {}, 0, 0, None), + (delete, "superuser", status.HTTP_302_FOUND, dict(key="hello"), 0, 1, None), + (delete, "owner", status.HTTP_302_FOUND, dict(key="hello"), 0, 1, None), + (delete, "normal_user", status.HTTP_403_FORBIDDEN, dict(key="hello"), 0, 0, None), ], ) def test_permission( @@ -95,11 +98,14 @@ def test_permission( data_input, fixture_create_update_var, fixture_delete_var, + create_call_count, + delete_call_count, + expected_data ): client.force_login(users[user]) response = view(client, app, data_input) assert response.status_code == expected_status - assert fixture_create_update_var.assert_called_once() - assert fixture_delete_var.assert_called_once() - if data_input: - assert fixture_create_update_var.called_once_with(data_input) + assert fixture_create_update_var.call_count == create_call_count + assert fixture_delete_var.call_count == delete_call_count + if data_input and create_call_count == 1: + fixture_create_update_var.assert_called_once_with(**expected_data) diff --git a/tests/frontend/views/test_secrets.py b/tests/frontend/views/test_secrets.py index 0acb94beb..4d36f7233 100644 --- a/tests/frontend/views/test_secrets.py +++ b/tests/frontend/views/test_secrets.py @@ -128,18 +128,19 @@ def test_add_secret_permissions( @pytest.mark.parametrize( # noqa: F405 - "user, key, data, expected_status", - [["superuser", "testing", {"env_name": "testing"}, 302], - ["app_admin", "testing", {}, 302], - ["normal_user", "testing", {}, 403]], + "user, key, data, expected_status, expected_calls", + [["superuser", "testing", {"env_name": "testing"}, 302, 1], + ["app_admin", "testing", {}, 302, 1], + ["normal_user", "testing", {}, 403, 0]], ) def test_delete_secret( - client, app, users, fixture_delete_secret, user, key, data, expected_status + client, app, users, fixture_delete_secret, user, key, data, expected_status, + expected_calls ): client.force_login(users[user]) response = delete_secret_post(client, app, key, data) assert response.status_code == expected_status - assert fixture_delete_secret.assert_called_once() + assert fixture_delete_secret.call_count == expected_calls def test_add_secret(fixture_create_update_secret, From d404b0b1f65d1ec293104d0a5c8e66754163cfeb Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Fri, 5 Apr 2024 10:31:52 +0100 Subject: [PATCH 27/31] updated to model-bakery --- requirements.dev.txt | 1 + requirements.txt | 1 - tests/api/cluster/test_access_to_s3buckets.py | 8 +++--- tests/api/cluster/test_bucket.py | 4 +-- tests/api/cluster/test_group.py | 4 +-- tests/api/filters/test_app_filter.py | 14 +++++----- tests/api/filters/test_apps3bucket_filter.py | 4 +-- tests/api/filters/test_s3bucket_filter.py | 8 +++--- tests/api/filters/test_users3bucket_filter.py | 24 ++++++++-------- tests/api/models/test_app.py | 20 ++++++------- tests/api/models/test_apps3bucket.py | 8 +++--- tests/api/models/test_ip_allowlist.py | 16 +++++------ tests/api/models/test_s3bucket.py | 6 ++-- tests/api/models/test_tool.py | 4 +-- tests/api/models/test_users3bucket.py | 6 ++-- tests/api/permissions/test_app_permissions.py | 13 ++++----- .../test_apps3bucket_permissions.py | 22 +++++++-------- .../permissions/test_s3bucket_permissions.py | 4 +-- .../test_users3bucket_permissions.py | 10 +++---- .../tasks/test_create_app_auth_settings.py | 6 ++-- tests/api/tasks/test_create_app_aws_role.py | 4 +-- tests/api/tasks/test_s3.py | 20 ++++++------- tests/api/test_pagination.py | 4 +-- tests/api/views/test_app.py | 10 +++---- tests/api/views/test_apps3bucket.py | 10 +++---- tests/api/views/test_customer.py | 4 +-- tests/api/views/test_s3bucket.py | 12 ++++---- tests/api/views/test_user.py | 6 ++-- tests/api/views/test_userapp.py | 6 ++-- tests/conftest.py | 8 +++--- tests/frontend/views/test_app.py | 24 ++++++++-------- tests/frontend/views/test_app_variables.py | 10 +++---- tests/frontend/views/test_datasource.py | 28 +++++++++---------- tests/frontend/views/test_ip_allowlist.py | 4 +-- tests/frontend/views/test_secrets.py | 16 +++++------ tests/frontend/views/test_user.py | 1 - tests/kubeapi/test_permissions.py | 6 ++-- 37 files changed, 177 insertions(+), 179 deletions(-) diff --git a/requirements.dev.txt b/requirements.dev.txt index f4873abbc..ad8f940c2 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -6,6 +6,7 @@ flake8==7.0.0 ipdb==0.13.13 ipython==8.21.0 isort==5.13.2 +model-bakery==1.17.0 pandas==2.2.0 pre-commit==3.6.1 pylint==3.0.3 diff --git a/requirements.txt b/requirements.txt index c7cf995b1..491f07686 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,6 @@ gunicorn==21.2.0 Jinja2==3.1.3 kubernetes==25.3.0 MarkupSafe==2.1.5 -model-mommy==2.0.0 moto==5.0.1 mozilla-django-oidc==4.0.0 psycopg2-binary==2.9.9 diff --git a/tests/api/cluster/test_access_to_s3buckets.py b/tests/api/cluster/test_access_to_s3buckets.py index 07f285f70..0655d1382 100644 --- a/tests/api/cluster/test_access_to_s3buckets.py +++ b/tests/api/cluster/test_access_to_s3buckets.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.cluster import App, RoleGroup, User @@ -47,14 +47,14 @@ def enable_db_for_all_tests(db): @pytest.fixture def bucket(): - return mommy.prepare("api.S3Bucket") + return baker.prepare("api.S3Bucket") @pytest.fixture def entities(bucket, users): return { - "app": App(mommy.prepare("api.App")), - "group": RoleGroup(mommy.prepare("api.IAMManagedPolicy")), + "app": App(baker.prepare("api.App")), + "group": RoleGroup(baker.prepare("api.IAMManagedPolicy")), "user": User(users["normal_user"]), } diff --git a/tests/api/cluster/test_bucket.py b/tests/api/cluster/test_bucket.py index 07f118409..423dcedfb 100644 --- a/tests/api/cluster/test_bucket.py +++ b/tests/api/cluster/test_bucket.py @@ -4,7 +4,7 @@ # Third-party import pytest from django.conf import settings -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api import cluster @@ -18,7 +18,7 @@ def enable_db_for_all_tests(db): @pytest.fixture def bucket(): - return mommy.prepare("api.S3Bucket", name="test-bucket") + return baker.prepare("api.S3Bucket", name="test-bucket") def test_arn(bucket): diff --git a/tests/api/cluster/test_group.py b/tests/api/cluster/test_group.py index 6160679fd..a86006e46 100644 --- a/tests/api/cluster/test_group.py +++ b/tests/api/cluster/test_group.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api import cluster @@ -16,7 +16,7 @@ def enable_db_for_all_tests(db): @pytest.fixture def iam_managed_policy(): - return mommy.make("api.IAMManagedPolicy", name="test") + return baker.make("api.IAMManagedPolicy", name="test") def test_arn(settings, iam_managed_policy): diff --git a/tests/api/filters/test_app_filter.py b/tests/api/filters/test_app_filter.py index 114cbdc91..fb7a3dd17 100644 --- a/tests/api/filters/test_app_filter.py +++ b/tests/api/filters/test_app_filter.py @@ -1,17 +1,17 @@ # Third-party -from model_mommy import mommy +from model_bakery import baker from rest_framework.reverse import reverse from rest_framework.test import APITestCase class AppFilterTest(APITestCase): def setUp(self): - self.superuser = mommy.make("api.User", is_superuser=True) - self.app_admin = mommy.make("api.User", is_superuser=False) - self.app_1 = mommy.make("api.App", name="App 1") - self.app_2 = mommy.make("api.App", name="App 2") - mommy.make("api.UserApp", user=self.app_admin, app=self.app_1, is_admin=True) - mommy.make("api.UserApp", user=self.app_admin, app=self.app_2, is_admin=True) + self.superuser = baker.make("api.User", is_superuser=True) + self.app_admin = baker.make("api.User", is_superuser=False) + self.app_1 = baker.make("api.App", name="App 1") + self.app_2 = baker.make("api.App", name="App 2") + baker.make("api.UserApp", user=self.app_admin, app=self.app_1, is_admin=True) + baker.make("api.UserApp", user=self.app_admin, app=self.app_2, is_admin=True) def test_everyone_see_everything(self): for user in [self.superuser, self.app_admin]: diff --git a/tests/api/filters/test_apps3bucket_filter.py b/tests/api/filters/test_apps3bucket_filter.py index b779d396a..2daa715c0 100644 --- a/tests/api/filters/test_apps3bucket_filter.py +++ b/tests/api/filters/test_apps3bucket_filter.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework.reverse import reverse NUM_APPS3BUCKETS = 2 @@ -13,7 +13,7 @@ def apps3buckets(s3, sqs): with patch("controlpanel.api.aws.AWSBucket.create"),\ patch("controlpanel.api.aws.AWSRole.grant_bucket_access"): - mommy.make("api.AppS3Bucket", NUM_APPS3BUCKETS) + baker.make("api.AppS3Bucket", NUM_APPS3BUCKETS) def list(client, *args): diff --git a/tests/api/filters/test_s3bucket_filter.py b/tests/api/filters/test_s3bucket_filter.py index 54095db48..34e31f976 100644 --- a/tests/api/filters/test_s3bucket_filter.py +++ b/tests/api/filters/test_s3bucket_filter.py @@ -1,6 +1,6 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -13,14 +13,14 @@ def enable_db_for_all_tests(db): @pytest.fixture(autouse=True) def buckets(db): return { - 1: mommy.make("api.S3Bucket", name="test-bucket-1"), - 2: mommy.make("api.S3Bucket", name="test-bucket-2"), + 1: baker.make("api.S3Bucket", name="test-bucket-1"), + 2: baker.make("api.S3Bucket", name="test-bucket-2"), } @pytest.fixture(autouse=True) def users3bucket(db, buckets, users): - return mommy.make( + return baker.make( "api.UserS3Bucket", user=users["normal_user"], s3bucket=buckets[1], diff --git a/tests/api/filters/test_users3bucket_filter.py b/tests/api/filters/test_users3bucket_filter.py index 2071d92cd..efafb2089 100644 --- a/tests/api/filters/test_users3bucket_filter.py +++ b/tests/api/filters/test_users3bucket_filter.py @@ -1,5 +1,5 @@ # Third-party -from model_mommy import mommy +from model_bakery import baker from rest_framework.reverse import reverse from rest_framework.status import HTTP_200_OK from rest_framework.test import APITestCase @@ -10,39 +10,39 @@ class UserS3BucketFilterTest(APITestCase): def setUp(self): - self.superuser = mommy.make("api.User", is_superuser=True) - self.normal_user = mommy.make("api.User", is_superuser=False) - self.other_user = mommy.make("api.User", is_superuser=False) + self.superuser = baker.make("api.User", is_superuser=True) + self.normal_user = baker.make("api.User", is_superuser=False) + self.other_user = baker.make("api.User", is_superuser=False) - self.s3bucket_1 = mommy.make("api.S3Bucket", name="test-bucket-1") - self.s3bucket_2 = mommy.make("api.S3Bucket", name="test-bucket-2") - self.s3bucket_3 = mommy.make("api.S3Bucket", name="test-bucket-3") + self.s3bucket_1 = baker.make("api.S3Bucket", name="test-bucket-1") + self.s3bucket_2 = baker.make("api.S3Bucket", name="test-bucket-2") + self.s3bucket_3 = baker.make("api.S3Bucket", name="test-bucket-3") - self.s3bucket_1_normal_user_access = mommy.make( + self.s3bucket_1_normal_user_access = baker.make( "api.UserS3Bucket", user=self.normal_user, s3bucket=self.s3bucket_1, access_level=UserS3Bucket.READONLY, ) - self.s3bucket_1_other_user_access = mommy.make( + self.s3bucket_1_other_user_access = baker.make( "api.UserS3Bucket", user=self.other_user, s3bucket=self.s3bucket_1, access_level=UserS3Bucket.READONLY, ) - self.s3bucket_2_normal_user_access = mommy.make( + self.s3bucket_2_normal_user_access = baker.make( "api.UserS3Bucket", user=self.normal_user, s3bucket=self.s3bucket_2, access_level=UserS3Bucket.READONLY, ) - self.s3bucket_2_superuser_access = mommy.make( + self.s3bucket_2_superuser_access = baker.make( "api.UserS3Bucket", user=self.superuser, s3bucket=self.s3bucket_2, access_level=UserS3Bucket.READWRITE, ) - self.s3bucket_3_superuser_access = mommy.make( + self.s3bucket_3_superuser_access = baker.make( "api.UserS3Bucket", user=self.superuser, s3bucket=self.s3bucket_3, diff --git a/tests/api/models/test_app.py b/tests/api/models/test_app.py index 58d5e8d8c..11f6c1cca 100644 --- a/tests/api/models/test_app.py +++ b/tests/api/models/test_app.py @@ -4,7 +4,7 @@ # Third-party import pytest from django.conf import settings -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.auth0 import Auth0Error @@ -28,7 +28,7 @@ def update_aws_secrets_manager(): @pytest.fixture def app(): - app = mommy.make("api.App") + app = baker.make("api.App") app.repo_url = "https://github.com/example.com/repo_name" auth_settings = dict( client_id="testing_client_id", @@ -129,7 +129,7 @@ def test_delete_customers(auth0, app): ], ) def test_repo_name(url, expected_name): - app = mommy.prepare("api.App") + app = baker.prepare("api.App") app.repo_url = url assert app._repo_name == expected_name @@ -144,7 +144,7 @@ def test_repo_name(url, expected_name): def test_delete_customer_by_email_error_getting_user(auth0, side_effect, error_message): authz = auth0.ExtendedAuth0.return_value authz.users.get_users_email_search.side_effect = side_effect - app = mommy.prepare("api.App") + app = baker.prepare("api.App") with pytest.raises(app.DeleteCustomerError, match=error_message): app.delete_customer_by_email("foo@email.com", group_id="123") @@ -157,7 +157,7 @@ def test_delete_customer_by_email_user_missing_group(auth0): authz.users.get_users_email_search.return_value = [user] authz.users.get_user_groups.return_value = [user_groups] - app = mommy.prepare("api.App") + app = baker.prepare("api.App") with pytest.raises( app.DeleteCustomerError, match="User foo@email.com cannot be found in this application group" @@ -173,7 +173,7 @@ def test_delete_customer_by_email_success(auth0): authz.users.get_users_email_search.return_value = [user] authz.users.get_user_groups.return_value = [user_groups] - app = mommy.prepare("api.App") + app = baker.prepare("api.App") with patch.object(app, "delete_customers") as delete_customers: app.delete_customer_by_email("foo@email.com", group_id="123") @@ -186,12 +186,12 @@ def test_delete_customer_by_email_success(auth0): @pytest.mark.django_db def test_app_allowed_ip_ranges(): ip_allow_lists = [ - mommy.make("api.IPAllowlist", allowed_ip_ranges="127.0.0.1, 128.10.10.100"), - mommy.make("api.IPAllowlist", allowed_ip_ranges=" 123.0.0.122,152.0.0.1"), + baker.make("api.IPAllowlist", allowed_ip_ranges="127.0.0.1, 128.10.10.100"), + baker.make("api.IPAllowlist", allowed_ip_ranges=" 123.0.0.122,152.0.0.1"), ] - app = mommy.make("api.App") # noqa:F841 + app = baker.make("api.App") # noqa:F841 for item in ip_allow_lists: - mommy.make( + baker.make( "api.AppIPAllowList", app_id=app.id, ip_allowlist_id=item.id, diff --git a/tests/api/models/test_apps3bucket.py b/tests/api/models/test_apps3bucket.py index 2d27bf3e4..f9dca4dab 100644 --- a/tests/api/models/test_apps3bucket.py +++ b/tests/api/models/test_apps3bucket.py @@ -5,7 +5,7 @@ import pytest from django.conf import settings from django.db.utils import IntegrityError -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import AppS3Bucket @@ -13,12 +13,12 @@ @pytest.fixture def app(): - return mommy.make("api.App", name="app_1") + return baker.make("api.App", name="app_1") @pytest.fixture def bucket(): - return mommy.make("api.S3Bucket", name="test-bucket-1") + return baker.make("api.S3Bucket", name="test-bucket-1") @pytest.mark.django_db @@ -57,7 +57,7 @@ def test_delete_revoke_permissions(app, bucket): with patch( "controlpanel.api.tasks.S3BucketRevokeAppAccess" ) as revoke_bucket_access_task: - apps3bucket = mommy.make( + apps3bucket = baker.make( "api.AppS3Bucket", app=app, s3bucket=bucket, diff --git a/tests/api/models/test_ip_allowlist.py b/tests/api/models/test_ip_allowlist.py index 78555b76d..182bd6c72 100644 --- a/tests/api/models/test_ip_allowlist.py +++ b/tests/api/models/test_ip_allowlist.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker @pytest.fixture(autouse=True) @@ -20,8 +20,8 @@ def update_aws_secrets_manager(): # def test_ip_allowlist_save_updates_aws_secrets_manager(update_aws_secrets_manager): -# ip_allowlist = mommy.make("api.IPAllowlist", allowed_ip_ranges="123") -# app = mommy.make("api.App", ip_allowlists=[ip_allowlist]) # noqa:F841 +# ip_allowlist = baker.make("api.IPAllowlist", allowed_ip_ranges="123") +# app = baker.make("api.App", ip_allowlists=[ip_allowlist]) # noqa:F841 # # ip_allowlist.save() # @@ -35,7 +35,7 @@ def update_aws_secrets_manager(): def test_ip_allowlist_without_app_save_does_not_update_aws_secrets_manager( update_aws_secrets_manager, ): - ip_allowlist = mommy.make("api.IPAllowlist") + ip_allowlist = baker.make("api.IPAllowlist") ip_allowlist.save() @@ -44,10 +44,10 @@ def test_ip_allowlist_without_app_save_does_not_update_aws_secrets_manager( # def test_ip_allowlist_delete_updates_aws_secrets_manager(update_aws_secrets_manager): # ip_allowlists = [ -# mommy.make("api.IPAllowlist", allowed_ip_ranges="xyz"), -# mommy.make("api.IPAllowlist", allowed_ip_ranges="123"), +# baker.make("api.IPAllowlist", allowed_ip_ranges="xyz"), +# baker.make("api.IPAllowlist", allowed_ip_ranges="123"), # ] -# apps = mommy.make("api.App", ip_allowlists=ip_allowlists, _quantity=2) # noqa:F841 +# apps = baker.make("api.App", ip_allowlists=ip_allowlists, _quantity=2) # noqa:F841 # # ip_allowlists[0].delete() # @@ -59,7 +59,7 @@ def test_ip_allowlist_without_app_save_does_not_update_aws_secrets_manager( def test_ip_allowlist_without_app_delete_does_not_update_aws_secrets_manager( update_aws_secrets_manager, ): - ip_allowlist = mommy.make("api.IPAllowlist") + ip_allowlist = baker.make("api.IPAllowlist") ip_allowlist.delete() diff --git a/tests/api/models/test_s3bucket.py b/tests/api/models/test_s3bucket.py index 63c999cbb..c97d32427 100644 --- a/tests/api/models/test_s3bucket.py +++ b/tests/api/models/test_s3bucket.py @@ -5,7 +5,7 @@ import pytest from botocore.exceptions import ClientError from django.conf import settings -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api import cluster @@ -26,8 +26,8 @@ def test_delete_revokes_permissions(bucket): with patch("controlpanel.api.models.AppS3Bucket.revoke_bucket_access") as app_revoke_bucket_access, \ patch("controlpanel.api.models.UserS3Bucket.revoke_bucket_access") as user_revoke_user_bucket_access: # link the bucket with an UserS3Bucket and AppS3Bucket - mommy.make("api.UserS3Bucket", s3bucket=bucket) - mommy.make("api.AppS3Bucket", s3bucket=bucket) + baker.make("api.UserS3Bucket", s3bucket=bucket) + baker.make("api.AppS3Bucket", s3bucket=bucket) # delete the source S3Bucket bucket.delete() # check that related objects revoke access methods were called diff --git a/tests/api/models/test_tool.py b/tests/api/models/test_tool.py index c31631602..bff8b481a 100644 --- a/tests/api/models/test_tool.py +++ b/tests/api/models/test_tool.py @@ -4,7 +4,7 @@ # Third-party import pytest from django.conf import settings -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import HomeDirectory, Tool, ToolDeployment, User @@ -12,7 +12,7 @@ @pytest.fixture def tool(db): - return mommy.make("api.Tool") + return baker.make("api.Tool") def test_deploy_for_generic(helm, tool, users): diff --git a/tests/api/models/test_users3bucket.py b/tests/api/models/test_users3bucket.py index c67b80ea0..afec60512 100644 --- a/tests/api/models/test_users3bucket.py +++ b/tests/api/models/test_users3bucket.py @@ -5,7 +5,7 @@ import pytest from django.conf import settings from django.db.utils import IntegrityError -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import UserS3Bucket @@ -14,12 +14,12 @@ @pytest.fixture def user(): - return mommy.make("api.User", auth0_id="github|user_1", username="user_1") + return baker.make("api.User", auth0_id="github|user_1", username="user_1") @pytest.fixture def bucket(): - return mommy.make("api.S3Bucket", name="test-bucket-1") + return baker.make("api.S3Bucket", name="test-bucket-1") @pytest.fixture diff --git a/tests/api/permissions/test_app_permissions.py b/tests/api/permissions/test_app_permissions.py index b7e53725b..0a169a1de 100644 --- a/tests/api/permissions/test_app_permissions.py +++ b/tests/api/permissions/test_app_permissions.py @@ -6,9 +6,8 @@ # Third-party import pytest -from django.contrib import auth from django.contrib.auth import get_user -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse from rules import perm_exists @@ -17,12 +16,12 @@ @pytest.fixture def users(users): users.update({ - "app_admin": mommy.make( + "app_admin": baker.make( "api.User", username="dave", auth0_id="github|user_4", ), - "app_user": mommy.make( + "app_user": baker.make( "api.User", username="testing", auth0_id="github|user_5", @@ -33,12 +32,12 @@ def users(users): @pytest.fixture(autouse=True) def app(users): - app = mommy.make("api.App", name="Test App 1") + app = baker.make("api.App", name="Test App 1") user = users["app_admin"] - mommy.make("api.UserApp", user=user, app=app, is_admin=True) + baker.make("api.UserApp", user=user, app=app, is_admin=True) user = users["app_user"] - mommy.make("api.UserApp", user=user, app=app, is_admin=False) + baker.make("api.UserApp", user=user, app=app, is_admin=False) return app diff --git a/tests/api/permissions/test_apps3bucket_permissions.py b/tests/api/permissions/test_apps3bucket_permissions.py index c4fb865d9..0c72d0e2f 100644 --- a/tests/api/permissions/test_apps3bucket_permissions.py +++ b/tests/api/permissions/test_apps3bucket_permissions.py @@ -4,7 +4,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -16,17 +16,17 @@ def users(users): users.update( { - "app_admin": mommy.make( + "app_admin": baker.make( "api.User", username="dave", auth0_id="github|user_4", ), - "bucket_admin": mommy.make( + "bucket_admin": baker.make( "api.User", username="ethel", auth0_id="github|user_5", ), - "app_bucket_admin": mommy.make( + "app_bucket_admin": baker.make( "api.User", username="fred", auth0_id="github|user_6", @@ -38,14 +38,14 @@ def users(users): @pytest.fixture def app(users): - app = mommy.make("api.App", name="App 1") - mommy.make( + app = baker.make("api.App", name="App 1") + baker.make( "api.UserApp", app=app, user=users["app_admin"], is_admin=True, ) - mommy.make( + baker.make( "api.UserApp", app=app, user=users["app_bucket_admin"], @@ -58,16 +58,16 @@ def app(users): def buckets(users): with patch("controlpanel.api.aws.AWSBucket.create"): buckets = { - "first": mommy.make("api.S3Bucket", is_data_warehouse=False), - "other": mommy.make("api.S3Bucket", is_data_warehouse=False), + "first": baker.make("api.S3Bucket", is_data_warehouse=False), + "other": baker.make("api.S3Bucket", is_data_warehouse=False), } - mommy.make( + baker.make( "api.UserS3Bucket", user=users["bucket_admin"], s3bucket=buckets["first"], is_admin=True, ) - mommy.make( + baker.make( "api.UserS3Bucket", user=users["app_bucket_admin"], s3bucket=buckets["first"], diff --git a/tests/api/permissions/test_s3bucket_permissions.py b/tests/api/permissions/test_s3bucket_permissions.py index ab3571b22..ce6a8fa06 100644 --- a/tests/api/permissions/test_s3bucket_permissions.py +++ b/tests/api/permissions/test_s3bucket_permissions.py @@ -5,7 +5,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -19,7 +19,7 @@ def users(users): for i, role in enumerate(("viewer", "editor", "admin")): users.update( { - f"bucket_{role}": mommy.make( + f"bucket_{role}": baker.make( "api.User", username=role, auth0_id=f"github|user_{5 + i}", diff --git a/tests/api/permissions/test_users3bucket_permissions.py b/tests/api/permissions/test_users3bucket_permissions.py index 617e3ded8..1a06f9c38 100644 --- a/tests/api/permissions/test_users3bucket_permissions.py +++ b/tests/api/permissions/test_users3bucket_permissions.py @@ -4,7 +4,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -16,7 +16,7 @@ def users(users): users.update( { - "bucket_admin": mommy.make( + "bucket_admin": baker.make( "api.User", username="bucket_admin", auth0_id="github|user_4", @@ -29,19 +29,19 @@ def users(users): @pytest.fixture def bucket(): with patch("controlpanel.api.aws.AWSBucket.create"): - return mommy.make("api.S3Bucket") + return baker.make("api.S3Bucket") @pytest.fixture def users3bucket(bucket, users): - mommy.make( + baker.make( "api.UserS3Bucket", user=users["bucket_admin"], s3bucket=bucket, access_level=UserS3Bucket.READONLY, is_admin=True, ) - return mommy.make( + return baker.make( "api.UserS3Bucket", user=users["normal_user"], s3bucket=bucket, diff --git a/tests/api/tasks/test_create_app_auth_settings.py b/tests/api/tasks/test_create_app_auth_settings.py index f2f5acfce..609dbf66a 100644 --- a/tests/api/tasks/test_create_app_auth_settings.py +++ b/tests/api/tasks/test_create_app_auth_settings.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import App @@ -26,7 +26,7 @@ def test_cluster_not_called_without_valid_app(cluster, complete, users): @patch("controlpanel.api.tasks.handlers.app.cluster") @patch("controlpanel.api.models.user.User.github_api_token", new=PropertyMock(return_value=None)) # noqa def test_cluster_not_called_without_github_api_token(cluster, complete, users): - app = mommy.make("api.App") + app = baker.make("api.App") user = users["superuser"] create_app_auth_settings( @@ -44,7 +44,7 @@ def test_cluster_not_called_without_github_api_token(cluster, complete, users): @patch("controlpanel.api.tasks.handlers.app.cluster") @patch("controlpanel.api.models.user.User.github_api_token", new=PropertyMock(return_value="dummy-token")) # noqa def test_valid_app_and_user(cluster, complete, users): - app = mommy.make("api.App") + app = baker.make("api.App") create_app_auth_settings( app.pk, diff --git a/tests/api/tasks/test_create_app_aws_role.py b/tests/api/tasks/test_create_app_aws_role.py index b377323fa..985bed078 100644 --- a/tests/api/tasks/test_create_app_aws_role.py +++ b/tests/api/tasks/test_create_app_aws_role.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import App @@ -29,7 +29,7 @@ def test_cluster_not_called_without_valid_app(cluster, complete, users): @patch("controlpanel.api.tasks.handlers.base.BaseModelTaskHandler.complete") @patch("controlpanel.api.tasks.handlers.app.cluster") def test_valid_app_and_user(cluster, complete, users): - app = mommy.make("api.App") + app = baker.make("api.App") create_app_aws_role(app.pk, users["superuser"].pk) diff --git a/tests/api/tasks/test_s3.py b/tests/api/tasks/test_s3.py index b8134dd01..7a001e00e 100644 --- a/tests/api/tasks/test_s3.py +++ b/tests/api/tasks/test_s3.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api.models import AppS3Bucket, S3Bucket, UserS3Bucket @@ -37,7 +37,7 @@ def test_exception_raised_when_called_without_valid_app( @patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.complete") @patch("controlpanel.api.models.s3bucket.cluster") def test_bucket_created(cluster, complete, users): - s3bucket = mommy.make("api.S3Bucket", bucket_owner="APP") + s3bucket = baker.make("api.S3Bucket", bucket_owner="APP") create_s3bucket( s3bucket.pk, users["superuser"].pk, bucket_owner=s3bucket.bucket_owner @@ -60,7 +60,7 @@ def test_bucket_created(cluster, complete, users): def test_access_granted( cluster, complete, users, task_method, model_class, cluster_class ): - obj = mommy.make(model_class) + obj = baker.make(model_class) task_method(obj.pk, users["superuser"].pk) @@ -78,7 +78,7 @@ def test_access_granted( @patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.complete") @patch("controlpanel.api.tasks.handlers.s3.cluster") def test_revoke_user_access(cluster, complete, bucket_name, is_folder): - user_bucket_access = mommy.make("api.UserS3Bucket", s3bucket__name=bucket_name) + user_bucket_access = baker.make("api.UserS3Bucket", s3bucket__name=bucket_name) s3bucket = user_bucket_access.s3bucket bucket_identifier = s3bucket.name if is_folder else s3bucket.arn revoke_user_s3bucket_access( @@ -104,7 +104,7 @@ def test_revoke_user_access(cluster, complete, bucket_name, is_folder): @patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.complete") @patch("controlpanel.api.tasks.handlers.s3.cluster") def test_revoke_app_access(cluster, complete): - app_bucket_access = mommy.make("api.AppS3Bucket") + app_bucket_access = baker.make("api.AppS3Bucket") revoke_app_s3bucket_access( bucket_arn=app_bucket_access.s3bucket.arn, app_pk=app_bucket_access.app.pk, @@ -123,11 +123,11 @@ def test_revoke_app_access(cluster, complete): @patch("controlpanel.api.models.PolicyS3Bucket.revoke_bucket_access", new=MagicMock()) @patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.complete") def test_revoke_all_access(complete, users): - bucket = mommy.make("api.S3Bucket") - user_access = mommy.make("api.UserS3Bucket", s3bucket=bucket) - app_access = mommy.make("api.AppS3Bucket", s3bucket=bucket) - policy_access = mommy.make("api.PolicyS3Bucket", s3bucket=bucket) - task = mommy.make("api.Task", user_id=users["superuser"].pk) + bucket = baker.make("api.S3Bucket") + user_access = baker.make("api.UserS3Bucket", s3bucket=bucket) + app_access = baker.make("api.AppS3Bucket", s3bucket=bucket) + policy_access = baker.make("api.PolicyS3Bucket", s3bucket=bucket) + task = baker.make("api.Task", user_id=users["superuser"].pk) revoke_all_access_s3bucket(bucket.pk, task.user_id) diff --git a/tests/api/test_pagination.py b/tests/api/test_pagination.py index 4bfd4721f..67d24813c 100644 --- a/tests/api/test_pagination.py +++ b/tests/api/test_pagination.py @@ -1,6 +1,6 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework.reverse import reverse DEFAULT_PAGE_SIZE = 100 @@ -23,7 +23,7 @@ def login_superuser(client, superuser): ) @pytest.mark.django_db def test_pagination(client, urlparams, page_size, next, prev, sqs): - mommy.make("api.App", DEFAULT_PAGE_SIZE + 20) + baker.make("api.App", DEFAULT_PAGE_SIZE + 20) app_list_url = reverse("app-list") response = client.get(app_list_url, urlparams) diff --git a/tests/api/views/test_app.py b/tests/api/views/test_app.py index 7aaa6da7b..008c7031f 100644 --- a/tests/api/views/test_app.py +++ b/tests/api/views/test_app.py @@ -3,7 +3,7 @@ # Third-party from botocore.exceptions import ClientError -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.exceptions import ValidationError from rest_framework.reverse import reverse @@ -16,7 +16,7 @@ @pytest.fixture # noqa: F405 def app(): - return mommy.make( + return baker.make( "api.App", repo_url="https://github.com/ministryofjustice/example.git", ) @@ -26,9 +26,9 @@ def app(): def models(app, users): with patch("controlpanel.api.aws.AWSRole.grant_bucket_access"), \ patch("controlpanel.api.aws.AWSBucket.create"): - mommy.make("api.App") - mommy.make("api.AppS3Bucket", app=app) - mommy.make("api.UserApp", app=app, user=users["superuser"]) + baker.make("api.App") + baker.make("api.AppS3Bucket", app=app) + baker.make("api.UserApp", app=app, user=users["superuser"]) def test_list(client): diff --git a/tests/api/views/test_apps3bucket.py b/tests/api/views/test_apps3bucket.py index fd52cd79a..90c89f5f9 100644 --- a/tests/api/views/test_apps3bucket.py +++ b/tests/api/views/test_apps3bucket.py @@ -5,7 +5,7 @@ # Third-party import pytest from django.conf import settings -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -16,8 +16,8 @@ @pytest.fixture def apps(): return { - 1: mommy.make("api.App", name="app_1"), - 2: mommy.make("api.App", name="app_2"), + 1: baker.make("api.App", name="app_1"), + 2: baker.make("api.App", name="app_2"), } @@ -133,7 +133,7 @@ def test_update_bad_requests(client, apps, apps3buckets, buckets): def test_create_with_s3_data_warehouse_not_allowed(client, apps): with patch("controlpanel.api.aws.AWSRole.grant_bucket_access"), \ patch("controlpanel.api.aws.AWSBucket.create"): - s3_bucket_app = mommy.make( + s3_bucket_app = baker.make( "api.S3Bucket", is_data_warehouse=False, ) @@ -146,7 +146,7 @@ def test_create_with_s3_data_warehouse_not_allowed(client, apps): response = client.post(reverse("apps3bucket-list"), data) assert response.status_code == status.HTTP_201_CREATED - s3_bucket = mommy.make("api.S3Bucket", is_data_warehouse=True) + s3_bucket = baker.make("api.S3Bucket", is_data_warehouse=True) data = { "app": apps[1].id, diff --git a/tests/api/views/test_customer.py b/tests/api/views/test_customer.py index 044c025c1..2cbdf461a 100644 --- a/tests/api/views/test_customer.py +++ b/tests/api/views/test_customer.py @@ -6,7 +6,7 @@ import pytest from auth0.rest import Auth0Error from bs4 import BeautifulSoup -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -16,7 +16,7 @@ @pytest.fixture def app(): - app = mommy.make("api.App") + app = baker.make("api.App") dev_auth_settings = dict( client_id="dev_client_id", group_id=str(uuid.uuid4()) diff --git a/tests/api/views/test_s3bucket.py b/tests/api/views/test_s3bucket.py index 00e3baa85..da9c27347 100644 --- a/tests/api/views/test_s3bucket.py +++ b/tests/api/views/test_s3bucket.py @@ -6,7 +6,7 @@ import pytest from botocore.exceptions import ClientError from django.conf import settings -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -18,17 +18,17 @@ @pytest.fixture def bucket(): with patch("controlpanel.api.aws.AWSBucket.create"): - return mommy.make("api.S3Bucket", name="test-bucket-1") + return baker.make("api.S3Bucket", name="test-bucket-1") @pytest.fixture(autouse=True) def models(bucket): with patch("controlpanel.api.aws.AWSRole.grant_bucket_access"), \ patch("controlpanel.api.aws.AWSBucket.create"): - mommy.make("api.S3Bucket") - mommy.make("api.S3Bucket", is_data_warehouse=True) - mommy.make("api.AppS3Bucket", s3bucket=bucket) - mommy.make("api.UserS3Bucket", s3bucket=bucket) + baker.make("api.S3Bucket") + baker.make("api.S3Bucket", is_data_warehouse=True) + baker.make("api.AppS3Bucket", s3bucket=bucket) + baker.make("api.UserS3Bucket", s3bucket=bucket) def test_list(client): diff --git a/tests/api/views/test_user.py b/tests/api/views/test_user.py index 20f3d6760..7f66f2864 100644 --- a/tests/api/views/test_user.py +++ b/tests/api/views/test_user.py @@ -6,7 +6,7 @@ # Third-party import pytest from botocore.exceptions import ClientError -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -17,8 +17,8 @@ @pytest.fixture(autouse=True) def models(users): with patch("controlpanel.api.aws.AWSBucket.create"): - mommy.make("api.UserS3Bucket", user=users["normal_user"]) - mommy.make("api.UserApp", user=users["normal_user"]) + baker.make("api.UserS3Bucket", user=users["normal_user"]) + baker.make("api.UserApp", user=users["normal_user"]) @pytest.fixture diff --git a/tests/api/views/test_userapp.py b/tests/api/views/test_userapp.py index 41ac9feda..5b9f5c97c 100644 --- a/tests/api/views/test_userapp.py +++ b/tests/api/views/test_userapp.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @@ -14,8 +14,8 @@ @pytest.fixture def apps(): return { - 1: mommy.make("api.App", name="app_1"), - 2: mommy.make("api.App", name="app_2"), + 1: baker.make("api.App", name="app_1"), + 2: baker.make("api.App", name="app_2"), } diff --git a/tests/conftest.py b/tests/conftest.py index bc3e1e5b7..15a3b88b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ # Third-party import pytest import yaml -from model_mommy import mommy +from model_bakery import baker # First-party/Local from controlpanel.api import auth0 @@ -92,7 +92,7 @@ def slack_WebClient(): def superuser( db, slack_WebClient, iam, managed_policy, airflow_dev_policy, airflow_prod_policy ): - return mommy.make( + return baker.make( "api.User", auth0_id="github|user_1", is_superuser=True, @@ -104,13 +104,13 @@ def superuser( def users(db, superuser, iam, managed_policy, airflow_dev_policy, airflow_prod_policy): return { "superuser": superuser, - "normal_user": mommy.make( + "normal_user": baker.make( "api.User", auth0_id="github|user_2", username="bob", is_superuser=False, ), - "other_user": mommy.make( + "other_user": baker.make( "api.User", username="carol", auth0_id="github|user_3", diff --git a/tests/frontend/views/test_app.py b/tests/frontend/views/test_app.py index d25ed94ff..2e7b698c0 100644 --- a/tests/frontend/views/test_app.py +++ b/tests/frontend/views/test_app.py @@ -8,7 +8,7 @@ from bs4 import BeautifulSoup from django.contrib.messages import get_messages from django.urls import reverse -from model_mommy import mommy +from model_bakery import baker from rest_framework import status # First-party/Local @@ -43,7 +43,7 @@ def github_api_token(): def users(users): users.update( { - "app_admin": mommy.make("api.User", username="app_admin"), + "app_admin": baker.make("api.User", username="app_admin"), } ) return users @@ -52,10 +52,10 @@ def users(users): @pytest.fixture def app(users): ip_allowlists = [ - mommy.make("api.IPAllowlist", allowed_ip_ranges="xyz"), + baker.make("api.IPAllowlist", allowed_ip_ranges="xyz"), ] - mommy.make("api.App", NUM_APPS - 1) - app = mommy.make("api.App") + baker.make("api.App", NUM_APPS - 1) + app = baker.make("api.App") app.repo_url = "https://github.com/github_org/testing_repo" dev_auth_settings = dict( client_id="dev_client_id", @@ -74,7 +74,7 @@ def app(users): } app.save() AppIPAllowList.objects.update_records(app, "dev_env", ip_allowlists) - mommy.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) + baker.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) return app @@ -180,8 +180,8 @@ def repos_for_redundant_auth(githubapi): def s3buckets(app, users): with patch("controlpanel.api.aws.AWSBucket.create") as _: buckets = { - "not_connected": mommy.make("api.S3Bucket", created_by=users["app_admin"]), - "connected": mommy.make("api.S3Bucket", created_by=users["app_admin"]), + "not_connected": baker.make("api.S3Bucket", created_by=users["app_admin"]), + "connected": baker.make("api.S3Bucket", created_by=users["app_admin"]), } return buckets @@ -189,7 +189,7 @@ def s3buckets(app, users): @pytest.fixture def apps3bucket(app, s3buckets): with patch("controlpanel.api.aws.AWSRole.grant_bucket_access"): - return mommy.make("api.AppS3Bucket", app=app, s3bucket=s3buckets["connected"]) + return baker.make("api.AppS3Bucket", app=app, s3bucket=s3buckets["connected"]) def list_apps(client, *args): @@ -528,7 +528,7 @@ def test_app_detail_display_all_envs(client, app, users, repos): def test_app_detail_with_missing_auth_client(client, users, repos_for_missing_auth): - app = mommy.make("api.App") + app = baker.make("api.App") app.repo_url = "https://github.com/github_org/testing_repo_with_auth" app.save() @@ -549,7 +549,7 @@ def test_app_detail_with_auth_client_redundant(client, users, app, repos_for_red @pytest.fixture def app_being_migrated(users): - app = mommy.make("api.App") + app = baker.make("api.App") app.repo_url = "https://github.com/new_github_org/testing_repo" app_old_repo_url = "https://github.com/github_org/testing_repo" migration_json = dict( @@ -563,7 +563,7 @@ def app_being_migrated(users): ) app.description = json.dumps(app_info) app.save() - mommy.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) + baker.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) return app diff --git a/tests/frontend/views/test_app_variables.py b/tests/frontend/views/test_app_variables.py index 1e5f5fd96..cff459549 100644 --- a/tests/frontend/views/test_app_variables.py +++ b/tests/frontend/views/test_app_variables.py @@ -4,7 +4,7 @@ # Third-party import pytest from django.urls import reverse -from model_mommy import mommy +from model_bakery import baker from rest_framework import status NUM_APPS = 3 @@ -14,7 +14,7 @@ def users(users): users.update( { - "owner": mommy.make("api.User", username="owner"), + "owner": baker.make("api.User", username="owner"), } ) return users @@ -22,9 +22,9 @@ def users(users): @pytest.fixture(autouse=True) def app(users): - mommy.make("api.App", NUM_APPS - 1) - app = mommy.make("api.App") - mommy.make("api.UserApp", user=users["owner"], app=app, is_admin=True) + baker.make("api.App", NUM_APPS - 1) + app = baker.make("api.App") + baker.make("api.UserApp", user=users["owner"], app=app, is_admin=True) return app diff --git a/tests/frontend/views/test_datasource.py b/tests/frontend/views/test_datasource.py index 2f4a892ac..d1733f70e 100644 --- a/tests/frontend/views/test_datasource.py +++ b/tests/frontend/views/test_datasource.py @@ -5,7 +5,7 @@ import pytest from django.conf import settings from django.urls import reverse, reverse_lazy -from model_mommy import mommy +from model_bakery import baker from rest_framework import status # First-party/Local @@ -23,8 +23,8 @@ def enable_db_for_all_tests(db): def users(users): users.update( { - "bucket_viewer": mommy.make("api.User", username="bucket_viewer"), - "bucket_admin": mommy.make("api.User", username="bucket_admin"), + "bucket_viewer": baker.make("api.User", username="bucket_viewer"), + "bucket_admin": baker.make("api.User", username="bucket_admin"), } ) return users @@ -34,53 +34,53 @@ def users(users): def buckets(db): with patch("controlpanel.api.aws.AWSBucket.create"): return { - "app_data1": mommy.make("api.S3Bucket", is_data_warehouse=False), - "app_data2": mommy.make("api.S3Bucket", is_data_warehouse=False), - "warehouse1": mommy.make("api.S3Bucket", is_data_warehouse=True), - "warehouse2": mommy.make("api.S3Bucket", is_data_warehouse=True), - "other": mommy.make("api.S3Bucket"), + "app_data1": baker.make("api.S3Bucket", is_data_warehouse=False), + "app_data2": baker.make("api.S3Bucket", is_data_warehouse=False), + "warehouse1": baker.make("api.S3Bucket", is_data_warehouse=True), + "warehouse2": baker.make("api.S3Bucket", is_data_warehouse=True), + "other": baker.make("api.S3Bucket"), } @pytest.fixture(autouse=True) def users3buckets(buckets, users): return { - "app_data_admin": mommy.make( + "app_data_admin": baker.make( "api.UserS3Bucket", user=users["bucket_admin"], s3bucket=buckets["app_data1"], access_level=UserS3Bucket.READWRITE, is_admin=True, ), - "app_data_admin2": mommy.make( + "app_data_admin2": baker.make( "api.UserS3Bucket", user=users["bucket_admin"], s3bucket=buckets["app_data2"], access_level=UserS3Bucket.READWRITE, is_admin=True, ), - "app_data_readonly": mommy.make( + "app_data_readonly": baker.make( "api.UserS3Bucket", user=users["bucket_viewer"], s3bucket=buckets["app_data2"], access_level=UserS3Bucket.READONLY, is_admin=False, ), - "warehouse_admin": mommy.make( + "warehouse_admin": baker.make( "api.UserS3Bucket", s3bucket=buckets["warehouse1"], user=users["bucket_admin"], access_level=UserS3Bucket.READWRITE, is_admin=True, ), - "warehouse_admin2": mommy.make( + "warehouse_admin2": baker.make( "api.UserS3Bucket", s3bucket=buckets["warehouse2"], user=users["bucket_admin"], access_level=UserS3Bucket.READWRITE, is_admin=True, ), - "warehouse_readonly": mommy.make( + "warehouse_readonly": baker.make( "api.UserS3Bucket", s3bucket=buckets["warehouse1"], user=users["bucket_viewer"], diff --git a/tests/frontend/views/test_ip_allowlist.py b/tests/frontend/views/test_ip_allowlist.py index 3f1f1a57d..ffe957d31 100644 --- a/tests/frontend/views/test_ip_allowlist.py +++ b/tests/frontend/views/test_ip_allowlist.py @@ -1,13 +1,13 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status from rest_framework.reverse import reverse @pytest.fixture(autouse=True) def ip_allowlist(): - allowlist = mommy.make( + allowlist = baker.make( "api.IPAllowlist", name="Test IP allowlist 1", allowed_ip_ranges="192.168.0.0/28", diff --git a/tests/frontend/views/test_secrets.py b/tests/frontend/views/test_secrets.py index 4d36f7233..2bffc027a 100644 --- a/tests/frontend/views/test_secrets.py +++ b/tests/frontend/views/test_secrets.py @@ -3,7 +3,7 @@ # Third-party from django.urls import reverse -from model_mommy import mommy +from model_bakery import baker # First-party/Local from tests.api.fixtures.aws import * @@ -34,7 +34,7 @@ def github_api_token(): def users(users): users.update( { - "app_admin": mommy.make("api.User", username="app_admin"), + "app_admin": baker.make("api.User", username="app_admin"), } ) return users @@ -42,9 +42,9 @@ def users(users): @pytest.fixture(autouse=True) # noqa: F405 def app(users): - mommy.make("api.App", NUM_APPS - 1) - app = mommy.make("api.App") - mommy.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) + baker.make("api.App", NUM_APPS - 1) + app = baker.make("api.App") + baker.make("api.UserApp", user=users["app_admin"], app=app, is_admin=True) return app @@ -73,15 +73,15 @@ def repos(githubapi): def s3buckets(app): with patch("controlpanel.api.aws.AWSBucket.create"): buckets = { - "not_connected": mommy.make("api.S3Bucket"), - "connected": mommy.make("api.S3Bucket"), + "not_connected": baker.make("api.S3Bucket"), + "connected": baker.make("api.S3Bucket"), } return buckets @pytest.fixture # noqa: F405 def apps3bucket(app, s3buckets): - return mommy.make("api.AppS3Bucket", app=app, s3bucket=s3buckets["connected"]) + return baker.make("api.AppS3Bucket", app=app, s3bucket=s3buckets["connected"]) @pytest.fixture diff --git a/tests/frontend/views/test_user.py b/tests/frontend/views/test_user.py index d7b4252e5..6042f7c6a 100644 --- a/tests/frontend/views/test_user.py +++ b/tests/frontend/views/test_user.py @@ -4,7 +4,6 @@ # Third-party import pytest from django.urls import reverse -from model_mommy import mommy from rest_framework import status diff --git a/tests/kubeapi/test_permissions.py b/tests/kubeapi/test_permissions.py index 0a7188f9a..6f8cdedb1 100644 --- a/tests/kubeapi/test_permissions.py +++ b/tests/kubeapi/test_permissions.py @@ -3,7 +3,7 @@ # Third-party import pytest -from model_mommy import mommy +from model_bakery import baker from rest_framework import status @@ -32,13 +32,13 @@ def k8s_api(): @pytest.fixture def users(): return { - "superuser": mommy.make( + "superuser": baker.make( "api.User", auth0_id="github|0", is_superuser=True, username="alice", ), - "normal_user": mommy.make( + "normal_user": baker.make( "api.User", username="bob", auth0_id="github|1", From 527d03e3c3a81ce80f5a29dcf3e10b9fe5c2c00a Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Fri, 5 Apr 2024 10:40:51 +0100 Subject: [PATCH 28/31] moved model-bakery to requirements --- requirements.dev.txt | 1 - requirements.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.dev.txt b/requirements.dev.txt index ad8f940c2..f4873abbc 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -6,7 +6,6 @@ flake8==7.0.0 ipdb==0.13.13 ipython==8.21.0 isort==5.13.2 -model-bakery==1.17.0 pandas==2.2.0 pre-commit==3.6.1 pylint==3.0.3 diff --git a/requirements.txt b/requirements.txt index 491f07686..65bbe5513 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,7 @@ gunicorn==21.2.0 Jinja2==3.1.3 kubernetes==25.3.0 MarkupSafe==2.1.5 +model-bakery==1.0.0 moto==5.0.1 mozilla-django-oidc==4.0.0 psycopg2-binary==2.9.9 From 7440b58c6564f9380434e6d5274e999170df909b Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Fri, 5 Apr 2024 10:54:36 +0100 Subject: [PATCH 29/31] bumped model-bakery to latest version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 65bbe5513..436b0fae2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ gunicorn==21.2.0 Jinja2==3.1.3 kubernetes==25.3.0 MarkupSafe==2.1.5 -model-bakery==1.0.0 +model-bakery==1.17.0 moto==5.0.1 mozilla-django-oidc==4.0.0 psycopg2-binary==2.9.9 From 85c76f098d3873c8147eab1b5661cc577ffd36c3 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Mon, 8 Apr 2024 12:15:22 +0100 Subject: [PATCH 30/31] updated node package --- Dockerfile | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index b10555791..341f4deb2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM 593291632749.dkr.ecr.eu-west-1.amazonaws.com/node:18.12.1-slim AS jsdep +FROM public.ecr.aws/docker/library/node:20.11.1 AS build-node COPY package.json package-lock.json ./ COPY jest.config.js controlpanel/frontend/static /src/ @@ -67,13 +67,13 @@ COPY docker docker COPY tests tests # install javascript dependencies -COPY --from=jsdep dist/app.css dist/app.js static/ -COPY --from=jsdep node_modules/accessible-autocomplete/dist/ static/accessible-autocomplete -COPY --from=jsdep node_modules/govuk-frontend static/govuk-frontend -COPY --from=jsdep node_modules/@ministryofjustice/frontend/moj static/ministryofjustice-frontend -COPY --from=jsdep node_modules/html5shiv/dist static/html5-shiv -COPY --from=jsdep node_modules/jquery/dist static/jquery -COPY --from=jsdep node_modules/jquery-ui/dist/ static/jquery-ui +COPY --from=build-node dist/app.css dist/app.js static/ +COPY --from=build-node node_modules/accessible-autocomplete/dist/ static/accessible-autocomplete +COPY --from=build-node node_modules/govuk-frontend static/govuk-frontend +COPY --from=build-node node_modules/@ministryofjustice/frontend/moj static/ministryofjustice-frontend +COPY --from=build-node node_modules/html5shiv/dist static/html5-shiv +COPY --from=build-node node_modules/jquery/dist static/jquery +COPY --from=build-node node_modules/jquery-ui/dist/ static/jquery-ui # empty .env file to prevent warning messages RUN touch .env From 14e09ba477d6f5b6c04b09911f3bd6de7c02ee81 Mon Sep 17 00:00:00 2001 From: jamesstottmoj Date: Tue, 9 Apr 2024 10:15:05 +0100 Subject: [PATCH 31/31] Removed commented out tests. Updated running.md to specify python 3.12 --- doc/running.md | 4 ++-- tests/api/models/test_ip_allowlist.py | 27 --------------------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/doc/running.md b/doc/running.md index 3837fc3bb..31533d307 100644 --- a/doc/running.md +++ b/doc/running.md @@ -27,8 +27,8 @@ versions of the services. ## 1. Required Dependencies -The Control Panel app requires Python 3.8+ It has been confirmed to work -with Python 3.8.12. +The Control Panel app requires Python 3.12. It has been confirmed to work +with Python 3.12.2. Install python dependencies with the following command: ```sh diff --git a/tests/api/models/test_ip_allowlist.py b/tests/api/models/test_ip_allowlist.py index 182bd6c72..56d6a98fb 100644 --- a/tests/api/models/test_ip_allowlist.py +++ b/tests/api/models/test_ip_allowlist.py @@ -19,19 +19,6 @@ def update_aws_secrets_manager(): yield update_aws_secrets_manager -# def test_ip_allowlist_save_updates_aws_secrets_manager(update_aws_secrets_manager): -# ip_allowlist = baker.make("api.IPAllowlist", allowed_ip_ranges="123") -# app = baker.make("api.App", ip_allowlists=[ip_allowlist]) # noqa:F841 -# -# ip_allowlist.save() -# -# update_aws_secrets_manager.assert_has_calls( -# [ -# call({"allowed_ip_ranges": "123"}), -# ] -# ) -# - def test_ip_allowlist_without_app_save_does_not_update_aws_secrets_manager( update_aws_secrets_manager, ): @@ -42,20 +29,6 @@ def test_ip_allowlist_without_app_save_does_not_update_aws_secrets_manager( update_aws_secrets_manager.assert_not_called() -# def test_ip_allowlist_delete_updates_aws_secrets_manager(update_aws_secrets_manager): -# ip_allowlists = [ -# baker.make("api.IPAllowlist", allowed_ip_ranges="xyz"), -# baker.make("api.IPAllowlist", allowed_ip_ranges="123"), -# ] -# apps = baker.make("api.App", ip_allowlists=ip_allowlists, _quantity=2) # noqa:F841 -# -# ip_allowlists[0].delete() -# -# update_aws_secrets_manager.assert_has_calls( -# [call({"allowed_ip_ranges": "123"}), call({"allowed_ip_ranges": "123"})] -# ) -# - def test_ip_allowlist_without_app_delete_does_not_update_aws_secrets_manager( update_aws_secrets_manager, ):