Skip to content

nocloud: handle dict-of-strings public-keys format#198

Open
mattia-eleuteri wants to merge 1 commit into
cloudbase:masterfrom
mattia-eleuteri:fix-nocloud-public-keys-dict-of-strings
Open

nocloud: handle dict-of-strings public-keys format#198
mattia-eleuteri wants to merge 1 commit into
cloudbase:masterfrom
mattia-eleuteri:fix-nocloud-public-keys-dict-of-strings

Conversation

@mattia-eleuteri
Copy link
Copy Markdown

Summary

NoCloudConfigDriveService.get_public_keys() assumed public-keys in the metadata was either a list or an OpenStack-style dict of {<name>: {'openssh-key': <key>}}, and called .get('openssh-key') on each value. KubeVirt's accessCredentials.sshPublicKey.propagationMethod.noCloud serialises the keys as map[string]string, producing {'key0': '<ssh-rsa …>', 'key1': '<ssh-rsa …>'} — a dict whose values are plain SSH key strings. With that input the plugin crashed:

ERROR cloudbaseinit.init [-] plugin 'SetUserSSHPublicKeysPlugin' failed with error
    "'str' object has no attribute 'get'"
  File ".../nocloudservice.py", line 698, in get_public_keys
    public_keys = service.get_public_keys()

SetUserSSHPublicKeysPlugin aborted and no key was ever written to either C:\Users\<user>\.ssh\authorized_keys or C:\ProgramData\ssh\administrators_authorized_keys, so SSH key authentication never worked.

This PR accepts both shapes: if a dict value is a string, it is treated as the SSH key directly; otherwise the previous OpenStack-style extraction (v.get('openssh-key')) is kept.

How to reproduce

  • Windows Server 2022 / 2025 image with cloudbase-init 1.1.8 deployed on KubeVirt ≥ 1.0 via NoCloud cidata.
  • KubeVirt VMI spec includes accessCredentials.sshPublicKey with source.secret and propagationMethod.noCloud: {}.
  • KubeVirt source for the format: pkg/cloud-init/cloud-init.go, type NoCloudMetadata struct { … PublicSSHKeys map[string]string \json:"public-keys,omitempty"` }`.

Test plan

  • New unit test test_get_public_keys_dict_of_strings mirroring the existing test_get_public_keys / test_get_public_keys_alt_fmt pattern.
  • Existing tests still pass:
    python -m unittest \
      cloudbaseinit.tests.metadata.services.test_nocloudservice.TestNoCloudConfigDriveService.test_get_public_keys \
      cloudbaseinit.tests.metadata.services.test_nocloudservice.TestNoCloudConfigDriveService.test_get_public_keys_alt_fmt \
      cloudbaseinit.tests.metadata.services.test_nocloudservice.TestNoCloudConfigDriveService.test_get_public_keys_dict_of_strings
    # → Ran 3 tests in 0.002s, OK
    
  • DCO sign-off on the commit.

Related

Copy link
Copy Markdown
Member

@ader1990 ader1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thank you for the fix. Can you please confirm that this code is your own contribution and not AI created? If AI assisted, please state in the commit message that you abide by the current DCO terms: https://github.com/cloudbase/cloudbase-init/blob/master/DCO#L16 and that your contribution is yours.

Also, if you can remove the AI bloat from the commit message and keep it as short as possible with real links pointing to the actual KubeVirt documentation or KubeVirt implementation.


@mock.patch(MODULE_PATH + '.NoCloudConfigDriveService._get_meta_data')
def test_get_public_keys_dict_of_strings(self, mock_get_metadata):
# KubeVirt serialises accessCredentials sshPublicKey noCloud as
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests should not have the same copy-pasted AI information as the initial implementation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the comment; the test now keeps only the fixture and the assertion.

return raw_ssh_keys

return [raw_ssh_keys[key].get('openssh-key') for key in raw_ssh_keys]
# raw_ssh_keys may be a dict in two shapes:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thanks for the fix. the code was complicated to read as it were, now it is even more complicated to read. Can you please update the code with logical sequential steps?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to a sequential for loop with explicit if/else. Let me know if it reads better.

Some sources serialise public-keys as a dict of {name: key-string}
rather than {name: {'openssh-key': key-string}}. KubeVirt's
accessCredentials sshPublicKey noCloud propagation does this, defining
NoCloudMetadata.PublicSSHKeys as map[string]string [1] and assigning
it directly in readCloudInitNoCloudMetaData [2]. The current third
branch of get_public_keys() then calls .get('openssh-key') on a string
value and raises "'str' object has no attribute 'get'". Accept both
shapes.

[1] https://github.com/kubevirt/kubevirt/blob/623bf15e7553875860ece1a382a9e35bfbd61a26/pkg/cloud-init/cloud-init.go#L84
[2] https://github.com/kubevirt/kubevirt/blob/623bf15e7553875860ece1a382a9e35bfbd61a26/pkg/cloud-init/cloud-init.go#L402-L408

AI-assisted contribution: I reviewed the change and submit it under
the DCO terms at https://github.com/cloudbase/cloudbase-init/blob/master/DCO#L16
(clause (a) — the contribution is mine).

Signed-off-by: mattia-eleuteri <mattia@hidora.io>
@mattia-eleuteri mattia-eleuteri force-pushed the fix-nocloud-public-keys-dict-of-strings branch from 5960904 to fe5a68e Compare June 5, 2026 12:16
@mattia-eleuteri
Copy link
Copy Markdown
Author

AI-assisted, disclosure added to the commit message per the DCO clause you linked. Commit body trimmed; KubeVirt source now linked at the type definition (pkg/cloud-init/cloud-init.go#L84) and the assignment in readCloudInitNoCloudMetaData (#L402-L408). Force-pushed fe5a68e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants