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

Fix firewall #2038

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix firewall #2038

wants to merge 2 commits into from

Conversation

Salamandar
Copy link
Contributor

for protocol, ports in firewall.config.items():
for port, info in ports.items():
for protocol in ["tcp", "udp"]:
for port, info in firewall.config[protocol].items():
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm that kind of stuff makes me paranoid about wether or not the expected keys does exist in the file ... We've seen in the past situation where yaml file (app settings) became empty because at some point yunohost tries to update the file, but there's no space left on the server (eg because a backup went crazy and filled the disk) and then somehow the file still exists but is empty ...

I suppose one day or another, we'll also have users randomly editing the file with typos so I would also make sure that somehow the expected keys (tcp, udp, ..?) do exist in the file

Can we iterate on the def read(...) method for YunoFirewall adding failsafes such as returning a dict with the appropriate structure ? 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we iterate on the def read(...) method for YunoFirewall adding failsafes such as returning a dict with the appropriate structure ? 😬

You mean just to ensure the dict in memory is valid ? Or to return an error if the file read doesn't comply to the expected structure ?

Copy link
Member

Choose a reason for hiding this comment

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

Hmyeah I'd say return an error (or a warning and then some failsafe values) if for some reason we're unable to read the file or it doesn't have the expected structure

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.

2 participants