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 support for reading hosts/subnets from a file #130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

urhot
Copy link

@urhot urhot commented Apr 20, 2023

This pull requests adds new argument to allow reading the hosts/subnets from a file.

This is sort of a duplicate of another pull request #42, but with a different approach. I leave it up to the author to decide if he wants to go forward with either of these.

Usage

New parameter: --routes-file <filename> or -f <filename>

For example:

vpn-slice --routes-file=/home/bob/.vpn-routes

The routes file should have one host per line, with optional comments:

# This is a comment
192.0.2.1
# Another comment
test1.example.com # one more comment
test2.example.com

To be considered

  • Are the argument names good? Is it acceptable to dedicate -f for this feature?

Differences to the implementation in #42

  • The provided file is not called a "config file", but just a list of hosts
  • Does not use fromfile_prefix_chars, but instead a simple custom parser that knows how to handle comments

New parameter: --routes-file <filename> or -f <filename>

Syntax is one host per line. Single-line or postfix comments are
handled and ignored.

For example:

1.2.3.4
host1.example.com # This is one more comment
host2.example.com
@nstp11
Copy link

nstp11 commented Aug 2, 2023

Hi all, is there any update on this case?
Will we add this enhancement?

Regards

@oliveirarleo
Copy link

Hi guys.
I see the repo is being maintained. Any reason to not adopt this? in my case the routes grew so much the command cannot be run anymore.

@@ -444,6 +465,7 @@ def parse_args_and_env(args=None, environ=os.environ):
p = argparse.ArgumentParser()
p.add_argument('routes', nargs='*', type=net_or_host_param, help='List of VPN-internal hostnames, included subnets (e.g. 192.168.0.0/24), excluded subnets (e.g. %%8.0.0.0/8), or aliases (e.g. host1=192.168.1.2) to add to routing and /etc/hosts.')
g = p.add_argument_group('Subprocess options')
p.add_argument('-f', '--routes-file', default=None, type=file_path_param, help='Path to List of VPN-internal hostnames, subnets (e.g. 192.168.0.0/24), or aliases (e.g. host1=192.168.1.2) to add to routing and /etc/hosts.')

Choose a reason for hiding this comment

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

FileType might be better for handling file as arguments, like this one:

Suggested change
p.add_argument('-f', '--routes-file', default=None, type=file_path_param, help='Path to List of VPN-internal hostnames, subnets (e.g. 192.168.0.0/24), or aliases (e.g. host1=192.168.1.2) to add to routing and /etc/hosts.')
p.add_argument('-f', '--routes-file', default=None, type=argparse.FileType('r'), help='Path to List of VPN-internal hostnames, subnets (e.g. 192.168.0.0/24), or aliases (e.g. host1=192.168.1.2) to add to routing and /etc/hosts.')

Comment on lines +120 to +121
lines = [x.strip() for x in f.readlines()]
for line in lines:

Choose a reason for hiding this comment

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

No need to read the entire file in memory beforehand, you can just iterate over the lines like this:

Suggested change
lines = [x.strip() for x in f.readlines()]
for line in lines:
for line in f:
line = line.strip()

lines = [x.strip() for x in f.readlines()]
for line in lines:
# ignore empty lines
if not len(line.strip()):

Choose a reason for hiding this comment

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

Redundant strip:

Suggested change
if not len(line.strip()):
if not line:

if not len(line.strip()):
continue
# ignore comment lines
if line.strip().startswith('#'):

Choose a reason for hiding this comment

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

Redundant strip:

Suggested change
if line.strip().startswith('#'):
if line.startswith('#'):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants