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

Initial version of External Certificates integration #98

Merged
merged 6 commits into from
May 9, 2024

Conversation

mateoflorido
Copy link
Member

@mateoflorido mateoflorido commented May 3, 2024

Applicable spec

KU083 - Vault integration

Overview

Implement the External Certificates relation using TLSCertificatesV3.

Rationale

This pull request introduces the capability of integrating the charm with external certificate providers like Vault or charms that use the TLSCertificatesV3 library, such as self-signed-certificates.

This pull request only tackles the bootstrap certificates generation. The control plane and worker certificates will be implemented in separate pull requests in the future.

Module Changes

  • Added a K8sCertificates module, which manages the request process for the Kubernetes' components certificates.

Library Changes

  • Added TLSCertificatesV3.

Copy link
Contributor

github-actions bot commented May 3, 2024

Everyone contributing to this PR have now signed the CLA. Thanks!

@mateoflorido mateoflorido changed the title Initial version of Vault integration Initial version of External Certificates integration May 7, 2024
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from f571351 to 139a91a Compare May 7, 2024 20:45
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from 139a91a to 14a739b Compare May 7, 2024 20:56
@addyess
Copy link
Contributor

addyess commented May 7, 2024

/canonical/self-hosted-runners/run-workflows 14a739b

@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from 1296c3a to 74237df Compare May 7, 2024 21:25
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from 74237df to b387b1a Compare May 8, 2024 13:28
@mateoflorido mateoflorido marked this pull request as ready for review May 8, 2024 13:30
@mateoflorido mateoflorido requested a review from a team as a code owner May 8, 2024 13:30
Copy link
Contributor

@addyess addyess left a comment

Choose a reason for hiding this comment

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

I have questions concerning the expiration of certificates, testing of those situations, and testing of other ubuntu bases. I don't expect we address all of them at once.

the most worrisome to me is certificate expiration

charms/worker/k8s/requirements.txt Outdated Show resolved Hide resolved
charms/worker/k8s/src/charm.py Outdated Show resolved Hide resolved
charms/worker/k8s/src/charm.py Outdated Show resolved Hide resolved
charms/worker/k8s/src/certificates.py Show resolved Hide resolved
charms/worker/k8s/src/certificates.py Outdated Show resolved Hide resolved
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from 656c1be to fdbb09a Compare May 8, 2024 19:15
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from fdbb09a to e7c7d29 Compare May 8, 2024 19:18
@mateoflorido mateoflorido force-pushed the KU-417/external-certs branch from a76ae26 to d838d04 Compare May 8, 2024 21:45
@mateoflorido mateoflorido requested a review from addyess May 9, 2024 14:06
Copy link
Contributor

@addyess addyess left a comment

Choose a reason for hiding this comment

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

amazing 😍 Lgtm

Comment on lines -313 to +307
config (BootstrapConfig|UpdateClusterConfigRequst):
config (BootstrapConfig|UpdateClusterConfigRequest):
Copy link
Contributor

Choose a reason for hiding this comment

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

😃

Comment on lines -173 to +176
app = self.applications[name]
app["charm"] = str(path.resolve())
app["channel"] = None
# FIXME: Omit non present charms
try:
app = self.applications[name]
app["charm"] = str(path.resolve())
app["channel"] = None
except KeyError:
log.warning("Application %s not found in bundle", name)
Copy link
Contributor

Choose a reason for hiding this comment

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

whats this for? oh? for test bundles where theres no worker charm?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it should be there until we add support for certificates in k8s-worker

@mateoflorido mateoflorido merged commit 5d77610 into certificates May 9, 2024
41 checks passed
@mateoflorido mateoflorido deleted the KU-417/external-certs branch May 9, 2024 17:47
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.

2 participants