Skip to content

test: add unit tests for client certificate PEM/PKCS helpers#793

Merged
weinong merged 2 commits into
Azure:mainfrom
NaeemH:test/clientcert-helper-unit-tests
Jul 14, 2026
Merged

test: add unit tests for client certificate PEM/PKCS helpers#793
weinong merged 2 commits into
Azure:mainfrom
NaeemH:test/clientcert-helper-unit-tests

Conversation

@NaeemH

@NaeemH NaeemH commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This adds unit-test coverage for the pure certificate-parsing helpers in pkg/internal/token/clientcertcredential.go, which were previously untested.

What is covered

Table-driven tests for:

  • isPublicKeyEqual — identical / differing / nil-modulus keys
  • splitPEMBlock — separates cert vs. key blocks, ignores unknown block types, handles input with no PEM blocks
  • parseRsaPrivateKey — PKCS#1, PKCS#8 (RSA), PKCS#8 (non-RSA → error), non-PEM / empty input
  • parseKeyPairFromPEMBlock — matching pair, mismatched cert/key, missing key
  • decodePkcs12 — invalid input error path
  • readCertificate — valid .pem, missing .pem, missing .pfx

Notes

  • Certificates and keys are generated in-test (crypto/x509 self-signed), so no fixtures and no new dependencies are introduced.
  • Style matches the existing package tests (table-driven, testify).

Coverage impact

Package pkg/internal/token coverage improves 64.1% → 65.8%. Targeted helpers go from 0% to:

Function Coverage
isPublicKeyEqual 100%
splitPEMBlock 100%
parseRsaPrivateKey 92%
parseKeyPairFromPEMBlock 96%
readCertificate 89%
decodePkcs12 43% (error path; the success path needs a real PFX fixture)

Related to #7.

Adds table-driven unit tests for the previously untested pure helpers in
clientcertcredential.go: isPublicKeyEqual, splitPEMBlock,
parseRsaPrivateKey, parseKeyPairFromPEMBlock, decodePkcs12 (error path),
and readCertificate. Certificates and keys are generated in-test, so no
fixtures or new dependencies are introduced.

Package coverage improves from 64.1% to 65.8%; the targeted helpers go
from 0% to 89-100% (decodePkcs12 error path only, as the success path
needs a real PFX).

Related to Azure#7

Signed-off-by: Naeem Hossain <naeemhossain@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.61%. Comparing base (15edb67) to head (520db10).
⚠️ Report is 106 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #793      +/-   ##
==========================================
- Coverage   63.43%   59.61%   -3.83%     
==========================================
  Files          37       49      +12     
  Lines        2210     2481     +271     
==========================================
+ Hits         1402     1479      +77     
- Misses        733      844     +111     
- Partials       75      158      +83     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Adds tests that decode a password-protected PKCS#12 (PFX) via decodePkcs12
and readCertificate: correct password returns the cert/key, wrong password
errors. The fixture is an embedded, base64-encoded throwaway self-signed
cert using legacy SHA1/3DES PBE (the only algorithms x/crypto/pkcs12
supports); a comment documents how to regenerate it.

This exercises the client-certificate password path end to end, taking
decodePkcs12 0%->100% and readCertificate 89%->100%. The
--client-certificate-password flag requested in Azure#127 is already
implemented; these tests lock in its behavior.

Signed-off-by: Naeem Hossain <naeemhossain@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds unit-test coverage for the client certificate parsing helpers in pkg/internal/token, exercising PEM splitting, RSA key parsing, certificate/key matching, and PKCS#12 decode/read paths.

Changes:

  • Introduces table-driven tests for PEM/keypair helper functions (RSA PKCS#1/PKCS#8, mismatch/error cases).
  • Adds PKCS#12 (PFX) tests covering both correct/incorrect password behavior and .pfx reading via readCertificate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/internal/token/clientcertcredential_helpers_test.go Adds table-driven unit tests for PEM/keypair parsing helpers and error paths.
pkg/internal/token/clientcertcredential_pfx_test.go Adds PKCS#12/PFX-focused tests, including a known-good PFX blob to validate successful decode and readCertificate behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@weinong weinong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@weinong weinong merged commit 53de973 into Azure:main Jul 14, 2026
6 checks passed
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.

5 participants