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

Standardize the generation and management of certificates #2097

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

lonelyCZ
Copy link
Member

Signed-off-by: lonelyCZ 531187475@qq.com

What type of PR is this?

/kind feature

What this PR does / why we need it:

Standardize the generation and management of certificates to prepare for certificate authentication and certificate rotation.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@karmada-bot karmada-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 30, 2022
@karmada-bot karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 30, 2022
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 6, 2022
@lonelyCZ lonelyCZ changed the title [WIP]Standardize the generation and management of certificates Standardize the generation and management of certificates Jul 6, 2022
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 6, 2022
@lonelyCZ
Copy link
Member Author

lonelyCZ commented Jul 6, 2022

I have updated the generated cert directory, and make the CN of CA clearer.

[root@master67 .karmada]# tree
.
├── apiserver.crt
├── apiserver-etcd-client.crt
├── apiserver-etcd-client.key
├── apiserver.key
├── ca-config.json
├── ca.crt
├── ca.key
├── etcd
│   ├── ca-config.json
│   ├── ca.crt
│   ├── ca.key
│   ├── server.crt
│   └── server.key
├── front-proxy-ca-config.json
├── front-proxy-ca.crt
├── front-proxy-ca.key
├── front-proxy-client.crt
├── front-proxy-client.key
├── karmada.crt
└── karmada.key
[root@master67 .karmada]# openssl x509 -in ca.crt -text -noout
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            bb:b2:a7:ba:72:60:31:53
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN=karmada
        Validity
            Not Before: Jul  5 17:09:14 2022 GMT
            Not After : Jul  2 17:09:14 2032 GMT

@lonelyCZ
Copy link
Member Author

lonelyCZ commented Jul 6, 2022

/retest

@lonelyCZ
Copy link
Member Author

lonelyCZ commented Jul 6, 2022

helm and karmadactl init will be changed in next prs. These are same as kubernetes/pki

[root@master67 pki]# tree
.
├── apiserver.crt
├── apiserver-etcd-client.crt
├── apiserver-etcd-client.key
├── apiserver.key
├── apiserver-kubelet-client.crt
├── apiserver-kubelet-client.key
├── ca.crt
├── ca.key
├── etcd
│   ├── ca.crt
│   ├── ca.key
│   ├── healthcheck-client.crt
│   ├── healthcheck-client.key
│   ├── peer.crt
│   ├── peer.key
│   ├── server.crt
│   └── server.key
├── front-proxy-ca.crt
├── front-proxy-ca.key
├── front-proxy-client.crt
├── front-proxy-client.key
├── sa.key
└── sa.pub

I generate some common components certificates, but don't generate exclusive certs for karmada-search and karmada-aggregated-apiserver. Do we need generate exclusive certs for them?

/cc @RainbowMango @XiShanYongYe-Chang

@XiShanYongYe-Chang
Copy link
Member

I generate some common components certificates, but don't generate exclusive certs for karmada-search and karmada-aggregated-apiserver. Do we need generate exclusive certs for them?

Actually, I'm not so sure. Is it necessary for us to use an exclusive cert?

@lonelyCZ
Copy link
Member Author

lonelyCZ commented Aug 2, 2022

Unify format,

