Proposal: Support Custom CA Certificate for Karmada Instance in Operator #5794

jabellard · 2024-11-08T00:25:49Z

What type of PR is this?

/kind design

What this PR does / why we need it:

Details are contained in the proposal.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

codecov-commenter · 2024-11-08T00:56:20Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.55%. Comparing base (0cc294f) to head (22ddd9d).
Report is 31 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5794      +/-   ##
==========================================
+ Coverage   42.41%   45.55%   +3.13%     
==========================================
  Files         656      658       +2     
  Lines       55884    53926    -1958     
==========================================
+ Hits        23705    24565     +860     
+ Misses      30659    27755    -2904     
- Partials     1520     1606      +86

Flag	Coverage Δ
unittests	`45.55% <ø> (+3.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

RainbowMango

Looks great, thank you @jabellard.

Just want to give another look regarding to certificate rotation, we probably need to reserve extension for that.
Let me know your thoughts about rotation feature.

docs/proposals/karmada-operator/custom_ca_cert/README.md

jabellard · 2024-11-08T12:54:59Z

@RainbowMango , @zhzhuang-zju : Thanks for your comments and questions. Just pushed a change to refine this proposal based on your feedback. Just let me know if you have other comments.

zhzhuang-zju · 2024-11-11T02:34:03Z

docs/proposals/karmada-operator/custom_ca_cert/README.md

+   // +kubebuilder:validation:Enum=Auto;Custom
+   // +kubebuilder:default=Auto
+   Mode CertificateMode `json:"mode,omitempty"`
+
+   // Custom contains the configuration for user-provided certificates when in Custom mode.
+   // +optional
+   Custom *CustomCertConfig `json:"custom,omitempty"`


Just a question, if the CA that can be customized is not limited to KarmadaCertRootCA and the API also allows customization of FrontProxyCA, how should the Mode be used in conjunction with Custom? For example, in a scenario where KarmadaCertRootCA needs to be customized, but FrontProxyCA does not need to be customized?

@zhzhuang-zju , good question. Given that the front proxy CA is used exclusively for in-cluster communication, I don't have a use case for customizing it, so right now, the plan is to add support only for customizing the Root/Kubernetes CA. However, if a use case for customizing it does arise, then we can easily add support for. It's a completely independent, self-singed CA:

Certificate: Data: Version: 3 (0x2) Serial Number: 4436933017513869186 (0x3d932712511dbf82) Signature Algorithm: sha256WithRSAEncryption Issuer: CN=front-proxy-ca Validity Not Before: Nov 2 17:08:48 2024 GMT Not After : Oct 31 17:08:48 2034 GMT Subject: CN=front-proxy-ca Subject Public Key Info: Public Key Algorithm: rsaEncryption Public-Key: (3072 bit) ...

Thanks for the explanation. It makes sense. As long as the extensibility is ensured, we can consider supporting more certificate configs when there are relevant use cases in the future.

@RainbowMango , @zhzhuang-zju , any more questions/comments?

Will make name of CA cert more specific. Maybe KarmadaRootCACert???

Add optional tag to secret ref for Karmada root CA cert as it is optional.

jabellard · 2024-11-14T13:55:46Z

@RainbowMango , I pushed changes. Any other comments?

Also, out of curiosity, is any of the Karmada maintainers part of the multi cluster SIG? I'm currently at KubeCon and have been talking to some of the members of that group. Looks like they're some overlap in the work that they do.

RainbowMango · 2024-11-15T01:55:10Z

It's still in my queue, I will get back on this ASAP.

Looks like they're some overlap in the work that they do.

I contributed a lot to that SIG, what kind of work overlap you are talking about?

jabellard · 2024-11-15T02:05:33Z

It's still in my queue, I will get back on this ASAP.

Looks like they're some overlap in the work that they do.

I contributed a lot to that SIG, what kind of work overlap you are talking about?

Yeah. I was wondering if you've contributed to that SIG and just found out you've contributed to the Work API. That's awesome!

RainbowMango · 2024-11-15T09:01:44Z

Hi @jabellard I worked out another approach that doesn't need the CertificateMode, please take a look:

// KarmadaSpec is the specification of the desired behavior of the Karmada.
type KarmadaSpec struct {
	// CustomCertificate specifies the configuration to customize the certificates
	// for Karmada components or control the certificate generation process, such as
	// the algorithm, validity period, etc.
	// Currently, it only supports customizing the CA certificate for limited components.
	// +optional
	CustomCertificate *CustomCertificate `json:"customCertificate,omitempty"`
}

// CustomCertificate holds the configuration for generating the certificate.
type CustomCertificate struct {
	// APIServerCACert references a Kubernetes secret containing the CA certificate
	// for component karmada-apiserver.
	// The secret must contain the following data keys:
	// - tls.crt: The TLS certificate.
	// - tls.key: The TLS private key.
	// If specified, this CA will be used to issue client certificates for
	// all components that access the APIServer as clients.
	// +optional
	APIServerCACert *LocalSecretReference `json:"apiServerCACert,omitempty"`
}

I'm concerned about the CertificateMode as it might bring ambitions, since Custom seems like all certificates will be customized.

jabellard · 2024-11-15T11:37:28Z

@RainbowMango , I think that's a valid concern. Thanks for thinking about how to remedy it. This design is also a bit simpler. Will push an update to incorporate it.

RainbowMango

/lgtm
Thank you @jabellard for the effort.

I think it's ready to go after squashing the commits.

Signed-off-by: Joe Nathan Abellard <[email protected]>

jabellard · 2024-11-16T13:03:28Z

/lgtm Thank you @jabellard for the effort.

I think it's ready to go after squashing the commits.

@RainbowMango , thanks for the constructive feedback. Just squashed.

RainbowMango

/lgtm
/approve
Thanks.

karmada-bot · 2024-11-16T13:46:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jabellard · 2024-11-20T00:48:56Z

Hey @RainbowMango , I just submitted the implementation PR for this. Can you please take a look?

RainbowMango · 2024-11-20T01:49:49Z

Sure. I hope this can be included in the coming release (v1.12.0) which is planed at the end of this month.

karmada-bot added the kind/design Categorizes issue or PR as related to design. label Nov 8, 2024

karmada-bot requested review from Poor12 and Tingtal November 8, 2024 00:25

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 8, 2024

jabellard mentioned this pull request Nov 8, 2024

Add Support for Custom Certificates in Karmada Operator #5784

Closed

RainbowMango reviewed Nov 8, 2024

View reviewed changes

jabellard force-pushed the karmada_custom_ca branch from 0602866 to 907b4fc Compare November 8, 2024 12:49

zhzhuang-zju reviewed Nov 11, 2024

View reviewed changes

RainbowMango reviewed Nov 16, 2024

View reviewed changes

karmada-bot assigned RainbowMango Nov 16, 2024

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2024

Propose support for custom CA cert

22ddd9d

Signed-off-by: Joe Nathan Abellard <[email protected]>

jabellard force-pushed the karmada_custom_ca branch from 807cb7c to 22ddd9d Compare November 16, 2024 13:01

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2024

RainbowMango reviewed Nov 16, 2024

View reviewed changes

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2024

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2024

karmada-bot merged commit c7bc870 into karmada-io:master Nov 16, 2024
18 checks passed

jabellard mentioned this pull request Nov 20, 2024

Support Custom API Server CA Certificate for Karmada Instance in Operator #5842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal: Support Custom CA Certificate for Karmada Instance in Operator #5794

Proposal: Support Custom CA Certificate for Karmada Instance in Operator #5794

jabellard commented Nov 8, 2024

codecov-commenter commented Nov 8, 2024 •

edited

Loading

RainbowMango left a comment

jabellard commented Nov 8, 2024

zhzhuang-zju Nov 11, 2024

jabellard Nov 11, 2024

zhzhuang-zju Nov 12, 2024

jabellard Nov 12, 2024

jabellard Nov 12, 2024

jabellard Nov 12, 2024

jabellard commented Nov 14, 2024

RainbowMango commented Nov 15, 2024

jabellard commented Nov 15, 2024

RainbowMango commented Nov 15, 2024

jabellard commented Nov 15, 2024

RainbowMango left a comment

jabellard commented Nov 16, 2024

RainbowMango left a comment

karmada-bot commented Nov 16, 2024

jabellard commented Nov 20, 2024

RainbowMango commented Nov 20, 2024

Proposal: Support Custom CA Certificate for Karmada Instance in Operator #5794

Proposal: Support Custom CA Certificate for Karmada Instance in Operator #5794

Conversation

jabellard commented Nov 8, 2024

codecov-commenter commented Nov 8, 2024 • edited Loading

Codecov Report

RainbowMango left a comment

Choose a reason for hiding this comment

jabellard commented Nov 8, 2024

zhzhuang-zju Nov 11, 2024

Choose a reason for hiding this comment

jabellard Nov 11, 2024

Choose a reason for hiding this comment

zhzhuang-zju Nov 12, 2024

Choose a reason for hiding this comment

jabellard Nov 12, 2024

Choose a reason for hiding this comment

jabellard Nov 12, 2024

Choose a reason for hiding this comment

jabellard Nov 12, 2024

Choose a reason for hiding this comment

jabellard commented Nov 14, 2024

RainbowMango commented Nov 15, 2024

jabellard commented Nov 15, 2024

RainbowMango commented Nov 15, 2024

jabellard commented Nov 15, 2024

RainbowMango left a comment

Choose a reason for hiding this comment

jabellard commented Nov 16, 2024

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Nov 16, 2024

jabellard commented Nov 20, 2024

RainbowMango commented Nov 20, 2024

codecov-commenter commented Nov 8, 2024 •

edited

Loading