Skip to content

refactor(azure): Extract SSH key cert handling to new certs module#6561

Closed
peytonr18 wants to merge 4 commits intocanonical:mainfrom
peytonr18:probertson-extract-ssh-key-cert
Closed

refactor(azure): Extract SSH key cert handling to new certs module#6561
peytonr18 wants to merge 4 commits intocanonical:mainfrom
peytonr18:probertson-extract-ssh-key-cert

Conversation

@peytonr18
Copy link
Copy Markdown
Contributor

@peytonr18 peytonr18 commented Nov 6, 2025

Proposed Commit Message

refactor(azure): Extract SSH key cert handling to new certs module

Refactor x509 certificate and SSH key handling logic from 
OpenSSLManager and DataSourceAzure into a new dedicated 
module cloudinit/sources/azure/certs.py with three utility functions:

  - is_openssh_formatted(): Validates OpenSSH format keys
  - is_x509_certificate(): Validates x509 certificates via openssl
  - convert_x509_to_openssh(): Converts x509 certs to OpenSSH keys

This refactoring maintains existing functionality without changes while 
preparing the codebase for upcoming work to support x509 certificates 
from Azure IMDS.

Additional Context

Purpose:
Refactors x509 certificate and SSH key handling logic from OpenSSLManager and DataSourceAzure into a dedicated module (cloudinit/sources/azure/certs.py).

Changes:

  • New module: cloudinit/sources/azure/certs.py with three functions:
  • is_openssh_formatted() - Validates OpenSSH format keys
  • is_x509_certificate() - Validates x509 certificates using openssl
  • convert_x509_to_openssh() - Converts x509 certs to OpenSSH keys
  • Updated: cloudinit/sources/helpers/azure.py --> _get_ssh_key_from_cert() now uses new module
  • Updated: cloudinit/sources/DataSourceAzure.py --> _key_is_openssh_formatted() now uses new module

Test Coverage:

  • Unit tests for OpenSSH key validation and basic checks (various types, edge cases, markers, empty strings)
  • Integration tests with openssl for x509 validation & with openssl + ssh-keygen for cert conversion
  • Mocked tests for error handling (openssl/ssh-keygen failures)

Merge type

  • Squash merge using "Proposed Commit Message"

@peytonr18 peytonr18 marked this pull request as ready for review November 6, 2025 21:54
Comment thread cloudinit/sources/azure/certs.py Outdated
Comment thread cloudinit/sources/azure/certs.py
@peytonr18 peytonr18 force-pushed the probertson-extract-ssh-key-cert branch 3 times, most recently from 45f78e2 to f4e3af9 Compare November 9, 2025 07:21
…sity

Added extract_x509_certificate() to validate certificates in bundles, integrated validation into parse_certificates(), and toned down debug logs to avoid sounding like failures.
@peytonr18 peytonr18 force-pushed the probertson-extract-ssh-key-cert branch from f4e3af9 to 90eb57b Compare November 9, 2025 08:50
@peytonr18
Copy link
Copy Markdown
Contributor Author

The following was discussed offline, and these changes are reflected in the most recent commit:

Re: The current parse_certificates() really extracts the x509 certificate from whatever bundle it might be in, I'm inclined to think the safest bet is to essentially extract this into another interface:
extract_x509_certificate(key_data: str) -> str | None:

def parse_certificates(self, certificates_xml):

        """Given the Certificates XML document, return a dictionary of
        fingerprints and associated SSH keys derived from the certs."""
        out = self._decrypt_certs_from_xml(certificates_xml)
        current = []
        keys = {}
        for line in out.splitlines():
            current.append(line)
            if re.match(r"[-]+END .*?KEY[-]+$", line):
                # ignore private_keys
                current = []
            elif re.match(r"[-]+END .*?CERTIFICATE[-]+$", line):
                certificate = "\n".join(current)
                ssh_key = self._get_ssh_key_from_cert(certificate)
                fingerprint = self._get_fingerprint_from_cert(certificate)
                keys[fingerprint] = ssh_key
                current = []
        return keys

Comment thread cloudinit/sources/azure/certs.py Outdated
@holmanb
Copy link
Copy Markdown
Member

holmanb commented Nov 20, 2025

It looks like this is waiting for updates per the latest review, so I'll hold off on reviewing it for now.

Fold the regex-based certificate parsing into cloudinit.sources.azure.certs so
helpers no longer reimplement it, and wire OpenSSLManager.parse_certificates to
loop over the helper. Added a regression test that confirms CRLF-mixed bundles
still yield every fingerprint + key pair.
@peytonr18 peytonr18 force-pushed the probertson-extract-ssh-key-cert branch from 4ad543b to e401a2e Compare November 21, 2025 21:39
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 6, 2025

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 blackboxsw, 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 blackboxsw to reopen it.)

@github-actions github-actions Bot added the stale-pr Pull request is stale; will be auto-closed soon label Dec 6, 2025
@github-actions github-actions Bot closed this Dec 13, 2025
@peytonr18
Copy link
Copy Markdown
Contributor Author

We are making changes on our end before reviewing this one to make sure our testing infrastructure is prepared to handle this change. Once those are finished, I'll reopen this PR for review.

@peytonr18
Copy link
Copy Markdown
Contributor Author

@blackboxsw Looking to reopen this PR now that we have completed the basework, so I can continue working on it. Thanks!

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

Labels

stale-pr Pull request is stale; will be auto-closed soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants