Skip to content

Commit 6f2fcb3

Browse files
grugnagrugna
authored
Merge pull request #57 from chicagopcdc/pcdc_dev
Pcdc dev
2 parents 369464c + 6318795 commit 6f2fcb3

19 files changed

+113
-148
lines changed

‎.github/workflows/ci.yaml

+9-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ concurrency:
99
group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}'
1010
cancel-in-progress: true
1111

12-
1312
jobs:
1413
Security:
1514
name: Security Pipeline
@@ -18,7 +17,15 @@ jobs:
1817
python-poetry: 'false'
1918
secrets: inherit
2019

21-
ci:
20+
UnitTest:
21+
name: Python Unit Test with Postgres
22+
uses: uc-cdis/.github/.github/workflows/python_unit_test.yaml@master
23+
with:
24+
setup-script: 'tests/ci_setup.sh'
25+
test-script: 'tests/ci_commands_script.sh'
26+
python-version: '3.9'
27+
28+
ImageBuildAndPush:
2229
name: Build Image and Push
2330
uses: uc-cdis/.github/.github/workflows/image_build_push.yaml@master
2431
secrets:

‎.github/workflows/wool.yaml

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
on:
2+
pull_request
3+
4+
name: Wool
5+
6+
jobs:
7+
runWool:
8+
name: Run black
9+
runs-on: ubuntu-latest
10+
steps:
11+
- uses: actions/checkout@master
12+
13+
- uses: uc-cdis/wool@master
14+
env:
15+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

‎.secrets.baseline

+2-2
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@
109109
"filename": "bin/settings.py",
110110
"hashed_secret": "347cd9c53ff77d41a7b22aa56c7b4efaf54658e3",
111111
"is_verified": false,
112-
"line_number": 44
112+
"line_number": 43
113113
}
114114
],
115115
"docs/local_dev_environment.md": [
@@ -327,5 +327,5 @@
327327
}
328328
]
329329
},
330-
"generated_at": "2023-05-10T15:28:32Z"
330+
"generated_at": "2023-10-06T21:59:13Z"
331331
}

‎.travis.yml

-48
This file was deleted.

‎Dockerfile

+1-3
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@
1111
# "db_database": "",
1212
# "gdcapi_secret_key": "",
1313
# "indexd_password": "",
14-
# "hostname": "",
15-
# "oauth2_client_id": "",
16-
# "oauth2_client_secret": ""
14+
# "hostname": ""
1715
# }
1816
# - Build the image: `docker build . -t sheepdog -f Dockerfile`
1917
# - Run: `docker run -v /full/path/to/creds.json:/var/www/sheepdog/creds.json -p 81:80 sheepdog`

‎README.md

+49
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,52 @@ see the README in that folder for more details.
5656

5757
## Submitter ID
5858
Sheepdog requires the `submitter_id` to be unique per node per project. It means that, the `submitter_id` of all `case` nodes must be unique per project. This constraint was technically enforced by the unique index of `(project_id, submitter_id)` in every node table.
59+
60+
## Local Test Run Using CI Scripts
61+
62+
If you want to locally replicate what GH Actions is doing more closely, follow
63+
these steps.
64+
65+
Ensure you've run `poetry install`.
66+
67+
Ensure you have Postgresql 13 set up and running.
68+
69+
Ensure there is a postgres user `postgres` *and* `test` setup with password `test`:
70+
71+
```
72+
CREATE USER postgres WITH PASSWORD 'test';
73+
```
74+
75+
Then run:
76+
77+
```bash
78+
bash tests/ci_setup.sh
79+
```
80+
81+
If the above fails due to postgres errors, your postgresql setup may need some
82+
fixing, it should finish with the following:
83+
84+
```
85+
Setting up test database
86+
Dropping old test data
87+
WARNING:root:Unable to drop test data:(psycopg2.errors.InvalidCatalogName) database "sheepdog_automated_test" does not exist
88+
89+
[SQL: DROP DATABASE "sheepdog_automated_test"]
90+
(Background on this error at: http://sqlalche.me/e/13/f405)
91+
Creating tables in test database
92+
Creating indexes
93+
writing RSA key
94+
```
95+
96+
The WARNING will show up the first time you run this, it's safe to ignore.
97+
98+
That sets up the database so if you run into postgres errors, you'll want to
99+
double check your postgres setup.
100+
101+
After that you can run unit tests with:
102+
103+
```bash
104+
bash tests/ci_commands_script.sh
105+
```
106+
107+
> You can see more detailed information on local dev setup in the docs/local_dev_environment.md

