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.

Sign up for GitHub

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

Jump to bottom

Encapsulate Token operations #5502

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

Encapsulate Token operations #5502

wants to merge 1 commit into from

Conversation

WillardHu
Copy link
Collaborator

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Encapsulate Token operations to improve readability and maintainability.

Which issue(s) this PR fixes:

A part of issue #5498

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@kubeedge-bot kubeedge-bot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Apr 1, 2024
@kubeedge-bot kubeedge-bot requested a review from GsssC April 1, 2024 08:19
@kubeedge-bot kubeedge-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2024
@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign willardhu after the PR has been reviewed.
You can assign the PR to them by writing /assign @willardhu in a comment when ready.

The full list of commands accepted by this bot can be found 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

@kubeedge-bot kubeedge-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 1, 2024
@WillardHu WillardHu mentioned this pull request Apr 1, 2024
Open
6 tasks
@WillardHu WillardHu changed the title [WIP] Encapsulate Token operations Encapsulate Token operations Apr 8, 2024
@WillardHu WillardHu mentioned this pull request Apr 17, 2024
Open
fisherxu
fisherxu reviewed Apr 19, 2024
View reviewed changes
Copy link
Member

@fisherxu fisherxu left a comment

Choose a reason for hiding this comment

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

Is this ready for review? :) @WillardHu

@WillardHu
Copy link
Collaborator Author

Is this ready for review? :) @WillardHu

Yes, this task has been completed.

@WillardHu WillardHu closed this May 9, 2024
@WillardHu WillardHu reopened this May 9, 2024
@kubeedge-bot kubeedge-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 9, 2024
@WillardHu WillardHu closed this May 9, 2024
@WillardHu WillardHu reopened this May 9, 2024
@WillardHu
Copy link
Collaborator Author

This change passed some tests in my k8s cluster

  • Cloudcore generates a self-signed ca certificate initially ✅
  • An new joined edge node(Edgecore) can connect to Cloudcore and obtains a service certificate ✅
  • The token can be correctly verified when the edge node request to the Cloudcore with a Authentication header ✅

PTAL @fisherxu @Shelley-BaoYue

Shelley-BaoYue
Shelley-BaoYue reviewed May 10, 2024
View reviewed changes
cloud/pkg/cloudhub/servers/httpserver/server.go Outdated
@@ -141,7 +142,9 @@ func verifyCertSubject(cert *x509.Certificate, nodeName string) error {
// verifyAuthorization verifies the token from EdgeCore CSR
func verifyAuthorization(w http.ResponseWriter, r *http.Request) bool {
authorizationHeader := r.Header.Get("authorization")
klog.V(4).Info("authorization token is: ", authorizationHeader)
Copy link
Collaborator

Choose a reason for hiding this comment

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

log the token info may cause the security problem

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The log print has been deleted, thanks

@WillardHu
Encapsulate Token operations
5c96472 
Signed-off-by: WillardHu <wei.hu@daocloud.io>
@WillardHu WillardHu closed this May 10, 2024
@WillardHu WillardHu reopened this May 10, 2024
@WillardHu WillardHu closed this May 10, 2024
@WillardHu WillardHu reopened this May 10, 2024
@WillardHu WillardHu requested a review from fisherxu May 10, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GsssC GsssC Awaiting requested review from GsssC

@Poor12 Poor12 Awaiting requested review from Poor12

@sids-b sids-b Awaiting requested review from sids-b

@Shelley-BaoYue Shelley-BaoYue Awaiting requested review from Shelley-BaoYue

@fisherxu fisherxu Awaiting requested review from fisherxu

Assignees
No one assigned
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@WillardHu @kubeedge-bot @fisherxu @Shelley-BaoYue