Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anpl 862 configure pre commit for dev #1079

Merged
merged 85 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
85bdd88
Bump django-filter from 2.3.0 to 22.1
dependabot[bot] Sep 27, 2022
a4174e4
added precommit
ahbensiali Oct 14, 2022
07590e9
ANPL-862 fixed requirements
ahbensiali Oct 14, 2022
5f0470c
ANPL-862 added instructions
ahbensiali Oct 14, 2022
36f8be0
ANPL-862 remove to tty
ahbensiali Oct 17, 2022
37b353e
ANPL-862 removed warnings
ahbensiali Oct 17, 2022
d66a3d7
ANPL-862 removed pytest step
ahbensiali Oct 25, 2022
7a03bcf
Update .flake8
ahbensiali Oct 24, 2022
3280087
test only staged
ahbensiali Oct 26, 2022
e368b4f
testing black changes
ahbensiali Oct 26, 2022
8a2ab61
ANPL-862 conflict fix
ahbensiali Oct 28, 2022
e01a1cc
ANPL-1257 Added nomis login option for dashboard app
Nov 4, 2022
0efbc16
removed unused code
Nov 4, 2022
4d83ed7
added codes for handling the possible exception when registering an app
Nov 4, 2022
5d1ac24
Fixed the failed tests and added extra tests to cover the new changes
Nov 5, 2022
b6172f5
ANPL-1228 Fixed the bucket permission issue
Nov 7, 2022
78ce4d4
Changed the implementation to make the checking func more general bas…
Nov 7, 2022
5c7ceeb
Added a test to cover such scenario
Nov 7, 2022
4eba11a
Fixed the failed tests
Nov 7, 2022
d2a06a8
ANPL-1258 Improve the tooling page
Nov 14, 2022
88311ee
Changed the form action into post api call
Nov 14, 2022
452d6e1
Fixed the failed tests and added new tests
Nov 14, 2022
be7662f
a few changes based on feedback
Nov 15, 2022
7c519d3
another minor change based on feedback
Nov 15, 2022
aa06e46
Changed the logic order for setting client to avoid the global limit …
Nov 15, 2022
33db42f
Fixed the failed test
Nov 15, 2022
d69da8c
ANPL-985 Adding the check for bucket's existence
Nov 15, 2022
4df2c51
Added 2 unit tests to cover the changes
Nov 16, 2022
f0ff939
Update controlpanel/api/validators.py
Nov 17, 2022
3a03c5e
changes based on the feedbacks
Nov 17, 2022
42d29ec
Fixed the failed test
Nov 17, 2022
e3152fb
Update controlpanel/api/serializers.py
Nov 17, 2022
7f51fc1
Update controlpanel/frontend/jinja2/release-create.html
Nov 17, 2022
31b9c26
changes based feedback
Nov 17, 2022
401094d
Merge branch 'anpl-1258-status-message-not-refresh' of https://github…
Nov 17, 2022
9548fe8
merged the change
Nov 17, 2022
ddb0359
Merge pull request #1091 from ministryofjustice/anpl-985-check-duplic…
Nov 18, 2022
9d26e16
Merge pull request #1089 from ministryofjustice/anpl-1228-bucket-perm…
Nov 21, 2022
9e475ca
Update controlpanel/frontend/views/app.py
Nov 21, 2022
438e07f
Merge pull request #1086 from ministryofjustice/anpl-1257-add-nomis-l…
Nov 21, 2022
c6431ef
ANPL-862 pre-commit docs and example of use. listed libraries used. c…
ahbensiali Nov 22, 2022
22ab912
ANPL-823 updated requirements.txt to lock in python-dotenv
ahbensiali Nov 22, 2022
92d1dd5
ANPL-823 removed poetry from build
ahbensiali Nov 22, 2022
83cb1d2
ANPL-823 changed dockerfile to use venv used in build. dev-packages i…
ahbensiali Nov 22, 2022
843a7f9
ANPL-823 test location of python used
ahbensiali Nov 22, 2022
f82d46f
ANPL-823 changed func to use python-dotenv using load_dotenv instead …
ahbensiali Nov 22, 2022
b728162
ANPL-823 reverted back to original library requirements.txt
ahbensiali Nov 22, 2022
aa5c392
Merge pull request #1036 from ministryofjustice/dependabot/pip/django…
rossjones Nov 24, 2022
837d7ca
Hotfix for tooling page after deploying idler
Nov 29, 2022
147e369
Hotfix for tooling page after deploying idler
Nov 30, 2022
925bc67
try to fix the failed tests
Nov 30, 2022
caaa3e8
fixed another minor issue for this PR and added more wording
Nov 30, 2022
667b12f
Tested on dev, got error and changed the code more robust
Nov 30, 2022
8e439dd
Another minor fixes related to jupyter release
Nov 30, 2022
801ec2f
Merge pull request #1097 from ministryofjustice/hotfix-tooling-env-pa…
Nov 30, 2022
0c27ad3
ANPL-862 added hooks to generic actions
ahbensiali Nov 30, 2022
82d0572
ANPL-862 added check handle to black and isort
ahbensiali Nov 30, 2022
806297d
Merge branch 'main' into anpl-1258-status-message-not-refresh
Nov 30, 2022
18c62f9
ANPL-862 changed pre-commit to fetch branch name and find the files t…
ahbensiali Nov 30, 2022
d6874da
ANPL-862 isort formatted wsgi.py file
ahbensiali Nov 30, 2022
dcb1dda
ANPL-823 removed build-system section of pyproject.toml
ahbensiali Nov 30, 2022
9330403
ANPL-823 removed poetry from pyproject.toml
ahbensiali Dec 2, 2022
c925e99
Solved the issues after merge
Dec 2, 2022
b4884c6
Give a try to see whether the env can pick up the javascript
Dec 2, 2022
ab8d751
Merge pull request #1090 from ministryofjustice/anpl-1258-status-mess…
Dec 5, 2022
f52755f
ANPL-823 removed pip install from docs
ahbensiali Dec 6, 2022
129afee
ANPL-862 changed back to check on commit
ahbensiali Dec 6, 2022
da0e248
Merge pull request #1093 from ministryofjustice/ANPL-823-replace-djan…
ahbensiali Dec 7, 2022
fa3ffbf
added precommit
ahbensiali Oct 14, 2022
119800b
ANPL-862 fixed requirements
ahbensiali Oct 14, 2022
d9d0a4d
ANPL-862 added instructions
ahbensiali Oct 14, 2022
355ce9a
ANPL-862 remove to tty
ahbensiali Oct 17, 2022
5d2c3e5
ANPL-862 removed warnings
ahbensiali Oct 17, 2022
210bead
ANPL-862 removed pytest step
ahbensiali Oct 25, 2022
c0e4277
Update .flake8
ahbensiali Oct 24, 2022
5212143
test only staged
ahbensiali Oct 26, 2022
783d997
testing black changes
ahbensiali Oct 26, 2022
f8446af
ANPL-862 conflict fix
ahbensiali Oct 28, 2022
4f69d9e
ANPL-862 pre-commit docs and example of use. listed libraries used. c…
ahbensiali Nov 22, 2022
e48b0a0
ANPL-862 added hooks to generic actions
ahbensiali Nov 30, 2022
7d5a314
ANPL-862 added check handle to black and isort
ahbensiali Nov 30, 2022
35a65c7
ANPL-862 changed pre-commit to fetch branch name and find the files t…
ahbensiali Nov 30, 2022
233649f
ANPL-862 isort formatted wsgi.py file
ahbensiali Nov 30, 2022
1559024
ANPL-862 changed back to check on commit
ahbensiali Dec 6, 2022
56ea1d8
ANPL-862 merge conflict fix
ahbensiali Dec 7, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[flake8]
ignore = E203, E266, E501, W503, F403, F401
max-line-length = 88
max-complexity = 18
select = B,C,E,F,W,T4,B9
LouiseABowler marked this conversation as resolved.
Show resolved Hide resolved
43 changes: 43 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
default_stages: [push]
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: requirements-txt-fixer
stages: [push]
- id: check-yaml
stages: [push]
- id: end-of-file-fixer
stages: [push]
- id: trailing-whitespace
stages: [push]

