[sssd PR#5754][opened] Sources cleanup.
by alexey-tikhonov
URL: https://github.com/SSSD/sssd/pull/5754
Author: alexey-tikhonov
Title: #5754: Sources cleanup.
Action: opened
PR body:
"""
Those are initial patches in the "cleanup" set.
I expect to add more to this PR, but those already published are ready for review.
Warning: this PR targets 2.6 upstream release, so it should be deferred until 2.5.3 is released.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5754/head:pr5754
git checkout pr5754
2 years, 7 months
[sssd PR#5766][comment] [WiP] Sources cleanup - part 3.
by pbrezina
URL: https://github.com/SSSD/sssd/pull/5766
Title: #5766: [WiP] Sources cleanup - part 3.
pbrezina commented:
"""
> > > * incorporate "lib secrets" into KCM since there are no other users
> >
> >
> > If you want to touch this, it might be good to investigate if it is still needed. libsecrets was created to share code between secrets responder and kcm (since first implementation of kcm talked to secrets via its rest api). It is however affected by the rest api usage, for example it converts key to urls to dn which is not needed anymore. Perhaps it would be enough to use sysdb instead and remove libsecrets completely.
>
> Right, once `libsecrets` drops encryption support entirely, it might happen we can just add some helpers to sysdb instead of `libsecrets`. It's just easier for me to do this step by step.
>
> > > * to check if `kcm_ops_queue*` layer is still needed (most probably answer is "no" because only sync backends are now available) and remove it not
> >
> >
> > This is questionable. Once you remove secrets backend, we can (and should) convert all tevent-based api of the backend to sync calls. But perhaps we will need some async calls in the future as well so it might be beneficial to keep the queue mechanism (or make it easily revertable).
>
> I can leave this intact.
> I worry it adds some latency (but not sure).
It can be removed for sync api, I would just like to be able to reimplement it easily if it is ever needed again. But perhaps it can be replaced by tevent_queue in which case it does not matter, I'm not sure.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5766#issuecomment-908352571
2 years, 7 months
[sssd PR#5766][comment] [WiP] Sources cleanup - part 3.
by alexey-tikhonov
URL: https://github.com/SSSD/sssd/pull/5766
Title: #5766: [WiP] Sources cleanup - part 3.
alexey-tikhonov commented:
"""
> > * incorporate "lib secrets" into KCM since there are no other users
>
> If you want to touch this, it might be good to investigate if it is still needed. libsecrets was created to share code between secrets responder and kcm (since first implementation of kcm talked to secrets via its rest api). It is however affected by the rest api usage, for example it converts key to urls to dn which is not needed anymore. Perhaps it would be enough to use sysdb instead and remove libsecrets completely.
Right, once `libsecrets` drops encryption support entirely, it might happen we can just add some helpers to sysdb instead of `libsecrets`. It's just easier for me to do this step by step.
> > * to check if `kcm_ops_queue*` layer is still needed (most probably answer is "no" because only sync backends are now available) and remove it not
>
> This is questionable. Once you remove secrets backend, we can (and should) convert all tevent-based api of the backend to sync calls. But perhaps we will need some async calls in the future as well so it might be beneficial to keep the queue mechanism (or make it easily revertable).
I can leave this intact.
I worry it adds some latency (but not sure).
"""
See the full comment at https://github.com/SSSD/sssd/pull/5766#issuecomment-908273496
2 years, 7 months
[sssd PR#5766][comment] [WiP] Sources cleanup - part 3.
by pbrezina
URL: https://github.com/SSSD/sssd/pull/5766
Title: #5766: [WiP] Sources cleanup - part 3.
pbrezina commented:
"""
> Sorry I didn't indicate this is work-in-progress yet.
>
> Remaining steps here are:
>
> * incorporate "lib secrets" into KCM since there are no other users
If you want to touch this, it might be good to investigate if it is still needed. libsecrets was created to share code between secrets responder and kcm (since first implementation of kcm talked to secrets via its rest api). It is however affected by the rest api usage, for example it converts key to urls to dn which is not needed anymore. Perhaps it would be enough to use sysdb instead and remove libsecrets completely.
> * to check if `kcm_ops_queue*` layer is still needed (most probably answer is "no" because only sync backends are now available) and remove it not
This is questionable. Once you remove secrets backend, we can (and should) convert all tevent-based api of the backend to sync calls. But perhaps we will need some async calls in the future as well so it might be beneficial to keep the queue mechanism (or make it easily revertable).
> * get rid of encrypted/JSON payload support in KCM
+1
> Then "lib secrets" can drop support of encrypted payload, and probably utils crypto can drop some helpers, etc, but I didn't check it yet.
+1 (if not replaced by sysdb)
"""
See the full comment at https://github.com/SSSD/sssd/pull/5766#issuecomment-908254353
2 years, 7 months
[sssd PR#5766][comment] [WiP] Sources cleanup - part 3.
by alexey-tikhonov
URL: https://github.com/SSSD/sssd/pull/5766
Title: #5766: [WiP] Sources cleanup - part 3.
alexey-tikhonov commented:
"""
Also there is an issues with a spec file:
```
/usr/bin/install -p -m 644 src/sysv/systemd/sssd.service src/sysv/systemd/sssd-nss.socket src/sysv/systemd/sssd-nss.service src/sysv/systemd/sssd-pam.socket src/sysv/systemd/sssd-pam-priv.socket src/sysv/systemd/sssd-pam.service src/sysv/systemd/sssd-autofs.socket src/sysv/systemd/sssd-autofs.service src/sysv/systemd/sssd-ifp.service src/sysv/systemd/sssd-pac.socket src/sysv/systemd/sssd-pac.service src/sysv/systemd/sssd-ssh.socket src/sysv/systemd/sssd-ssh.service src/sysv/systemd/sssd-sudo.socket src/sysv/systemd/sssd-sudo.service ./src/sysv/systemd/sssd-kcm.socket src/sysv/systemd/sssd-kcm.service '/builddir/build/BUILDROOT/sssd-2.5.3-0.210827.210238.git9841f13e6.fc34.x86_64/usr/lib/systemd/system'
/usr/bin/install: cannot stat './src/sysv/systemd/sssd-kcm.socket': No such file or directory
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5766#issuecomment-908203913
2 years, 7 months
[sssd PR#5766][comment] [WiP] Sources cleanup - part 3.
by alexey-tikhonov
URL: https://github.com/SSSD/sssd/pull/5766
Title: #5766: [WiP] Sources cleanup - part 3.
alexey-tikhonov commented:
"""
Sorry I didn't indicate this is work-in-progress yet.
Remaining steps here are:
- incorporate "lib secrets" into KCM since there are no other users
- to check if `kcm_ops_queue*` layer is still needed (most probably answer is "no" because only sync backends are now available) and remove it not
- get rid of encrypted/JSON payload support in KCM
Then "lib secrets" can drop support of encrypted payload, and probably utils crypto can drop some helpers, etc, but I didn't check it yet.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5766#issuecomment-908192570
2 years, 7 months