URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5450
* `master`
* ec932d35172819ac68343355faaad4dc6ffae688 - KCM: Disable responder idle
timeout with renewals
* ddc
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
Thank you. Ack.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-836525263
___
sssd-devel mail
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Thank you for your patience, last two nitpick in code and we can push.
>
> And perhaps different wording for the release notes (fill in the version):
>
> ```
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
Thank you for your patience, last two nitpick in code and we can push.
And perhaps different wording for the release notes (fill in the version):
```
:feature: Added
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> And one more thing - add [release
> notes](https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7) and
> upstream ticket to the commit messages, e.g
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> > And one more thing - add [release
> > notes](https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7)
> > and upstream ticket to the commit messages, e.g.
>
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> And one more thing - add [release
> notes](https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7) and
> upstream ticket to the commit messages, e.g
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
And one more thing - add [release
notes](https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7) and
upstream ticket to the commit messages, e.g.
*
https://gi
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> In this case the hash table is completely redundant. I think we have two
> options:
>
> 1. Keep the hash table. But in this case it would be better to mai
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> > Also what do we need the renew hash table for? I don't see where you take
> > advantage of it? You always call `kcm_ccdb_renew_init` from
> > `kcm_renew_tgt_timer_ha
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Also what do we need the renew hash table for? I don't see where you take
> advantage of it? You always call `kcm_ccdb_renew_init` from
> `kcm_renew_tgt_timer
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Also what do we need the renew hash table for? I don't see where you take
> advantage of it? You always call `kcm_ccdb_renew_init` from
> `kcm_renew_tgt_timer
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
Sorry, it still does not work correctly. Right now you don't continue with next
user if you did not find the current. You want:
```diff
diff --git a/src/responder/kcm/kc
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Hi Justin,
> I'm sorry it takes so long, but there are few more thinks. They are mostly
> minor, but one makes kcm fail to start.
>
These issues are addressed
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> Thanks a lot Pavel for the further review, it is better to get everything
> resolved now than having to fix issues later.
>
> > If the cache contains uid that is not r
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Thanks a lot Pavel for the further review, it is better to get everything
resolved now than having to fix issues later.
> If the cache contains uid that is not
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
I pushed a new version addressing the latest review items from Pavel. The CI
failure `Details` link shows no actual files, just an empty header. Is it
expected?
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> > It's not sleeping, it still spins in tevent loop doing stuff which may have
> > a negative impact on battery.
>
> That's exactly my question: what is it doing? IIUC,
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Ah, I missed the last patch: `KCM: Disable responder idle timeout with
> renewals`. So it will work correclty. But I wonder if it would be better to
> keep th
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> Ah, I missed the last patch: `KCM: Disable responder idle timeout with
> renewals`. So it will work correclty. But I wonder if it would be better to
> keep th
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
alexey-tikhonov commented:
"""
> It's not sleeping, it still spins in tevent loop doing stuff which may have a
> negative impact on battery.
That's exactly my question: what is it doing? IIUC,
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> > But I wonder if it would be better to keep the idle timeout enabled.
>
> What's wrong with keeping an idle process "running"? Sleeping process with
> small memory fo
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
alexey-tikhonov commented:
"""
> But I wonder if it would be better to keep the idle timeout enabled.
What's wrong with keeping an idle process "running"? Sleeping process with
small memory fo
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
pbrezina commented:
"""
> There are few minor comments... but:
>
> If I understand it correctly, this functionality requires KCM to be running.
> KCM is currently socket activated so I see two
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
sumit-bose commented:
"""
Hi,
thanks for updating the patches so that the build works automatically on
platforms without the new libkrb5 calls. The CI failures with rawhide and F34
are (unfort
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
It looks like most the CI failures are unrelated to this PR, @sumit-bose could
you please confirm this?
"""
See the full comment at
https://github.com/SSSD/sss
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Disregard my previous comment, I added a custom pytest marker to skip the intg
test if not built with renewal support
"""
See the full comment at
https://githu
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
@sumit-bose How do I conditionally exclude the `test_kcm_renewals` integration
test from `src/tests/intg/test_kcm.py` when kcm renewals are not being built?
See
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Changes made as requested.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-779398475
___
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
sumit-bose commented:
"""
Hi,
@justin-stephenson, you can add `--disable-kcm-renewal` for platforms where the
calls are not available in `contrib/ci/configure.sh`
@pbrezina, can you update the
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
CI fails as expected with `configure: error: krb5 marshalling functions not
available, --disable-kcm-renewal should be used`
"""
See the full comment at
https:
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Sorry for the delay. I updated the PR to
* Use the exported krb5 marshalling credentials functions
* Build KCM renewals code conditionally, if the krb5 marshal
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Thank you, I'll work on it and update the PR.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-769816677
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
sumit-bose commented:
"""
> > thanks for the effort. What would be your suggestion for the way forward?
> > We can add a configure check if the new functions are already available.
> > But if t
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
frozencemetery commented:
"""
> thanks for the effort. What would be your suggestion for the way forward? We
> can add a configure check if the new functions are already available. But if
> the
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
sumit-bose commented:
"""
> The krb5 PR has merged upstream. For convenience, I've backported the two
> functions to Fedora rawhide starting in krb5-1.19-0.beta2.3.fc34.
Hi Robbie,
thanks for
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
frozencemetery commented:
"""
The krb5 PR has merged upstream. For convenience, I've backported the two
functions to Fedora rawhide starting in krb5-1.19-0.beta2.3.fc34.
"""
See the full comme
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
I addressed several of the aforementioned issues, I'll make a second pass
tomorrow. I could not find any instances of `Mid-function variable declaration`
but ma
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
> I've started discussion upstream in krb5 about making public functions for
> cred marshalling in krb5. This would allow SSSD to avoid reimplementing
> several h
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
frozencemetery commented:
"""
Hi, you've written "fixed" below several things, but the code isn't change.
Are you missing a push?
"""
See the full comment at
https://github.com/SSSD/sssd/pull
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
frozencemetery commented:
"""
(krb5 discussion PR: https://github.com/krb5/krb5/pull/1153 )
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-760546977
___
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
This is ready for review, latest CI failures are unrelated.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-760264220
__
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
Latest CI failures are unrelated.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-760264220
URL: https://github.com/SSSD/sssd/pull/5450
Title: #5450: kcm: add support for kerberos tgt renewals
frozencemetery commented:
"""
(I'll want to review this once it passes CI.)
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5450#issuecomment-756832041
___
44 matches
Mail list logo
