CKS: generate a random UUID as password of CKS user in project#11639
Conversation
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
DaanHoogland
left a comment
There was a problem hiding this comment.
lgtm, but how about when the min pw length is set beyond a UUID length?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11639 +/- ##
============================================
- Coverage 16.17% 16.17% -0.01%
+ Complexity 13298 13297 -1
============================================
Files 5656 5656
Lines 498151 498221 +70
Branches 60441 60452 +11
============================================
- Hits 80589 80581 -8
- Misses 408591 408671 +80
+ Partials 8971 8969 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
then #11626 appears 😆 this is just a quick workaround. generate a more strong password matching the policy, or ignore the password policy for CKS users |
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm ( but a real solution should be created in a call that generates a password from a password policy in the end )
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15014 |
shwstppr
left a comment
There was a problem hiding this comment.
change looks good but maybe for a complete solution we can bypass PasswordPolicy here. Kubernetes project account is created with System account call context so maybe we can bypass policy validations when System is creating accounts
thanks @shwstppr |
|
@weizhouapache best case would have been creating a password compliant to the password policy. But because of regex config it would be difficult and error-prone. Next best is bypassing policy and creating a stronger password (which would never be used though). So I think maybe we keep this usage of complete UUID and then bypass the policy for system created account (which I feel would be more generic than adding a User.Source.CKS) |
thanks @shwstppr |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15179 |
shwstppr
left a comment
There was a problem hiding this comment.
@weizhouapache I've not tested but I guess we didn't need all the changes to pass the caller.
We could use CallContext.getCallingAccount(). KubernetesClusterManager was already using the system callcontext at line 1550
thanks @shwstppr , good point |
…y system account" This reverts commit 2493a59.
reverted the last commit, added a simple check on the caller tested ok
thanks @shwstppr , good advise |
|
cool, thanks for the testing @shwstppr |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15234 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14487)
|
|
merging based on approvals and manua test, thanks @shwstppr |





Description
This PR fixes the password CKS user in project
UuidUtils.firstreturns only the first part of UUID , which contains 8 letters/digits in total.If the min length is set to a value more than 8, the CKS cluster cannot be created in project
To be consistent with CKS user in non-project, use UUID instead of first 8 chars of UUID as password of CKS user in project
This fixes #11626
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?