Skip to content

Commit

Permalink
Work in progress
Browse files Browse the repository at this point in the history
  • Loading branch information
clarktsiory committed Mar 8, 2024
1 parent 60a4451 commit 7e838ff
Show file tree
Hide file tree
Showing 10 changed files with 249 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -607,23 +607,34 @@ object JsonResponseObjects {
}
}

final case class JRPropertyHierarchyStatus(
hasChildTypeConflicts: Boolean,
fullHierarchy: List[JRParentProperty]
)

// similar to JRGlobalParameter but s/id/name and no changeRequestId
final case class JRProperty(
name: String,
value: ConfigValue,
description: Option[String],
inheritMode: Option[InheritMode],
provider: Option[PropertyProvider],
hierarchy: Option[JRPropertyHierarchy],
origval: Option[ConfigValue]
name: String,
value: ConfigValue,
description: Option[String],
inheritMode: Option[InheritMode],
provider: Option[PropertyProvider],
hierarchy: Option[JRPropertyHierarchy],
hierarchyStatus: Option[JRPropertyHierarchyStatus],
origval: Option[ConfigValue]
)
object JRProperty {
def fromGroupProp(p: GroupProperty): JRProperty = {
val desc = if (p.description.trim.isEmpty) None else Some(p.description)
JRProperty(p.name, p.value, desc, p.inheritMode, p.provider, None, None)
JRProperty(p.name, p.value, desc, p.inheritMode, p.provider, None, None, None)
}

def fromNodePropertyHierarchy(prop: NodePropertyHierarchy, renderInHtml: RenderInheritedProperties): JRProperty = {
def fromNodePropertyHierarchy(
prop: NodePropertyHierarchy,
hasChildTypeConflicts: Boolean,
fullHierarchy: List[ParentProperty],
renderInHtml: RenderInheritedProperties
): JRProperty = {
val (parents, origval) = prop.hierarchy.reverse match {
case Nil => (None, None)
case list =>
Expand All @@ -641,8 +652,23 @@ object JsonResponseObjects {
}
(Some(parents), prop.hierarchy.headOption.map(_.value))
}
val hierarchyStatus = Some(
JRPropertyHierarchyStatus(
hasChildTypeConflicts,
fullHierarchy.map(JRParentProperty.fromParentProperty(_))
)
)
val desc = if (prop.prop.description.trim.isEmpty) None else Some(prop.prop.description)
JRProperty(prop.prop.name, prop.prop.value, desc, prop.prop.inheritMode, prop.prop.provider, parents, origval)
JRProperty(
prop.prop.name,
prop.prop.value,
desc,
prop.prop.inheritMode,
prop.prop.provider,
parents,
hierarchyStatus,
origval
)
}
}

Expand Down Expand Up @@ -683,13 +709,20 @@ object JsonResponseObjects {

object JRGroupInheritedProperties {
def fromGroup(
groupId: NodeGroupId,
properties: List[NodePropertyHierarchy],
groupId: NodeGroupId,
properties: List[
(NodePropertyHierarchy, List[ParentProperty], Boolean)
], // parent properties, child properties, has conflicts
renderInHtml: RenderInheritedProperties
): JRGroupInheritedProperties = {
JRGroupInheritedProperties(
groupId.serialize,
properties.sortBy(_.prop.name).map(JRProperty.fromNodePropertyHierarchy(_, renderInHtml))
properties
.sortBy(_._1.prop.name)
.map {
case (parentProperties, childProperties, hasConflicts) =>
JRProperty.fromNodePropertyHierarchy(parentProperties, hasConflicts, childProperties, renderInHtml)
}
)
}
}
Expand Down Expand Up @@ -921,6 +954,7 @@ trait RudderJsonEncoders {
}
}
}
implicit val propertyHierarchyStatusEncoder: JsonEncoder[JRPropertyHierarchyStatus] = DeriveJsonEncoder.gen
implicit val propertyEncoder: JsonEncoder[JRProperty] = DeriveJsonEncoder.gen
implicit val criteriumEncoder: JsonEncoder[JRCriterium] = DeriveJsonEncoder.gen
implicit val queryEncoder: JsonEncoder[JRQuery] = DeriveJsonEncoder.gen
Expand All @@ -936,10 +970,11 @@ trait RudderJsonEncoders {
object JsonResponseObjectDecodes extends RudderJsonDecoders {
import JsonResponseObjects._

implicit lazy val decodeJRParentProperty: JsonDecoder[JRParentProperty] = DeriveJsonDecoder.gen
implicit lazy val decodeJRPropertyHierarchy: JsonDecoder[JRPropertyHierarchy] = DeriveJsonDecoder.gen
implicit lazy val decodePropertyProvider: JsonDecoder[PropertyProvider] = JsonDecoder.string.map(s => PropertyProvider(s))
implicit lazy val decodeJRProperty: JsonDecoder[JRProperty] = DeriveJsonDecoder.gen
implicit lazy val decodeJRParentProperty: JsonDecoder[JRParentProperty] = DeriveJsonDecoder.gen
implicit lazy val decodeJRPropertyHierarchy: JsonDecoder[JRPropertyHierarchy] = DeriveJsonDecoder.gen
implicit lazy val decodePropertyProvider: JsonDecoder[PropertyProvider] = JsonDecoder.string.map(s => PropertyProvider(s))
implicit lazy val decodeJRPropertyHierarchyStatus: JsonDecoder[JRPropertyHierarchyStatus] = DeriveJsonDecoder.gen
implicit lazy val decodeJRProperty: JsonDecoder[JRProperty] = DeriveJsonDecoder.gen

implicit lazy val decodeJRCriterium: JsonDecoder[JRCriterium] = DeriveJsonDecoder.gen
implicit lazy val decodeJRDirectiveSectionVar: JsonDecoder[JRDirectiveSectionVar] = DeriveJsonDecoder.gen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
package com.normation.rudder.services.nodes

import cats.implicits._
import com.normation.GitVersion
import com.normation.errors._
import com.normation.inventory.domain.NodeId
import com.normation.rudder.domain.nodes.NodeGroup
Expand All @@ -59,7 +58,7 @@ import com.normation.rudder.domain.properties.PropertyProvider
import com.normation.rudder.domain.queries._
import com.normation.rudder.services.nodes.GroupProp._
import com.softwaremill.quicklens._
import com.typesafe.config.ConfigParseOptions
import com.typesafe.config.ConfigRenderOptions
import org.jgrapht.alg.connectivity.ConnectivityInspector
import org.jgrapht.graph.AsSubgraph
import org.jgrapht.graph.DefaultDirectedGraph
Expand Down Expand Up @@ -430,17 +429,7 @@ object MergeNodeProperties {
merged <- mergeAll(flatten)
globals = globalParams.toList.map {
case (n, v) =>
// TODO: no version in param for now
val config = GenericProperty.toConfig(
n,
GitVersion.DEFAULT_REV,
v.value,
v.inheritMode,
Some(INHERITANCE_PROVIDER),
None,
ConfigParseOptions.defaults().setOriginDescription(s"Global parameter '${n}'")
)
(n, NodePropertyHierarchy(NodeProperty(config), ParentProperty.Global(v.value) :: Nil))
(n, NodePropertyHierarchy(NodeProperty(v.config), ParentProperty.Global(v.value) :: Nil))
}
} yield {
// here, we add global parameters as a first default
Expand Down Expand Up @@ -567,4 +556,34 @@ object MergeNodeProperties {
} yield sorted
}

/**
* Check that all properties have the same type in all down the hierarchy (comparing valueType of config).
* If not, report all downward elements that have overrides with a different type :
* - inheriting groups that override the proprety
* - nodes that override the property
*/
def checkValueTypes(properties: List[NodePropertyHierarchy]): PureResult[Unit] = {
properties
.traverse(v => {
val valueType = v.prop.value.valueType
val overrides = v.hierarchy.collect {
case ParentProperty.Group(name, id, value) =>
s"Group '${name}' (${id.serialize}) with value '${value.render(ConfigRenderOptions.concise())}'"
case ParentProperty.Node(name, id, value) =>
s"Node '${name}' (${id.value}) with value '${value.render(ConfigRenderOptions.concise())}'"
}
if (v.hierarchy.exists(_.value.valueType != valueType)) {
Left(
Inconsistency(
s"Property '${v.prop.name}' has different types in the hierarchy. It's not allowed. " +
s"Downward elements with different types: ${overrides.mkString(", ")}"
)
)
} else {
Right(())
}
})
.void
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,29 @@ class TestMergeGroupProperties extends Specification {
merged must beRight(expected)
}

"global parameter are inherited and cannot be overridden with a different type of property" >> {
val g = "bar".toConfigValue
val parentP2PropJson =
GroupProperty("foo", GitVersion.DEFAULT_REV, GenericProperty.parseValue("""{"foo":"bar"}""").forceGet, None, None)
val p2 = parent2
.modify(_.properties)
.setTo(parentP2PropJson :: Nil)
.modify(_.query)
.setTo(Some(Query(NodeReturnType, And, Identity, List(parent1.toCriterion))))
val merged = MergeNodeProperties.checkPropertyMerge(List(p2, parent1, child).map(_.toTarget), Map("foo" -> g.toGP("foo")))
merged must beLeft[RudderError].like {
case e =>
e.fullMsg must contain(
"""Downward elements with different types: Group 'child' (child) with value '"baz"', Group 'parent2' (parent2) with value '{"foo":"bar"}', Group 'parent1' (parent1) with value '"bar1"'"""
)
}
}

"when overriding json we" should {
def getOverrides(groups: List[NodeGroup]): Map[String, String] = {

MergeNodeProperties.checkPropertyMerge(groups.map(_.toTarget), Map()) match {
case Left(_) => throw new IllegalArgumentException(s"Error when overriding properties")
case Left(e) => throw new IllegalArgumentException(s"Error when overriding properties: ${e.fullMsg}")
case Right(v) => v.map(p => (p.prop.name, GenericProperty.serializeToHocon(p.prop.value))).toMap
}
}
Expand Down Expand Up @@ -388,7 +406,7 @@ class TestMergeGroupProperties extends Specification {
}
"override whatever by simple values" in {
val child =
Map("s" -> "c", "arr" -> "c", "obj" -> "c")
Map("s" -> "c", "arr" -> """[-1]""", "obj" -> """{"a":"c"}""")
val props = checkOverrides(
Map("s" -> "p", "arr" -> "[1,2]", "obj" -> """{"a":"b"}"""),
child
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ import com.normation.rudder.apidata.implicits._
import com.normation.rudder.batch.AsyncDeploymentActor
import com.normation.rudder.batch.AutomaticStartDeployment
import com.normation.rudder.domain.nodes._
import com.normation.rudder.domain.properties.NodePropertyHierarchy
import com.normation.rudder.facts.nodes.NodeFactRepository
import com.normation.rudder.facts.nodes.QueryContext
import com.normation.rudder.repository.CategoryAndNodeGroup
import com.normation.rudder.repository.RoNodeGroupRepository
import com.normation.rudder.repository.RoParameterRepository
Expand Down Expand Up @@ -436,6 +439,7 @@ class GroupsApi(
params: DefaultParams,
authzToken: AuthzToken
): LiftResponse = {
implicit val qc: QueryContext = authzToken.qc
NodeGroupId
.parse(sid)
.toIO
Expand All @@ -454,6 +458,7 @@ class GroupsApi(
params: DefaultParams,
authzToken: AuthzToken
): LiftResponse = {
implicit val qc: QueryContext = authzToken.qc
NodeGroupId
.parse(sid)
.toIO
Expand Down Expand Up @@ -1047,6 +1052,7 @@ class GroupApiService6(
}

class GroupApiService14(
nodeFactRepo: NodeFactRepository,
readGroup: RoNodeGroupRepository,
writeGroup: WoNodeGroupRepository,
paramRepo: RoParameterRepository,
Expand Down Expand Up @@ -1309,13 +1315,47 @@ class GroupApiService14(
def getNodePropertiesTree(
groupId: NodeGroupId,
renderInHtml: RenderInheritedProperties
): IOResult[JRGroupInheritedProperties] = {
)(implicit qc: QueryContext): IOResult[JRGroupInheritedProperties] = {
for {
allGroups <- readGroup.getFullGroupLibrary().map(_.allGroups)
params <- paramRepo.getAllGlobalParameters()
properties <- MergeNodeProperties.forGroup(groupId, allGroups, params.map(p => (p.name, p)).toMap).toIO
groupLibrary <- readGroup.getFullGroupLibrary()
allGroups = groupLibrary.allGroups
serverList = allGroups.get(groupId).map(_.nodeGroup.serverList).getOrElse(Set.empty)

nodes <- nodeFactRepo.getAll().map(_.filterKeys(serverList.contains(_)).values.toList)

params <- paramRepo.getAllGlobalParameters().map(_.map(p => (p.name, p)).toMap)
parentProperties <- MergeNodeProperties.forGroup(groupId, allGroups, params).toIO
properties <- ZIO.foreach(nodes) { nodeFact =>
MergeNodeProperties
.forNode(
nodeFact.toNodeInfo,
groupLibrary.getTarget(nodeFact).map(_._2).toList,
params
)
.map(childProperties => {
parentProperties.map(p => {
val matchingChildProperties = childProperties.collect {
case cp if cp.prop.name == p.prop.name => cp.hierarchy
}
val hasConflicts = MergeNodeProperties
.checkValueTypes(matchingChildProperties.map(NodePropertyHierarchy(p.prop, _)))
.isLeft
(
p,
matchingChildProperties.flatten,
hasConflicts
)
})
})
.toIO
}

} yield {
JRGroupInheritedProperties.fromGroup(groupId, properties, renderInHtml)
JRGroupInheritedProperties.fromGroup(
groupId,
properties.flatten, // TODO: instead of flatten we need to sort and have a 3-level hierarchy : "node"s, "group"s, "global"s
renderInHtml
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,24 @@ class NodeApiService(
keyInfo = getKeyInfo(restNode)
updated = updateNode(nodeFact, restNode, newProperties, keyInfo._1, keyInfo._2)
_ <- if (CoreNodeFact.same(updated, nodeFact)) ZIO.unit
else nodeFactRepository.save(NodeFact.fromMinimal(updated)).unit
else {
for {
targets <- groupRepo.getFullGroupLibrary().map(_.getTarget(nodeFact).map(_._2).toList)
globalParameters <- paramRepo.getAllGlobalParameters()
// check that properties are of the right type
_ <- MergeNodeProperties
.forNode(
updated.toNodeInfo,
targets,
globalParameters.map(p => (p.name, p)).toMap
)
.flatMap(
MergeNodeProperties.checkValueTypes(_)
)
.toIO
_ <- nodeFactRepository.save(NodeFact.fromMinimal(updated))
} yield ()
}
} yield {
updated
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ class RestTestSetUp {
)
val groupService6 = new GroupApiService6(mockNodeGroups.groupsRepo, mockNodeGroups.groupsRepo, restDataSerializer)
val groupService14 = new GroupApiService14(
mockNodes.nodeFactRepo,
mockNodeGroups.groupsRepo,
mockNodeGroups.groupsRepo,
mockParameters.paramsRepo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,19 @@ type alias Property =
, value : Value
, provider : Maybe String
, hierarchy : Maybe String
, hierarchyStatus : Maybe HierarchyStatus
, origval : Maybe Value
}

type alias HierarchyStatus =
{ hasChildTypeConflicts : Bool
, fullHierarchy : List ParentProperty
}

type alias ParentGroupProperty = { id : String, name : String }
type alias ParentNodeProperty = { id : String, name : String }
type ParentProperty = ParentGlobal | ParentGroup ParentGroupProperty | ParentNode ParentNodeProperty


type SortOrder = Asc | Desc

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,29 @@ decodeProperty =
|> required "value" value
|> optional "provider" (map Just string) Nothing
|> optional "hierarchy" (map Just string) Nothing
|> optional "hierarchyStatus" (map Just decodeHierarchyStatus) Nothing
|> optional "origval" (map Just value) Nothing

decodeHierarchyStatus : Decoder HierarchyStatus
decodeHierarchyStatus =
succeed HierarchyStatus
|> required "hasChildTypeConflicts" bool
|> required "fullHierarchy" (list decodeParentProperty)

decodeParentProperty : Decoder ParentProperty
decodeParentProperty =
field "kind" string |> andThen (\s ->
case s of
"group" ->
map2 ParentGroupProperty
(field "id" string)
(field "name" string)
|> map ParentGroup
"node" ->
map2 ParentNodeProperty
(field "id" string)
(field "name" string)
|> map ParentNode
"global" -> succeed ParentGlobal
_ -> fail "Invalid parent property kind"
)
Loading

0 comments on commit 7e838ff

Please sign in to comment.