- repo: https://github.com/psf/black
rev: 22.8.0
hooks:
- id: black
name: black formatting
entry: bash -c 'black --check $(git diff --name-only origin/$(git rev-parse --abbrev-ref HEAD)..HEAD | grep .py)'
ahbensiali marked this conversation as resolved.
Show resolved Hide resolved

- repo: https://github.com/pycqa/isort
ahbensiali marked this conversation as resolved.
Show resolved Hide resolved
rev: 5.10.1
hooks:
- id: isort
name: isort (python)
entry: bash -c 'isort --check-only $(git diff --name-only origin/$(git rev-parse --abbrev-ref HEAD)..HEAD | grep .py)'

- repo: https://github.com/pycqa/flake8
rev: 5.0.4
hooks:
- id: flake8
name: flake8 format check
entry: bash -c 'flake8 --config=.flake8 $(git diff --name-only origin/$(git rev-parse --abbrev-ref HEAD)..HEAD | grep .py)'

- repo: local
hooks:
- id: jira-ticket
name: Check for Jira ticket
language: pygrep
entry: '\A(?!ANPL+-[0-9]+)'
args: [--multiline]
stages: [commit-msg]
20 changes: 10 additions & 10 deletions controlpanel/settings/test.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
# First-party/Local
from controlpanel.settings.common import *


