#3713 SPEC: Move secrets responder to the package sssd-kcm
Closed by fidencio. Opened by lslebodn.
SSSD/ lslebodn/sssd spec_secrets  into  master

Download 3713.patch

The sssd secrets responder is used mainly by sssd-kcm and it is not
used by any service which is in the sub-package sssd-common.
Therefore it make more sense to have secrets responder in the pacakge
sssd-kcm and reduce dependencies of sssd-common package
(http-parser, jansson)
note: libcurl is installed anyway on fedora due to other dependencies

There are basically 3 options for packaging sssd-secrets.

  1. in sssd-common (current state)
  2. create a new package (proposed by @jhrozek)
  3. in sssd-kcm (proposed in this PR)

IMHO, the 1st option is the less ideal. Because sssd-kcm is not default on f26 and el<=7

The second option would make sense if there were huge plans with sssd-secrets and there was a demand to have minimal dependency for sssd-secrets (-ldap, -krb5, -cares, -pcre ...)

It might think that 3rd option is not ideal in context of #3685. However, sssd-secrets is ideal way how to debug content of secrets db which will probably stays the same. (ldb-tools are note very helpful)

sh# curl -H "Content-Type: application/octet-stream" -XGET --output - --unix-socket /var/run/secrets.socket http://localhost/kcm/persistent/1000/default
33dc5d0b-2217-4161-b581-d8e2298e0628

vs

sh# ldbsearch -H /var/lib/sss/secrets/secrets.ldb -b cn=default,cn=1000,cn=persistent,cn=kcm
# record 1
dn: cn=default,cn=1000,cn=persistent,cn=kcm
creationTime: 1520888990
enctype: masterkey
secret: uRdGAK4JP71zjgzdguT6ubd7rK7EdLbTnydz2y1Xn27P6U/YK0yoQosDEMZTdUpEYBUuX4
 aKxEcdh8tQ7zZNgCtLw1KHVWBXCMMrAbRRzJEGScL97UwSAuEl8nbuBlhrPLea5ePO6hRaRD2A6aN
 nyw==
type: simple
distinguishedName: cn=default,cn=1000,cn=persistent,cn=kcm

And therefore I think that 3rd option is more suitable and simpler then 2nd.

This is the very same as https://github.com/SSSD/sssd/pull/529 which has been closed already.

This was @jhrozek's comment:
I understand the point about minimal dependencies of sssd-common, but I wonder if it was better to create a separate subpackage for the secrets binary. This is for two reasons, a) the proposed packaging is not intuitive, b) we are actually thinking about not talking to the secrets REST API from KCM to secrets, but writing to the secrets database directly. And then bundling the two responders together wouldn't make sense.

And this was my message when closing the PR:
Based on @jhrozek's comment and considering we may even deprecate the secrets responder soon, I'm closing this PR as "Rejected".

Nevertheless, thanks for the contribution!

Pull-Request has been closed by fidencio

Lukas, I'm now working on deprecating sssd-secrets. It will not be built by default and it won't be included in downstreams at all. KCM will communicate directly with the secrets database. So I'm not sure this PR is needed, if you don't mind I would table it for now and check if it makes sense after we merge the direct-to-db patches.

@jhrozek,
deprecating != removing

Moreover, this is a trivial patch and already improve something. It will take sometime when you implement your changes.

And if you decide completely remove sssd-secrets another tool will need to be implemented to debug content of secrets ldb database.

Thes sssd-secrets is already ready and it is quite trivial toto debug content of secrets database. (As it was already shown in previous comments)

This PR was prematurely closed before reaching consensus between @lslebodn and @jhrozek. It is not possible to reopen PR ATM in pagure therefore new PR #3714 was created.

@jhrozek, I think we can continue in discussion in any PR.

On 19 Apr 2018, at 20:20, Lukas Slebodnik pagure@pagure.io wrote:
=20
=20
lslebodn commented on the pull-request: SPEC: Move secrets responder = to the package sssd-kcm that you are following:
``
@jhrozek,
deprecating !=3D removing

In this case, deprecating =3D=3D not building by default.

I need to think about this, mainly to see if your approach would make =
sense for 1.16 (because RHEL can=E2=80=99t just deprecate =
functionality).=

@lslebodn, @jhrozek,

As the person who closed the PR (both here and on github), I'd like to apologize for that and make it clear that I did not blindly close the PRs (actually, it was even discussed on #sssd).

So, I'm stepping out and leaving it for you two to decide what's better.

Best Regards,

On 19 Apr 2018, at 23:18, Fabiano Fid=C3=AAncio pagure@pagure.io =
wrote:
=20
=20
fidencio commented on the pull-request: SPEC: Move secrets responder = to the package sssd-kcm that you are following:
``
@lslebodn, @jhrozek,
=20
As the person who closed the PR (both here and on github), I'd like to =
apologize for that and make it clear that I did not blindly close the =
PRs (actually, it was even discussed on #sssd).
=20

I=E2=80=99m not blaming you for closing the PR, in fact I told you to =
close it on #sssd, I just think @lslebodn might have a point for the =
1.16 branch. I=E2=80=99ll decide a bit later, currently I=E2=80=99m =
reviewing some urgent PRs.

So, I'm stepping out and leaving it for you two to decide what's =
better.
=20
Best Regards,
``
=20
To reply, visit the link below or just reply to this email
https://pagure.io/SSSD/sssd/pull-request/3713

On 19 Apr 2018, at 20:20, Lukas Slebodnik pagure@pagure.io wrote:

@jhrozek,
deprecating != removing

In this case, deprecating == not building by default.
I need to think about this, mainly to see if your approach would make
sense for 1.16 (because RHEL can't just deprecate functionality)

It is not just about RHEL. Fedora has update policy[1] as well. And removing daemon in stable release does not fit there.

This PR does not want to remove anything. But if you decide later to do that then it can easily be done in spec. It is already optional there

%if (0%{?fedora} || (0%{?rhel} >= 7 && 0%{rhel7_minor} >= 4))
    %global with_secrets 1
%else
    %global with_secret_responder --without-secrets
%endif

[1] http://fedoraproject.org/wiki/Updates_Policy

Metadata