Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improvement(tls-certs): collect and keep TLS/SSL artifacts #9219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dimakr
Copy link
Contributor

@dimakr dimakr commented Nov 14, 2024

Collect TLS/SSL artifacts (server, client certificates; CA certificate; etc.) and keep them after a test is finished, similarly to how logs are collected and published.
This will facilitate root cause analysis of SCT failures caused by certificate related issues (e.g. "Unknown Subject Alternative name in X.509 certificate" kind of errors, etc.).

Closes: #9133

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@dimakr dimakr marked this pull request as ready for review November 15, 2024 09:36
@dimakr dimakr requested review from soyacz and fruch and removed request for soyacz November 15, 2024 09:36
sdcm/logcollector.py Outdated Show resolved Hide resolved
@@ -2986,6 +2991,7 @@ def tearDown(self):
time.sleep(1) # Sleep is needed to let final event being saved into files
self.save_email_data()
self.argus_collect_gemini_results()
self.collect_certs()
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be called before the line 2988 (self.clean_resources())?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certs are collected from SCT runner, so cleaning cluster resources doesn't affect collecting certs.

Copy link
Contributor

Choose a reason for hiding this comment

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

we might also consider collecting from nodes, to validate we didn't mess-up transferring them to the correct nodes ?

Copy link
Contributor

@vponomaryov vponomaryov Nov 18, 2024

Choose a reason for hiding this comment

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

Gathering certs from nodes is also interesting, IMHO.
But it can be done later.

sdcm/logcollector.py Outdated Show resolved Hide resolved
vponomaryov
vponomaryov previously approved these changes Nov 18, 2024
Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

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

LGTM

Collect SSL configuration from SCT runner (node certificates/keys; CA certificate/key;
etc.) and node specific certificates from DB/loader nodes. Keep them after a test is
finished, similarly to how logs are collected and published.
This will facilitate root cause analysis of SCT failures caused by certificate
related issues.

Closes: scylladb#9133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node failed to run because of X.509 certificate error
3 participants