Skip to content

Commit

Permalink
network: auto-disable dead interfaces on configure
Browse files Browse the repository at this point in the history
It's possible that clients do not interact with the network controller -
even if the controller is interactive - and simply mark the controller
as configured (currently the desktop installer does this). Subiquity's
policy is to disable interfaces without a global IP address by the
time the network screen is shown. However, other than special handling
for autoinstall cases, this logic is only invoked the first time
the screen is shown. This makes sure the config written to the target
device adheres to policy.

This change purposefully does not apply the config in the live
environment. It's likely Desktop does not interact with the network
controller to avoid interactions between networkd and NetworkManager.
Handling of this logic should be for future commits.

(cherry picked from commit e22946c)
  • Loading branch information
Chris-Peterson444 committed Jul 2, 2024
1 parent b27e35c commit 2ffce2e
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 1 deletion.
8 changes: 8 additions & 0 deletions subiquity/server/controllers/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,14 @@ async def GET(self) -> NetworkStatus:
)

async def configured(self):
# There may be some instances in which the network controller
# is interactive but is only ever marked configured (through a
# POST to either /network or /meta/mark_configured) and is never
# interacted with otherwise (via GET) such that we don't disable
# interfaces which don't have a global IP.
if self.interactive() and not self.view_shown:
self.update_initial_configs()

self.model.has_network = self.network_event_receiver.has_default_route
self.model.needs_wpasupplicant = (
self.wlan_support_install_state() == PackageInstallState.DONE
Expand Down
62 changes: 61 additions & 1 deletion subiquity/server/controllers/tests/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from unittest.mock import ANY, Mock, patch
from copy import copy
from unittest.mock import ANY, AsyncMock, Mock, patch

import jsonschema
from jsonschema.validators import validator_for

from subiquity.models.network import NetworkModel
from subiquity.server.controllers.network import NetworkController
from subiquitycore.models.network import NetworkDev
from subiquitycore.tests import SubiTestCase
from subiquitycore.tests.mocks import make_app
from subiquitycore.tests.parameterized import parameterized


class TestNetworkController(SubiTestCase):
Expand Down Expand Up @@ -57,3 +61,59 @@ def test_valid_schema(self):
)

JsonValidator.check_schema(NetworkController.autoinstall_schema)


class TestNetworkAutoDisableInterfaces(SubiTestCase):
"""Tests for automatic disabling of disconnected interfaces."""

def setUp(self):
app = make_app()
app.note_file_for_apport = Mock()
app.opts.output_base = self.tmp_dir()
app.opts.project = "subiquity"
app.package_installer = Mock()
app.state_path = Mock()
app.hub = AsyncMock()
self.controller = NetworkController(app)
self.controller.model = NetworkModel()

@parameterized.expand(
(
# Interactive, view_shown, expect_updates
(True, False, True), # Interactive but not modified: update configs
(False, False, False), # Autoinstall case: Don't do anything.
# Should be handled by autoinstall logic
(True, True, False), # Edits made in UI: Don't update
)
)
async def test_disable_on_configured(self, interactive, view_shown, modify):
"""Test disconnected interfaces are disabled on marked configured."""
# It's possible that we only ever mark the network controller
# configured (e.g., Desktop) and don't otherwise interact with it
# when it was supposed to be interactive. Test to make sure
# disconnected interfaces are still disabled internally.
self.controller.interactive = Mock(return_value=interactive)
self.controller.view_shown = view_shown

live_dev = NetworkDev(self.controller.model, "testdev0", "eth")
live_config = {"dhcp4": True}
live_dev.config = copy(live_config)
live_dev.info = Mock(addresses={"addr": Mock(scope="global")})
self.controller.model.devices_by_name["testdev0"] = live_dev

dead_dev = NetworkDev(self.controller.model, "testdev1", "eth")
dead_config = {"dhcp4": True}
dead_dev.config = copy(dead_config)
dead_dev.info = Mock(addresses={})
self.controller.model.devices_by_name["testdev1"] = dead_dev

with patch("subiquity.server.controller.open"):
await self.controller.configured()

# Live config shouldn't be modified no matter what
self.assertEqual(live_dev.config, live_config)

if modify:
self.assertEqual(dead_dev.config, {})
else:
self.assertEqual(dead_dev.config, dead_config)

0 comments on commit 2ffce2e

Please sign in to comment.