ENV = 'test'
ENV = "test"

AWS_COMPUTE_ACCOUNT_ID = "test_compute_account_id"
AWS_DATA_ACCOUNT_ID = "123456789012" # XXX DO NOT CHANGE - it will break moto tests
K8S_WORKER_ROLE_NAME = "nodes.example.com"
SAML_PROVIDER = "test-saml"

LOGGING["loggers"]["django_structlog"]["level"] = "WARNING"
LOGGING["loggers"]["controlpanel"]["level"] = "WARNING"
LOGGING["loggers"]["django_structlog"]["level"] = "WARNING" # noqa: F405
LOGGING["loggers"]["controlpanel"]["level"] = "WARNING" # noqa: F405

AUTHENTICATION_BACKENDS = [
'rules.permissions.ObjectPermissionBackend',
'django.contrib.auth.backends.ModelBackend',
"rules.permissions.ObjectPermissionBackend",
"django.contrib.auth.backends.ModelBackend",
]
MIDDLEWARE.remove('mozilla_django_oidc.middleware.SessionRefresh')
REST_FRAMEWORK['DEFAULT_AUTHENTICATION_CLASSES'].remove(
'mozilla_django_oidc.contrib.drf.OIDCAuthentication',
MIDDLEWARE.remove("mozilla_django_oidc.middleware.SessionRefresh") # noqa: F405
REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"].remove( # noqa: F405
"mozilla_django_oidc.contrib.drf.OIDCAuthentication",
)
OIDC_OP_JWKS_ENDPOINT = "https://example.com/.well-known/jwks.json"
OIDC_ALLOW_UNSECURED_JWT = True
OIDC_DOMAIN = "oidc.idp.example.com"

TOOLS_DOMAIN = 'example.com'
TOOLS_DOMAIN = "example.com"

