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: Unable to login and missing keypairs when a user does not have any project #1360

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Jul 2, 2023

follow-up #1022

If a user is not associated with any group (project), the user's keypairs are not included in the KeyPairList query result, with a mismatch of count and slice resolvers.

If one or more user is not associated with any group (project), all users cannot login to the web UI. At least, the login error should be confined to the specific user, without affecting other users with proper project memberships.

Suggestion

How about deprecating projects fields of Keypairs?
I made a commit to test it

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • API server-client counterparts (e.g., manager API -> client SDK)

@fregataa fregataa added the urgency:3 Must be finished within a certain time frame. label Jul 2, 2023
@fregataa fregataa added this to the 23.03 milestone Jul 2, 2023
@fregataa fregataa self-assigned this Jul 2, 2023
@github-actions github-actions bot added comp:manager Related to Manager component size:S 10~30 LoC labels Jul 2, 2023
@fregataa fregataa requested a review from adrysn July 2, 2023 12:28
@github-actions github-actions bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Jul 2, 2023
Comment on lines 328 to 334
sa.join(keypairs, users, keypairs.c.user == users.c.uuid, isouter=True)
.join(
association_groups_users,
users.c.uuid == association_groups_users.c.user_id,
isouter=True,
)
.join(groups, association_groups_users.c.group_id == groups.c.id, isouter=True)
Copy link
Member

Choose a reason for hiding this comment

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

We should never OUTER JOIN three tables in any case.

@adrysn adrysn added the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Jul 6, 2023
@github-actions github-actions bot added size:S 10~30 LoC and removed size:M 30~100 LoC labels Jul 6, 2023
@achimnol achimnol removed the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Jul 31, 2023
@fregataa fregataa requested a review from kyujin-cho August 10, 2023 08:40
@kyujin-cho kyujin-cho modified the milestones: 23.03, 23.09 Aug 30, 2023
@achimnol achimnol changed the title fix: Keypair graphene query mismatch between count and slice fix: KeyPair GraphQL query mismatch between count and slice Oct 24, 2023
achimnol added a commit that referenced this pull request Oct 24, 2023
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

image

In my setup, the PR seems not to work as expected...

@achimnol achimnol added the type:bug Reports about that are not working label Oct 24, 2023
@achimnol
Copy link
Member

Another issue to look together: if there exists a user who does not belong to any project, all user's login is blocked due to a query error.

@achimnol achimnol changed the title fix: KeyPair GraphQL query mismatch between count and slice fix: Unable to login and missing keypairs when a user does not have any project Oct 25, 2023
@achimnol achimnol added urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! and removed urgency:3 Must be finished within a certain time frame. labels Oct 25, 2023
@kyujin-cho
Copy link
Member

kyujin-cho commented Nov 1, 2023

This should be resoled before shipping 23.09.5.

@kyujin-cho kyujin-cho removed the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Nov 1, 2023
@fregataa fregataa added the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Jan 9, 2024
@fregataa fregataa requested a review from achimnol January 9, 2024 02:50
@fregataa
Copy link
Member Author

fregataa commented Jan 9, 2024

@achimnol I resolved the issue that unable to fetch keypairs whose user is not associate with any project.
But I still cannot reproduce the login issue. it just work when such non-associate users exist

image

@github-actions github-actions bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Jan 11, 2024
@fregataa fregataa added urgency:4 As soon as feasible, implementation is essential. and removed urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! labels Jan 11, 2024
@fregataa fregataa marked this pull request as draft March 8, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component size:M 30~100 LoC type:bug Reports about that are not working urgency:4 As soon as feasible, implementation is essential.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants