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

chore: support to update and delete system accounts #8756

Merged
merged 14 commits into from
Jan 21, 2025
Merged

chore: support to update and delete system accounts #8756

merged 14 commits into from
Jan 21, 2025

Conversation

leon-inf
Copy link
Contributor

@leon-inf leon-inf commented Jan 7, 2025

No description provided.

@github-actions github-actions bot added the size/XL Denotes a PR that changes 500-999 lines. label Jan 7, 2025
@leon-inf leon-inf force-pushed the support/system-account branch from 7afc8b9 to 74eff1d Compare January 7, 2025 06:54
@leon-inf leon-inf force-pushed the support/system-account branch from 74eff1d to 791e7a4 Compare January 7, 2025 08:33
@codecov Codecov
Copy link

codecov bot commented Jan 7, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 68.48485% with 104 lines in your changes missing coverage. Please review.

Project coverage is 60.45%. Comparing base (fbd2ea0) to head (823dfd9).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...mponent/transformer_component_account_provision.go 67.92% 40 Missing and 11 partials ⚠️
...rs/apps/component/transformer_component_account.go 75.35% 21 Missing and 14 partials ⚠️
...ps/cluster/transformer_cluster_sharding_account.go 0.00% 7 Missing ⚠️
pkg/controller/component/its_convertor.go 0.00% 4 Missing ⚠️
pkg/controller/component/component_version.go 0.00% 3 Missing ⚠️
controllers/apps/componentdefinition_controller.go 77.77% 1 Missing and 1 partial ⚠️
pkg/controller/lifecycle/lfa_account.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8756      +/-   ##
==========================================
+ Coverage   60.06%   60.45%   +0.39%     
==========================================
  Files         382      382              
  Lines       46123    46303     +180     
==========================================
+ Hits        27704    27994     +290     
+ Misses      15748    15630     -118     
- Partials     2671     2679       +8
Flag Coverage Δ
unittests 60.45% <68.48%> (+0.39%) ⬆️

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.

@github-actions github-actions bot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Jan 8, 2025
@leon-inf leon-inf marked this pull request as ready for review January 8, 2025 08:16
@leon-inf leon-inf requested review from Y-Rookie and a team as code owners January 8, 2025 08:16
zjx20
zjx20 reviewed Jan 10, 2025
View reviewed changes
controllers/apps/component/transformer_component_account.go

func signatureSystemAccountPassword(secret *corev1.Secret) error {
password := secret.Data[constant.AccountPasswdForSecret]
hashedPassword, err := bcrypt.GenerateFromPassword(password, bcrypt.DefaultCost)
Copy link
Contributor

Choose a reason for hiding this comment

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

GenerateFromPassword() can't handle long passwords (>72bytes), should we handle the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a maximum length limit of 64 bytes for user-specified passwords.

@leon-inf leon-inf force-pushed the support/system-account branch from 803cc26 to 0e630f9 Compare January 21, 2025 06:55
@apecloud-bot apecloud-bot added the approved PR Approved Test label Jan 21, 2025
@apecloud-bot apecloud-bot removed the approved PR Approved Test label Jan 21, 2025
@leon-inf
Copy link
Contributor Author

/approve

@apecloud-bot apecloud-bot added the approved PR Approved Test label Jan 21, 2025
@apecloud-bot apecloud-bot removed the approved PR Approved Test label Jan 21, 2025
Y-Rookie
Y-Rookie reviewed Jan 21, 2025
View reviewed changes
controllers/apps/component/transformer_component_account.go
}
ctrlutil.MergeMetadataMapInplace(secret.Labels, &runningCopy.Labels)
ctrlutil.MergeMetadataMapInplace(secret.Annotations, &runningCopy.Annotations)
if !reflect.DeepEqual(running, runningCopy) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's recommended to add some logs when updating passwords to facilitate debugging or track user changes

@apecloud-bot apecloud-bot added the approved PR Approved Test label Jan 21, 2025
@leon-inf leon-inf merged commit 2b312c3 into main Jan 21, 2025
37 checks passed
@leon-inf leon-inf deleted the support/system-account branch January 21, 2025 10:50
@leon-inf
Copy link
Contributor Author

/cherry-pick release-1.0-beta

@github-actions github-actions bot added this to the Release 0.9.3 milestone Jan 21, 2025
@apecloud-bot
Copy link
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/12885373248

apecloud-bot pushed a commit that referenced this pull request Jan 21, 2025
@leon-inf
chore: support to update and delete system accounts (#8756)
238b285 
(cherry picked from commit 2b312c3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apecloud-bot apecloud-bot apecloud-bot approved these changes

@Y-Rookie Y-Rookie Y-Rookie approved these changes

@zjx20 zjx20 zjx20 approved these changes

Assignees
No one assigned
Labels
approved PR Approved Test size/XXL Denotes a PR that changes 1000+ lines.
Projects
None yet
Milestone
Release 0.9.3
Development

Successfully merging this pull request may close these issues.
4 participants
@leon-inf @apecloud-bot @zjx20 @Y-Rookie