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

Add /api/ctf/start and /api/ctf/stop routes #5

Merged
merged 32 commits into from
Nov 20, 2023
Merged

Conversation

mradigen
Copy link
Member

This PR adds the following:

  • Starting and stopping of Docker containers
  • Allow for response messages to be configured from config.py
  • Tortoise Models for Container and CTF
  • Restructure DB to run asynchronously with FastAPI without multiple event loops
  • Update docs

I could not add tests cause I was unable to figure out how to run asynchronous tests with pytest.

@mradigen
Copy link
Member Author

Not sure why mypy is considering image_config["ports"] as a str when it is a dict. Additionally it seems that the docker module doesn't have a py.typed, hence mypy throws an error for that.

src/pwncore/db.py Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/start.py Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/start.py Outdated Show resolved Hide resolved
src/pwncore/config.py Outdated Show resolved Hide resolved
src/pwncore/config.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/start.py Outdated Show resolved Hide resolved
src/pwncore/db.py Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/start.py Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/start.py Outdated Show resolved Hide resolved
WizzyGeek added a commit to WizzyGeek/pwncore that referenced this pull request Nov 15, 2023
Missing: Config value for constraint
Waiting-On: lugvitc#5 369802e
@WizzyGeek WizzyGeek mentioned this pull request Nov 16, 2023
pyproject.toml Show resolved Hide resolved
src/pwncore/container.py Outdated Show resolved Hide resolved
src/pwncore/routes/ctf/__init__.py Outdated Show resolved Hide resolved
@WizzyGeek
Copy link
Contributor

Also run, tox run -e black
In order to fix the type errors you will have to jump through some hoops, it should become easier to pass through after #7

Copy link
Contributor

@WizzyGeek WizzyGeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, except the docker client being initialised before the app runs, in the future it might break if someone decides to change the eventloop, but we can ignore that if it works rn

@WizzyGeek
Copy link
Contributor

oh yah, get atleast 3.11 to pass before merging, for any type issues you can't resolve after #7 lemme know I will do a PR to your fork

db_url=config.db_url,
modules={"models": ["pwncore.models"]}
)
await Tortoise.generate_schemas()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need safe=True here?

"""
return {"status": "CTF started"}
if config.development:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just mark this for future removal with a comment

@nexxeln nexxeln merged commit 76692d6 into lugvitc:master Nov 20, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants