cc_ssh_authkey_fingerprints.py: prevent duplicate messages on console

[dermotbradley]

Proposed Commit Message

cc_ssh_authkey_fingerprints.py: prevent duplicate messages on console

When cloud-init is configured to show SSH user key fingerprints during
boot two of the same message appears for each user. This appears to be as
the util.multi_log call defaults to send to both console directly and to
stderr (which also goes to console).

This change sends them only to console directly.

Additional Context

Test Steps

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[TheRealFalcon]

@blackboxsw , can you take a look at this as well? I feel like I'm missing something. If all stdout/stderr automatically goes to the console, what's the point of having a multi_log and logging to console and stderr simultaneously?

[dermotbradley]

@TheRealFalcon @blackboxsw

Just to clarify the before/after situation:

Existing behaviour is that for every user who does not have any SSH key the "ci-info: no authorized SSH keys fingerprints found for user XYZ." message appears on the console twice and in cloud-init-output.log only once.

This PR has the result that the message appears only once on the console and not at all in the logfile.

[blackboxsw]

Thanks for the PR @dermotbradley and review @TheRealFalcon. What I'm not understanding about this PR is how this patch "fixes" the behavior mentioned in the suggested commit message:

When cloud-init is configured to show SSH user key fingerprints during boot two of the same message appears for each user.

I want to make sure the duplicate messages you are referring to are the following format (and not the other key related messages):
ci-info: no authorized SSH keys fingerprints found for user chad.
[338079.648348] cloud-init[732]: ci-info: no authorized SSH keys fingerprints found for user chad.

And nevermind, saw the trailing comment @dermotbradley already made

[blackboxsw]

I'm +1 on @dermotbradley's logic if James agrees with my point above.

But, I do think we minimally need an integration test since we have no unit test coverage on this feature anyway.

Here's a diff that'll assert that we only see one log instead of two in the console

commit b39bfd3daf32d7c8894a8c60ad76569bfdbc652b (HEAD)
Author: Chad Smith <chad.smith@canonical.com>
Date:   Tue Nov 9 16:11:42 2021 -0700

    integration test: assert not duplicate messaging in console for absent SSH keys

diff --git a/tests/integration_tests/modules/test_keys_to_console.py b/tests/integration_tests/modules/test_keys_to_console.py
index 56dff9a0..7a724ae2 100644
--- a/tests/integration_tests/modules/test_keys_to_console.py
+++ b/tests/integration_tests/modules/test_keys_to_console.py
@@ -21,6 +21,15 @@ ssh:
   emit_keys_to_console: false
 """
 
+ENABLE_KEYS_TO_CONSOLE_USER_DATA = """\
+#cloud-config
+ssh:
+  emit_keys_to_console: true
+users:
+ - default
+ - name: barfoo
+"""
+
 
 @pytest.mark.user_data(BLACKLIST_USER_DATA)
 class TestKeysToConsoleBlacklist:
@@ -65,3 +74,32 @@ class TestKeysToConsoleDisabled:
     def test_footer_excluded(self, class_client):
         syslog = class_client.read_from_file("/var/log/syslog")
         assert "END SSH HOST KEY FINGERPRINTS" not in syslog
+
+
+@pytest.mark.user_data(ENABLE_KEYS_TO_CONSOLE_USER_DATA)
+@pytest.mark.ec2
+@pytest.mark.lxd_container
+@pytest.mark.oci
+@pytest.mark.openstack
+class TestKeysToConsoleEnabled:
+    """Test that output can be enabled disabled."""
+
+    def test_duplicate_messaging_console_log(self, class_client):
+        class_client.execute('cloud-init status --wait --long').ok
+        try:
+            console_log = class_client.instance.console_log()
+        except NotImplementedError:
+            # Assume that an exception here means that we can't use the console
+            # log
+            pytest.skip("NotImplementedError when requesting console log")
+            return
+        if console_log.lower() == 'no console output':
+            # This test retries because we might not have the full console log
+            # on the first fetch. However, if we have no console output
+            # at all, we don't want to keep retrying as that would trigger
+            # another 5 minute wait on the pycloudlib side, which could
+            # leave us waiting for a couple hours
+            pytest.fail('no console output')
+            return
+        msg = "no authorized SSH keys fingerprints found for user barfoo."
+        assert 1 == console_log.count(msg)

This is testable (and will fail on focal daily images)
CLOUD_INIT_PLATFORM=lxd_container .tox/integration-tests/bin/pytest tests/integration_tests/modules/test_keys_to_console.py::TestKeysToConsoleEnabled

[github-actions]

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging mitechie, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag mitechie to reopen it.)

[dermotbradley]

@blackboxsw @TheRealFalcon What's the current situation regarding this PR?

[TheRealFalcon]

@dermotbradley, I think we're just waiting on an integration test.

[blackboxsw]

@dermotbradley: As James mentioned, if you are ok with including my suggested integration test suggestion or writing your own for this, I think we are good to land this branch and get it into the next release.

[dermotbradley]

@dermotbradley: As James mentioned, if you are ok with including my suggested integration test suggestion or writing your own for this, I think we are good to land this branch and get it into the next release.

@blackboxsw Yes I've added your test now, thanks.

I was confused by your comment 3 weeks back which I thought meant you were waiting for James' feedback.

Anyway this should be ready to go now.

[TheRealFalcon]

Ah, sorry about that. I probably should have responded then. I'll take a look here ASAP, and we should be able to get it merged soon.

[TheRealFalcon]

Thanks!

Changes have been addressed