From 9458f825f3fedf2ac5774ec76de4f7676d3ad7c4 Mon Sep 17 00:00:00 2001 From: Will Bennett <77282969+willbennett-devops@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:48:19 -0400 Subject: [PATCH] systemvm: don't restart xl2tpd when the L2TP secrets file is unchanged configure_l2tpIpsec() called secret.empty() before rewriting ipsec.any.secrets. empty() also clears the copy read from disk, so the file always compared as changed, and since #12422 every iptables pass restarted xl2tpd and dropped all remote access VPN sessions on any LB, ACL, port forward or firewall change. Use repopulate(), which keeps the loaded copy, so xl2tpd is restarted only when the PSK or the L2TP configuration really changed. --- systemvm/debian/opt/cloud/bin/configure.py | 2 +- systemvm/test/TestCsRemoteAccessVpn.py | 70 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 systemvm/test/TestCsRemoteAccessVpn.py diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index 95cb73608468..b0c1793590b2 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -1274,7 +1274,7 @@ def configure_l2tpIpsec(self, left, obj): l2tp_changed = l2tpfile.commit() secret = CsFile(vpnsecretfilte) - secret.empty() + secret.repopulate() secret.addeq(": PSK \"%s\"" % (psk)) secret_changed = secret.commit() diff --git a/systemvm/test/TestCsRemoteAccessVpn.py b/systemvm/test/TestCsRemoteAccessVpn.py new file mode 100644 index 000000000000..e7efa2beff51 --- /dev/null +++ b/systemvm/test/TestCsRemoteAccessVpn.py @@ -0,0 +1,70 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import os +import secrets +import shutil +import tempfile +import unittest +from unittest import mock + +import configure +from configure import CsRemoteAccessVpn + +# random per run, never a real key +PSK1 = secrets.token_hex(8) +PSK2 = secrets.token_hex(8) + + +class TestCsRemoteAccessVpn(unittest.TestCase): + + def setUp(self): + self.dir = tempfile.mkdtemp() + self.vpn = CsRemoteAccessVpn.__new__(CsRemoteAccessVpn) + self.vpn.VPNCONFDIR = self.dir + self.obj = {"local_ip": "10.1.2.1", "local_cidr": "198.51.100.0/24", "public_interface": "eth1", + "ip_range": "10.1.2.2-10.1.2.8", "preshared_key": PSK1, "vpn_server_ip": "203.0.113.10"} + real = configure.CsFile + # keep every file configure_l2tpIpsec() writes inside the test directory + patcher = mock.patch("configure.CsFile", side_effect=lambda name: real(os.path.join(self.dir, os.path.basename(name)))) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(shutil.rmtree, self.dir) + + def configure(self): + return self.vpn.configure_l2tpIpsec("203.0.113.10", self.obj) + + def secret(self): + with open(os.path.join(self.dir, "ipsec.any.secrets")) as f: + return f.read() + + def test_unchanged_config_is_not_reported_as_changed(self): + # an unchanged pass must not restart xl2tpd, which drops every connected client + self.assertTrue(self.configure()) + self.assertFalse(self.configure()) + self.assertEqual(self.secret(), ': PSK "%s"\n' % PSK1) + + def test_changed_psk_is_reported_as_changed(self): + self.configure() + self.obj["preshared_key"] = PSK2 + self.assertTrue(self.configure()) + self.assertEqual(self.secret(), ': PSK "%s"\n' % PSK2) + self.assertFalse(self.configure()) + + +if __name__ == '__main__': + unittest.main()