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

IFP: GetUserGroups() returns origPrimaryGroupGidNumber #5272

Closed
wants to merge 2 commits into from
Closed

IFP: GetUserGroups() returns origPrimaryGroupGidNumber #5272

wants to merge 2 commits into from

Conversation

ikerexxe
Copy link
Contributor

@ikerexxe ikerexxe commented Aug 13, 2020 •
edited
Loading

There was a mismatch between the information provided by NSS and IFP
interfaces. nss_protocol_fill_initgr() returned
origPrimaryGroupGidNumber as one of the group members of a user, but
GetUserGroups() didn't. This commit makes GetUserGroups() also return
origPrimaryGroupGidNumber value.

Moreover, new infopipe test case to check:
Given auto_private_groups is enabled
When GetUserGroups is called
Then the origPrimaryGroupGidNumber is returned as part of the group memberships

Resolves:
#4569

@ikerexxe ikerexxe changed the title WIP! responder_utils: GetUserGroups() returns SYSDB_PRIMARY_GROUP_GIDNUM IFP: GetUserGroups() returns origPrimaryGroupGidNumber Aug 24, 2020
@ikerexxe
Copy link
Contributor Author

ikerexxe commented Sep 7, 2020

CI failures are not related with the change itself.

@sumit-bose
Copy link
Contributor

Hi,

thanks for the patch and your patience. I tested the patch and it worked as expected but please see my inline comment.

bye,
Sumit

@ikerexxe
IFP: GetUserGroups() returns origPrimaryGroupGidNumber
34de034 
There was a mismatch between the information provided by NSS and IFP
interfaces. nss_protocol_fill_initgr() returned
origPrimaryGroupGidNumber as one of the group members of a user, but
GetUserGroups() didn't. This commit makes GetUserGroups() also return
origPrimaryGroupGidNumber value.

Resolves:
SSSD#4569
@ikerexxe
IFP-TESTS: GetUserGroups() returns origPrimaryGroupGidNumber
7b95c2f 
New infopipe test case to check:
Given auto_private_groups is enabled
When GetUserGroups is called
Then the origPrimaryGroupGidNumber is returned as part of the group
memberships

Resolves:
SSSD#4569
@sumit-bose
Copy link
Contributor

Hi,

thank you, I have no further comments, the CI failure on F32 seems unrelated. ACK.

bye,
Sumit

@ikerexxe
Copy link
Contributor Author

Thank you for having a look at it. I was already doing it but I was unable to find any connection between my changes and the failure.

@sumit-bose
Copy link
Contributor

Thank you for having a look at it. I was already doing it but I was unable to find any connection between my changes and the failure.

The issue can be seen in other CI runs as well, currently I have no idea for the reason.

@alexey-tikhonov
Copy link
Member

Thank you for having a look at it. I was already doing it but I was unable to find any connection between my changes and the failure.

The issue can be seen in other CI runs as well, currently I have no idea for the reason.

It fails with

FAILED test_session_recording.py::test_none - AssertionError: list mismatch:

@justin-stephenson , could you please take a look?

@justin-stephenson
Copy link
Contributor

Thank you for having a look at it. I was already doing it but I was unable to find any connection between my changes and the failure.

The issue can be seen in other CI runs as well, currently I have no idea for the reason.

It fails with

FAILED test_session_recording.py::test_none - AssertionError: list mismatch:

@justin-stephenson , could you please take a look?

Yes, let me do some investigation

@justin-stephenson
Copy link
Contributor

This test_session_recording.py::test_none runs successfully on my Fedora 32 system. I tried to execute the CI tests following https://github.com/SSSD/sssd/blob/master/contrib/test-suite/README.md using sssd-vagrant/fedora32-client in the config.json but it fails here:

[sssd-test-suite] [tesẗ́-suite]   [3/5] Running ./contrib/ci/run
pep8:                   failure  00:00:03 ci-pep8.log
FAILURE
[sssd-test-suite] [tesẗ́-suite]   ERROR ShellCommandError: Command returned non-zero status code: 1                    
[sssd-test-suite] [tesẗ́-suite]   [4/5] Archive artifacts (skipped on error)                                           
[sssd-test-suite] [tesẗ́-suite]   [5/5] Halting guests: ['client'] (finalizing)                                        
==> client: Halting domain...

Is there something I'm missing to execute the tests the same way as the CI is running them(in order to reproduce the failure)?

@pbrezina
Copy link
Member

pbrezina commented Oct 1, 2020

Please ping me or IRC or on e-mail (or on the team list) and I'll help you with it.

@pbrezina pbrezina added the Ready to push Ready to push label Oct 1, 2020
@pbrezina
Copy link
Member

pbrezina commented Oct 1, 2020

Pushed PR: #5272

  • master
    • 5ddabed - IFP-TESTS: GetUserGroups() returns origPrimaryGroupGidNumber
    • 49481da - IFP: GetUserGroups() returns origPrimaryGroupGidNumber

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Oct 1, 2020
@pbrezina pbrezina closed this Oct 1, 2020
@pbrezina pbrezina mentioned this pull request Oct 1, 2020
Closed
@ikerexxe ikerexxe deleted the get_user_groups branch October 1, 2020 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sumit-bose sumit-bose sumit-bose left review comments

Assignees

@sumit-bose sumit-bose

Labels
Pushed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ikerexxe @sumit-bose @alexey-tikhonov @justin-stephenson @pbrezina