[root@master67 .karmada]# ll
total 76
-rw-r--r--. 1 root root 1354 Aug  2 09:42 apiserver.crt
-rw-------. 1 root root 1679 Aug  2 09:42 apiserver.key
-rw-r--r--. 1 root root  112 Aug  2 09:42 ca-config.json
-rw-r--r--. 1 root root 1090 Aug  2 09:42 ca.crt
-rw-r--r--. 1 root root 1704 Aug  2 09:42 ca.key
-rw-r--r--. 1 root root  112 Aug  2 09:42 etcd-ca-config.json
-rw-r--r--. 1 root root 1090 Aug  2 09:42 etcd-ca.crt
-rw-r--r--. 1 root root 1704 Aug  2 09:42 etcd-ca.key
-rw-r--r--. 1 root root 1346 Aug  2 09:42 etcd-client.crt
-rw-------. 1 root root 1679 Aug  2 09:42 etcd-client.key
-rw-r--r--. 1 root root 1387 Aug  2 09:42 etcd-server.crt
-rw-------. 1 root root 1679 Aug  2 09:42 etcd-server.key
-rw-r--r--. 1 root root  112 Aug  2 09:42 front-proxy-ca-config.json
-rw-r--r--. 1 root root 1107 Aug  2 09:42 front-proxy-ca.crt
-rw-r--r--. 1 root root 1708 Aug  2 09:42 front-proxy-ca.key
-rw-r--r--. 1 root root 1407 Aug  2 09:42 front-proxy-client.crt
-rw-------. 1 root root 1675 Aug  2 09:42 front-proxy-client.key
-rw-r--r--. 1 root root 1432 Aug  2 09:42 karmada.crt
-rw-------. 1 root root 1675 Aug  2 09:42 karmada.key

@lonelyCZ
Copy link
Member Author

I generate some common components certificates, but don't generate exclusive certs for karmada-search and karmada-aggregated-apiserver. Do we need generate exclusive certs for them?

Actually, I'm not so sure. Is it necessary for us to use an exclusive cert?

For simplicity, we can use unifily etcd-client.crt for other components to connect etcd.

@lonelyCZ
Copy link
Member Author

/cc @RainbowMango

Signed-off-by: lonelyCZ <531187475@qq.com>
@RainbowMango
Copy link
Member

Generally looks good to me.
I can start working on it tomorrow.
cc @mrlihanbo Could you please help to take a look?

@RainbowMango
Copy link
Member

/assign

@RainbowMango
Copy link
Member

Looks good!
question on #2097 (comment).

What's the difference between apiserver.crt/apiserver.key and karmada.crt/karmada.key?

@lonelyCZ
Copy link
Member Author

What's the difference between apiserver.crt/apiserver.key and karmada.crt/karmada.key?

Apiserver maintains mutual authentication with other components, so the certificates should preferably be different. The apiserver.crt is specially used by karmada apiserver side.

Each component should have its own certificate, but for convenience, the non-core components will use the Karmda.crt certificate directly, because they don't need to interact with too many other components.

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/approve

Leave LGTM to @mrlihanbo

@karmada-bot
Copy link
Collaborator

[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:

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

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2022
@mrlihanbo
Copy link

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022
@karmada-bot karmada-bot merged commit f80070f into karmada-io:master Aug 18, 2022
@lonelyCZ lonelyCZ deleted the pr-normalize-cert branch August 21, 2022 09:25
@RainbowMango RainbowMango added this to the v1.3 milestone Aug 27, 2022
@RainbowMango
Copy link
Member

Hi @lonelyCZ,

May this change block users from upgrading, especially from v1.2 to the incoming v1.3? Or, is there anything special we need to explain in upgrading docs? like
https://karmada.io/docs/administrator/upgrading/v1.1-v1.2

@RainbowMango
Copy link
Member

@yy158775 is trying to verify if v1.2 can be smoothly upgraded from v1.2 to v1.3, he met some issues, but not sure if it is caused by this issue yet.

@lonelyCZ
Copy link
Member Author

May this change block users from upgrading, especially from v1.2 to the incoming v1.3? Or, is there anything special we need to explain in upgrading docs?

In principle, we just need to regenerate the certificate and then modify the 'Karmada-Kubeconfig' secret and the YAML of each component.

In fact, we can upgrade the version without modifying the certificate, because the PR just standardizes the certificate format.

@yy158775 is trying to verify if v1.2 can be smoothly upgraded from v1.2 to v1.3, he met some issues, but not sure if it is caused by this issue yet.

What are the specific problems?Perhaps, I could also investigate it.

@RainbowMango
Copy link
Member

In fact, we can upgrade the version without modifying the certificate, because the PR just standardizes the certificate format.

Yes, this PR just updated the path related. Should not block upgrading.

@yy158775
Copy link
Contributor

yy158775 commented Aug 30, 2022

The upgrading on the other machine has no problem.My previous problem has nothing to do with this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants