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

refactor: use TLS library v4 #206

Merged
merged 11 commits into from
Aug 8, 2024
Merged

refactor: use TLS library v4 #206

merged 11 commits into from
Aug 8, 2024

Conversation

gruyaume
Copy link
Contributor

@gruyaume gruyaume commented Jul 30, 2024

Description

Bump the TLS library to V4.

Addition of certificate helper methods

The addition of src/certificates.py is because the TLS library V4 does not provide the generate_certificate, generate_private_key, and generate_ca helpers as those were specific to this self-signed certificates charm.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have bumped the version of the library

Signed-off-by: guillaume <[email protected]>
@gruyaume gruyaume changed the title chore: bump tls lib to v4 chore: refactor code to use TLS library to v4 Jul 31, 2024
@gruyaume gruyaume changed the title chore: refactor code to use TLS library to v4 chore: refactor code to use TLS library v4 Jul 31, 2024
@gruyaume gruyaume changed the title chore: refactor code to use TLS library v4 chore: refactor to use TLS library v4 Jul 31, 2024
@gruyaume gruyaume changed the title chore: refactor to use TLS library v4 refactor: use TLS library v4 Jul 31, 2024
Signed-off-by: guillaume <[email protected]>
@gruyaume gruyaume marked this pull request as ready for review August 6, 2024 15:26
@gruyaume gruyaume requested a review from a team as a code owner August 6, 2024 15:26
Copy link
Contributor

@saltiyazan saltiyazan left a comment

Choose a reason for hiding this comment

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

I went through it and it looks really good. I have a few comments some of which might not be directly on the PR but I think they could be included here.

src/certificates.py Show resolved Hide resolved
tests/unit/certificates_helpers.py Outdated Show resolved Hide resolved
src/certificates.py Show resolved Hide resolved
src/certificates.py Outdated Show resolved Hide resolved
src/certificates.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/unit/test_certificates.py Outdated Show resolved Hide resolved
tests/unit/test_certificates.py Outdated Show resolved Hide resolved
@gruyaume gruyaume requested a review from saltiyazan August 7, 2024 17:57
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

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

one small comment

src/charm.py Show resolved Hide resolved
Copy link
Contributor

@saltiyazan saltiyazan left a comment

Choose a reason for hiding this comment

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

Looks good. I created tickets for the comments that would not be addressed as part of this PR.
For the comment on get_certificate_extensions I prefer a better name, but I'll leave it up to you to resolve the comment either accepting or rejecting the new suggestion.

@gruyaume gruyaume merged commit d7fbd76 into main Aug 8, 2024
12 checks passed
@gruyaume gruyaume deleted the dev-v4 branch August 8, 2024 12:16
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.

3 participants