CSRF_COOKIE_SECURE = False
SESSION_COOKIE_SECURE = False
Expand Down
4 changes: 4 additions & 0 deletions controlpanel/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
https://docs.djangoproject.com/en/2.1/howto/deployment/wsgi/
"""

# Standard library
import os

# Third-party
from django.core.wsgi import get_wsgi_application

# First-party/Local
from controlpanel.utils import load_app_conf_from_file

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "controlpanel.settings")
Expand Down
50 changes: 42 additions & 8 deletions doc/running.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ source venv/bin/activate
pip3 install -r requirements.txt
pip3 install -r requirements.dev.txt
pip3 uninstall python-dotenv # see ANPL-823
pre-commit install --hook-type commit-msg
pre-commit install
ahbensiali marked this conversation as resolved.
Show resolved Hide resolved
```

In addition, you must have:
Expand Down Expand Up @@ -83,7 +85,7 @@ and have [cluster admin access to Kubernetes](https://silver-dollop-30c6a355.pag
### AWS Configuration

In order to run the app you'll need various permissions set up for you in the
wider infrastructure of the project, mainly for AWS platform.
wider infrastructure of the project, mainly for AWS platform.

As the docs for AWS (linked above) mention, you'll need to add yourself an AWS
user account linked to your MoJ email address via the
Expand Down Expand Up @@ -258,14 +260,14 @@ and then ask a colleague for help.

### Local AWS profile setup (on first run only)
This app needs to interact with multiple AWS accounts in order to support the users' needs.
The AWS resources like IAM, s3 buckets are under our data account and will be managed by
The AWS resources like IAM, s3 buckets are under our data account and will be managed by
app through [boto3](https://boto3.amazonaws.com/v1/documentation/api/latest/index.html). In order to make sure the boto3 can obtain the right profile for local env.
The following steps will show how to create it.

Assume that the name of profile for our aws data account is ```admin-data```

#### Add the AWS credential into .aws/credentials
it should look like below
it should look like below
```
[admin-data]
aws_access_key_id = <your aws_access_key_id>
Expand Down Expand Up @@ -318,17 +320,17 @@ If you want to run the control panel app to manage AWS resources under single ro
following environment variable to define the profile you want to use
- ```AWS_PROFILE```: The profile which will be used for ```boto3``` auth
export AWS_PROFILE = "admin-data"
- Make sure there is NO other AWS boto3 environment variables defined.
- Make sure there is NO other AWS boto3 environment variables defined.

#### AWS credential setting for multiple AWS roles
If you want to run the app to manage the AWS resources cross different AWS accounts by assuming
If you want to run the app to manage the AWS resources cross different AWS accounts by assuming
different roles, then
- Check whether following 2 more environment variables have been setup in the env file or not
- `AWS_DATA_ACCOUNT_ROLE`: The role_arn of admin-data account
- `AWS_DEV_ACCOUNT_ROLE` : The role_arn of admin-dev account

if you are not sure what the value of role_arn of those two accounts is, you can find them out by
checking the aws config file.
checking the aws config file.

More detail about the settings for mult-account is [here](architecture.md) (last section)
- Make sure other AWS boto3 settings e.g. ```AWS_PROFILE``` are NOT defined in your env, otherwise the app will
Expand Down Expand Up @@ -371,7 +373,7 @@ Go to http://localhost:8000/, sign in via Auth0 and marvel at your locally
running control panel.

NOTES: if you use aws-vault to manage your AWS credentials, during the running process of the app,
you may encounter a popup window for asking you to provide key-chain password from time to time,
you may encounter a popup window for asking you to provide key-chain password from time to time,
which is normal.

### Loading tools
Expand All @@ -390,3 +392,35 @@ Check that you have `<TOOL>_AUTH_CLIENT_DOMAIN`, `<TOOL>_AUTH_CLIENT_ID` and `<T

Even though your instance of Control Panel is running locally, it will still interact with the remote AWS data account and development Kubernetes cluster.
The data account is also used by our production environment, so take care when interacting with our AWS resources directly.


## Development Practices

### pre-commit

`pre-commit` is a package manager for git hooks that we use during local development.

Current checks are:-
- requirements.txt library sort and check
- yaml file check
- end-of-file must have white line
- trailing white spaces check
- `black` library (formats Python code)
- `isort` library (standardises the order of Python imports)
- `flake8` library (formats Python code and also improves code style)
- Jira ticket reference (commits must reference the ticket number)

To override the above for whatever reason (maybe you don't have a ticket number and because you are working on hotfix) you can use the following command.

`PRE_COMMIT_ALLOW_NO_CONFIG=1 git push ...`

### Git commit message

Commit messages should follow the appropriate format.
All commits must begin with the Jira ticket they are associated with.

format: `ANPL-[int]`

e.g.

`git commit -m "ANPL-1234 insert message here"`
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ exclude = '''
'''

[tool.isort]
profile = 'black'
import_heading_firstparty = 'First-party/Local'
import_heading_future = 'Future'
import_heading_stdlib = 'Standard library'
import_heading_thirdparty = 'Third-party'
line_length = 88
multi_line_output = 3
no_lines_before = 'LOCALFOLDER'
profile = 'black'
12 changes: 7 additions & 5 deletions requirements.dev.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
black==22.8.0
django-debug-toolbar==3.2.4
django-debug-toolbar-requests==1.0.5
django-elasticsearch-debug-toolbar==2.0.0
pylint==2.12.2
pylint-django==2.4.4
ipython==7.31.1
flake8==5.0.4
ipdb==0.13.9
black==22.8.0
isort==5.10.1
ipython==7.31.1
isort==5.10.1
pre-commit==2.20.0
pylint==2.15.4
pylint-django==2.4.4