Skip to content

Commit

Permalink
fix: fixed issues where --is-registered would print incorrect outputs (
Browse files Browse the repository at this point in the history
…#288)

## Manual Testing
### Verify Bug
- Register a new client to a server, without accepting the new computer
on the server side yet
- Run `landscape-config --is-registered`, verify output says True
### Verify Fix
**_NOTE:_** In order to not disrupt any workflows we added new command
line arguments rather than editing existing ones
- Repeat registration without accepting step
- Run `landscape-config --actively-registered` and verify output is
false
- Run `landscape-config --registration-sent` and verify output is true
- Accept the new computer on server end, and rerun `landscape-config
--actively-registered`, verify output is True
  • Loading branch information
david-mclain authored Nov 26, 2024
1 parent 1656a23 commit 3018d6f
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 15 deletions.
2 changes: 2 additions & 0 deletions landscape/client/broker/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class Identity:
@ivar secure_id: A server-provided ID for secure message exchange.
@ivar insecure_id: Non-secure server-provided ID, mainly used with
the ping server.
@ivar accepted_types: The types of messages the server will accept
from the client during exchange
@ivar computer_title: See L{BrokerConfiguration}.
@ivar account_name: See L{BrokerConfiguration}.
@ivar registration_password: See L{BrokerConfiguration}.
Expand Down
53 changes: 48 additions & 5 deletions landscape/client/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,20 @@ def make_parser(self):
"Send a new registration request only if one has not been sent"
),
)
parser.add_argument(
"--actively-registered",
action="store_true",
help="Exit with code 0 (success) if client is "
"registered else returns {}. Displays "
"registration info.".format(EXIT_NOT_REGISTERED),
)
parser.add_argument(
"--registration-sent",
action="store_true",
help="Exit with code 0 (success) if client is "
"registered else returns {}. Displays "
"registration info.".format(EXIT_NOT_REGISTERED),
)
return parser


