fix: openvpn_server - generate #89

genofire · 2023-09-08T16:14:24Z

opoplawski · 2023-12-19T02:40:57Z

Thanks for the PR. Sorry for the long delay in responding. Any chance this could get rebased and checks fixed? Thanks.

genofire · 2024-01-02T21:15:59Z

done

ERROR: Found 1 pylint issue(s) which need to be resolved:
ERROR: plugins/module_utils/openvpn_server.py:319:17: disallowed-name: Disallowed name "_"

genofire · 2024-01-02T21:21:13Z

plugins/module_utils/openvpn_server.py

+        for param in ['shared_key', 'tls']:
+            before_value = self.pfsense.get_element(param, target_elt)
+            if before_value is None and self.params[param] == 'generate':
+                (_, key, stderr) = self.module.run_command('/usr/local/sbin/openvpn --genkey secret /dev/stdout')


linter say _ is bad -> how should an unused variable be named?

i name it unused now: https://peps.python.org/pep-0640/

FWIW - that PEP was rejected. We are using dummy elsewhere in the code, so let's stick with that.

opoplawski · 2024-01-03T04:42:15Z

So, the current docs state: If set to 'generate' it will create a key if one does not already exist. I think this is the proper behavior. However, currently if there is an existing OpenVPN server that matches the config we get:

TASK [Define OpenVPN Server generate] ******************************************************************************************************************************************************************************************************
[WARNING]: Using a variable for a task's 'args' is unsafe in some situations (see https://docs.ansible.com/ansible/devel/reference_appendices/faq.html#argsplat-unsafe)
--- before
+++ after
@@ -35,7 +35,7 @@
     "protocol": "UDP4",
     "remote_network": "",
     "remote_networkv6": "",
-    "tls": "IwojIDIwNDggYml0IE9wZW5WUE4gc3RhdGljIGtleQojCi0tLS0tQkVHSU4gT3BlblZQTiBTdGF0aWMga2V5IFYxLS0tLS0KOWJkNzhmNWQ3ODM4NjIxZmQxNmJmNWQ2MzU4NGY0MGYKODllZmQ3MDRiMjc4MzU2YjJhMjM0MTljZjhmNjgzM2QKMTJlMGI5MTE0NTBiMzA4NjNkZDYxNWEyMzRhOTliYWUKMWQzODZmODNjZDQ2ZGQ2MGQ3NTUzOWI2ZGE2YzMxY2YKZDMxMWM0ZDNmYTU3N2RkZDUzM2I4Y2VmYWFhYzhmNDIKNWM3N2M3OTgxYzU4Mjk1OTRiNjQyOTYxMWJhYzEzMWYKMGU4NWFhYjIyNzlkNmMyMWVhNGUzNjNjMjBhNmI1MTEKMDlhZDBhY2ViZTZiNjg3ZGM5YmYwYWU1MzJmNDBmMjMKZDYyMWI0YzM5NjBkODg1OTMzM2E0MDg3NWE2NGFhOGUKOTJlZjhkM2VkNWIwNTk1MjI0Yzg1NGU2NGE2MDUyZjYKNTJjMjRlODBkMmFhZDEyYzExYTUyYTNhMDg5YmQyNGIKNDRkMDdmNzVmZjRjOTBiYThkMmFmOWQ5ZGRlMTZiNGIKYjQ3NWU1OGM1Nzk4NzU2OTMwMzJiZjdhOGI1YTRkNmEKNTYzZTY2NTM3ZmE3MjUwYTYwNmQ3MmZlYmU0MTA4ZmQKOTIxMjkwYTNlZWVmOTVlMzI4MGVmNjgyNTk2NGJmNDYKYjM0NzFjNDZmN2JiNWQzYjVlMTU0NDcwMDNjMTc4NWIKLS0tLS1FTkQgT3BlblZQTiBTdGF0aWMga2V5IFYxLS0tLS0K",
+    "tls": "generate",
     "tls_type": "auth",
     "topology": "subnet",
     "tunnel_network": "10.100.1.0/24",

changed: [pfsense-test] => {"changed": true, "commands": ["update openvpn_server 'OpenVPN Server generate' set "], "stderr": "", "stderr_lines": [], "stdout": "pfSense shell: global $debug;\npfSense shell: $debug = 1;\npfSense shell: \npfSense shell: require_once('openvpn.inc');\npfSense shell: init_config_arr(array('openvpn', 'openvpn-server'));\npfSense shell: $a = &$config['openvpn']['openvpn-server'];\npfSense shell: $ovpn = $a[3];\npfSense shell: \npfSense shell: openvpn_resync('server',$ovpn);\npfSense shell: openvpn_resync_csc_all();\npfSense shell: \npfSense shell: exec\npfSense shell: exit\n", "stdout_lines": ["pfSense shell: global $debug;", "pfSense shell: $debug = 1;", "pfSense shell: ", "pfSense shell: require_once('openvpn.inc');", "pfSense shell: init_config_arr(array('openvpn', 'openvpn-server'));", "pfSense shell: $a = &$config['openvpn']['openvpn-server'];", "pfSense shell: $ovpn = $a[3];", "pfSense shell: ", "pfSense shell: openvpn_resync('server',$ovpn);", "pfSense shell: openvpn_resync_csc_all();", "pfSense shell: ", "pfSense shell: exec", "pfSense shell: exit"], "vpnid": 5}

And the config.xml ends up with:

                        <tls>generate</tls>

We shouldn't report changed in this case and nothing should get updated on the pfSense box.

plugins/module_utils/openvpn_server.py

opoplawski · 2024-01-03T14:29:51Z

