Skip to content

Commit

Permalink
vagrant: Improve network interfaces support
Browse files Browse the repository at this point in the history
Current code doesn't ensure that the network name is valid. According
to current Vagrant documentation, it can only be:
- public_network
- private_network
- forwarded_port

Moreover, the same documentation is saying that public network configuration
may be reduced to:

config.vm.network "public_network"

So add needed code to support interfaces without any options.

On the test side:
- modify the network test to check that the generated Vagrantfile is correct
when no option is specified
- add a test to ensure invalid network name are caught.

Fixes: #99
Signed-off-by: Arnaud Patard <[email protected]>
  • Loading branch information
apatard committed Jun 26, 2023
1 parent cc94045 commit 8be3b3e
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 5 deletions.
13 changes: 10 additions & 3 deletions src/molecule_plugins/vagrant/modules/vagrant.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
See doc/source/configuration.rst
"""

VAGRANT_VALID_NETNAMES = ["private_network", "public_network", "forwarded_port"]
VAGRANTFILE_TEMPLATE = """
{%- macro ruby_format(value) -%}
{%- if value is boolean -%}
Expand Down Expand Up @@ -244,7 +245,7 @@
# Network
##
{% for n in instance.networks %}
c.vm.network "{{ n.name }}", {{ dict2args(n.options) }}
c.vm.network "{{ n.name }}"{% if 'options' in n %}, {{ dict2args(n.options) }}{% endif %}
{% endfor %}
##
Expand Down Expand Up @@ -617,10 +618,16 @@ def _get_instance_vagrant_config_dict(self, instance):
networks = []
if "interfaces" in instance:
for iface in instance["interfaces"]:
net_name = iface["network_name"]
if net_name not in VAGRANT_VALID_NETNAMES:
self._module.fail_json(
msg=f"Invalid network_name value {net_name}."
)
net = {}
net["name"] = iface["network_name"]
net["name"] = net_name
iface.pop("network_name")
net["options"] = iface
if len(iface) > 0:
net["options"] = iface
networks.append(net)

# compat
Expand Down
16 changes: 16 additions & 0 deletions test/vagrant/functional/test_func.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,22 @@ def test_invalid_settings(temp_dir):
not is_vagrant_supported(),
reason="vagrant not supported on this machine",
)
def test_invalid_network_name(temp_dir):
scenario_directory = os.path.join(
os.path.dirname(util.abs_path(__file__)), os.path.pardir, "scenarios"
)

with change_dir_to(scenario_directory):
cmd = ["molecule", "create", "--scenario-name", "invalid_net"]
result = run_command(cmd)
assert result.returncode == 2

assert "Invalid network_name value my_network." in result.stdout


@pytest.mark.skipif(
not is_vagrant_supported(), reason="vagrant not supported on this machine"
)
@pytest.mark.parametrize(
"scenario",
[
Expand Down
10 changes: 10 additions & 0 deletions test/vagrant/scenarios/molecule/invalid_net/converge.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
- name: Converge
hosts: all
gather_facts: false
become: true
tasks:
- name: Sample task # noqa command-instead-of-shell
ansible.builtin.shell:
cmd: uname
changed_when: false
11 changes: 11 additions & 0 deletions test/vagrant/scenarios/molecule/invalid_net/molecule.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
dependency:
name: galaxy
driver:
name: vagrant
platforms:
- name: instance/foo
interfaces:
- network_name: my_network
provisioner:
name: ansible
1 change: 1 addition & 0 deletions test/vagrant/scenarios/molecule/network/molecule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ platforms:
- network_name: private_network
ip: 192.168.56.4
auto_config: true
- network_name: public_network
provisioner:
name: ansible
verifier:
Expand Down
4 changes: 2 additions & 2 deletions test/vagrant/scenarios/molecule/network/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
gather_subset:
- network
tasks:
- name: Check that there are 3 interfaces
- name: Check that there are 4 interfaces
ansible.builtin.assert:
that:
- "{{ ansible_interfaces | length == 3 }}"
- "{{ ansible_interfaces | length == 4 }}"

0 comments on commit 8be3b3e

Please sign in to comment.