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.

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

[8.0] feat (VOMS2IAM): add options to sync from IAM #7612

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

chaen
Copy link
Contributor

@chaen chaen commented May 14, 2024

Addresses part of #7416

This does not yet work, because there are still too many bugs in the voms-importer https://github.com/indigo-iam/voms-importer/

Wht this PR does is to add 2 options to the VOMS2CSAgent`:

  • CompareWithIAM: when set to true, we will dump the users from IAM, and print a comparison with what VOMS sees
  • UseIAM: not query voms-admin anymore, but the IAM endpoint

BEGINRELEASENOTES

  • Configuration
    NEW: VOMS2CSAgent can query IAM

ENDRELEASENOTES

@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label May 14, 2024
@chaen chaen force-pushed the v8.0_feat_voms2Iam branch 2 times, most recently from 741d4d5 to bba8478 Compare May 14, 2024 13:34
@chaen chaen marked this pull request as ready for review June 5, 2024 07:26
def compare_entry(self, iam_entry, voms_entry, is_robot):
"""Compare a VOMS and IAM entry"""

if not iam_entry.get("mail") == voms_entry.get("mail"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not iam_entry.get("mail") == voms_entry.get("mail"):
if iam_entry.get("mail") != voms_entry.get("mail"):

if not iam_entry.get(field) == voms_entry.get(field):
self.log.info(f"{iam_entry['nickname']} - {field} : {iam_entry.get(field)} vs {voms_entry.get(field)}")

if not sorted(iam_entry["Roles"]) == sorted(voms_entry["Roles"]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not sorted(iam_entry["Roles"]) == sorted(voms_entry["Roles"]):
if sorted(iam_entry["Roles"]) != sorted(voms_entry["Roles"]):

"""Compare a VOMS and IAM entry"""

if not iam_entry.get("mail") == voms_entry.get("mail"):
self.log.info(f"{iam_entry['nickname']} - mail : {iam_entry.get('mail')} vs {voms_entry.get('mail')}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this logs (message, varmsg)?

self.log.info("No extra entry entries in IAM, GOOD !")

# We are waiting for IAM to synchronize also suspended people
# https://github.com/indigo-iam/voms-importer/pull/22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be in a release?

iam_list_url = f"{self.iam_url}/scim/Users"
iam_users = []
startIndex = 1
totalResults = 1000 # total number of users
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

errors = 0
for user in self.iam_users_raw:
try:
users.update(self.convert_iam_to_voms(user))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So IAM returns a list of dictionaries where each dictionary has one key (the username)?

Might be nice to put some example responses in docstrings to make it easier to follow.

@chrisburr chrisburr merged commit 97bf94b into DIRACGrid:rel-v8r0 Jun 5, 2024
26 checks passed
@DIRACGridBot DIRACGridBot added sweep:done All sweeping actions have been done for this PR sweep:failed Sweeping failed and needs manual intervention labels Jun 5, 2024
@DIRACGridBot
Copy link

Sweep summary

Sweep ran in https://github.com/DIRACGrid/DIRAC/actions/runs/9382608769

Failed:

  • integration
    cherry-pick 97bf94b into integration failed
    check merge conflicts on a local copy of this repository
    git fetch upstream
    git checkout upstream/integration -b cherry-pick-2-97bf94bd5-integration
    git cherry-pick -x -m 1 97bf94bd5
    # Fix the conflicts
    git cherry-pick --continue
    git commit --amend -m 'sweep: #7612 feat (VOMS2IAM): add options to sync from IAM' --author='Christophe Haen <christophe.haen@cern.ch>'
    git push -u origin cherry-pick-2-97bf94bd5-integration
    
    # If you have the GitHub CLI installed the PR can be made with
    gh pr create \
         --label 'sweep:from rel-v8r0' \
         --base integration \
         --repo DIRACGrid/DIRAC \
         --title '[sweep:integration] feat (VOMS2IAM): add options to sync from IAM' \
         --body 'Sweep #7612 `feat (VOMS2IAM): add options to sync from IAM` to `integration`.
    
    Adding original author @chaen as watcher.
    
    BEGINRELEASENOTES
    
    
    * Configuration
    NEW: VOMS2CSAgent can query IAM
    
    ENDRELEASENOTES
    Closes #7647'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge sweep:done All sweeping actions have been done for this PR sweep:failed Sweeping failed and needs manual intervention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants