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

fix(keycloak): serialize the calls to get groups and users #2038

Closed
wants to merge 5 commits into from

Conversation

PatAKnight
Copy link
Contributor

@PatAKnight PatAKnight commented Nov 20, 2024

Hey, I just made a Pull Request!

Serialize the calls to get groups and users from Keycloak. In cases where there was a large number of users and / or groups, originally the keycloak plugin would make many async calls that would lead to DOS of the keycloak server.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@PatAKnight PatAKnight requested review from schultzp2020 and a team as code owners November 20, 2024 18:33
@PatAKnight PatAKnight requested a review from vinzscam November 20, 2024 18:33
@backstage-goalie
Copy link
Contributor

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage-community/plugin-catalog-backend-module-keycloak workspaces/keycloak/plugins/catalog-backend-module-keycloak patch v3.2.2

@PatAKnight PatAKnight force-pushed the keycloak-fix branch 2 times, most recently from 23e94e6 to 872eb90 Compare November 20, 2024 18:47
Signed-off-by: Kashish Mittal <[email protected]>
Signed-off-by: Kashish Mittal <[email protected]>
Signed-off-by: Kashish Mittal <[email protected]>
Copy link
Member

@vinzscam vinzscam left a comment

Choose a reason for hiding this comment

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

nice! Just a small suggestion and we can :shipit:

max: entityQuerySize,
first: i * entityQuerySize,
});
entityResults.push(...entitiesPage);
Copy link
Member

Choose a reason for hiding this comment

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

so that we avoid a possible stack overflow 😅

Suggested change
entityResults.push(...entitiesPage);
entityResults.concat(entitiesPage);

@PatAKnight
Copy link
Contributor Author

Closing in favor of #2121

@PatAKnight PatAKnight closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keycloak backend plugin leads to DOS of Keycloak server
4 participants