From 6322eaa0c9386b82a6af89ba58489f8facec59de Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Fri, 16 Dec 2022 15:25:45 +0100 Subject: [PATCH 1/7] Update repo README to include a link to the Sphinx docs --- README.adoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.adoc b/README.adoc index b824ecde632..032cd8eb5da 100644 --- a/README.adoc +++ b/README.adoc @@ -1,6 +1,8 @@ = Wat is OpenKAT? +*NOTE: You can find the most recent English documentation at https://minvws.github.io/nl-kat-coordination/* + OpenKAT heeft als doel het monitoren, registreren en analyseren van de status van informatiesystemen. Het uitgangspunt is dat veel van de grote beveiligingsincidenten worden veroorzaakt door kleine fouten en bekende kwetsbaarheden en dat als je die tijdig kunt vinden je systemen en infrastructuur een stuk veiliger worden. OpenKAT scant, verzamelt, analyseert en rapporteert in een permanent proces: From 375a18b75d909dd2aea09f68dfc0dde213b9ee15 Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Fri, 16 Dec 2022 15:26:02 +0100 Subject: [PATCH 2/7] Add GitHub templates --- .github/ISSUE_TEMPLATE/bug_report.md | 43 ++++++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 16 ++++++ .github/ISSUE_TEMPLATE/feature_request.md | 22 ++++++++ .github/pull_request_template.md | 63 +++++++++++++++++++++++ 4 files changed, 144 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 .github/pull_request_template.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000000..d3219019940 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,43 @@ +--- +name: Bug report +about: Create a bug report to help us improve +title: '' +labels: 'bug' +assignees: '' +--- + +_Please add `bug` and one or more of the following labels to your issue:_ +`frontend backend community dependencies` + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Screenshots** +If applicable, add screenshots to help explain your problem. + +**OpenKAT version** +Should contain at least the commit or release of Rocky. If you think another module may be the cause of the bug, note that module's commit or release here as well. + +**Desktop (please complete the following information):** + - OS: [e.g. iOS] + - Browser: [e.g. chrome, safari] + - Version: [e.g. 22] + +**Smartphone (please complete the following information):** + - Device: [e.g. iPhone6] + - OS: [e.g. iOS8.1] + - Browser: [e.g. stock browser, safari] + - Version: [e.g. 22] + +**Additional context** +Add any other context about the problem here. \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000000..8c19463e672 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,16 @@ +blank_issues_enabled: false +contact_links: + - name: KAT coordination repository + url: https://github.com/minvws/nl-kat-coordination + - name: KAT security issue reporting + url: https://github.com/underdarknl + about: If you find any serious security issues, please contact Jan Klopper (@underdarknl) directly and do NOT create a public issue. + - name: KAT documentation + url: https://minvws.github.io/nl-kat-coordination + about: You can find all (developer) documentation on this webpage. + - name: KAT security policy + url: https://github.com/minvws/nl-kat-coordination/security/policy + - name: KAT code of conduct + url: https://github.com/minvws/.github/blob/master/CODE_OF_CONDUCT.md + - name: KAT contributor agreement + url: https://github.com/minvws/.github/blob/master/CONTRIBUTING.md \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000000..c3e4ce980cd --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,22 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: 'feature' +assignees: 'underdarknl' +--- + +_Please add one or more of the following labels to your issue:_ +`frontend backend community dependencies` + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. \ No newline at end of file diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000000..113f6f94155 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,63 @@ +## Changes +_Please describe the essence of this PR in a few sentences._ + +## Issue ticket number and link +_Please paste a link to the issue on the project board here. Alternatively, if there was no submitted issue prior to this PR, you may add this PR to the project board directly._ + +## Proof +_Please paste some screenshots or other proof of your (working) change here. If you feel that this is not required (e.g. this PR is trivial), note that here._ + +## Extra instructions for others +_This section may be skipped or omitted. Uncomment and answer the below questions if relevant._ + + + +## Checklist for author(s): +- [ ] This PR comes from a `feature` or `hotfix` branch, in line with our git branching strategy; +- [ ] This PR is "bite-sized" and only focuses on a single issue, problem, or feature; +- [ ] I am not reinventing the wheel: there is no high-quality library that already has this feature; +- [ ] I have changed the example `.env` files if I added, removed, or changed any config options, and I have informed others that they need to modify their `.env` files if required; +- [ ] I have performed a self-review of my own code; +- [ ] I have commented my code, particularly in hard-to-understand areas; +- [ ] I have made corresponding changes to the documentation, if necessary; +- [ ] I have written unit, integration, and end-to-end tests for the change that I made; + +If a non-trivial PR: +- [ ] This PR is part of a milestone and has appropriate labels; +- [ ] This PR is properly linked to the project board (either directly or via an issue); +- [ ] I have added screenshots or some other proof that my code does what it is supposed to do; + + +``` +## Checklist for functional reviewer(s): +- [ ] If a non-trivial PR: This PR is properly linked to an issue on the project board; +- [ ] I have checked out this branch, and successfully ran `make kat`; +- [ ] I have ran `make test-rf` and all end-to-end Robot Framework tests pass; +- [ ] I confirmed that the PR's advertised `feature` or `hotfix` works as intended; +- [ ] I confirmed that there are no unintended functional regressions in this branch; + +### What works: +* _bullet point + screenshot (if useful) per tested functionality_ + +### What doesn't work: +* _bullet point + screenshot (if useful) per tested functionality_ + +### Bug or feature?: +* _bullet point + screenshot (if useful) if it is unclear whether something is a bug or an intended feature._ +``` + +``` +## Checklist for code reviewer(s): +- [ ] The code passes the CI tests and linters; +- [ ] The code does not bypass authentication or security mechanisms; +- [ ] The code does not introduce any dependency on a library that has not been properly vetted; +- [ ] The code does not violate Model-View-Template and our other architectural principles; +- [ ] The code contains docstrings, comments, and documentation where needed; +- [ ] The code prioritizes readability over performance where appropriate; +- [ ] The code conforms to our agreed coding standards. +``` \ No newline at end of file From 51bf72687dc1fb77cc9708734f401c57091f5874 Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Fri, 16 Dec 2022 15:26:23 +0100 Subject: [PATCH 3/7] Update docs with logging guidelines --- docs/source/conf.py | 2 +- docs/source/containers.md | 212 ----------------------------- docs/source/guidelines/index.rst | 1 + docs/source/guidelines/logging.rst | 83 +++++++++++ docs/source/readme.rst | 4 - 5 files changed, 85 insertions(+), 217 deletions(-) delete mode 100644 docs/source/containers.md create mode 100644 docs/source/guidelines/logging.rst delete mode 100644 docs/source/readme.rst diff --git a/docs/source/conf.py b/docs/source/conf.py index 7ad9e200c1e..de643641d4c 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -48,7 +48,7 @@ "display_github": True, "github_user": "minvws", "github_repo": "nl-kat-coordination", - "github_version": "develop", + "github_version": "main", "conf_py_path": "/docs/source/", } diff --git a/docs/source/containers.md b/docs/source/containers.md deleted file mode 100644 index 374990b8e10..00000000000 --- a/docs/source/containers.md +++ /dev/null @@ -1,212 +0,0 @@ -# Container deployment - -OpenKAT can be deployed using containers. We aim to support both simple docker / -docker-compose setups and container orchestration systems like Kubernetes and -Nomad. - -There is a docker-compose.release-example.yml in the root directory that can be -used as an example how to deploy using docker-compose. - -## Container images - -The container images can be found here: - -- https://github.com/minvws/nl-kat-boefjes/pkgs/container/nl-kat-boefjes -- https://github.com/minvws/nl-kat-bytes/pkgs/container/nl-kat-bytes -- https://github.com/minvws/nl-kat-mula/pkgs/container/nl-kat-mula -- https://github.com/minvws/nl-kat-octopoes/pkgs/container/nl-kat-octopoes -- https://github.com/minvws/nl-kat-rocky/pkgs/container/nl-kat-rocky -- https://github.com/minvws/nl-kat-keiko/pkgs/container/nl-kat-keiko - -## Setup - -To set up an installation with pre-built containers, you can pull the repository using: - -```shell -git clone https://github.com/minvws/nl-kat-coordination.git -``` - -If this is your first install, and you do not have an .env file yet, you can create an .env file using the following command: - -```shell -make env -``` - -This will create an .env file with the default values. You can edit this file to change the default values. Now you can pull and start the containers using the following command: - -```shell -docker-compose -f docker-compose.release-example.yml up -d -``` - - -The container image run the necessary database migration commands in the -entrypoint if DATABASE_MIGRATION is set. You manually need to run setup commands -in the katalogus and rocky containers to initialize everything. In the katalogus -container we need to create an organisation, we can do this by running the -following in the katalogus container: - -```shell -python3 -m boefjes.seed -``` - -With docker-compose you would run this as: - -```shell -docker-compose -f docker-compose.release-example.yml exec katalogus python3 -m boefjes.seed -``` - -In the rocky container we first need to import the OOI database seed: - -```shell -python3 manage.py loaddata OOI_database_seed.json -``` - -With docker-compose you would run this as: - -```shell -docker-compose -f docker-compose.release-example.yml exec rocky python3 manage.py loaddata OOI_database_seed.json -``` - -Next we need to create the superuser, this will prompt for the e-mail address and password: - -```shell -python3 manage.py createsuperuser -``` - -With docker-compose you would run this as: - -```shell -docker-compose -f docker-compose.release-example.yml exec rocky python3 manage.py createsuperuser -``` - - -We also need to create an organisation, this command will create a development organisation: - -```shell -python3 manage.py setup_dev_account -``` - -With docker-compose you would run this as: - -```shell -docker-compose -f docker-compose.release-example.yml exec rocky python3 manage.py setup_dev_account -``` - -## Container commands - -We have two container images that are used to run multiple containers. What the container runs is be specified by overriding the CMD of the container. - -| Container image | CMD | Description | -|-----------------|-------------|-----------------------------------------------------------------------------------| -| boefjes | boefje | Boefjes runtime | -| boefjes | normalizer | Normalizers runtime | -| boefjes | katalogus | Katalogus API | -| octopoes | web | Octopoes API | -| octopoes | worker-beat | Celery worker running beat. There must only be exactly one container of this type | -| octopoes | worker | Celery worker. Use this if you need to more than one work container for scaling | - -## Env variables - -Each container needs to be configured using a set of environment variables - -### Boefjes / Katalogus - -| Environment variable | Required | Default Value | Format | Description | -|------------------------------|----------|---------------|--------------------------------------|----------------------------------------------------------------------------------------| -| `WORKER_CONCURRENCY` | no | 10 | | Number of worker processes to start | -| `QUEUE_NAME_BOEFJES` | no | boefjes | | Queue name for boefjes | -| `QUEUE_NAME_NORMALIZERS` | no | normalizers | | Queue name for normalizers | -| `QUEUE_URI` | yes | | amqp://user:pass@host:5672/vhost | RabbitMQ used by celery, should be the same as Mula `SCHEDULER_DSP_BROKER_URL` | -| `BYTES_API` | yes | | http://bytes:8000 | URI for the Bytes API | -| `BYTES_USERNAME` | yes | | | Username for Bytes API | -| `BYTES_PASSWORD` | yes | | | Password for Bytes API | -| `KATALOGUS_API` | yes | | http://katalogus:8000 | URI for the Katalogus API | -| `OCTOPOES_API` | yes | | http://octopoes_api:8000 | URI for the Octopoes API | -| `KATALOGUS_DB_URI` | yes | | postgresql://user:paswd@host:5432/db | URI for the Postgresql DB | -| `WP_SCAN_API` | no | | | A token needed by WP Scan boefje | -| `ENCRYPTION_MIDDLEWARE` | no | IDENTITY | | Encryption to use for the katalogus settings: IDENTITY (no encryption) or NACL_SEALBOX | -| `KATALOGUS_PRIVATE_KEY_B_64` | no | | | KATalogus NaCl Sealbox base-64 private key string | -| `KATALOGUS_PUBLIC_KEY_B_64` | no | | | KATalogus NaCl Sealbox base-64 public key string | -| `WP_SCAN_API` | no | | | | -| `SHODAN_API` | no | | | | -| `BINARYEDGE_API` | no | | | | -| `LEAKIX_API` | no | | | | -| `REMOTE_NS` | no | 8.8.8.8 | | | -| `LXD_ENDPOINT` | no | | | | -| `LXD_PASSWORD` | no | | | | -| `DATABASE_MIGRATION` | no | false | | Container entrypoint will run database migrations if set to "true" | - -See also https://github.com/minvws/nl-kat-boefjes#environment-variables - -## Bytes - -| Environment variable | Required | Default Value | Format | Description | -|-------------------------|----------|----------------|--------------------------------------|--------------------------------------------------------------------| -| `SECRET` | yes | | | Secret used for JWT | -| `BYTES_USERNAME` | yes | | | Username for Bytes API | -| `BYTES_PASSWORD` | yes | | | Password for Bytes API | -| `QUEUE_URI` | no | | | RabbitMQ queue to send events to | -| `BYTES_DB_URI` | yes | | postgresql://user:paswd@host:5432/db | URI for the Postgresql DB | -| `BYTES_DATA_DIR` | yes | | | Directory to store files | -| `ENCRYPTION_MIDDLEWARE` | yes | `NACL_SEALBOX` | | Encryption to use: IDENTITY (no encryption) or NACL_SEALBOX | -| `DATABASE_MIGRATION` | no | false | | Container entrypoint will run database migrations if set to "true" | - -See also https://github.com/minvws/nl-kat-bytes#configuration - -## Octopoes - -| Environment variable | Required | Default Value | Format | Description | -|----------------------|----------|---------------|-----------------------|---------------------------| -| `XTDB_URI` | yes | | http://crux:3000 | XTDB uri | -| `XTDB_TYPE` | no | crux | | crux or xtdb | -| `QUEUE_URI` | yes | | | RabbitMQ queue | -| `KATALOGUS_API` | yes | | http://katalogus:8000 | URI for the Katalogus API | - -See also https://github.com/minvws/nl-kat-octopoes#environment-variables - -## Mula - -| Environment variable | Required | Default Value | Format | Description | -|---------------------------------|----------|---------------|--------------------------------------|--------------------------------------------------------------------| -| `SCHEDULER_BOEFJE_POPULATE` | no | False | | Set to True to enable queueing of boefjes | -| `SCHEDULER_NORMALIZER_POPULATE` | no | True | | Set to True to enable queueing of normalizers | -| `SCHEDULER_DSP_BROKER_URL` | yes | | amqp://user:pass@host:5672/vhost | RabbitMQ instance used by celery | -| `SCHEDULER_RABBITMQ_DSN` | yes | | amqp://user:pass@host:5672/vhost | RabbitMQ instance used by scheduler, can be the same as celery | -| `SCHEDULER_DB_DSN` | yes | | postgresql://user:paswd@host:5432/db | URI for scheduler DB | -| `BYTES_API` | yes | | http://bytes:8000 | URI for the Bytes API | -| `BYTES_USERNAME` | yes | | | Username for Bytes API | -| `BYTES_PASSWORD` | yes | | | Password for Bytes API | -| `KATALOGUS_API` | yes | | http://katalogus:8000 | URI for the Katalogus API | -| `OCTOPOES_API` | yes | | http://octopoes_api:8000 | URI for the Octopoes API | -| `DATABASE_MIGRATION` | no | false | | Container entrypoint will run database migrations if set to "true" | - -See also https://github.com/minvws/nl-kat-mula/blob/main/docs/configuration.md - -## Rocky - -| Environment variable | Required | Default Value | Format | Description | -|--------------------------|----------|---------------|----------------------------------|---------------------------------------------------------------------------------------------------| -| `ROCKY_DB_HOST` | yes | | | Postgres host | -| `ROCKY_DB_PORT` | yes | | | Postgres port | -| `ROCKY_DB` | yes | | | Postgres database database | -| `ROCKY_DB_USER` | yes | | | Postgres username | -| `ROCKY_DB_PASSWORD` | yes | | | Postgres password | -| `SECRET_KEY` | yes | | String | Key of at least 50 characters, see https://docs.djangoproject.com/en/4.1/ref/settings/#secret-key | -| `QUEUE_NAME_BOEFJES` | no | boefjes | | Queue name for boefjes | -| `QUEUE_NAME_NORMALIZERS` | no | normalizers | | Queue name for normalizers | -| `QUEUE_URI` | yes | | amqp://user:pass@host:5672/vhost | RabbitMQ used by celery, should be the same as Mula `SCHEDULER_DSP_BROKER_URL` | -| `BYTES_API` | yes | | http://bytes:8000 | URI for the Bytes API | -| `BYTES_USERNAME` | yes | | | Username for Bytes API | -| `BYTES_PASSWORD` | yes | | | Password for Bytes API | -| `KATALOGUS_API` | yes | | http://katalogus:8000 | URI for the Katalogus API | -| `OCTOPOES_API` | yes | | http://octopoes_api:8000 | URI for the Octopoes API | -| `SCHEDULER_API` | yes | | http://scheduler:8000 | URI for the scheduler API | -| `EMAIL_HOST` | no | | | Hostname of mail server to use to send e-mails | -| `EMAIL_PORT` | no | 25 | | Mail server port | -| `EMAIL_HOST_USER` | no | | | Username to use to connect to mail server | -| `EMAIL_HOST_PASSWORD` | no | | | Password to use to connect to mail server | -| `DEFAULT_FROM_EMAIL` | no | | | https://docs.djangoproject.com/en/4.1/ref/settings/#default-from-email | -| `SERVER_EMAIL` | no | | | https://docs.djangoproject.com/en/4.1/ref/settings/#server-email | -| `EMAIL_USE_TLS` | no | | | https://docs.djangoproject.com/en/4.1/ref/settings/#email-use-tls | -| `EMAIL_USE_SSL` | no | | | https://docs.djangoproject.com/en/4.1/ref/settings/#email-use-ssl | -| `DATABASE_MIGRATION` | no | false | | Container entrypoint will run database migrations if set to "true" | diff --git a/docs/source/guidelines/index.rst b/docs/source/guidelines/index.rst index b15ecf3f739..0afac32191f 100644 --- a/docs/source/guidelines/index.rst +++ b/docs/source/guidelines/index.rst @@ -10,4 +10,5 @@ Contains documentation for developers and contributors. management development contributions + logging project_statuses \ No newline at end of file diff --git a/docs/source/guidelines/logging.rst b/docs/source/guidelines/logging.rst new file mode 100644 index 00000000000..3b58446c386 --- /dev/null +++ b/docs/source/guidelines/logging.rst @@ -0,0 +1,83 @@ +Logging +####### + +Some general guidelines for how to log within your applications: + +- Log after, not before. This communicates what operation was performed, and what the result was. + +.. code-block:: python + + # Don't do this + log.info("Doing stuff") + some_stuff() + + # Do this + some_stuff() + log.info("Did stuff") + +- Separate parameters and messages. + This will make sure that logs are parseable, searchable, easy to extend, and read. + A python package called `structlog `_ can help with this. + This is one example on how to separate parameters and messages, you can also decide to for instance use a json output: + +.. code-block:: python + + # Don't do this + some_stuff() + log.info("Did stuff with %s", url) + + # Do this + some_stuff() + log.info("Did stuff with %s [url=%s]", url, url) + + +* Use ``FATAL`` when immediate intervention is needed and the system can't + continue running. + +* Distinguish between ``WARNING`` and ``ERROR``. Use ``WARNING`` when you did + something but an error occurred. Use ``ERROR`` when something wasn't done and + went wrong. These severities need attention and fixing. + +* ``INFO`` is for business, ``DEBUG`` is for technology. The ``INFO`` log should look + like a book. + +.. code-block:: text + + INFO | User registered for newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + INFO | Newsletter sent to user. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + INFO | User unsubscribed from newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + +.. code-block:: text + + DEBUG | Saved user to newsletter list. [user_id=3a937a10-da92-49b6-9350-6f2c74bcb34c] + DEBUG | Sent welcome email. [user_id=3a937a10-da92-49b6-9350-6f2c74bcb34c] + INFO | User registered for newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + DEBUG | Started cron job to send newsletter of the day. [subscribers=12345] + INFO | Newsletter sent to user. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + INFO | User unsubscribed from newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] + +- Design log to be chainable. When using parameters in logging make sure that + it can be aggregated, over multiple logs. E.g. + ``[user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"]`` allows you to aggregate + logs from multiple logs with the same ``user_id`` parameter. + +- Never log any personal identifiable information, and confidential + information. So no email addresses, names, credit card numbers, etc. + +- Always log in UTC, log in milliseconds. Synchronize your servers with a NTP + daemon. + +.. + - TODO: request guid, see if this is already done by log aggregation of them + used cloud provider. + +.. + - TODO: what always should be present: time, severity, process ID, thread ID, + application identifier, request identifier, (user identifier), and message. + +**Sources** + +- ``_ +- ``_ +- ``_ +- ``_ \ No newline at end of file diff --git a/docs/source/readme.rst b/docs/source/readme.rst deleted file mode 100644 index 1b8cb583201..00000000000 --- a/docs/source/readme.rst +++ /dev/null @@ -1,4 +0,0 @@ -README -====== - -todo \ No newline at end of file From 245bb057725c1901894785a1adffc7eaf7e69256 Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Fri, 16 Dec 2022 15:56:06 +0100 Subject: [PATCH 4/7] Add line breaks and make a note about basing work off release tags --- docs/source/guidelines/contributions.rst | 18 +++++-- docs/source/guidelines/development.rst | 66 +++++++++++++++++------- docs/source/guidelines/management.rst | 22 +++++--- 3 files changed, 76 insertions(+), 30 deletions(-) diff --git a/docs/source/guidelines/contributions.rst b/docs/source/guidelines/contributions.rst index a448644425f..457de1fb1eb 100644 --- a/docs/source/guidelines/contributions.rst +++ b/docs/source/guidelines/contributions.rst @@ -1,7 +1,8 @@ Contributions ############# -All contributions, bug reports, bug fixes, documentation improvements, enhancements and ideas are welcome. You can directly join and be involved in the development of OpenKAT: +All contributions, bug reports, bug fixes, documentation improvements, enhancements and ideas are welcome. +You can directly join and be involved in the development of OpenKAT: - Install and use OpenKAT and provide feedback - Development of boefje, normalizer and bit plugins @@ -10,7 +11,8 @@ All contributions, bug reports, bug fixes, documentation improvements, enhanceme - Solve tickets with a ``good first issue`` label - Port OpenKAT to other systems -Note that it is required to sign a `Contributor License Agreement `_ when submitting. The ``CLAassitant`` bot will request this automatically on your first Pull Request. +Note that it is required to sign a `Contributor License Agreement `_ when submitting. +The ``CLAassitant`` bot will request this automatically on your first Pull Request. Contribute to Codebase ====================== @@ -19,7 +21,7 @@ Contribute to Codebase See :ref:`Development` for our code style, coding conventions, and overall workflow. - Fork the right repository in GitHub -- Create a new branch from ``develop`` (and make sure it is always up-to-date with the OpenKAT ``develop`` branch!) +- Create a new branch from either ``main`` or a release tag. Note that ``main`` changes rapidly, and as such may not be a suitable basis for your work. - This branch should be in the following format: - ``[feature|enhancement|bug|hotfix]/random-cat-popup-on-screen`` - Commit and push the code @@ -35,7 +37,11 @@ See :ref:`Development` for our code style, coding conventions, and overall workf Contribute Documentation ======================== -Contributing to the documentation benefits everyone who uses OpenKAT. We encourage you to help us improve the documentation, and you don't have to be an expert using OpenKAT to do so. There are many sections that are better off written by non-experts. If something in the docs doesn't make sense to you, updating the relevant section might be a great way to ensure it will help the next person. You're welcome to propose edits to almost every text, including comments and docstrings in the code, the wiki, and other files. +Contributing to the documentation benefits everyone who uses OpenKAT. +We encourage you to help us improve the documentation, and you don't have to be an expert using OpenKAT to do so. +There are many sections that are better off written by non-experts. +If something in the docs doesn't make sense to you, updating the relevant section might be a great way to ensure it will help the next person. +You're welcome to propose edits to almost every text, including comments and docstrings in the code, the wiki, and other files. You could help us out with the following sections: @@ -48,7 +54,9 @@ All documentation should be placed in a repository's ``docs`` folder. Contributor Social Contract =========================== -All contributors (including, but not limited to, developers and issue reporters) promise to do their best to adhere to the guidelines in :ref:`Guidelines`. Everyone is encouraged to politely and constructively point out guidelines violations to others. Actively enforcing these guidelines makes that the entire project benefits in quality control. +All contributors (including, but not limited to, developers and issue reporters) promise to do their best to adhere to the guidelines in :ref:`Guidelines`. +Everyone is encouraged to politely and constructively point out guidelines violations to others. +Actively enforcing these guidelines makes that the entire project benefits in quality control. Code of Conduct =============== diff --git a/docs/source/guidelines/development.rst b/docs/source/guidelines/development.rst index 9e501d4e5fe..6fafcbdae68 100644 --- a/docs/source/guidelines/development.rst +++ b/docs/source/guidelines/development.rst @@ -4,14 +4,18 @@ Development Code ==== -We strive to keep the code compatible with the Python versions used in Debian Stable and the last two Ubuntu LTS releases. As of writing, these are Python 3.8, 3.9, and 3.10. +We strive to keep the code compatible with the Python versions used in Debian Stable and the last two Ubuntu LTS releases. +As of writing, these are Python 3.8, 3.9, and 3.10. -To improve readability and consistency we use the `PEP 8 `_ guidelines. Developers are encouraged to write their code as strictly compliant as possible. +To improve readability and consistency we use the `PEP 8 `_ guidelines. +Developers are encouraged to write their code as strictly compliant as possible. Tools ===== -To make development and validation easier, we adopted :ref:`pre-commit` hooks to automate most of this. This will help identify broken/bad code, improve consistency and save time during code reviews. Some tools and hooks have been adopted for both local development as well as in our CI/CD pipeline as GitHub actions. +To make development and validation easier, we adopted :ref:`pre-commit` hooks to automate most of this. +This will help identify broken/bad code, improve consistency and save time during code reviews. +Some tools and hooks have been adopted for both local development as well as in our CI/CD pipeline as GitHub actions. Some of the tools are: @@ -19,7 +23,8 @@ Some of the tools are: - ``flake8`` for checking code style and programming errors - ``vulture`` for checking dead code -With some tools exceptions are made that differ a bit from the standard configuration such as line lengths, error codes, etc. See the different configuration files. A few more will be implemented later on including: +With some tools exceptions are made that differ a bit from the standard configuration such as line lengths, error codes, etc. +See the different configuration files. A few more will be implemented later on including: - ``pylint`` for checking code (and documentation) style and programming errors and measuring code quality (next to ``flake8``) - ``mypy`` for type checking @@ -32,7 +37,9 @@ For the frontend there are a few more tasks in the CI/CD pipeline: Pre-commit ---------- -Continuous Integration will run several checks as mentioned above, like black and flake8 and more using pre-commit hooks. Any warnings from these checks will cause the Continuous Integration to fail; therefore, it is helpful to run the check yourself before submitting code. This can be done by `installing pre-commit `_:: +Continuous Integration will run several checks as mentioned above, like black and flake8 and more using pre-commit hooks. +Any warnings from these checks will cause the Continuous Integration to fail; therefore, it is helpful to run the check yourself before submitting code. +This can be done by `installing pre-commit `_:: pip install pre-commit @@ -40,14 +47,19 @@ and then running:: pre-commit install -from the root directory of a repository. Now all of the checks will be run each time you commit changes without your needing to run each one manually. In addition, using pre-commit will also allow you to more easily remain up-to-date with our code checks as they change. +from the root directory of a repository. Now all of the checks will be run each time you commit changes without your needing to run each one manually. +In addition, using pre-commit will also allow you to more easily remain up-to-date with our code checks as they change. Type Hinting ============ -In this project we use strict type hinting where possible. This should make code easier to read and reason about and easier to use (external) libraries. Also, many modern frameworks today use type hints for e.g. runtime data validation and documentation. Static code analysis tools (including those in an IDE) and type linters can be used to validate typing and improve code qualtity. +In this project we use strict type hinting where possible. +This should make code easier to read and reason about and easier to use (external) libraries. +Also, many modern frameworks today use type hints for e.g. runtime data validation and documentation. +Static code analysis tools (including those in an IDE) and type linters can be used to validate typing and improve code qualtity. -Although we try to provide as much type hinting as possible, it may be harder to use type hints in some contexts because of mismatches between different type linters or external libraries that don't have type hints or typing stubs. Therefore we try to be less strict in e.g. (parts of) Django based applications, but enfore stricter type hinting in more isolated code such as clients and utility functions. +Although we try to provide as much type hinting as possible, it may be harder to use type hints in some contexts because of mismatches between different type linters or external libraries that don't have type hints or typing stubs. +Therefore we try to be less strict in e.g. (parts of) Django based applications, but enfore stricter type hinting in more isolated code such as clients and utility functions. In principle, all independent code (e.g. which does not depend on, or inherit from, external libraries) should be strictly typed. @@ -56,7 +68,10 @@ In practice, this means ``mypy --strict --ignore-missing-imports``; we do not ch Testing ======= -To prevent bugs and regressions, and to evaluate and verify that the products work, we test the codebase. We do this both manually and automated (preferred). Developers are encouraged to follow the `Test Driven Development `_ paradigm (although this is not strictly required). It is obligatory to write unit tests for each bug fix and implemented feature. +To prevent bugs and regressions, and to evaluate and verify that the products work, we test the codebase. +We do this both manually and automated (preferred). +Developers are encouraged to follow the `Test Driven Development `_ paradigm (although this is not strictly required). +It is obligatory to write unit tests for each bug fix and implemented feature. Code should be written in such a way that it is inherently testable. @@ -78,41 +93,53 @@ Unit tests should be: Integration Tests ----------------- -For integration testing the frontend we adopted `Robot Framework `_. It has a simple syntax and many plugins available that should improve our test coverage. +For integration testing the frontend we adopted `Robot Framework `_. +It has a simple syntax and many plugins available that should improve our test coverage. Development Environment ======================= -See the `KAT wiki `_ for the overall installation instructions. In a development context, we strongly recommend to use the Docker setup to test and make changes in the codebase (and not production packages). +See the `KAT wiki `_ for the overall installation instructions. +In a development context, we strongly recommend to use the Docker setup to test and make changes in the codebase (and not production packages). When it comes to development there is no specific IDE that must be used, although many of us would choose PyCharm as the preferred IDE. -``make`` is used for automating several tasks such as building, cloning, pulling changes and more. Developers are encouraged to implement any helper or convenience shell functionality through a ``Makefile``. +``make`` is used for automating several tasks such as building, cloning, pulling changes and more. +Developers are encouraged to implement any helper or convenience shell functionality through a ``Makefile``. Furthermore the different services are containerised using Docker and set up to run with ``docker-compose``. Merge Strategy ============== -**Commits should preferably be squashed** when merging a PR back into the primary branch. This helps to keep the git history clean and easier to digest. Multiple rework commits *may* be submitted (or also squashed together) to highlight the rework and give more transparency. +**Commits should preferably be squashed** when merging a PR back into the primary branch. +This helps to keep the git history clean and easier to digest. +Multiple rework commits *may* be submitted (or also squashed together) to highlight the rework and give more transparency. Branching --------- -In principle, all work-in-progress is based off the ``develop`` branch. Releases are tags on the ``main`` branch. +In principle, all work-in-progress by the core team is based off the ``main`` branch. Releases are tags on the ``main`` branch. +If you are a community contributor, it may be wise to use a release tag as the basis for your work instead of the ``main`` branch. +This is because that branch generally changes rapidly, and may require you to continuously pull and merge all changes into your PR. Reviews ------- -Code and functional reviewers are encouraged to be reasonably strict. **An approval should only be given after serious consideration**. Reviewers should not be tempted to accept "it works" contributions, and should consider whether the changes by the PR will lead to extra refactoring and maintenance down the road. -We believe that writing good, well thought-out code is more important than adding features as quickly as possible. Remember that writing tests and documentation (where necessary) are obligatory. That said, everyone should remember to be polite and constructive in their feedback and comments. +Code and functional reviewers are encouraged to be reasonably strict. **An approval should only be given after serious consideration**. +Reviewers should not be tempted to accept "it works" contributions, and should consider whether the changes by the PR will lead to extra refactoring and maintenance down the road. +We believe that writing good, well thought-out code is more important than adding features as quickly as possible. +Remember that writing tests and documentation (where necessary) are obligatory. +That said, everyone should remember to be polite and constructive in their feedback and comments. -``# noqa`` and/or ``# pylint: disable=error-msg`` may be used sparingly on a per-line basis if the CI encounters a false positive, or if it concerns a code style issue that is non-trivial to fix. Code reviewers are strongly encouraged to be sceptical of this. +``# noqa`` and/or ``# pylint: disable=error-msg`` may be used sparingly on a per-line basis if the CI encounters a false positive, or if it concerns a code style issue that is non-trivial to fix. +Code reviewers are strongly encouraged to be sceptical of this. Code commenting and documentation --------------------------------- Everyone is encouraged to write meaningful comments in their code where necessary, especially in complicated or abstract parts. -`PEP 257 `_ (as checked by ``pydocstyle``) is our preferred way of writing docstrings. Ideally, each public method, class, function, and module has one. +`PEP 257 `_ (as checked by ``pydocstyle``) is our preferred way of writing docstrings. +Ideally, each public method, class, function, and module has one. Using docstrings and type hints everywhere improves the quality of the automatically generated API documentation. @@ -121,7 +148,8 @@ Using docstrings and type hints everywhere improves the quality of the automatic Technical diagrams ================== -We prefer the use of `Mermaid `_ to create (technical) diagrams of things. These are automatically rendered by GitHub and the online Sphinx docs. +We prefer the use of `Mermaid `_ to create (technical) diagrams of things. +These are automatically rendered by GitHub and the online Sphinx docs. Mermaid has support for things like PlantUML and ERD's. diff --git a/docs/source/guidelines/management.rst b/docs/source/guidelines/management.rst index 301b3885f86..f32258b0327 100644 --- a/docs/source/guidelines/management.rst +++ b/docs/source/guidelines/management.rst @@ -1,33 +1,43 @@ Project management ################## -In principle, `all project management is handled in a central board `_. Issues and PRs may be created in the repositories of the KAT modules, as long as they are linked to the central board. All (linked) issues and PR's should be assigned to a status column, and have an assignee so we know who is chiefly responsible / who should take action. Developers are encouraged to use the Review column(s) to get their PR's merged, and to regularly check if they can help review something. +In principle, `all project management is handled in a central board `_. +Issues and PRs may be created in the repositories of the KAT modules, as long as they are linked to the central board. +All (linked) issues and PR's should be assigned to a status column, and have an assignee so we know who is chiefly responsible / who should take action. +Developers are encouraged to use the Review column(s) to get their PR's merged, and to regularly check if they can help review something. Feature Milestones ================== Although we have no fixed release schedules, we focus on a pre-defined list of tasks each iteration/cycle. -At the beginning of each release cycle, we take inventory of which (major) functionality we want to implement. This will most likely be based on cards in the "incoming features and refinements" column on the project board. We agree to prioritize our collective effort on implementing that functionality, although there is always time for bug fixing, testing, and quality control. All tasks that belong to that cycle should have an appropriate milestone label in the project board, and should be moved to the To-Do column. +At the beginning of each release cycle, we take inventory of which (major) functionality we want to implement. +This will most likely be based on cards in the "incoming features and refinements" column on the project board. +We agree to prioritize our collective effort on implementing that functionality, although there is always time for bug fixing, testing, and quality control. +All tasks that belong to that cycle should have an appropriate milestone label in the project board, and should be moved to the To-Do column. A cycle only ends if either: * functionality is no longer required (i.e. changing requirements); -* the changes on ``develop`` are complete, have been approved by QA, and have been released to a production tag. +* the changes on ``main`` are complete, have been approved by QA, and have been released to a production tag. When a cycle has been completed, we hold a quick retrospective to evaluate what we did and did not manage to complete, and any additional problems that we uncovered. Bugs and Feature Requests ========================= -For effective bug reporting and feature requests there are :ref:`Templates`. These should be used and submitted in the coordination repository's `issues board `_. Please make sure to link them to the central project board as an incoming feature/refinement. +For effective bug reporting and feature requests there are :ref:`Templates`. +These should be used and submitted in the coordination repository's `issues board `_. +Please make sure to link them to the central project board as an incoming feature/refinement. Pull Requests ============= -Each unit of work shall be submitted as a pull request using a :ref:`Pull Request Template` and reviewed by at least one developer. The checklist should be completed by a functional and a code reviewer. +Each unit of work shall be submitted as a pull request using a :ref:`Pull Request Template` and reviewed by at least one developer. +The checklist should be completed by a functional and a code reviewer. In-depth content discussions ============================ -All formal discussions about the direction of the project or about significant technical choices should be done through `GitHub Discussions `_. It is important that there is a paper trail about why certain decisions were made, and this is not guaranteed through e.g. Signal or Jitsi. \ No newline at end of file +All formal discussions about the direction of the project or about significant technical choices should be done through `GitHub Discussions `_. +It is important that there is a paper trail about why certain decisions were made, and this is not guaranteed through e.g. Signal or Jitsi. \ No newline at end of file From db99b7c6d7269aa82b3f27ec363b7f3ac91a2706 Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Thu, 29 Dec 2022 13:27:22 +0100 Subject: [PATCH 5/7] Remove logging document --- docs/source/guidelines/index.rst | 1 - docs/source/guidelines/logging.rst | 83 ------------------------------ 2 files changed, 84 deletions(-) delete mode 100644 docs/source/guidelines/logging.rst diff --git a/docs/source/guidelines/index.rst b/docs/source/guidelines/index.rst index 0afac32191f..b15ecf3f739 100644 --- a/docs/source/guidelines/index.rst +++ b/docs/source/guidelines/index.rst @@ -10,5 +10,4 @@ Contains documentation for developers and contributors. management development contributions - logging project_statuses \ No newline at end of file diff --git a/docs/source/guidelines/logging.rst b/docs/source/guidelines/logging.rst deleted file mode 100644 index 3b58446c386..00000000000 --- a/docs/source/guidelines/logging.rst +++ /dev/null @@ -1,83 +0,0 @@ -Logging -####### - -Some general guidelines for how to log within your applications: - -- Log after, not before. This communicates what operation was performed, and what the result was. - -.. code-block:: python - - # Don't do this - log.info("Doing stuff") - some_stuff() - - # Do this - some_stuff() - log.info("Did stuff") - -- Separate parameters and messages. - This will make sure that logs are parseable, searchable, easy to extend, and read. - A python package called `structlog `_ can help with this. - This is one example on how to separate parameters and messages, you can also decide to for instance use a json output: - -.. code-block:: python - - # Don't do this - some_stuff() - log.info("Did stuff with %s", url) - - # Do this - some_stuff() - log.info("Did stuff with %s [url=%s]", url, url) - - -* Use ``FATAL`` when immediate intervention is needed and the system can't - continue running. - -* Distinguish between ``WARNING`` and ``ERROR``. Use ``WARNING`` when you did - something but an error occurred. Use ``ERROR`` when something wasn't done and - went wrong. These severities need attention and fixing. - -* ``INFO`` is for business, ``DEBUG`` is for technology. The ``INFO`` log should look - like a book. - -.. code-block:: text - - INFO | User registered for newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - INFO | Newsletter sent to user. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - INFO | User unsubscribed from newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - -.. code-block:: text - - DEBUG | Saved user to newsletter list. [user_id=3a937a10-da92-49b6-9350-6f2c74bcb34c] - DEBUG | Sent welcome email. [user_id=3a937a10-da92-49b6-9350-6f2c74bcb34c] - INFO | User registered for newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - DEBUG | Started cron job to send newsletter of the day. [subscribers=12345] - INFO | Newsletter sent to user. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - INFO | User unsubscribed from newsletter. [user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"] - -- Design log to be chainable. When using parameters in logging make sure that - it can be aggregated, over multiple logs. E.g. - ``[user_id="3a937a10-da92-49b6-9350-6f2c74bcb34c"]`` allows you to aggregate - logs from multiple logs with the same ``user_id`` parameter. - -- Never log any personal identifiable information, and confidential - information. So no email addresses, names, credit card numbers, etc. - -- Always log in UTC, log in milliseconds. Synchronize your servers with a NTP - daemon. - -.. - - TODO: request guid, see if this is already done by log aggregation of them - used cloud provider. - -.. - - TODO: what always should be present: time, severity, process ID, thread ID, - application identifier, request identifier, (user identifier), and message. - -**Sources** - -- ``_ -- ``_ -- ``_ -- ``_ \ No newline at end of file From f09d45a0466d3b69b2c3f1c055dc84751bfbde88 Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Thu, 29 Dec 2022 13:27:33 +0100 Subject: [PATCH 6/7] Process bug report feedback --- .github/ISSUE_TEMPLATE/bug_report.md | 10 +++++----- docs/source/templates/bug_report_template.md | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index d3219019940..8bd76b68854 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -6,8 +6,7 @@ labels: 'bug' assignees: '' --- -_Please add `bug` and one or more of the following labels to your issue:_ -`frontend backend community dependencies` +_Please add `bug`, the name of any relevant modules (e.g. `rocky`), and any other relevant labels to your issue._ **Describe the bug** A clear and concise description of what the bug is. @@ -26,14 +25,15 @@ A clear and concise description of what you expected to happen. If applicable, add screenshots to help explain your problem. **OpenKAT version** -Should contain at least the commit or release of Rocky. If you think another module may be the cause of the bug, note that module's commit or release here as well. +Note the release tag (and if possible: the installation method) here. +If it concerns an in-development version, note the branch(es) and commit hash(es) here as well. -**Desktop (please complete the following information):** +**Desktop (please complete the following information if relevant):** - OS: [e.g. iOS] - Browser: [e.g. chrome, safari] - Version: [e.g. 22] -**Smartphone (please complete the following information):** +**Smartphone (please complete the following information if relevant):** - Device: [e.g. iPhone6] - OS: [e.g. iOS8.1] - Browser: [e.g. stock browser, safari] diff --git a/docs/source/templates/bug_report_template.md b/docs/source/templates/bug_report_template.md index 99630271e9e..17dea642920 100644 --- a/docs/source/templates/bug_report_template.md +++ b/docs/source/templates/bug_report_template.md @@ -9,8 +9,7 @@ labels: 'bug' assignees: '' --- -_Please add `bug` and one or more of the following labels to your issue:_ -`frontend backend community dependencies` +_Please add `bug`, the name of any relevant modules (e.g. `rocky`), and any other relevant labels to your issue._ **Describe the bug** A clear and concise description of what the bug is. @@ -29,14 +28,15 @@ A clear and concise description of what you expected to happen. If applicable, add screenshots to help explain your problem. **OpenKAT version** -Should contain at least the commit or release of Rocky. If you think another module may be the cause of the bug, note that module's commit or release here as well. +Note the release tag (and if possible: the installation method) here. +If it concerns an in-development version, note the branch(es) and commit hash(es) here as well. -**Desktop (please complete the following information):** +**Desktop (please complete the following information if relevant):** - OS: [e.g. iOS] - Browser: [e.g. chrome, safari] - Version: [e.g. 22] -**Smartphone (please complete the following information):** +**Smartphone (please complete the following information if relevant):** - Device: [e.g. iPhone6] - OS: [e.g. iOS8.1] - Browser: [e.g. stock browser, safari] From 34f3e6d20b418b3ab4fae2244cd6b7e73045014d Mon Sep 17 00:00:00 2001 From: Patrick Darwinkel Date: Thu, 29 Dec 2022 13:29:07 +0100 Subject: [PATCH 7/7] Fix trailing whitespace --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- docs/source/templates/bug_report_template.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 8bd76b68854..8f31247da6a 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -25,7 +25,7 @@ A clear and concise description of what you expected to happen. If applicable, add screenshots to help explain your problem. **OpenKAT version** -Note the release tag (and if possible: the installation method) here. +Note the release tag (and if possible: the installation method) here. If it concerns an in-development version, note the branch(es) and commit hash(es) here as well. **Desktop (please complete the following information if relevant):** diff --git a/docs/source/templates/bug_report_template.md b/docs/source/templates/bug_report_template.md index 17dea642920..305bc16e8a4 100644 --- a/docs/source/templates/bug_report_template.md +++ b/docs/source/templates/bug_report_template.md @@ -28,7 +28,7 @@ A clear and concise description of what you expected to happen. If applicable, add screenshots to help explain your problem. **OpenKAT version** -Note the release tag (and if possible: the installation method) here. +Note the release tag (and if possible: the installation method) here. If it concerns an in-development version, note the branch(es) and commit hash(es) here as well. **Desktop (please complete the following information if relevant):**