‎bin/settings.py

+1-18
Original file line numberDiff line numberDiff line change
@@ -46,25 +46,8 @@ def load_json(file_name):
4646
for key in ["fence_username", "fence_password", "fence_host", "fence_database"]
4747
]
4848
)
49-
config["OIDC_ISSUER"] = "https://%s/user" % conf_data["hostname"]
5049

51-
config["OAUTH2"] = {
52-
"client_id": conf_data.get("oauth2_client_id", "{{oauth2_client_id}}"),
53-
"client_secret": conf_data.get("oauth2_client_secret", "{{oauth2_client_secret}}"),
54-
"api_base_url": "https://%s/user/" % conf_data["hostname"],
55-
"authorize_url": "https://%s/user/oauth2/authorize" % conf_data["hostname"],
56-
"access_token_url": "https://%s/user/oauth2/token" % conf_data["hostname"],
57-
"refresh_token_url": "https://%s/user/oauth2/token" % conf_data["hostname"],
58-
"client_kwargs": {
59-
"redirect_uri": "https://%s/api/v0/oauth2/authorize" % conf_data["hostname"],
60-
"scope": "openid data user",
61-
},
62-
# deprecated key values, should be removed after all commons use new oidc
63-
"internal_oauth_provider": "http://fence-service/oauth2/",
64-
"oauth_provider": "https://%s/user/oauth2/" % conf_data["hostname"],
65-
"redirect_uri": "https://%s/api/v0/oauth2/authorize" % conf_data["hostname"],
66-
}
67-
config["USER_API"] = config["OIDC_ISSUER"] # for use by authutils
50+
config["USER_API"] = "https://%s/user" % conf_data["hostname"] # for use by authutils
6851
# use the USER_API URL instead of the public issuer URL to accquire JWT keys
6952
config["FORCE_ISSUER"] = True
7053
config["DICTIONARY_URL"] = environ.get(

‎bin/setup_psqlgraph.py

-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@ def create_indexes(host, port, user, password, database, use_ssl=False):
195195

196196

197197
if __name__ == "__main__":
198-
199198
parser = argparse.ArgumentParser()
200199
parser.add_argument(
201200
"--host", type=str, action="store", default="localhost", help="psql-server host"

‎poetry.lock

+1-17
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pyproject.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "sheepdog"
3-
version = "5.1.0"
3+
version = "5.1.1"
44
description = "Gen3 Submission Service"
55
authors = ["CTDS UChicago <cdis@uchicago.edu>"]
66
license = "Apache-2.0"
@@ -50,7 +50,6 @@ pytest = ">=4.6.5"
5050
pytest-cov = ">=2.5.1"
5151
requests_mock = ">=1.4.0"
5252
coverage = ">=5.3.0"
53-
cdis_oauth2client = ">=1.1.0"
5453
mock = ">=1.0.1"
5554
pytest-flask = ">=1.2.0"
5655
moto = ">=0.4.5"

‎sheepdog/api.py

+3-14
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
from flask import Flask, jsonify
88
from psqlgraph import PsqlGraphDriver
99

10-
from authutils.oauth2 import client as oauth2_client
11-
from authutils.oauth2.client import blueprint as oauth2_blueprint
1210
from authutils import AuthError
1311
from cdispyutils.log import get_handler
1412
from cdispyutils.uwsgi import setup_user_harakiri
@@ -56,20 +54,13 @@ def app_register_blueprints(app):
5654
models.init(md)
5755
validators.init(vd)
5856

59-
# register each blueprint twice (at `/` and at `/v0/`). Flask requires the
60-
# blueprint names to be unique, so rename them before registering the 2nd time
61-
v0 = "/v0"
62-
57+
# register the blueprint twice (at `/` and at `/v0/`). Flask requires the
58+
# blueprint names to be unique, so rename it before registering the 2nd time
6359
sheepdog_blueprint = sheepdog.create_blueprint("submission")
64-
app.register_blueprint(sheepdog_blueprint, url_prefix=v0 + "/submission")
60+
app.register_blueprint(sheepdog_blueprint, url_prefix="/v0/submission")
6561
sheepdog_blueprint.name += "_legacy"
6662
app.register_blueprint(sheepdog_blueprint, url_prefix="/submission")
6763

68-
# TODO we may be able to deprecate the oauth2 endpoints?
69-
app.register_blueprint(oauth2_blueprint.blueprint, url_prefix=v0 + "/oauth2")
70-
oauth2_blueprint.blueprint.name += "_legacy"
71-
app.register_blueprint(oauth2_blueprint.blueprint, url_prefix="/oauth2")
72-
7364

7465
def db_init(app):
7566
app.logger.info("Initializing PsqlGraph driver")
@@ -90,8 +81,6 @@ def db_init(app):
9081
if app.config.get("AUTO_MIGRATE_DATABASE"):
9182
migrate_database(app)
9283

93-
app.oauth_client = oauth2_client.OAuthClient(**app.config["OAUTH2"])
94-
9584
app.logger.info("Initializing index client")
9685
app.index_client = IndexClient(
9786
app.config["INDEX_CLIENT"]["host"],

‎sheepdog/dev_settings.py

-15
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,6 @@
7474
)
7575

7676
USER_API = "http://localhost/user/"
77-
OIDC_ISSUER = "http://localhost"
78-
OAUTH2 = {
79-
"client_id": os.environ.get("CDIS_GDCAPI_CLIENT_ID"),
80-
"client_secret": os.environ.get("CDIS_GDCAPI_CLIENT_SECRET"),
81-
"api_base_url": USER_API,
82-
"authorize_url": "http://localhost/user/oauth2/authorize",
83-
"access_token_url": "http://localhost/user/oauth2/token",
84-
"refresh_token_url": "http://localhost/user/oauth2/token",
85-
"client_kwargs": {
86-
"redirect_uri": os.environ.get(
87-
"CDIS_GDCAPI_OAUTH_REDIRECT", "http://localhost/api/v0/oauth2/authorize"
88-
),
89-
"scope": "openid data user",
90-
},
91-
}
9277

9378
SESSION_COOKIE_NAME = "sheepdog_session"
9479
# verify project existence in dbgap or not

‎sheepdog/test_settings.py

-7
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,6 @@
3838

3939
FLASK_SECRET_KEY = "flask_test_key" # nosec
4040

41-
OAUTH2 = {
42-
"client_id": "",
43-
"client_secret": "",
44-
"oauth_provider": "",
45-
"redirect_uri": "",
46-
}
47-
4841
USER_API = "localhost"
4942
BASE_URL = "localhost"
5043

‎tests/ci_commands_script.sh

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env bash
2+
3+
# since this whole thing is run as a bash {{this script}}, only the last pytest
4+
# command controls the exit code. We actually want to exit early if something fails
5+
set -e
6+
7+
# datadict and datadictwithobjid tests must run separately to allow
8+
# loading different datamodels
9+
poetry run pytest -vv --cov=sheepdog --cov-report xml tests/integration/datadict
10+
poetry run pytest -vv --cov=sheepdog --cov-report xml --cov-append tests/integration/datadictwithobjid
11+
poetry run pytest -vv --cov=sheepdog --cov-report xml --cov-append tests/unit

‎tests/ci_setup.sh

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/usr/bin/env bash
2+
poetry run python bin/setup_test_database.py
3+
mkdir -p tests/integration/resources/keys
4+
cd tests/integration/resources/keys
5+
openssl genrsa -out test_private_key.pem 2048
6+
openssl rsa -in test_private_key.pem -pubout -out test_public_key.pem
7+
cd -

‎tests/integration/README.md

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
TODO: Why are we integration testing alongside unit tests like this? We need to migrate
2+
these tests elsewhere b/c it's not really integration testing if we're
3+
configuring postgres exactly the way it expects in the CI running this.
4+
Since this CI setup is completely separate from how we run postgres in
5+
production, most of these tests are pointless.
6+
7+
As of 13 SEP 23 I'm ripping out some of this to enable the migration
8+
to Github Actions from Travis CI.
9+
10+
- Alex

0 commit comments

Comments
 (0)