Skip to content

Commit

Permalink
Merge branch 'branches/rudder/8.1' into branches/rudder/8.2
Browse files Browse the repository at this point in the history
  • Loading branch information
fanf committed Sep 30, 2024
2 parents 9c7a62c + 67b2d36 commit 9e8873c
Show file tree
Hide file tree
Showing 16 changed files with 436 additions and 381 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ package com.normation.cfclerk.domain

import com.normation.inventory.domain.AgentType
import com.normation.rudder.domain.policies.PolicyTypes
import com.normation.rudder.services.policies.ComponentId
import com.normation.utils.Utils.*
import enumeratum.*
import org.apache.commons.text.StringEscapeUtils
Expand Down Expand Up @@ -215,8 +216,22 @@ final case class Technique(
*/
val templatesIds: Set[TechniqueResourceId] = agentConfigs.flatMap(cfg => cfg.templates.map(_.id)).toSet

val getAllVariableSpecs: Seq[VariableSpec] =
this.rootSection.getAllVariables ++ this.systemVariableSpecs :+ this.trackerVariableSpec
def getAllVariableSpecs: Map[ComponentId, VariableSpec] = {
val inputVars = rootSection
.getAllVariablesBySection(Nil)
.map {
case (parents, spec) =>
val reportId = spec.id.orElse(parents.headOption.flatMap(_.id))
(ComponentId(spec.name, parents.map(_.name), reportId), spec)
}
.toMap

val specialVars =
(this.systemVariableSpecs.toSeq :+ this.trackerVariableSpec).map(s => (ComponentId(s.name, Nil, None), s)).toMap

// we should not have redefinition - but with that, a special var could overwrite an input one
inputVars ++ specialVars
}

// Escape the description, so that text cannot be used to inject anything in display
def escapedDescription: String = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import com.normation.errors.Unexpected
/**
* This file define the model for metadata of object
* contained in SECTIONS.
* A section may contains other section or variables.
* A section may contain other section or variables.
*/

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,9 @@ final case class ParsedPolicyDraft(
*/
final case class BoundPolicyDraft(
id: PolicyId,
ruleName: String, // human readable name of the original rule, for ex for log
ruleName: String, // human-readable name of the original rule, for ex for log

directiveName: String, // human readable name of the original directive, for ex for log
directiveName: String, // human-readable name of the original directive, for ex for log

technique: Technique,
acceptationDate: DateTime,
Expand Down Expand Up @@ -581,58 +581,114 @@ final case class BoundPolicyDraft(
* for the targeted agent.
* This is the place where we can check for variable consistency too, since we have the final
* techniques and expanded variables (like: check that a mandatory variable was expanded to "").
*
* It's also the place where we add missing variables that are well defined, and where we check for
* constrains.
*
* If there is variable defined here but not in the technique, they are removed.
*/
def toPolicy(agent: AgentType): Either[String, Policy] = {
PolicyTechnique.forAgent(technique, agent).flatMap { pt =>
val checkedMandatoryValues: Either[Accumulated[RudderError], List[(ComponentId, Variable)]] = expandedVars.accumulatePure {
case (cid, variable) =>
// check if there is mandatory variable with missing/blank values
(variable.spec.constraint.mayBeEmpty, variable.values.exists(v => v == null || v.isBlank)) match {
case (false, true) =>
// first, if we have a default value, use that
variable.spec.constraint.default match {
case Some(d) =>
variable
.copyWithSavedValues(variable.values.map(v => if (v.isBlank) d else v))
.map(v => (cid, v))
// else it's an error
case None =>
Left(
Inconsistency(
s"Error for policy for directive '${directiveName}' [${id.directiveId.debugString}] in rule '${ruleName}' [${id.ruleId.serialize}]: " +
s"a non optional value is missing for parameter '${variable.spec.description}' [param ID: ${variable.spec.name}]"
)
)
}
case _ => Right((cid, variable))
}
}

checkedMandatoryValues
.map(withDefaultVals => {
Policy(
id,
ruleName,
directiveName,
pt,
acceptationDate,
NonEmptyList.of(
PolicyVars(
id,
policyMode,
withDefaultVals.toMap,
originalVars,
trackerVariable
)
),
priority,
policyMode,
ruleOrder,
directiveOrder,
overrides
)
})
.left
// check that all variables from root section are here and well defined
// the lookup for variables is a bit complicated. We have unique variable by name, so for now, we must to only match on that.
// Since we use the same technique to build section path, it should be the same (apart in tests) but
// I prefer to do the lookup on name
val techVarSpecs = technique.getAllVariableSpecs.map { case (cid, s) => (cid.value, (cid, s)) }

val allVars = expandedVars + (ComponentId(trackerVariable.spec.name, Nil, None) -> trackerVariable)

(for {
checkedExistingValues <- allVars.accumulatePure {
case (cid, variable) =>
techVarSpecs.get(cid.value) match {
case None =>
Left(
Inconsistency(
s"Error for policy for directive '${directiveName}' [${id.directiveId.debugString}] in rule '${ruleName}' [${id.ruleId.serialize}]: " +
s"a value is defined but the technique doesn't specify a variable with name '${variable.spec.name}' in section '${cid.parents.reverse
.mkString("/")}'"
)
)
case Some((cid2, _)) =>
if (cid != cid2) {
PolicyGenerationLogger.debug(
s"In '${directiveName}' [${id.directiveId.debugString}] in rule '${ruleName}' [${id.ruleId.serialize}]: component Id are " +
s"not the same for technique and var for '${cid.value}'. In technique: '${cid2.parents.reverse
.mkString("/")}' ; In var: '${cid.parents.reverse.mkString("/")}'"
)
}
// check if there is mandatory variable with missing/blank values
(
variable.spec.constraint.mayBeEmpty,
variable.values.exists(v => v == null || v.isEmpty)
) match {
// simple case: it's optional, we don't care if empty or not.
case (true, _) => Right((cid, variable))

// simple case: all mandatory are filled
case (false, false) => Right((cid, variable))

// complicate case: mandatory with unfilled value: check if a default is available
case (false, true) =>
// first, if we have a default value, use that
variable.spec.constraint.default match {
case Some(d) =>
variable
.copyWithSavedValues(variable.values.map(v => if (v.isEmpty) d else v))
.map(v => (cid, v))
// else it's an error
case None =>
Left(
Inconsistency(
s"Error for policy for directive '${directiveName}' [${id.directiveId.debugString}] in rule '${ruleName}' [${id.ruleId.serialize}]: " +
s"a non optional value is missing for parameter '${variable.spec.description}' [param ID: ${variable.spec.name}]"
)
)
}

}
}
}
missing = techVarSpecs -- checkedExistingValues.map(_._1.value).toSet
// add missing if possible
added <- missing.accumulatePure {
case (_, (cid, spec)) =>
(spec.constraint.mayBeEmpty, spec.constraint.default) match {
case (true, _) => Right((cid, spec.toVariable()))
case (false, Some(d)) => Right((cid, spec.toVariable(Seq(d))))
case (false, None) =>
Left(
Inconsistency(
s"Error for policy for directive '${directiveName}' [${id.directiveId.debugString}] in rule '${ruleName}' [${id.ruleId.serialize}]: " +
s"a non optional value is missing for parameter '${spec.description}' [param ID: ${spec.name}]"
)
)
}

}
} yield {
Policy(
id,
ruleName,
directiveName,
pt,
acceptationDate,
NonEmptyList.of(
PolicyVars(
id,
policyMode,
(added ++ checkedExistingValues).toMap,
originalVars,
trackerVariable
)
),
priority,
policyMode,
ruleOrder,
directiveOrder,
overrides
)
}).left
.map(_.deduplicate.msg)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1301,8 +1301,8 @@ object BuildNodeConfiguration extends Loggable {

val t0 = System.nanoTime()
// group by nodes
// no consistancy / unicity check is done here, it will be done
// in an other phase. We are just switching to a node-first view.
// no consistency / unicity check is done here, it will be done
// in another phase. We are just switching to a node-first view.
val policyDraftByNode: Map[NodeId, Seq[ParsedPolicyDraft]] = {
val byNodeDrafts = scala.collection.mutable.Map.empty[NodeId, Vector[ParsedPolicyDraft]]
ruleVals.foreach { rule =>
Expand Down Expand Up @@ -1338,7 +1338,7 @@ object BuildNodeConfiguration extends Loggable {
val toRemove = filteredTechniques.getOrElse(nodeId, Nil)
parsedDrafts.filterNot(d => toRemove.contains(d.technique.id.name))
}
// if a node is in state "emtpy policies", we only keep system policies + log
// if a node is in state "empty policies", we only keep system policies + log
filteredDrafts = if (context.nodeInfo.rudderSettings.state == NodeState.EmptyPolicies) {
PolicyGenerationLogger.info(
s"Node '${context.nodeInfo.fqdn}' (${context.nodeInfo.id.value}) is in '${context.nodeInfo.rudderSettings.state.name}' state, keeping only system policies for it"
Expand Down Expand Up @@ -1769,12 +1769,12 @@ object RuleExpectedReportBuilder extends Loggable {
case (m, originalVariables) if (m.isEmpty && originalVariables.nonEmpty) =>
(Seq(DEFAULT_COMPONENT_KEY), originalVariables.values.flatMap(_.values).toSeq) // this is an autobounding policy
case (variables, m) if (m.isEmpty) =>
PolicyGenerationLogger.warn(
PolicyGenerationLogger.debug(
s"Somewhere in the expansion of variables, the bounded variable ${boundingVar} for ${vars.trackerVariable.spec.name} in Directive ${directiveId.serialize} appeared, but was not originally there"
)
(variables.values.flatMap(_.values).toSeq, variables.values.flatMap(_.values).toSeq) // this is an autobounding policy
case (variables, originalVariables) =>
PolicyGenerationLogger.warn(
PolicyGenerationLogger.debug(
s"Expanded and unexpanded values for bounded variable ${boundingVar} for ${vars.trackerVariable.spec.name} in Directive ${directiveId.serialize} have not the same size : ${variables.values} and ${originalVariables.values}"
)
(variables.values.flatMap(_.values).toSeq, originalVariables.values.flatMap(_.values).toSeq)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ object MergePolicyService {
boundedPolicyDrafts: Seq[BoundPolicyDraft]
): Box[List[Policy]] = {

// now manage merge of mutli-instance mono-policy techniques
// now manage merge of multi-instance mono-policy techniques
// each merge can fails because of a consistency error, so we are grouping merge and checks
// for a technique in a failable function.
// must give at least one element in parameter
Expand All @@ -127,7 +127,7 @@ object MergePolicyService {
): Box[Policy] = {

//
// ACTUALY check and merge if needed
// ACTUALLY check and merge if needed
//
drafts match {
case Nil =>
Expand Down Expand Up @@ -167,11 +167,11 @@ object MergePolicyService {
)
}
/*
* Here, we must go to some lengh to try to keep the "directive" policy mode (for the merge). So we need to distinguish
* Here, we must go to some length to try to keep the "directive" policy mode (for the merge). So we need to distinguish
* four cases:
* - all drafts have None policy mode => the result is none.
* - all drafts have Some(policy mode) and all the same => the directive get that Some(policy mode)
* - at least two draft have Some(policy mode) which are differents => it's an error, report it
* - at least two draft have Some(policy mode) which are different => it's an error, report it
* - we have some homogeneous Some(policy mode) with at least one None => we must compute the resulting mode with default (global, node) values.
* The last two can be computed with PolicyMode facility .
*/
Expand All @@ -188,7 +188,7 @@ object MergePolicyService {
drafts.map(d => d.id.ruleId.serialize + " / " + d.id.directiveId.serialize).mkString(" ; "))
}
// actually merge.
// Be carefull, there is TWO merge to consider:
// Be careful, there is TWO merge to consider:
// 1. the order on which the vars are merged. It must follow existing semantic, i.e:
// first by priority, then by rule order, then by directive order.
sortedVars = NonEmptyList(d, tail).sorted(priorityThenBundleOrder).map { d =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,15 @@ class RuleValServiceImpl(
Full(
variableSpecs.map {
case (parents, spec) =>
// be carefull, component id are not defined at the varspec level, but in its parent spec one
// be careful, component id are not defined at the varspec level, but in its parent spec one
val reportId = spec.id.orElse(parents.headOption.flatMap(_.id))
context.get(spec.name) match {
case None =>
spec.toVariable()
(ComponentId(spec.toVariable().spec.name, parents.map(_.name), reportId), spec.toVariable())
(ComponentId(spec.name, parents.map(_.name), reportId), spec.toVariable())
case Some(seqValues) =>
val newVar = spec.toVariable(seqValues)
assert(seqValues.toSet == newVar.values.toSet)
(ComponentId(newVar.spec.name, parents.map(_.name), reportId), newVar)
(ComponentId(spec.name, parents.map(_.name), reportId), newVar)
}
}.toMap
)
Expand Down
Loading

0 comments on commit 9e8873c

Please sign in to comment.