Make network API replace IP/WiFi settings #5283
Conversation
|
An alternative to this would be to allow However, the support for Also the CLI would need a way to set no nameserver (currently that is not possible since the Another option here would be to make the complete API replace style. For the frontend this would work without a problem as all aspects ( |
There was a problem hiding this comment.
I think this makes sense. Minor style change to the existing code.
One thing I was thinking about though, what if we mapped the existing code to PATCH /network/interface/{interface}/update? Like update POST /network/interface/{interface}/update exactly as you're showing but if we receive a PATCH request to that endpoint it does what it does today?
Then what we can do is simply update the CLI to make a PATCH call instead of a POST call to keep it working as it does today. And if we want we can add a --replace parameter or something that makes it do the POST call and clear existing values.
That way we don't have to try and collect the existing values and add them to the postbody to maintain existing behavior in the CLI, Supervisor will handle that. The frontend will continue to use POST and behave the way we want it to.
agners
force-pushed
the
make-network-api-replace-settings
branch
2 times, most recently
from
b5c9474
to
9c99758
Compare
Currently it is only possible to set DNS servers when in static mode. However, there are use cases to set DNS servers when in auto mode as well, e.g. if no local DNS server is provided by the DHCP, or the provided DNS turns out to be non-working.
Make sure gateway is correctly converted to the internal IP representation. Fix type info.
agners
force-pushed
the
make-network-api-replace-settings
branch
from
9c99758
to
c85bc69
Compare
|
Warning Rate limit exceeded@agners has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 43 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe changes involve updates to network configuration handling, specifically for IPv4, IPv6, and Wi-Fi settings. Default values for optional attributes have been added, and the logic for updating interface settings has been streamlined. Additionally, modifications to the DNS attribute type and gateway processing have been implemented to enhance robustness and prevent errors associated with missing values. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NetworkManager
participant Configuration
User->>NetworkManager: Request to update network settings
NetworkManager->>Configuration: Validate new settings
Configuration->>Configuration: Set defaults for optional attributes
Configuration->>NetworkManager: Return validated settings
NetworkManager->>User: Confirm settings updated
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Instead of using replace() simply set the API defaults in the API schema.
agners
force-pushed
the
make-network-api-replace-settings
branch
from
c85bc69
to
885506f
Compare
This reverts commit 885506f.
This avoid the unnecessary replaces from before. It also makes it more obvious that this part of the API doesn't patch existing settings.
Proposed change
This changes the network API behavior to replace IP configuration and WiFi settings instead of updating them.
It also adds support to set DNS (via
nameserversproperty) when methodautois used. This allows to set a DNS server in case DHCP does not provide one, or overrides theUpdate by replacing the whole IP configuration has advantages:
This also works out quite well with how the frontend behaves, it essentially keeps the behavior as it is today:
methodproperty. So this clears lingeringnameservers.method,addresses,gatewayandnameservers), so no change in behavior.However, this does change the CLI behavior: Missing options now really lead to clearing that particular field. However, this seems the more expected behavior, e.g. the following sequence:
With the patch style API, this would not have cleared the DNS. Clearing was only possible by switching to method
autoand then back tostatic.Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation