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

Fixes #24295: API export of groups doesn't export the categories as dependencies #5521

Conversation

fanf
Copy link
Member

@fanf fanf commented Mar 22, 2024

https://issues.rudder.io/issues/24295

The chosen new mapping is one that match the LDAP/UI representation of group categories and groups:

archive/
 `- groups
      |- category_with_name
      |     |- category.json // the category info: uuid, name, etc
      |     `- some_group.json // the group as before
      `- some_group_under_root_category.json

This mapping is so-so, since the group serialisation also contains the group category id, so we duplicate the information, and if we move a group, it doesn't actually change its category, which is surprising. But I wanted to have the same json serialisation than the one from API.

I hesitated with a mappring like:

archive/
 |- group-categories.json // one big json will the categories 
 `- groups
      |- some_group.json // the group as before
      `- some_group_under_root_category.json

But it was strang to have a mixe of "one item for each groups" and "one big json with lots of categories".
Still, maybe it would be easier to manipulate.

@fanf fanf requested a review from VinceMacBuche March 22, 2024 21:53
@fanf fanf force-pushed the bug_24295/api_export_of_groups_doesn_t_export_the_categories_as_dependancies branch from 8277e16 to aaea1fc Compare March 22, 2024 21:55
@@ -435,12 +464,18 @@ class ZipArchiveBuilderService(
val name = fileArchiveNameService.toFileName(origName) + extension

// find a free name, avoiding overwriting a previous similar one
usedNames.modify(m => {
val realName = if (m(category).contains(name)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

never do that, it breaks when the thing doesn't exist

usedNames: Ref[Map[String, Set[String]]]
): IOResult[Chunk[Zippable]] = {
for {
ref <- Ref.make(Chunk.empty[Zippable])
Copy link
Member Author

Choose a reason for hiding this comment

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

we use a ref to avoid building list of list and then flatmapping in the end

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5521
-- Your faithful QA
Kant merge: "Live your life as though your every act were to become a universal law."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/82359/console)

@fanf
Copy link
Member Author

fanf commented Mar 28, 2024

OK, merging this PR

@fanf fanf merged commit f095045 into Normation:branches/rudder/7.3 Mar 28, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants