Skip to content

Fix regression by forcing no IMDS ssh keys#760

Merged
OddBloke merged 2 commits into
canonical:masterfrom
trstringer:thstring/no-imds-ssh
Jan 11, 2021
Merged

Fix regression by forcing no IMDS ssh keys#760
OddBloke merged 2 commits into
canonical:masterfrom
trstringer:thstring/no-imds-ssh

Conversation

@trstringer
Copy link
Copy Markdown
Contributor

@trstringer trstringer commented Jan 11, 2021

Proposed Commit Message

Fix regression with handling of IMDS ssh keys.

With the changes for ssh public keys to be retrieved from IMDS as a first option, when a key is passed through not in the raw ssh public key format it causes an issue and the key is not added to the user's authorized_keys file.

This PR will temporarily disable this behavior until a permanent fix is put in place.

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

Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke left a comment

Choose a reason for hiding this comment

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

Thanks for this fix, Thomas! We'll need to discuss how to remediate this problem in a longer-term fashion, but this fix addresses our immediate need.

I have one important question inline: I want to make sure that the logging/diagnostics are aligned with your expectations. (I also have a side note, which doesn't really need addressing here.)

Comment thread cloudinit/sources/DataSourceAzure.py
Comment thread cloudinit/sources/DataSourceAzure.py
Comment thread cloudinit/sources/DataSourceAzure.py
@OddBloke
Copy link
Copy Markdown
Collaborator

Thanks for the clarifications! I'm happy with this code. I'm now performing some testing locally, I will approve once I've confirmed this works.

Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke left a comment

Choose a reason for hiding this comment

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

I've confirmed that this fixes the issue using the test that I have just proposed for review in #761 (your review over there would be appreciated!). Thanks!

@OddBloke OddBloke merged commit 4f62ae8 into canonical:master Jan 11, 2021
@trstringer
Copy link
Copy Markdown
Contributor Author

Thanks, @OddBloke!

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