Expand Down Expand Up @@ -777,8 +791,13 @@ def attempt_registration(
return 0


def is_registered(config):
"""Return whether the client is already registered."""
def registration_sent(config):
"""
Return whether the client has sent a registration request to the server.
For now does same thing as is_registered as to make function name more
clear with what is performed. This is the legacy behaviour of
--is-registered and the name will be changed in a future release.
"""
persist_filename = os.path.join(
config.data_path,
f"{BrokerService.service_name}.bpickle",
Expand All @@ -788,6 +807,19 @@ def is_registered(config):
return bool(identity.secure_id)


def actively_registered(config):
"""Return whether or not the client is currently registered with server"""
persist_filename = os.path.join(
config.data_path,
f"{BrokerService.service_name}.bpickle",
)
persist = Persist(filename=persist_filename, user=USER, group=GROUP)
accepted_types = persist.get("message-store.accepted-types")
if accepted_types is not None:
return len(accepted_types) > 1 and "register" not in accepted_types
return False


def registration_info_text(config, registration_status):
"""
A simple output displaying whether the client is registered or not, the
Expand Down Expand Up @@ -844,7 +876,7 @@ def get_secure_id(config):
return identity.secure_id


def main(args, print=print):
def main(args, print=print): # noqa: C901
"""Interact with the user and the server to set up client configuration."""
config = LandscapeSetupConfiguration()
try:
Expand All @@ -866,9 +898,9 @@ def main(args, print=print):
"and force registration together.",
)

already_registered = is_registered(config)
already_registered = registration_sent(config)

if config.is_registered:
if config.is_registered or config.registration_sent:

registration_status = already_registered

Expand All @@ -880,6 +912,17 @@ def main(args, print=print):
else:
sys.exit(EXIT_NOT_REGISTERED)

if config.actively_registered:
currently_registered = actively_registered(config)

info_text = registration_info_text(config, currently_registered)
print(info_text)

if currently_registered:
sys.exit(0)
else:
sys.exit(EXIT_NOT_REGISTERED)

if os.getuid() != 0:
sys.exit("landscape-config must be run as root.")

Expand Down
51 changes: 43 additions & 8 deletions landscape/client/tests/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@
from landscape.client import USER
from landscape.client.broker.registration import Identity
from landscape.client.broker.tests.helpers import BrokerConfigurationHelper
from landscape.client.configuration import actively_registered
from landscape.client.configuration import bootstrap_tree
from landscape.client.configuration import ConfigurationError
from landscape.client.configuration import EXIT_NOT_REGISTERED
from landscape.client.configuration import get_secure_id
from landscape.client.configuration import ImportOptionError
from landscape.client.configuration import is_registered
from landscape.client.configuration import LandscapeSetupConfiguration
from landscape.client.configuration import LandscapeSetupScript
from landscape.client.configuration import main
from landscape.client.configuration import print_text
from landscape.client.configuration import prompt_yes_no
from landscape.client.configuration import registration_info_text
from landscape.client.configuration import registration_sent
from landscape.client.configuration import restart_client
from landscape.client.configuration import set_secure_id
from landscape.client.configuration import setup
Expand Down Expand Up @@ -1182,7 +1183,7 @@ def test_main_register_if_needed_silent(
mock_input.assert_not_called()

@mock.patch(
"landscape.client.configuration.is_registered",
"landscape.client.configuration.registration_sent",
return_value=True,
)
@mock.patch("landscape.client.configuration.restart_client")
Expand Down Expand Up @@ -2282,19 +2283,53 @@ def setUp(self):
persist_file = os.path.join(self.config.data_path, "broker.bpickle")
self.persist = Persist(filename=persist_file)

def test_is_registered_false(self):
def test_registration_sent_false(self):
"""
If the client hasn't previously registered, is_registered returns False
If the client hasn't previously sent a registration request,
registration_sent returns False
"""
self.assertFalse(is_registered(self.config))
self.assertFalse(registration_sent(self.config))

def test_is_registered_true(self):
def test_registration_sent_true(self):
"""
If the client has previously registered, is_registered returns True.
If the client has previously sent a registration request,
registration_sent returns True.
"""
self.persist.set("registration.secure-id", "super-secure")
self.persist.save()
self.assertTrue(is_registered(self.config))
self.assertTrue(registration_sent(self.config))

def test_actively_registered_true(self):
"""
If the client is actively registered with the server returns True
"""
self.persist.set(
"message-store.accepted-types",
["test", "temperature"],
)
self.persist.save()
self.assertTrue(actively_registered(self.config))

def test_actively_registered_false(self):
"""
If the client is not actively registered with the server returns False
"""
self.persist.set("message-store.accepted-types", ["test", "register"])
self.persist.save()
self.assertFalse(actively_registered(self.config))

def test_actively_registered_false_only_test(self):
"""
If the client is not actively registered with the server returns False.
Here we check add only test to the accepted types as it is always an
accepted type by the server. In the actively_registered function we
check to see if the len(accepted_types) > 1 to make sure there are more
accepted types than just the test. This test case makes sure that we
fail the test case of only test if provided in accepted types
"""
self.persist.set("message-store.accepted-types", ["test"])
self.persist.save()
self.assertFalse(actively_registered(self.config))


class RegistrationInfoTest(LandscapeTest):
Expand Down
14 changes: 12 additions & 2 deletions man/landscape-config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,25 @@ OPTIONS
enter: ALL.
--include-manager-plugins=PLUGINS A comma-separated list of manager
plugins to load explicitly.
--manage-sources-list-d [MANAGE_SOURCES_LIST_D] Repository profiles manage
the files in ’etc/apt/sources.list.d'. (default: true)
--manage-sources-list-d [MANAGE_SOURCES_LIST_D] Repository profiles manage
the files in ’etc/apt/sources.list.d'. (default: true)
-n, --no-start Don't start the client automatically.
--ok-no-register Return exit code 0 instead of 2 if the client can't be
registered.
--silent Run without manual interaction.
--disable Stop running clients and disable start at boot.
--init Set up the client directories structure and exit.
--is-registered Exit with code 0 (success) if client
has sent registration request else returns 5.
Display registration sent info.
(NOTE: use --actively-registered to detect if
registration has been accepted server-side)
--registration-sent Exit with code 0 (success) if client
has sent registration request else returns 5.
Display registration sent info.
(NOTE: use --actively-registered to detect if
registration has been accepted server-side)
--actively-registered Exit with code 0 (success) if client
is registered else returns 5. Display
registration info.

Expand Down

0 comments on commit 3018d6f

Please sign in to comment.