From 38e098a41154e6561578cd723608fcd7577c3d01 Mon Sep 17 00:00:00 2001 From: Jonas L Date: Fri, 17 Nov 2023 15:05:52 +0100 Subject: [PATCH] feat: allow returning root_password in servers rebuild (#276) Fixes #181 Allow returning a full response during the server rebuild by passing an option to the rebuild method. The main reason for this is to not break user land and give users time to upgrade. We also don't want to introduce a new method `rebuild2` or `rebuild_with_full_response` to keep the API clean. The deprecation timeline is the following: - Introduce the `return_response` argument and deprecated the default behavior (`return_response=False`) - Wait for the next major release to remove the ability to only return the action and deprecated the usage of the `return_response` argument. - Wait for the next+1 major release to fully remove the `return_response` argument, or simply leave it unused. --- hcloud/servers/client.py | 42 ++++++++++++++++++++++++------- hcloud/servers/domain.py | 18 +++++++++++++ tests/unit/servers/test_client.py | 26 ++++++++++++++----- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/hcloud/servers/client.py b/hcloud/servers/client.py index ea72851..b6da0d3 100644 --- a/hcloud/servers/client.py +++ b/hcloud/servers/client.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from typing import TYPE_CHECKING, Any, NamedTuple from ..actions import ActionsPageResult, BoundAction, ResourceActionsClient @@ -21,6 +22,7 @@ PrivateNet, PublicNetwork, PublicNetworkFirewall, + RebuildResponse, RequestConsoleResponse, ResetPasswordResponse, Server, @@ -299,13 +301,18 @@ def create_image( """ return self._client.create_image(self, description, type, labels) - def rebuild(self, image: Image | BoundImage) -> BoundAction: + def rebuild( + self, + image: Image | BoundImage, + *, + return_response: bool = False, + ) -> RebuildResponse | BoundAction: """Rebuilds a server overwriting its disk with the content of an image, thereby destroying all data on the target server. - :param image: :class:`BoundImage ` or :class:`Image ` - :return: :class:`BoundAction ` + :param image: Image to use for the rebuilt server + :param return_response: Whether to return the full response or only the action. """ - return self._client.rebuild(self, image) + return self._client.rebuild(self, image, return_response=return_response) def change_type( self, @@ -930,12 +937,14 @@ def rebuild( self, server: Server | BoundServer, image: Image | BoundImage, - ) -> BoundAction: + *, + return_response: bool = False, + ) -> RebuildResponse | BoundAction: """Rebuilds a server overwriting its disk with the content of an image, thereby destroying all data on the target server. - :param server: :class:`BoundServer ` or :class:`Server ` - :param image: :class:`BoundImage ` or :class:`Image ` - :return: :class:`BoundAction ` + :param server: Server to rebuild + :param image: Image to use for the rebuilt server + :param return_response: Whether to return the full response or only the action. """ data: dict[str, Any] = {"image": image.id_or_name} response = self._client.request( @@ -943,7 +952,22 @@ def rebuild( method="POST", json=data, ) - return BoundAction(self._client.actions, response["action"]) + + rebuild_response = RebuildResponse( + action=BoundAction(self._client.actions, response["action"]), + root_password=response.get("root_password"), + ) + + if not return_response: + warnings.warn( + "Returning only the 'action' is deprecated, please set the " + "'return_response' keyword argument to 'True' to return the full " + "rebuild response and update your code accordingly.", + DeprecationWarning, + stacklevel=2, + ) + return rebuild_response.action + return rebuild_response def enable_backup(self, server: Server | BoundServer) -> BoundAction: """Enables and configures the automatic daily backup option for the server. Enabling automatic backups will increase the price of the server by 20%. diff --git a/hcloud/servers/domain.py b/hcloud/servers/domain.py index 2d55fd3..3802020 100644 --- a/hcloud/servers/domain.py +++ b/hcloud/servers/domain.py @@ -244,6 +244,24 @@ def __init__( self.password = password +class RebuildResponse(BaseDomain): + """Rebuild Response Domain + + :param action: Shows the progress of the server rebuild action + :param root_password: The root password of the server when not using SSH keys + """ + + __slots__ = ("action", "root_password") + + def __init__( + self, + action: BoundAction, + root_password: str | None, + ): + self.action = action + self.root_password = root_password + + class PublicNetwork(BaseDomain): """Public Network Domain diff --git a/tests/unit/servers/test_client.py b/tests/unit/servers/test_client.py index 2490ecf..a8ba355 100644 --- a/tests/unit/servers/test_client.py +++ b/tests/unit/servers/test_client.py @@ -307,15 +307,19 @@ def test_create_image( def test_rebuild(self, hetzner_client, bound_server, generic_action): hetzner_client.request.return_value = generic_action - action = bound_server.rebuild(Image(name="ubuntu-20.04")) + response = bound_server.rebuild( + Image(name="ubuntu-20.04"), + return_response=True, + ) hetzner_client.request.assert_called_with( url="/servers/14/actions/rebuild", method="POST", json={"image": "ubuntu-20.04"}, ) - assert action.id == 1 - assert action.progress == 0 + assert response.action.id == 1 + assert response.action.progress == 0 + assert response.root_password is None or isinstance(response.root_password, str) def test_enable_backup(self, hetzner_client, bound_server, generic_action): hetzner_client.request.return_value = generic_action @@ -1040,15 +1044,25 @@ def test_create_image(self, servers_client, server, response_server_create_image ) def test_rebuild(self, servers_client, server, generic_action): servers_client._client.request.return_value = generic_action - action = servers_client.rebuild(server, Image(name="ubuntu-20.04")) + response = servers_client.rebuild( + server, + Image(name="ubuntu-20.04"), + return_response=True, + ) servers_client._client.request.assert_called_with( url="/servers/1/actions/rebuild", method="POST", json={"image": "ubuntu-20.04"}, ) - assert action.id == 1 - assert action.progress == 0 + assert response.action.id == 1 + assert response.action.progress == 0 + assert response.root_password is None or isinstance(response.root_password, str) + + def test_rebuild_return_response_deprecation(self, servers_client, generic_action): + servers_client._client.request.return_value = generic_action + with pytest.deprecated_call(): + servers_client.rebuild(Server(id=1), Image(name="ubuntu-20.04")) @pytest.mark.parametrize( "server", [Server(id=1), BoundServer(mock.MagicMock(), dict(id=1))]