From 924efda616607d8547e9d533444921990520fccd Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Thu, 28 Nov 2024 22:03:03 +0100 Subject: [PATCH 1/4] Fixes #25977: Node update event does not correctly record old node --- .../facts/nodes/NodeFactRepository.scala | 2 +- .../rudder/facts/nodes/NodeFactStorage.scala | 6 +-- .../nodes/TestCoreNodeFactInventory.scala | 49 ++++++++++++------- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactRepository.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactRepository.scala index 1bd660d4b8e..585e76f07f3 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactRepository.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactRepository.scala @@ -687,7 +687,7 @@ class CoreNodeFactRepository( // first we persist on cold storage, which is more likely to fail. Plus, for history reason, some // mapping are not exactly isomorphic, and some normalization can happen - for ex, for missing machine. s <- storage.save(updated) - // then, we get the actual thing that was saved from the save even + // then, we get the actual thing that was saved from the save event up = s match { case StorageChangeEventSave.Created(node, attrs) => node diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala index b7cefb777b2..f0889a806f0 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala @@ -685,9 +685,6 @@ class LdapNodeFactStorage( override def save(nodeFact: NodeFact)(implicit attrs: SelectFacts): IOResult[StorageChangeEventSave] = { nodeLibMutex.writeLock(for { con <- ldap - _ <- con - .save(nodeMapper.nodeToEntry(nodeFact.toNode)) - .chainError(s"Cannot save node with id '${nodeFact.id.value}' in LDAP") mergedSoft <- if (LdapNodeFactStorage.needsSoftware(attrs)) { softwareSave.tryWith(nodeFact.software.map(_.toSoftware).toSet).map(Some(_)) } else None.succeed @@ -710,6 +707,9 @@ class LdapNodeFactStorage( nf.modify(_.software).setToIfDefined(optSoft) } ) + _ <- con + .save(nodeMapper.nodeToEntry(nodeFact.toNode)) + .chainError(s"Cannot save node with id '${nodeFact.id.value}' in LDAP") inv = SelectFacts .merge(nodeFact, optOld)(attrs) .toFullInventory diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala index ab6267c79cf..476bf0cccad 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala @@ -213,15 +213,10 @@ class TestCoreNodeFactInventory extends Specification with BeforeAfterAll { } // org.slf4j.LoggerFactory -// .getLogger("inventory-processing") +// .getLogger("nodes") // .asInstanceOf[ch.qos.logback.classic.Logger] // .setLevel(ch.qos.logback.classic.Level.TRACE) - org.slf4j.LoggerFactory - .getLogger("nodes.details.write") - .asInstanceOf[ch.qos.logback.classic.Logger] - .setLevel(ch.qos.logback.classic.Level.TRACE) - sequential // node7 has the following inventory info: @@ -593,10 +588,7 @@ class TestCoreNodeFactInventory extends Specification with BeforeAfterAll { "root status can not be modified" >> { val res = (for { r <- factRepo.get(Constants.ROOT_POLICY_SERVER_ID).notOptional("root must be here") - _ <- factRepo.save(NodeFact.fromMinimal(r.modify(_.rudderSettings.status).setTo(PendingInventory)))( - testChangeContext, - SelectFacts.none - ) + _ <- factRepo.save(r.modify(_.rudderSettings.status).setTo(PendingInventory))(testChangeContext) } yield ()).either.runNow res must beLeft @@ -605,18 +597,37 @@ class TestCoreNodeFactInventory extends Specification with BeforeAfterAll { "Update of policy mode to default mode after it was set to audit/enforce should be default (#25866)" >> { val res = (for { node <- factRepo.get(node7id).notOptional("node7 must be here") - _ <- factRepo.save(NodeFact.fromMinimal(node.modify(_.rudderSettings.policyMode).setTo(Some(PolicyMode.Audit))))( - testChangeContext, - SelectFacts.none - ) - _ <- factRepo.save(NodeFact.fromMinimal(node.modify(_.rudderSettings.policyMode).setTo(None)))( - testChangeContext, - SelectFacts.none - ) + _ <- factRepo.save(node.modify(_.rudderSettings.policyMode).setTo(Some(PolicyMode.Audit)))(testChangeContext) + _ <- factRepo.save(node.modify(_.rudderSettings.policyMode).setTo(None))(testChangeContext) updatedNode <- factRepo.get(node7id).notOptional("node7 must be here") } yield updatedNode.rudderSettings.policyMode).either.runNow - res must beRight(beNone) + (mockLdapFactStorage.testServer + .getEntry("nodeId=node7,ou=Nodes,cn=rudder-configuration") + .getAttribute("policyMode") + .getValue === """default""") and + (res must beRight(beNone)) + } + } + + "We must see change in state in the diff (#25704)" >> { + // node7 is "initializing" in ldap sample data + val res = (for { + node <- factRepo.get(node7id).notOptional("node7 must be here") + diff <- factRepo.save(node.modify(_.rudderSettings.state).setTo(NodeState.PreparingEOL))(testChangeContext) + } yield diff).either.runNow + + (mockLdapFactStorage.testServer + .getEntry("nodeId=node7,ou=Nodes,cn=rudder-configuration") + .getAttribute("state") + .getValue === """preparing-eol""") and + (res must beRight) and { + res.forceGet.event match { + case NodeFactChangeEvent.Updated(oldNode, newNode, _) => + (oldNode.rudderSettings.state === NodeState.Initializing) and (newNode.rudderSettings.state === NodeState.PreparingEOL) + + case x => ko(s"bad change event, get ${x}") + } } "the count of active nodes change if we disable one" >> { From 6b5da30bbb46a5ce6faa4bde90a870d8b2217a4c Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Thu, 28 Nov 2024 23:42:20 +0100 Subject: [PATCH 2/4] Fixes #25979: Migrate custom properties along with node properties for consistency --- .../inventory/ldap/core/InventoryMapper.scala | 3 +- .../rudder/facts/nodes/NodeFactStorage.scala | 6 +- .../repository/ldap/LDAPEntityMapper.scala | 4 +- .../rudder/tenants/TenantService.scala | 2 +- .../nodes/TestCoreNodeFactInventory.scala | 78 ++++++++++- .../CheckMigrateCustomProperties.scala | 123 ++++++++++++++++++ .../TestMigrateCustomProperties.scala | 114 ++++++++++++++++ 7 files changed, 322 insertions(+), 8 deletions(-) create mode 100644 webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala create mode 100644 webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala diff --git a/webapp/sources/ldap-inventory/inventory-repository/src/main/scala/com/normation/inventory/ldap/core/InventoryMapper.scala b/webapp/sources/ldap-inventory/inventory-repository/src/main/scala/com/normation/inventory/ldap/core/InventoryMapper.scala index 310cfda29ba..93db7aa1106 100644 --- a/webapp/sources/ldap-inventory/inventory-repository/src/main/scala/com/normation/inventory/ldap/core/InventoryMapper.scala +++ b/webapp/sources/ldap-inventory/inventory-repository/src/main/scala/com/normation/inventory/ldap/core/InventoryMapper.scala @@ -823,7 +823,8 @@ class InventoryMapper( root.resetValuesTo(A_TIMEZONE_NAME, timezone.name) root.resetValuesTo(A_TIMEZONE_OFFSET, timezone.offset) } - server.customProperties.foreach(cp => root.addValues(A_CUSTOM_PROPERTY, cp.toJson)) + // custom properties are not saved here anymore + root.deleteAttribute(A_CUSTOM_PROPERTY) server.softwareUpdates.foreach { s => import JsonSerializers.implicits.* root.addValues(A_SOFTWARE_UPDATE, s.toJson) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala index f0889a806f0..7478d398f9e 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala @@ -849,8 +849,10 @@ class LdapNodeFactStorage( needSoftware: Boolean ): IOResult[Option[NodeFact]] = { // mostly copied from com.normation.rudder.services.nodes.NodeInfoServiceCachedImpl # getBackendLdapNodeInfo - val ldapAttrs = - (if (needSoftware) Seq(A_SOFTWARE_DN) else Seq()) ++ NodeInfoService.nodeInfoAttributes :+ LDAPConstants.A_SOFTWARE_UPDATE + val ldapAttrs = { + (if (needSoftware) Seq(A_SOFTWARE_DN) + else Seq()) ++ NodeInfoService.nodeInfoAttributes :+ LDAPConstants.A_SOFTWARE_UPDATE :+ LDAPConstants.A_CUSTOM_PROPERTY + } con.get(inventoryDitService.getDit(status).NODES.NODE.dn(nodeId.value), ldapAttrs*).flatMap { case None => // end of game, no node here diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPEntityMapper.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPEntityMapper.scala index 3e9be1c9ffb..4ed7b55ae9f 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPEntityMapper.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/ldap/LDAPEntityMapper.scala @@ -135,11 +135,9 @@ class LDAPEntityMapper( case _ => } - // for node properties, we ALWAYS filter-out properties coming from inventory, - // because we don't want to store them there. entry.resetValuesTo( A_NODE_PROPERTY, - node.properties.collect { case p if (p.provider != Some(NodeProperty.customPropertyProvider)) => p.toData }* + node.properties.map(_.toData)* ) node.nodeReportingConfiguration.heartbeatConfiguration match { diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/tenants/TenantService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/tenants/TenantService.scala index f0aa4511758..f69e2cfc883 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/tenants/TenantService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/tenants/TenantService.scala @@ -118,7 +118,7 @@ object DefaultTenantService { /* * _tenantsEnabled is accessed in a lot of hot path, we prefer not to encapsulate it into a Ref. - * We still put its modification behind a eval. + * We still put its modification behind an eval. */ class DefaultTenantService(private var _tenantsEnabled: Boolean, val tenantIds: Ref[Set[TenantId]]) extends TenantService { diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala index 476bf0cccad..ab1f522fc61 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/facts/nodes/TestCoreNodeFactInventory.scala @@ -48,6 +48,8 @@ import com.normation.rudder.domain.Constants import com.normation.rudder.domain.nodes.MachineInfo import com.normation.rudder.domain.nodes.NodeState import com.normation.rudder.domain.policies.PolicyMode +import com.normation.rudder.domain.properties.GenericProperty.* +import com.normation.rudder.domain.properties.NodeProperty import com.normation.rudder.tenants.DefaultTenantService import com.normation.rudder.tenants.TenantId import com.normation.utils.DateFormaterService @@ -213,7 +215,12 @@ class TestCoreNodeFactInventory extends Specification with BeforeAfterAll { } // org.slf4j.LoggerFactory -// .getLogger("nodes") +// .getLogger("inventory-processing") +// .asInstanceOf[ch.qos.logback.classic.Logger] +// .setLevel(ch.qos.logback.classic.Level.TRACE) + +// org.slf4j.LoggerFactory +// .getLogger("nodes.details.write") // .asInstanceOf[ch.qos.logback.classic.Logger] // .setLevel(ch.qos.logback.classic.Level.TRACE) @@ -675,4 +682,73 @@ class TestCoreNodeFactInventory extends Specification with BeforeAfterAll { } } + + "Inventory properties must be retrieved, migrated and not seen new each time (#25704)" >> { + val existingProp1 = + NodeProperty.apply("datacenter", "Paris".toConfigValue, None, Some(NodeProperty.customPropertyProvider)) + val existingProp2 = { + NodeProperty.apply( + "from_inv", + Map("key1" -> "custom prop value", "key2" -> "some more json").toConfigValue, + None, + Some(NodeProperty.customPropertyProvider) + ) + } + val newProp = + NodeProperty.apply("new_inv", "inventory value".toConfigValue, None, Some(NodeProperty.customPropertyProvider)) + + // no provider in the json of custom properties + val beforeInv = mockLdapFactStorage.testServer + .getEntry("nodeId=node1,ou=Nodes,ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration") + .getAttribute("customProperty") + .getValues === Array( + """{"name":"datacenter", "value":"Paris"}""", + """{"name":"from_inv", "value":{ "key1":"custom prop value", "key2":"some more json"}}""" + ) + + val beforeNode = mockLdapFactStorage.testServer + .getEntry("nodeId=node1,ou=Nodes,cn=rudder-configuration") + .getAttribute("serializedNodeProperty") + .getValues === Array("""{"name":"foo","value":"bar"}""") + + val res = (for { + node <- factRepo.get(NodeId("node1")).notOptional("node1 must be here") + props = node.properties.appended(newProp) + // first time: change should be here + d1 <- factRepo.save(node.modify(_.properties).setTo(props))(testChangeContext) + // second time: should be noop + d2 <- factRepo.save(node.modify(_.properties).setTo(props))(testChangeContext) + } yield (d1, d2)).either.runNow + + // after a new save, even only touching core node fact, custom props are removed + val afterInv = mockLdapFactStorage.testServer + .getEntry("nodeId=node1,ou=Nodes,ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration") + .getAttribute("customProperty") === null + + // here, we now have the provider info + val afterNode = mockLdapFactStorage.testServer + .getEntry("nodeId=node1,ou=Nodes,cn=rudder-configuration") + .getAttribute("serializedNodeProperty") + .getValues === Array( + """{"name":"foo","value":"bar"}""", // this one is a node property + existingProp1.toData, + existingProp2.toData, + """{"name":"new_inv","provider":"inventory","value":"inventory value"}""" + ) + + beforeInv and beforeNode and afterInv and afterNode and { + res.forceGet match { + case ( + NodeFactChangeEventCC(NodeFactChangeEvent.Updated(oldNode, newNode, _), _), + secondChange + ) => + (oldNode.properties.size === 1) and // old node is incorrectly reporting 1 because of the bug: it doesn't see custom props + (newNode.properties.size === 4) and + (newNode.properties.last === newProp) and + (secondChange.event must beAnInstanceOf[NodeFactChangeEvent.Noop]) + + case x => ko(s"bad change event, got: ${x._1.event}") + } + } + } } diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala new file mode 100644 index 00000000000..880a18353dd --- /dev/null +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala @@ -0,0 +1,123 @@ +/* + ************************************************************************************* + * Copyright 2024 Normation SAS + ************************************************************************************* + * + * This file is part of Rudder. + * + * Rudder is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In accordance with the terms of section 7 (7. Additional Terms.) of + * the GNU General Public License version 3, the copyright holders add + * the following Additional permissions: + * Notwithstanding to the terms of section 5 (5. Conveying Modified Source + * Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU General + * Public License version 3, when you create a Related Module, this + * Related Module is not considered as a part of the work and may be + * distributed under the license agreement of your choice. + * A "Related Module" means a set of sources files including their + * documentation that, without modification of the Source Code, enables + * supplementary functions or services in addition to those offered by + * the Software. + * + * Rudder is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Rudder. If not, see . + + * + ************************************************************************************* + */ + +package bootstrap.liftweb.checks.migration + +import bootstrap.liftweb.BootstrapChecks +import com.normation.errors.* +import com.normation.inventory.domain.NodeId +import com.normation.inventory.ldap.core.InventoryDit +import com.normation.inventory.ldap.core.LDAPConstants.* +import com.normation.ldap.sdk.BuildFilter +import com.normation.ldap.sdk.LDAPConnectionProvider +import com.normation.ldap.sdk.RwLDAPConnection +import com.normation.rudder.domain.logger.MigrationLoggerPure +import com.normation.rudder.facts.nodes.ChangeContext +import com.normation.rudder.facts.nodes.NodeFactRepository +import com.normation.zio.* +import zio.* + +/* + * This migration check looks if there is still nodes with custom properties + * in the ou=Accepted Inventories branch, and if so, it save back them so that + * they are moved along with other properties. + * Added in rudder 8.1.8 (https://issues.rudder.io/issues/25978) + * Can be removed in Rudder 8.3 + */ + +class CheckMigrateCustomProperties( + ldap: LDAPConnectionProvider[RwLDAPConnection], + factRepo: NodeFactRepository, + inventoryDit: InventoryDit +) extends BootstrapChecks { + + override def description: String = + "Check if some nodes still have custom properties store in LDAP inventory and migrate them to LDAP node is so" + + override def checks(): Unit = { + migrateAll() + .catchAll(err => { + MigrationLoggerPure.error(s"Error when trying to migrate nodes' custom properties in LDAP: ${err.fullMsg}") + }) + .forkDaemon // make it async to avoid blocking startup, there can be a lot of nodes to migrate + .runNow + } + + /* + * The whole process + */ + def migrateAll(): IOResult[Unit] = { + for { + ids <- findNodeNeedingMigration() + _ <- migrateProperties(ids) + } yield () + } + + /* + * Look for nodes with customProperty attribute + */ + def findNodeNeedingMigration(): IOResult[Seq[NodeId]] = { + for { + con <- ldap + needMigration <- con + .searchOne(inventoryDit.NODES.dn, BuildFilter.HAS(A_CUSTOM_PROPERTY), A_NODE_UUID) + .chainError(s"Error when trying to get node entries with existing ${A_CUSTOM_PROPERTY} attributes.") + ids <- ZIO.foreach(needMigration)(e => inventoryDit.NODES.NODE.idFromDN(e.dn).toIO) + } yield ids + } + + /* + * Migrate node after node. We don't want that one failure stop the process + */ + def migrateProperties(nodeIds: Seq[NodeId]): UIO[Unit] = { + implicit val cc = ChangeContext.newForRudder(Some("Migrating custom properties LDAP storage")) + ZIO + .foreach(nodeIds) { id => + (for { + opt <- factRepo.get(id)(cc.toQuery) + _ <- opt match { + case Some(node) => factRepo.save(node) + case None => ZIO.unit + } + } yield ()).catchAll { + case e => MigrationLoggerPure.error(s"Error when migrating custom properties for node '${id.value}': ${e.fullMsg}") + } + } + .unit + } + +} diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala new file mode 100644 index 00000000000..8096aae5b0d --- /dev/null +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala @@ -0,0 +1,114 @@ +/* + ************************************************************************************* + * Copyright 2024 Normation SAS + ************************************************************************************* + * + * This file is part of Rudder. + * + * Rudder is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In accordance with the terms of section 7 (7. Additional Terms.) of + * the GNU General Public License version 3, the copyright holders add + * the following Additional permissions: + * Notwithstanding to the terms of section 5 (5. Conveying Modified Source + * Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU General + * Public License version 3, when you create a Related Module, this + * Related Module is not considered as a part of the work and may be + * distributed under the license agreement of your choice. + * A "Related Module" means a set of sources files including their + * documentation that, without modification of the Source Code, enables + * supplementary functions or services in addition to those offered by + * the Software. + * + * Rudder is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Rudder. If not, see . + + * + ************************************************************************************* + */ + +package bootstrap.liftweb.checks.migration + +import com.normation.errors +import com.normation.inventory.domain.NodeId +import com.normation.rudder.facts.nodes.CoreNodeFactRepository +import com.normation.rudder.facts.nodes.MockLdapFactStorage +import com.normation.rudder.facts.nodes.NoopGetNodesBySoftwareName +import com.normation.rudder.facts.nodes.QueryContext +import com.normation.rudder.facts.nodes.SelectFacts +import com.normation.rudder.tenants.DefaultTenantService + +import com.normation.zio.UnsafeRun +import org.junit.runner.RunWith +import org.specs2.mutable.* +import org.specs2.runner.JUnitRunner + +import zio.* + +@RunWith(classOf[JUnitRunner]) +class TestMigrateCustomProperties extends Specification { + + val mockLdapFactStorage = new MockLdapFactStorage() + + val factRepo: CoreNodeFactRepository = { + (for { + tenantService <- DefaultTenantService.make(Nil) + repo <- CoreNodeFactRepository.make(mockLdapFactStorage.nodeFactStorage, NoopGetNodesBySoftwareName, tenantService, Chunk()) + } yield repo).runNow + } + + val migration = new CheckMigrateCustomProperties(mockLdapFactStorage.ldap, factRepo, mockLdapFactStorage.acceptedDIT) + + org.slf4j.LoggerFactory + .getLogger("nodes") + .asInstanceOf[ch.qos.logback.classic.Logger] + .setLevel(ch.qos.logback.classic.Level.TRACE) + + sequential + + "Migrating custom" should { + /* + * Node1 contains: + * - custom properties 'datacenter' and 'from_inv' + * - node property 'foo' + */ + + "find some properties to migrate" in { + (migration.findNodeNeedingMigration().runNow must containTheSameElementsAs(List(NodeId("node1")))) and + (getNode1PropNamesAsSeenByLdap() must containTheSameElementsAs(List("foo", "datacenter", "from_inv"))) and + (getNode1PropNamesAsSeenByRepo() must containTheSameElementsAs(List("foo", "datacenter", "from_inv"))) + } + + "migrate all nodes" in { + migration.migrateAll().either.runNow must beRight() + } + + "not find any nodes to migrate after migration" in { + (migration.findNodeNeedingMigration().runNow must beEmpty) and + (getNode1PropNamesAsSeenByLdap() must containTheSameElementsAs(List("foo", "datacenter", "from_inv"))) and + (getNode1PropNamesAsSeenByRepo() must containTheSameElementsAs(List("foo", "datacenter", "from_inv"))) + } + } + + def getNode1PropNamesAsSeenByLdap(): Seq[String] = { + (for { + _ <- errors.effectUioUnit(println(s"****** ldap ********")) + n <- mockLdapFactStorage.nodeFactStorage.getAccepted(NodeId("node1"))(SelectFacts.none).notOptional("node1 must be present") + } yield n.properties.map(_.name)).runNow + } + + def getNode1PropNamesAsSeenByRepo(): Chunk[String] = { + (for { + _ <- errors.effectUioUnit(println(s"****** repos ********")) + n <- factRepo.get(NodeId("node1"))(QueryContext.testQC).notOptional("node1 must be present") + } yield n.properties.map(_.name)).runNow + } +} From c82d9521d1541800779d70605cec7294f80ceb8b Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Fri, 29 Nov 2024 10:28:36 +0100 Subject: [PATCH 3/4] fixup! Fixes #25979: Migrate custom properties along with node properties for consistency Fixes #25979: Migrate custom properties along with node properties for consistency --- .../src/main/scala/bootstrap/liftweb/RudderConfig.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala index 1a8f250a361..6c4b880db16 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala @@ -3269,6 +3269,7 @@ object RudderConfigInit { KEEP_DELETED_NODE_FACT_DURATION ), new CheckTableReportsExecutionTz(doobie), + new CheckMigrateCustomProperties(rwLdap, nodeFactRepository, acceptedNodesDitImpl), new CheckTechniqueLibraryReload( techniqueRepositoryImpl, uuidGen From 6e7c18c849533b8e08d4c2e060c2524108ac6de8 Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Fri, 29 Nov 2024 16:59:57 +0100 Subject: [PATCH 4/4] fixup! fixup! Fixes #25979: Migrate custom properties along with node properties for consistency Fixes #25979: Migrate custom properties along with node properties for consistency --- .../checks/migration/CheckMigrateCustomProperties.scala | 4 ++-- .../checks/migration/TestMigrateCustomProperties.scala | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala index 880a18353dd..6f5245534c5 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckMigrateCustomProperties.scala @@ -55,8 +55,8 @@ import zio.* * This migration check looks if there is still nodes with custom properties * in the ou=Accepted Inventories branch, and if so, it save back them so that * they are moved along with other properties. - * Added in rudder 8.1.8 (https://issues.rudder.io/issues/25978) - * Can be removed in Rudder 8.3 + * Added in rudder 8.3 (https://issues.rudder.io/issues/25979) + * Can be removed in Rudder 9.2. */ class CheckMigrateCustomProperties( diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala index 8096aae5b0d..bc135c69aba 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateCustomProperties.scala @@ -100,14 +100,12 @@ class TestMigrateCustomProperties extends Specification { def getNode1PropNamesAsSeenByLdap(): Seq[String] = { (for { - _ <- errors.effectUioUnit(println(s"****** ldap ********")) n <- mockLdapFactStorage.nodeFactStorage.getAccepted(NodeId("node1"))(SelectFacts.none).notOptional("node1 must be present") } yield n.properties.map(_.name)).runNow } def getNode1PropNamesAsSeenByRepo(): Chunk[String] = { (for { - _ <- errors.effectUioUnit(println(s"****** repos ********")) n <- factRepo.get(NodeId("node1"))(QueryContext.testQC).notOptional("node1 must be present") } yield n.properties.map(_.name)).runNow }