Skip to content

test: improve self-signed certificate cleanup#3706

Merged
reubenmiller merged 6 commits intothin-edge:mainfrom
reubenmiller:test-track-certificates
Jun 27, 2025
Merged

test: improve self-signed certificate cleanup#3706
reubenmiller merged 6 commits intothin-edge:mainfrom
reubenmiller:test-track-certificates

Conversation

@reubenmiller
Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller commented Jun 24, 2025 •
edited
Loading

Proposed changes

Improve the tracking of self-signed certificates within the tests. With the changes, the system tests no longer leave self-signed certificate which where generated within the tests.

  • New keyword to allow a certificate to be registered for cleanup
  • Record the certificates after each test (instead of assuming the cert still exists at the end of the suite)
  • Fix thumbprint parsing logic
  • Use Cumulocity CA to register devices in the HSM tests, however a the RSA private key with a bit length of 1024 had to be removed as it isn't supported by the Cumulocity CA and is anyway deemed insecure by today's security recommendatons

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 24, 2025 17:25 — with GitHub Actions Inactive
@reubenmiller reubenmiller added the theme:testing Theme: Testing label Jun 24, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 24, 2025 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
652 0 3 652 100 1h47m1.985242s

@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 24, 2025 18:47 — with GitHub Actions Inactive
@reubenmiller reubenmiller force-pushed the test-track-certificates branch from b2bf978 to cb685e2 Compare June 24, 2025 19:23
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 24, 2025 19:23 — with GitHub Actions Inactive
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 24, 2025 21:14 — with GitHub Actions Inactive
@reubenmiller
update test dependencies
51a3d2e 
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller reubenmiller force-pushed the test-track-certificates branch from 1ba70e3 to 9ddfb0f Compare June 25, 2025 07:25
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 25, 2025 07:25 — with GitHub Actions Inactive
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 26, 2025 07:21 — with GitHub Actions Inactive
@reubenmiller reubenmiller marked this pull request as ready for review June 26, 2025 07:59
@reubenmiller reubenmiller requested review from a team, albinsuresh and jarhodes314 as code owners June 26, 2025 07:59
@Bravo555 Bravo555 self-requested a review June 26, 2025 08:44
@Bravo555
Copy link
Copy Markdown
Member

Bravo555 commented Jun 26, 2025

Instead of manually tracking individual certificates, couldn't we perhaps just remove ALL the certificates from the tenant that match the test device name (and children)? These names should be globally unique, so no other device should be affected and additionally:

  • if I debug a suite and create more certificates from the shell these would also get cleaned up
  • if the test suite mistakenly didn't use the new keyword and uploaded the cert manually, this would also get cleaned up automatically
  • the implementation could be much simplified

I think this would be a good enough solution assuming that from within the tests we only create certs with the name of the device (and children) or I'm otherwise not missing anything else.

Bravo555
Bravo555 reviewed Jun 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Bravo555 Bravo555 left a comment

Choose a reason for hiding this comment

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

Looks alright, still have some suggestions and questions, will approve after they're addressed.

@reubenmiller reubenmiller mentioned this pull request Jun 26, 2025
Merged
11 tasks
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 26, 2025 19:41 — with GitHub Actions Inactive
@reubenmiller reubenmiller mentioned this pull request Jun 26, 2025
Merged
11 tasks
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 27, 2025 06:13 — with GitHub Actions Inactive
@reubenmiller
Copy link
Copy Markdown
Contributor Author

reubenmiller commented Jun 27, 2025

@Bravo555 I pushed a fix, ab51521, as I noticed that the Cumulocity device/user cleanup logic wasn't being run when self-signed certificates were not being used...but the latest run leaves a clean environment now.

Bravo555
Bravo555 approved these changes Jun 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Bravo555 Bravo555 left a comment

Choose a reason for hiding this comment

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

Approved

@reubenmiller
track certificates after each test and allow users to manually regist…
0f3a7a3 
…er certificates for cleanup

Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller
register self-signed certificates for cleanup within tests
e6d180a 
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller
skip bootstrapping to creating initial certs
45f8d25 
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller
use cumulocity certificate-authority for device registration to avoid…
9fe4f50 
… self-signed certificates

Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller
remove test case for RSA keys with a key length of 1024 bits
9842236 
RSA 1024 keys aren't currently supported by the Cumulocity Certificate Authority feature.
It is unclear whether they will be allowed or not in the future given that the key size
is considered to be insecure.

Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller reubenmiller force-pushed the test-track-certificates branch from ab51521 to 9842236 Compare June 27, 2025 08:19
@reubenmiller reubenmiller temporarily deployed to Test Pull Request June 27, 2025 08:19 — with GitHub Actions Inactive
@reubenmiller reubenmiller added this pull request to the merge queue Jun 27, 2025
Merged via the queue into thin-edge:main with commit 36d4d52 Jun 27, 2025
34 checks passed
@Bravo555 Bravo555 mentioned this pull request Jul 1, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bravo555 Bravo555 Bravo555 approved these changes

@albinsuresh albinsuresh Awaiting requested review from albinsuresh albinsuresh is a code owner

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

Assignees

No one assigned

Labels

theme:testing Theme: Testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@reubenmiller @Bravo555