Resolve "Importing group as a top level group targeting an existing namespace give false positive status"
What does this MR do and why?
This MR gives correct error message when you're trying to specify import target, which already exists in the system but is invisible to user (i.e. group which is private as an example) See #341185 (closed) for details
Screenshots or screen recordings
How to set up and validate locally
- Enable feature
bulk_import
viaFeature.enable(:bulk_import)
(should be enabled by default) - (we are assuming you have default GDK setup with private group
Commit451
available) - Impersonate to any non-admin user
- Open "New group" -> Import (
/groups/new
) - Use
https://gitlab.com
as source instance andGeK1Nis4j-SY1X4sqE5c
as personal access token (this token is from separate user on GitLab instance with 0 real data available, so we do not expose any security risks here) - Try to specify
Commit451
as import target name - Observe validation error
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Technical details
First of all, I'm super sorry for
It might be easier to get the logic behind code, if you just do a "review" of import_entities/import_groups
entire code
Why you are doing such major refactoring?
Well, it started pretty simple... Our group(...)
GraphQL query reports group as non-existent even if it exists (which is kind of logical). We have a separate REST api call /api/v4/namespaces/${namespaceName}/exists
which returns if group exists
Ok, let's make a new client resolver for this... except this will not work:
- You do not have an ability to cancel graphql query implemented by local resolver. And having "suggestions" api uncancellable creates all kind of the risks - starting from "out-of-order" requests to stuck requests
-
/api/v4/namespaces/${namespaceName}/exists
returnssuggestion
- this is the name which is guaranteed to be available. We want to consume this suggestion on initial load - because we could "auto-generate" import name which was already occupied. This was previously implemented in local resolvers, but what if our suggestion is one of the names which we already use as suggestion (I know it is complex, I hope video makes it clearer). To workaround this we need to iterate across all current suggestions and if they are located in local resolvers ... We simply do not have a clean way to do that - in order to repeatimportGroups
query we need to be aware of all parameters (filtering, pagination) inside resolver... which puts us to doom
So what was done?
- All validation logic moved from local resolvers to main table component. This simplified entire flow for understanding so much
- (while we are here) behavior of local GraphQL was normalized to conform GraphQL standards (so
webUrl
instead ofweb_url
) - Logic of "put group status to pending while XHR request is in flight" also removed from resolvers. Instead we track "pending requests" in component
All these changes unlock are future potential migration to real graphql queries and greatly improves readability of final result