But now if I go to change a different parameter:

The full traceback is:
Traceback (most recent call last):
  File "/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py", line 107, in <module>
    _ansiballz_main()
  File "/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py", line 99, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py", line 47, in invoke_module
    runpy.run_module(mod_name='ansible_collections.pfsensible.core.plugins.modules.pfsense_openvpn_server', init_globals=dict(_module_fqn='ansible_collections.pfsensible.core.plugins.modules.pfsense_openvpn_server', _modlib_path=modlib_path),
  File "<frozen runpy>", line 226, in run_module
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/modules/pfsense_openvpn_server.py", line 288, in <module>
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/modules/pfsense_openvpn_server.py", line 284, in main
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/module_utils/module_base.py", line 198, in commit_changes
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/basic.py", line 1521, in exit_json
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/basic.py", line 1514, in _return_formatted
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/common/parameters.py", line 927, in remove_values
  File "/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/common/parameters.py", line 470, in _remove_values_conditions
TypeError: Value of unknown type: <class 'xml.etree.ElementTree.Element'>, <Element 'tls' at 0x1d0c1442cfe0>
fatal: [pfsense-test]: FAILED! => {
    "changed": false,
    "module_stderr": "X11 forwarding request failed\r\nShared connection to 192.168.100.2 closed.\r\n",
    "module_stdout": "Traceback (most recent call last):\r\n  File \"/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py\", line 107, in <module>\r\n    _ansiballz_main()\r\n  File \"/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py\", line 99, in _ansiballz_main\r\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\r\n  File \"/root/.ansible/tmp/ansible-tmp-1704292126.0399086-667821-263900557354764/AnsiballZ_pfsense_openvpn_server.py\", line 47, in invoke_module\r\n    runpy.run_module(mod_name='ansible_collections.pfsensible.core.plugins.modules.pfsense_openvpn_server', init_globals=dict(_module_fqn='ansible_collections.pfsensible.core.plugins.modules.pfsense_openvpn_server', _modlib_path=modlib_path),\r\n  File \"<frozen runpy>\", line 226, in run_module\r\n  File \"<frozen runpy>\", line 98, in _run_module_code\r\n  File \"<frozen runpy>\", line 88, in _run_code\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/modules/pfsense_openvpn_server.py\", line 288, in <module>\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/modules/pfsense_openvpn_server.py\", line 284, in main\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible_collections/pfsensible/core/plugins/module_utils/module_base.py\", line 198, in commit_changes\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/basic.py\", line 1521, in exit_json\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/basic.py\", line 1514, in _return_formatted\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/common/parameters.py\", line 927, in remove_values\r\n  File \"/tmp/ansible_pfsensible.core.pfsense_openvpn_server_payload_b6tuub9e/ansible_pfsensible.core.pfsense_openvpn_server_payload.zip/ansible/module_utils/common/parameters.py\", line 470, in _remove_values_conditions\r\nTypeError: Value of unknown type: <class 'xml.etree.ElementTree.Element'>, <Element 'tls' at 0x1d0c1442cfe0>\r\n",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}

opoplawski · 2024-01-04T00:14:20Z

I think I figure out that issue.

Fixes pfsensible#81

opoplawski added this to the 0.6.0 milestone Dec 23, 2023

opoplawski added the bug Something isn't working label Dec 23, 2023

genofire force-pushed the patch-1 branch from 69a2599 to d443b9a Compare January 2, 2024 21:15

genofire commented Jan 2, 2024

View reviewed changes

genofire force-pushed the patch-1 branch from d443b9a to 2650898 Compare January 2, 2024 21:23

genofire commented Jan 3, 2024

View reviewed changes

plugins/module_utils/openvpn_server.py Outdated Show resolved Hide resolved

genofire force-pushed the patch-1 branch from 2650898 to 6d770e6 Compare January 3, 2024 11:31

opoplawski force-pushed the patch-1 branch from 6d770e6 to d512cfa Compare January 4, 2024 00:13

genofire and others added 3 commits January 3, 2024 18:53

[pfsense_openvpn_server] Handle generate value for tls, psk

7f019fc

Fixes pfsensible#81

[pfsense_openvpn_server] Add test for generate

aa6e842

[pfsense_openvpn_client] Handle generate value for tls, psk

7d9aaed

Fixes pfsensible#81

opoplawski force-pushed the patch-1 branch from 1412356 to 7d9aaed Compare January 4, 2024 01:54

opoplawski merged commit 90d5406 into pfsensible:master Jan 4, 2024
3 checks passed

genofire deleted the patch-1 branch January 4, 2024 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: openvpn_server - generate #89

fix: openvpn_server - generate #89

genofire commented Sep 8, 2023

opoplawski commented Dec 19, 2023

genofire commented Jan 2, 2024 •

edited

Loading

genofire Jan 2, 2024

genofire Jan 2, 2024

opoplawski Jan 3, 2024

opoplawski commented Jan 3, 2024

opoplawski commented Jan 3, 2024

opoplawski commented Jan 4, 2024

fix: openvpn_server - generate #89

fix: openvpn_server - generate #89

Conversation

genofire commented Sep 8, 2023

opoplawski commented Dec 19, 2023

genofire commented Jan 2, 2024 • edited Loading

genofire Jan 2, 2024

Choose a reason for hiding this comment

genofire Jan 2, 2024

Choose a reason for hiding this comment

opoplawski Jan 3, 2024

Choose a reason for hiding this comment

opoplawski commented Jan 3, 2024

opoplawski commented Jan 3, 2024

opoplawski commented Jan 4, 2024

genofire commented Jan 2, 2024 •

edited

Loading