Skip to content

Commit

Permalink
Fix SonarQube warnings and running tests with poetry (#253)
Browse files Browse the repository at this point in the history
* Fix running tests with poetry

* Disable some SonarQube warnings

* Fix type hint for lists with at least one item

* Drop using deprecated datetime.utcnow

* Add support for CSRF protection
  • Loading branch information
hluk authored Jan 2, 2025
1 parent 3694457 commit a5b982f
Show file tree
Hide file tree
Showing 14 changed files with 852 additions and 800 deletions.
10 changes: 1 addition & 9 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
[run]
branch = True
include =
waiverdb/*

[report]
fail_under = 75
exclude_lines =
pragma: no cover
if __name__ == .__main__.:
omit =
waiverdb/wsgi.py
waiverdb/migrations/*
fail_under = 80
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ dist
docker/home/
.idea
.coverage
coverage.xml
report.xml
4 changes: 2 additions & 2 deletions docker/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import os

DATABASE_URI = 'postgresql+psycopg2://waiverdb:waiverdb@waiverdb-db:5433/waiverdb'
DATABASE_URI = 'postgresql+psycopg2://waiverdb:waiverdb@waiverdb-db:5433/waiverdb' # NOSONAR

if os.getenv('TEST') == 'true':
DATABASE_URI += '_test'
Expand All @@ -10,7 +10,7 @@
AUTH_METHOD = 'dummy'
MESSAGE_BUS_PUBLISH = False
SUPERUSERS = ['dummy']
RESULTSDB_API_URL = 'http://resultsdb:5001/api/v2.0'
RESULTSDB_API_URL = 'http://resultsdb:5001/api/v2.0' # NOSONAR

AUTH_METHODS = ['OIDC']
OIDC_CLIENT_SECRETS = '/etc/secret/client_secrets.json'
Expand Down
1,566 changes: 800 additions & 766 deletions poetry.lock

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,20 @@ include = [
]

[tool.poetry.dependencies]
python = ">=3.8,<3.13"
python = ">=3.9,<3.13"

# https://github.com/lepture/authlib/pull/662
# fix authlib to use correct auth method for token introspection
# fixes submitting waivers with Bodhi. remove this when there is
# a fixed stable authlib release
authlib = {git = "https://github.com/AdamWill/authlib.git", branch = "oauth2-fix-introspect-endpoint"}

flask = "^2.2.5"
flask-oidc = "^2.2.0"
flask-session = ">=0.6.0"
flask = "^3.1.0"
flask-oidc = "^2.2.2"
flask-session = ">=0.8.0"
Flask-SQLAlchemy = "^3.1.1"
Flask-Cors = "^5.0.0"
Flask-Migrate = "^4.0.5"
itsdangerous = {version = "==2.0.1", optional = true}
Flask-Migrate = "^4.0.7"

gssapi = "^1.8.3"
python-ldap = "^3.4.3"
Expand Down Expand Up @@ -97,6 +96,7 @@ markupsafe = {version = "==2.1.5", optional = true}
pydantic = "^2.9.2"
Flask-Pydantic = "^0.12.0"
flask-restx = "^1.3.0"
flask-wtf = "^1.2.2"

[tool.poetry.extras]
test = [
Expand Down
15 changes: 9 additions & 6 deletions tests/test_api_v10.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0+

from datetime import datetime, timedelta
from datetime import timedelta
import json

import pytest
Expand All @@ -13,6 +13,7 @@
from .utils import create_waiver
from waiverdb import __version__
from waiverdb.models import Waiver
from waiverdb.models.waivers import utcnow_naive
from waiverdb.api_v1 import get_resultsdb_result, _authorization_warning


Expand Down Expand Up @@ -608,9 +609,10 @@ def test_filtering_waivers_by_username(client, session):


def test_filtering_waivers_by_since(client, session):
before1 = (datetime.utcnow() - timedelta(seconds=100)).isoformat()
before2 = (datetime.utcnow() - timedelta(seconds=99)).isoformat()
after = (datetime.utcnow() + timedelta(seconds=100)).isoformat()
now = utcnow_naive()
before1 = (now - timedelta(seconds=100)).isoformat()
before2 = (now - timedelta(seconds=99)).isoformat()
after = (now + timedelta(seconds=100)).isoformat()
create_waiver(session, subject_type='koji_build', subject_identifier='glibc-2.26-27.fc27',
testcase='testcase1', username='foo', product_version='foo-1')
r = client.get(f'/api/v1.0/waivers/?since={before1}')
Expand Down Expand Up @@ -639,19 +641,20 @@ def test_filtering_waivers_by_since(client, session):


def test_filtering_waivers_by_malformed_since(client, session):
now = utcnow_naive()
r = client.get('/api/v1.0/waivers/?since=123')
res_data = json.loads(r.get_data(as_text=True))
assert r.status_code == 400
assert res_data['message']['since'] == \
"Invalid isoformat string: '123'"

r = client.get(f'/api/v1.0/waivers/?since={datetime.utcnow().isoformat()},badend')
r = client.get(f'/api/v1.0/waivers/?since={now.isoformat()},badend')
res_data = json.loads(r.get_data(as_text=True))
assert r.status_code == 400
assert res_data['message']['since'] == \
"Invalid isoformat string: 'badend'"

r = client.get(f'/api/v1.0/waivers/?since={datetime.utcnow().isoformat()},too,many,commas')
r = client.get(f'/api/v1.0/waivers/?since={now.isoformat()},too,many,commas')
res_data = json.loads(r.get_data(as_text=True))
assert r.status_code == 400
assert res_data['message']['since'] == \
Expand Down
17 changes: 13 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ requires =
poetry

[testenv]
basepython = python3.12
allowlist_externals = poetry
skip_install = true
no_package = true
Expand All @@ -12,18 +13,18 @@ no_package = true
commands_pre =
poetry install --only=main --extras=test
commands =
pytest \
poetry run pytest \
--cov-reset \
--cov-config=.coveragerc \
--cov=waiverdb \
--cov=. \
--cov-report=term \
--cov-report=xml \
--cov-report=html \
--junit-xml=report.xml \
--ignore=functional-tests \
{posargs}

[testenv:functional]
skip_install = true
commands_pre =
poetry install --only=main --extras=test --extras=functional-test
setenv =
Expand All @@ -35,9 +36,17 @@ allowlist_externals =
docker/healthcheck.sh
commands =
docker/healthcheck.sh
pytest \
poetry run pytest \
--cov-reset \
--cov-config=.coveragerc \
--cov=. \
--cov-report=term \
--cov-report=xml \
--cov-report=html \
--junit-xml=report.xml \
--driver=Firefox \
functional-tests/ \
tests/ \
{posargs}

[testenv:bandit]
Expand Down
2 changes: 1 addition & 1 deletion waiverdb-messages/waiverdb_messages/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from fedora_messaging import message


SCHEMA_URL = "http://fedoraproject.org/message-schema/"
SCHEMA_URL = "https://fedoraproject.org/message-schema/"

WAIVERDB_MESSAGE_SCHEMA = {
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion waiverdb-messages/waiverdb_messages/waiverdb_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class WaiverDBMessageV1(WaiverDBMessage):

body_schema = {
"id": SCHEMA_URL + topic,
"$schema": "http://json-schema.org/draft-04/schema#",
"$schema": "https://json-schema.org/draft-04/schema#",
"description": "Schema for waiverdb messages",
"type": "object",
"properties": {
Expand Down
2 changes: 1 addition & 1 deletion waiverdb/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
Blueprint,
Response,
current_app,
escape,
redirect,
render_template,
request,
Expand All @@ -16,6 +15,7 @@
from flask_oidc import OpenIDConnect
from flask_pydantic import validate
from flask_restx import Resource, Api, marshal_with, marshal
from markupsafe import escape
from werkzeug.exceptions import (
BadRequest,
Forbidden,
Expand Down
4 changes: 4 additions & 0 deletions waiverdb/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from flask_migrate import Migrate
from flask_pydantic.exceptions import ValidationError
from flask_session import Session
from flask_wtf.csrf import CSRFProtect
from sqlalchemy import event, text
from sqlalchemy.exc import ProgrammingError
import requests
Expand All @@ -25,6 +26,8 @@
from werkzeug.exceptions import default_exceptions
from waiverdb.monitor import db_hook_event_listeners

csrf = CSRFProtect()


def enable_cors(app):
"""
Expand Down Expand Up @@ -89,6 +92,7 @@ def populate_db_config(app):
# applicaiton factory http://flask.pocoo.org/docs/0.12/patterns/appfactories/
def create_app(config_obj=None, create_session=True):
app = Flask(__name__)
csrf.init_app(app)

if config_obj:
app.config.from_object(config_obj)
Expand Down
2 changes: 2 additions & 0 deletions waiverdb/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class Config(object):
},
}

WTF_CSRF_ENABLED = False


class ProductionConfig(Config):
DEBUG = False
Expand Down
7 changes: 4 additions & 3 deletions waiverdb/models/requests.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: LGPL-2.0-or-later
import annotated_types
from typing import Annotated, List, Optional, Tuple, Union
from datetime import datetime

from pydantic import BaseModel, Field, StringConstraints, RootModel, conlist, model_validator
from pydantic import BaseModel, Field, StringConstraints, RootModel, model_validator
from werkzeug.exceptions import BadRequest


Expand Down Expand Up @@ -75,7 +76,7 @@ def subject_must_be_defined(self):
)


CreateWaiverList = RootModel[Union[CreateWaiver, conlist(CreateWaiver, min_length=1)]]
CreateWaiverList = RootModel[Union[CreateWaiver, List[CreateWaiver]]]


class GetWaivers(BaseModel):
Expand Down Expand Up @@ -109,7 +110,7 @@ class WaiverFilter(BaseModel):


class FilterWaivers(BaseModel):
filters: conlist(WaiverFilter, min_length=1)
filters: Annotated[List[WaiverFilter], annotated_types.Len(min_length=1)]
include_obsolete: bool = False


Expand Down
7 changes: 6 additions & 1 deletion waiverdb/models/waivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
from .requests import TestSubject, TestResult


def utcnow_naive():
"""Returns current UTC date/time without the timezone info."""
return datetime.datetime.now(datetime.UTC).replace(tzinfo=None)


def subject_dict_to_type_identifier(subject: TestSubject):
"""
WaiverDB < 0.11 accepted an arbitrary dict for the 'subject'.
Expand Down Expand Up @@ -54,7 +59,7 @@ class Waiver(db.Model):
waived = db.Column(db.Boolean, nullable=False, default=False)
scenario = db.Column(db.String(255), nullable=True)
comment = db.Column(db.Text)
timestamp = db.Column(db.DateTime, default=datetime.datetime.utcnow)
timestamp = db.Column(db.DateTime, default=utcnow_naive)
__table_args__ = (
db.Index('ix_waiver_subject_type_identifier', subject_type, subject_identifier),
)
Expand Down

0 comments on commit a5b982f

Please sign in to comment.