From 67b2d367e7c48bb07c7f91a90affec8ad39a70ac Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Fri, 27 Sep 2024 21:37:36 +0200 Subject: [PATCH] Fixes #25557: Spaces as a default value provided by node properties are now refused --- .../normation/cfclerk/domain/Technique.scala | 19 +- .../domain/VariableAndSectionSpec.scala | 2 +- .../services/policies/DataStructures.scala | 158 ++++++---- .../services/policies/DeploymentService.scala | 10 +- .../policies/MergePolicyService.scala | 10 +- .../services/policies/RuleValService.scala | 7 +- .../policies/write/BuildBundleSequence.scala | 112 ++++--- .../policies/write/PolicyWriterService.scala | 2 +- .../write/PrepareTemplateVariables.scala | 53 +--- .../cfengine-community/rudder-directives.cf | 4 +- .../Create_file/1.0/metadata.xml | 7 + .../com/normation/rudder/MockServices.scala | 25 +- .../services/policies/NodeConfigData.scala | 286 ++++++++++-------- .../policies/TestBuildNodeConfiguration.scala | 105 +++---- .../policies/write/PolicyAgregationTest.scala | 7 +- .../src/test/resources/api/api_directives.yml | 8 +- 16 files changed, 435 insertions(+), 380 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/Technique.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/Technique.scala index 7a64c7407dd..70869e45539 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/Technique.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/Technique.scala @@ -38,6 +38,7 @@ package com.normation.cfclerk.domain import com.normation.inventory.domain.AgentType +import com.normation.rudder.services.policies.ComponentId import com.normation.utils.Utils.* import enumeratum.* import org.apache.commons.text.StringEscapeUtils @@ -214,8 +215,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 = { diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/VariableAndSectionSpec.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/VariableAndSectionSpec.scala index e15e25d9a17..4c00aea4f76 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/VariableAndSectionSpec.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/VariableAndSectionSpec.scala @@ -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. */ /** diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DataStructures.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DataStructures.scala index 3036f4b1932..32b24dcb223 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DataStructures.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DataStructures.scala @@ -534,9 +534,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, @@ -579,58 +579,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) } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DeploymentService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DeploymentService.scala index 118d2d597bf..3f1fb092d39 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DeploymentService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DeploymentService.scala @@ -1280,8 +1280,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 => @@ -1317,7 +1317,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.state == NodeState.EmptyPolicies) { PolicyGenerationLogger.info( s"Node '${context.nodeInfo.hostname}' (${context.nodeInfo.id.value}) is in '${context.nodeInfo.state.name}' state, keeping only system policies for it" @@ -1748,12 +1748,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) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/MergePolicyService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/MergePolicyService.scala index 9f81fd235e2..8121b2191ae 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/MergePolicyService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/MergePolicyService.scala @@ -111,7 +111,7 @@ object MergePolicyService { */ def buildPolicy(nodeInfo: NodeInfo, mode: GlobalPolicyMode, 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 @@ -123,7 +123,7 @@ object MergePolicyService { ): Box[Policy] = { // - // ACTUALY check and merge if needed + // ACTUALLY check and merge if needed // drafts match { case Nil => @@ -163,11 +163,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 . */ @@ -184,7 +184,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 => diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/RuleValService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/RuleValService.scala index 3761219968c..bb1110648fa 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/RuleValService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/RuleValService.scala @@ -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 ) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/BuildBundleSequence.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/BuildBundleSequence.scala index dcb3a3e593b..24f40ce8546 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/BuildBundleSequence.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/BuildBundleSequence.scala @@ -40,9 +40,11 @@ package com.normation.rudder.services.policies.write import cats.implicits.* import com.normation.cfclerk.domain.BundleName import com.normation.cfclerk.domain.RunHook +import com.normation.cfclerk.domain.SectionSpec import com.normation.cfclerk.domain.SystemVariable import com.normation.cfclerk.domain.TechniqueGenerationMode import com.normation.cfclerk.domain.TechniqueId +import com.normation.cfclerk.domain.Variable import com.normation.cfclerk.services.SystemVariableSpecService import com.normation.errors.* import com.normation.inventory.domain.AgentType @@ -363,60 +365,78 @@ class BuildBundleSequence( val name = Promiser(policy.ruleName + "/" + policy.directiveName) // and for now, all bundle get the same reportKey - val techniqueBundles = policy.technique.agentConfig.bundlesequence.map { bundleName => - if (bundleName.value.trim.nonEmpty) { - val vars = { - policy.technique.generationMode match { - case TechniqueGenerationMode.MultipleDirectivesWithParameters => - for { - // Here we are looking for variables value from a Directive based on a ncf technique that has parameter - // ncf technique parameters ( having an id and a name, which is used inside the technique) were translated into Rudder variables spec - // (having a name, which acts as an id, and allow to do templating on techniques, and a description which is presented to users) with the following Rule - // ncf Parameter | Rudder variable - // id | name - // name | description - // So here we only have Rudder variables, and we want to create a new object BundleParam, which name needs to be the value used inside the technique (so the tehcnique paramter name) - // And we will get the value in the Directive by looking for it with the name of the Variable (which is the id of the parameter) - // ncf Parameter | Rudder variable | Bundle param - // id | name | N/A - // name | description | name = variable.description = parameter.name - // N/A | N/A | value = values.get(variable.name) = values.get(parameter.id) - // We would definitely be happier if we add a way to describe them as Rudder technique parameter and not as variable - - (varId, varName, variableName) <- - policy.technique.rootSection.copyWithoutSystemVars.getAllVariables.map(v => - (v.name, v.description, v.variableName) + def getVars( + generationMode: TechniqueGenerationMode, + rootSection: SectionSpec, + expandedVars: Map[String, Variable] + ): PureResult[List[BundleParam]] = { + generationMode match { + case TechniqueGenerationMode.MultipleDirectivesWithParameters => + rootSection.copyWithoutSystemVars.getAllVariables.accumulatePure { v => + // Here we are looking for variables value from a Directive based on a ncf technique that has parameter + // ncf technique parameters ( having an id and a name, which is used inside the technique) were translated into Rudder variables spec + // (having a name, which acts as an id, and allow to do templating on techniques, and a description which is presented to users) with the following Rule + // ncf Parameter | Rudder variable + // id | name + // name | description + // So here we only have Rudder variables, and we want to create a new object BundleParam, which name needs to be the value used inside the technique (so the technique parameter name) + // And we will get the value in the Directive by looking for it with the name of the Variable (which is the id of the parameter) + // ncf Parameter | Rudder variable | Bundle param + // id | name | N/A + // name | description | name = variable.description = parameter.name + // N/A | N/A | value = values.get(variable.name) = values.get(parameter.id) + // We would definitely be happier if we add a way to describe them as Rudder technique parameter and not as variable + val (varId, varName, variableName) = (v.name, v.description, v.variableName) + + for { + value <- + expandedVars + .get(varId) + .flatMap(_.values.headOption) + .notOptionalPure( + s"Missing variable value for '${varId}' (${varName}) in node ${nodeId.value} -> ${name}. This is " + + s"likely a bug, please report it to rudder devs" ) - } yield { - val value = policy.expandedVars.get(varId).map(_.values.headOption.getOrElse("")).getOrElse("") - BundleParam.DoubleQuote(value, varName, variableName) - } - case TechniqueGenerationMode.MergeDirectives | TechniqueGenerationMode.MultipleDirectives => - Nil + } yield { + BundleParam.DoubleQuote(value, varName, variableName) + } } - } + case TechniqueGenerationMode.MergeDirectives | TechniqueGenerationMode.MultipleDirectives => + Right(Nil) + } + } - List(Bundle(Some(policy.id), bundleName, vars.toList)) + val techniqueBundles = policy.technique.agentConfig.bundlesequence.accumulatePure { bundleName => + if (bundleName.value.trim.nonEmpty) { + for { + vars <- getVars(policy.technique.generationMode, policy.technique.rootSection, policy.expandedVars) + } yield { + List(Bundle(Some(policy.id), bundleName, vars)) + } } else { - PolicyGenerationLogger.warn( + val msg = s"Technique '${policy.technique.id}' used in node '${nodeId.value}' contains some bundle with empty name, which is forbidden and so they are ignored in the final bundle sequence" - ) - Nil + PolicyGenerationLogger.warn(msg) + Left(Inconsistency(msg)) } - }.flatten.toList + }.map(_.flatten) for { policyMode <- PolicyMode.computeMode(globalPolicyMode, nodePolicyMode, policy.policyMode :: Nil) - } yield { // we must update technique bundle in case policy generation is multi-instance - val bundles = policy.technique.generationMode match { - case TechniqueGenerationMode.MultipleDirectives => - techniqueBundles.map(b => - b.copy(name = BundleName(b.name.value.replaceAll(Policy.TAG_OF_RUDDER_MULTI_POLICY, policy.id.getRudderUniqueId))) - ) - case _ => - techniqueBundles - } + bundles <- policy.technique.generationMode match { + case TechniqueGenerationMode.MultipleDirectives => + techniqueBundles.map( + _.map(b => { + b.copy(name = + BundleName(b.name.value.replaceAll(Policy.TAG_OF_RUDDER_MULTI_POLICY, policy.id.getRudderUniqueId)) + ) + }) + ) + case _ => + techniqueBundles + } + } yield { TechniqueBundles( name, policy.id.directiveId, @@ -581,8 +601,8 @@ object CfengineBundleVariables { } /* - * utilitary method for formating an input list - * For the CFengine agent, we are waiting ONE string of the fully + * utility method for formating an input list + * For the CFEngine agent, we are waiting ONE string of the fully * formatted result, ie. something like: """ * "common/1.0/update.cf", * "rudder-directives.cf", diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PolicyWriterService.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PolicyWriterService.scala index bce76c68327..93c5ae83db5 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PolicyWriterService.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PolicyWriterService.scala @@ -731,7 +731,7 @@ class PolicyWriterServiceImpl( /* * Idea: sort template by content (ie what is cached). Put each unique cache in a seq with the corresponding list of * template to fill. - * Get the the corresponding list of filled template. + * Get the corresponding list of filled template. * Write it. * No semaphore, no synchronisation. */ diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PrepareTemplateVariables.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PrepareTemplateVariables.scala index fd8a71f14d7..f18c40f23e2 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PrepareTemplateVariables.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/write/PrepareTemplateVariables.scala @@ -265,7 +265,7 @@ class PrepareTemplateVariablesImpl( .findHandler(agentNodeProps) .toIO .chainError(s"Error when trying to fetch variable escaping method for node ${agentNodeProps.nodeId.value}") - // here, `traverse` seems to give similar but more consistant results than `traverseParN` + // here, `traverse` seems to give similar but more consistent results than `traverseParN` preparedTechniques <- ZIO.foreach(policies) { p => for { _ <- @@ -357,54 +357,9 @@ class PrepareTemplateVariablesImpl( // we can't do it when we built node configuration because some (system variable at least (note: but only? If so, we could just check // them here, not everything). - val variables = policy.expandedVars + ((policy.trackerVariable.spec.name, policy.trackerVariable)) - - for { - stVariables <- policy.technique.getAllVariableSpecs.accumulate { variableSpec => - variableSpec match { - case x: TrackerVariableSpec => - variables.get(x.name) match { - case None => - Inconsistency( - s"[${agentNodeProps.nodeId.value}:${policy.technique.id.debugString}] Misssing mandatory value for tracker variable: '${x.name}'" - ).fail - case Some(v) => - stVariableFromVariable(v, agentVariableHandler, agentNodeProps.nodeId, policy.technique.id) - .map(Some(_)) - } - case x: SystemVariableSpec => - systemVars.get(x.name) match { - case None => - if (x.constraint.mayBeEmpty) { // ok, that's expected - PolicyGenerationLoggerPure.trace( - s"[${agentNodeProps.nodeId.value}:${policy.technique.id.debugString}] Variable system named '${x.name}' not found in the extended variables environnement" - ) *> - None.succeed - } else { - Inconsistency( - s"[${agentNodeProps.nodeId.value}:${policy.technique.id.debugString}] Missing value for system variable: '${x.name}'" - ).fail - } - case Some(sysvar) => - stVariableFromVariable(sysvar, agentVariableHandler, agentNodeProps.nodeId, policy.technique.id) - .map(Some(_)) - } - case x: SectionVariableSpec => - variables.get(x.name) match { - case None => - Inconsistency( - s"[${agentNodeProps.nodeId.value}:${policy.technique.id.debugString}] Misssing value for standard variable: '${x.name}'" - ).fail - case Some(v) => - stVariableFromVariable(v, agentVariableHandler, agentNodeProps.nodeId, policy.technique.id) - .map(Some(_)) - } - } - } - } yield { - // return the collection without the none - stVariables.flatten - } + ZIO.foreach(policy.trackerVariable :: (systemVars.values ++ policy.expandedVars.values).toList)(v => + stVariableFromVariable(v, agentVariableHandler, agentNodeProps.nodeId, policy.technique.id) + ) } } diff --git a/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/expected-share/node-cfe-with-two-directives/rules/cfengine-community/rudder-directives.cf b/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/expected-share/node-cfe-with-two-directives/rules/cfengine-community/rudder-directives.cf index ad9f1b52d87..a27701bf192 100644 --- a/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/expected-share/node-cfe-with-two-directives/rules/cfengine-community/rudder-directives.cf +++ b/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/expected-share/node-cfe-with-two-directives/rules/cfengine-community/rudder-directives.cf @@ -33,8 +33,8 @@ bundle agent run_16d86a56_93ef_49aa_86b7_0d10102e4ea9 methods: "50-rule-technique-ncf/Create a file" usebundle => rudder_reporting_context_v4("16d86a56-93ef-49aa-86b7-0d10102e4ea9","208716db-2675-43b9-ab57-bfbab84346aa","Create_file","","","16d86a56-93ef-49aa-86b7-0d10102e4ea9208716db-2675-43b9-ab57-bfbab84346aa"); "50-rule-technique-ncf/Create a file" usebundle => enable_reporting; - "50-rule-technique-ncf/Create a file" usebundle => Create_file("\"foo"); - "50-rule-technique-ncf/Create a file" usebundle => Create_file_rudder_reporting("\"foo"); + "50-rule-technique-ncf/Create a file" usebundle => Create_file("\"foo"," "); + "50-rule-technique-ncf/Create a file" usebundle => Create_file_rudder_reporting("\"foo"," "); "50-rule-technique-ncf/Create a file" usebundle => clean_reporting_context; } bundle agent run_16617aa8_1f02_4e4a_87b6_d0bcdfb4019f diff --git a/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/techniques/ncf_techniques/Create_file/1.0/metadata.xml b/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/techniques/ncf_techniques/Create_file/1.0/metadata.xml index 243796c9979..8fa90f5b3f0 100644 --- a/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/techniques/ncf_techniques/Create_file/1.0/metadata.xml +++ b/webapp/sources/rudder/rudder-core/src/test/resources/configuration-repository/techniques/ncf_techniques/Create_file/1.0/metadata.xml @@ -69,6 +69,13 @@ file_name + + 3021FC4F-DA33-4D84-8991-C42EBAB2335F + test_space + + + + diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/MockServices.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/MockServices.scala index 5164c97dae9..6693fa68995 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/MockServices.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/MockServices.scala @@ -130,6 +130,7 @@ import com.normation.rudder.rule.category.* import com.normation.rudder.services.marshalling.NodeGroupCategoryUnserialisationImpl import com.normation.rudder.services.marshalling.NodeGroupUnserialisationImpl import com.normation.rudder.services.marshalling.RuleUnserialisationImpl +import com.normation.rudder.services.policies.ComponentId import com.normation.rudder.services.policies.NodeConfigData import com.normation.rudder.services.policies.NodeConfiguration import com.normation.rudder.services.policies.ParameterForConfiguration @@ -375,20 +376,18 @@ class MockDirectives(mockTechniques: MockTechniques) { * in class TestNodeConfiguration */ - val commonTechnique: Technique = techniqueRepos.unsafeGet(TechniqueId(TechniqueName("common"), TV("1.0"))) - def commonVariables(nodeId: NodeId, allNodeInfos: Map[NodeId, NodeInfo]): Map[String, VariableSpec#V] = { - val spec = commonTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("OWNER").toVariable(Seq(allNodeInfos(nodeId).localAdministratorAccountName)), - spec("UUID").toVariable(Seq(nodeId.value)), - spec("POLICYSERVER_ID").toVariable(Seq(allNodeInfos(nodeId).policyServerId.value)), - spec("POLICYSERVER_ADMIN").toVariable( - Seq(allNodeInfos(allNodeInfos(nodeId).policyServerId).localAdministratorAccountName) - ), - spec("ALLOWEDNETWORK").toVariable(Seq("")) - ).map(v => (v.spec.name, v)).toMap + val commonTechnique: Technique = techniqueRepos.unsafeGet(TechniqueId(TechniqueName("common"), TV("1.0"))) + def commonVariables(nodeId: NodeId, allNodeInfos: Map[NodeId, NodeInfo]): Map[ComponentId, VariableSpec#V] = { + commonTechnique.getAllVariableSpecs.collect { + case (c @ ComponentId("OWNER", _, _), s) => (c, s.toVariable(Seq(allNodeInfos(nodeId).localAdministratorAccountName))) + case (c @ ComponentId("UUID", _, _), s) => (c, s.toVariable(Seq(nodeId.value))) + case (c @ ComponentId("POLICYSERVER_ID", _, _), s) => (c, s.toVariable(Seq(allNodeInfos(nodeId).policyServerId.value))) + case (c @ ComponentId("POLICYSERVER_ADMIN", _, _), s) => + (c, s.toVariable(Seq(allNodeInfos(allNodeInfos(nodeId).policyServerId).localAdministratorAccountName))) + case (c @ ComponentId("ALLOWEDNETWORK", _, _), s) => (c, s.toVariable(Seq(""))) + } } - val commonDirective: Directive = Directive( + val commonDirective: Directive = Directive( DirectiveId(DirectiveUid("common-root"), GitVersion.DEFAULT_REV), TV("1.0"), Map( diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/NodeConfigData.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/NodeConfigData.scala index 9507bb54c52..f9bf42d80fd 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/NodeConfigData.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/NodeConfigData.scala @@ -48,7 +48,6 @@ import com.normation.cfclerk.domain.TechniqueVersion import com.normation.cfclerk.domain.TechniqueVersionHelper import com.normation.cfclerk.domain.TrackerVariableSpec import com.normation.cfclerk.domain.Variable -import com.normation.cfclerk.domain.VariableSpec import com.normation.cfclerk.services.impl.GitTechniqueReader import com.normation.cfclerk.services.impl.SystemVariableSpecServiceImpl import com.normation.cfclerk.services.impl.TechniqueRepositoryImpl @@ -647,9 +646,9 @@ ootapja6lKOaIpqp0kmmYN7gFIhp implicit def toRID(id: String): RuleId = RuleId(RuleUid(id)) implicit def toRCID(id: String): RuleCategoryId = RuleCategoryId(id) val t1: Technique = Technique(("t1", "1.0"), "t1", "t1", Nil, TrackerVariableSpec(None, None), SectionSpec("root"), None) - val d1: Directive = Directive("d1", "1.0", Map("foo1" -> Seq("bar1")), "d1", "d1", None) - val d2: Directive = Directive("d2", "1.0", Map("foo2" -> Seq("bar2")), "d2", "d2", Some(PolicyMode.Enforce)) - val d3: Directive = Directive("d3", "1.0", Map("foo3" -> Seq("bar3")), "d3", "d3", Some(PolicyMode.Audit)) + val d1: Directive = Directive("d1", "1.0", Map("foo1" -> Seq("bar1")), "d1", "d1", None, _isEnabled = true) + val d2: Directive = Directive("d2", "1.0", Map("foo2" -> Seq("bar2")), "d2", "d2", Some(PolicyMode.Enforce), _isEnabled = true) + val d3: Directive = Directive("d3", "1.0", Map("foo3" -> Seq("bar3")), "d3", "d3", Some(PolicyMode.Audit), _isEnabled = true) val fat1: FullActiveTechnique = FullActiveTechnique( "d1", "t1", @@ -914,7 +913,7 @@ class TestNodeConfiguration( .openOrThrowException("I should get global system variable in test!") val t9: Long = System.currentTimeMillis() - NodeConfigData.logger.trace(s"Nodes & groupes : ${t9 - t8} ms") + NodeConfigData.logger.trace(s"Nodes & groups : ${t9 - t8} ms") // // root has 4 system directive, let give them some variables @@ -960,14 +959,16 @@ class TestNodeConfiguration( val commonTechnique: Technique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("common"), TechniqueVersionHelper("1.0"))) def commonVariables(nodeId: NodeId, allNodeInfos: Map[NodeId, NodeInfo]): Map[ComponentId, Variable] = { - val spec = commonTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("OWNER").toVariable(Seq(allNodeInfos(nodeId).localAdministratorAccountName)), - spec("UUID").toVariable(Seq(nodeId.value)), - spec("POLICYSERVER_ID").toVariable(Seq(allNodeInfos(nodeId).policyServerId.value)), - spec("POLICYSERVER_ADMIN").toVariable(Seq(allNodeInfos(allNodeInfos(nodeId).policyServerId).localAdministratorAccountName)), - spec("ALLOWEDNETWORK").toVariable(Seq("")) - ).map(v => (ComponentId(v.spec.name, Nil, None), v)).toMap // None because no reportId for old var + getVariablesFromSpec( + commonTechnique, + List( + ("OWNER", Seq(allNodeInfos(nodeId).localAdministratorAccountName)), + ("UUID", Seq(nodeId.value)), + ("POLICYSERVER_ID", Seq(allNodeInfos(nodeId).policyServerId.value)), + ("POLICYSERVER_ADMIN", Seq(allNodeInfos(allNodeInfos(nodeId).policyServerId).localAdministratorAccountName)), + ("ALLOWEDNETWORK", Seq("")) + ) + ) } val commonDirective: Directive = Directive( @@ -1016,31 +1017,39 @@ class TestNodeConfiguration( // we have one rule with several system technique for root server config - // get variable's values based on the kind of spec for that: if the values are provided, use them. - def getVariables( - techiqueDebugId: String, - spec: Map[String, VariableSpec], - variables: List[String] + // a version of get with a default value "value for ..." + def getVariablesFromSpec( + technique: Technique, + variables: List[(String, Seq[String])] ): Map[ComponentId, Variable] = { - variables - .map(name => { - spec - .get(name) + val specs = technique.getAllVariableSpecs + // add system variable as a base, we don't build them correctly because we skip NodeConfiguration set-up + // in that test, see TestBuildNodeConfiguration to see how it should be done + specs.collect { case (cid, s) if (s.isSystem) => (cid, s.toVariable(Seq(s"dummy_system_${s.name}"))) } ++ + variables.map { + case (name, values) => + specs + .find(s => s._1.value == name) .getOrElse( - throw new RuntimeException(s"Missing variable spec '${name}' in technique ${techiqueDebugId} in test") + throw new RuntimeException(s"Missing variable spec '${name}' in technique ${technique.id.debugString} in test") ) match { - case p: PredefinedValuesVariableSpec => p.toVariable(p.providedValues._1 :: p.providedValues._2.toList) - case s => s.toVariable(Seq(s"value for '${name}'")) + case (cid, p: PredefinedValuesVariableSpec) => (cid, p.toVariable(p.providedValues._1 :: p.providedValues._2.toList)) + case (cid, s) => (cid, s.toVariable(values)) } - }) - .map(v => (ComponentId(v.spec.name, Nil, None), v)) - .toMap + }.toMap + } + + // get variable's values based on the kind of spec for that: if the values are provided, use them. + def getVariables( + technique: Technique, + variables: List[String] + ): Map[ComponentId, Variable] = { + getVariablesFromSpec(technique, variables.map(name => (name, Seq(s"value for '${name}'")))) } def simpleServerPolicy(name: String, variables: List[String] = List()): (Technique, BoundPolicyDraft) = { val technique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName(s"${name}"), TechniqueVersionHelper("1.0"))) - val spec = technique.getAllVariableSpecs.map(s => (s.name, s)).toMap - val vars = getVariables(technique.id.serialize, spec, variables) + val vars = getVariables(technique, variables) val policy = { val id = PolicyId(RuleId("policy-server-root"), DirectiveId(DirectiveUid(s"${name}-root")), TechniqueVersionHelper("1.0")) draft( @@ -1084,8 +1093,7 @@ class TestNodeConfiguration( val inventoryTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("inventory"), TechniqueVersionHelper("1.0"))) val inventoryVariables: Map[ComponentId, Variable] = { - val spec = inventoryTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - getVariables(inventoryTechnique.id.serialize, spec, List("expectedReportKey Inventory")) + getVariables(inventoryTechnique, List("expectedReportKey Inventory")) } val inventoryAll: BoundPolicyDraft = { @@ -1108,14 +1116,16 @@ class TestNodeConfiguration( lazy val clockTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("clockConfiguration"), TechniqueVersionHelper("3.0"))) lazy val clockVariables: Map[ComponentId, Variable] = { - val spec = clockTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("CLOCK_FQDNNTP").toVariable(Seq("")), // testing mandatory value with default "true" - spec("CLOCK_HWSYNC_ENABLE").toVariable(Seq("true")), - spec("CLOCK_NTPSERVERS").toVariable(Seq("${rudder.param.ntpserver}")), - spec("CLOCK_SYNCSCHED").toVariable(Seq("240")), - spec("CLOCK_TIMEZONE").toVariable(Seq("dontchange")) - ).map(v => (ComponentId(v.spec.name, Nil, None), v)).toMap + getVariablesFromSpec( + clockTechnique, + List( + ("CLOCK_FQDNNTP", Seq("")), // testing mandatory value with default "true" + ("CLOCK_HWSYNC_ENABLE", Seq("true")), + ("CLOCK_NTPSERVERS", Seq("${rudder.param.ntpserver}")), + ("CLOCK_SYNCSCHED", Seq("240")), + ("CLOCK_TIMEZONE", Seq("dontchange")) + ) + ) } lazy val clock: BoundPolicyDraft = { val id = PolicyId(RuleId("rule1"), DirectiveId(DirectiveUid("directive1+rev1")), TechniqueVersionHelper("1.0+rev2")) @@ -1139,17 +1149,19 @@ class TestNodeConfiguration( lazy val rpmTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("rpmPackageInstallation"), TechniqueVersionHelper("7.0"))) lazy val rpmVariables: Map[ComponentId, Variable] = { - val spec = rpmTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("RPM_PACKAGE_CHECK_INTERVAL").toVariable(Seq("5")), - spec("RPM_PACKAGE_POST_HOOK_COMMAND").toVariable(Seq("", "", "")), - spec("RPM_PACKAGE_POST_HOOK_RUN").toVariable(Seq("false", "false", "false")), - spec("RPM_PACKAGE_REDACTION").toVariable(Seq("add", "add", "add")), - spec("RPM_PACKAGE_REDLIST").toVariable(Seq("plop", "foo", "bar")), - spec("RPM_PACKAGE_VERSION").toVariable(Seq("", "", "")), - spec("RPM_PACKAGE_VERSION_CRITERION").toVariable(Seq("==", "==", "==")), - spec("RPM_PACKAGE_VERSION_DEFINITION").toVariable(Seq("default", "default", "default")) - ).map(v => (ComponentId(v.spec.name, Nil, None), v)).toMap + getVariablesFromSpec( + rpmTechnique, + List( + ("RPM_PACKAGE_CHECK_INTERVAL", Seq("5")), + ("RPM_PACKAGE_POST_HOOK_COMMAND", Seq("", "", "")), + ("RPM_PACKAGE_POST_HOOK_RUN", Seq("false", "false", "false")), + ("RPM_PACKAGE_REDACTION", Seq("add", "add", "add")), + ("RPM_PACKAGE_REDLIST", Seq("plop", "foo", "bar")), + ("RPM_PACKAGE_VERSION", Seq("", "", "")), + ("RPM_PACKAGE_VERSION_CRITERION", Seq("==", "==", "==")), + ("RPM_PACKAGE_VERSION_DEFINITION", Seq("default", "default", "default")) + ) + ) } def rpmDirective(id: String, pkg: String): Directive = Directive( DirectiveId(DirectiveUid(id), GitVersion.DEFAULT_REV), @@ -1167,7 +1179,8 @@ class TestNodeConfiguration( id, "", None, - "" + "", + _isEnabled = true ) lazy val rpm: BoundPolicyDraft = { val id = PolicyId(RuleId("rule2"), DirectiveId(DirectiveUid("directive2")), TechniqueVersionHelper("1.0")) @@ -1185,17 +1198,19 @@ class TestNodeConfiguration( lazy val pkgTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("packageManagement"), TechniqueVersionHelper("1.0"))) lazy val pkgVariables: Map[ComponentId, Variable] = { - val spec = pkgTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("PACKAGE_LIST").toVariable(Seq("htop")), - spec("PACKAGE_STATE").toVariable(Seq("present")), - spec("PACKAGE_VERSION").toVariable(Seq("latest")), - spec("PACKAGE_VERSION_SPECIFIC").toVariable(Seq("")), - spec("PACKAGE_ARCHITECTURE").toVariable(Seq("default")), - spec("PACKAGE_ARCHITECTURE_SPECIFIC").toVariable(Seq("")), - spec("PACKAGE_MANAGER").toVariable(Seq("default")), - spec("PACKAGE_POST_HOOK_COMMAND").toVariable(Seq("")) - ).map(v => (ComponentId(v.spec.name, Nil, None), v)).toMap + getVariablesFromSpec( + pkgTechnique, + List( + ("PACKAGE_LIST", Seq("htop")), + ("PACKAGE_STATE", Seq("present")), + ("PACKAGE_VERSION", Seq("latest")), + ("PACKAGE_VERSION_SPECIFIC", Seq("")), + ("PACKAGE_ARCHITECTURE", Seq("default")), + ("PACKAGE_ARCHITECTURE_SPECIFIC", Seq("")), + ("PACKAGE_MANAGER", Seq("default")), + ("PACKAGE_POST_HOOK_COMMAND", Seq("")) + ) + ) } lazy val pkg: BoundPolicyDraft = { val id = PolicyId( @@ -1217,19 +1232,21 @@ class TestNodeConfiguration( lazy val fileTemplateTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("fileTemplate"), TechniqueVersionHelper("1.0"))) lazy val fileTemplateVariables1: Map[ComponentId, Variable] = { - val spec = fileTemplateTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("FILE_TEMPLATE_RAW_OR_NOT").toVariable(Seq("Raw")), - spec("FILE_TEMPLATE_TEMPLATE").toVariable(Seq("")), - spec("FILE_TEMPLATE_RAW_TEMPLATE").toVariable(Seq("some content")), - spec("FILE_TEMPLATE_AGENT_DESTINATION_PATH").toVariable(Seq("/tmp/destination.txt")), - spec("FILE_TEMPLATE_TEMPLATE_TYPE").toVariable(Seq("mustache")), - spec("FILE_TEMPLATE_OWNER").toVariable(Seq("root")), - spec("FILE_TEMPLATE_GROUP_OWNER").toVariable(Seq("root")), - spec("FILE_TEMPLATE_PERMISSIONS").toVariable(Seq("700")), - spec("FILE_TEMPLATE_PERSISTENT_POST_HOOK").toVariable(Seq("false")), - spec("FILE_TEMPLATE_TEMPLATE_POST_HOOK_COMMAND").toVariable(Seq("")) - ).map(v => (ComponentId(v.spec.name, Nil, None), v)).toMap + getVariablesFromSpec( + fileTemplateTechnique, + List( + ("FILE_TEMPLATE_RAW_OR_NOT", Seq("Raw")), + ("FILE_TEMPLATE_TEMPLATE", Seq("")), + ("FILE_TEMPLATE_RAW_TEMPLATE", Seq("some content")), + ("FILE_TEMPLATE_AGENT_DESTINATION_PATH", Seq("/tmp/destination.txt")), + ("FILE_TEMPLATE_TEMPLATE_TYPE", Seq("mustache")), + ("FILE_TEMPLATE_OWNER", Seq("root")), + ("FILE_TEMPLATE_GROUP_OWNER", Seq("root")), + ("FILE_TEMPLATE_PERMISSIONS", Seq("700")), + ("FILE_TEMPLATE_PERSISTENT_POST_HOOK", Seq("false")), + ("FILE_TEMPLATE_TEMPLATE_POST_HOOK_COMMAND", Seq("")) + ) + ) } lazy val fileTemplate1: BoundPolicyDraft = { val id = PolicyId( @@ -1248,19 +1265,21 @@ class TestNodeConfiguration( ) } lazy val fileTemplateVariables2 = { - val spec = fileTemplateTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("FILE_TEMPLATE_RAW_OR_NOT").toVariable(Seq("Raw")), - spec("FILE_TEMPLATE_TEMPLATE").toVariable(Seq("")), - spec("FILE_TEMPLATE_RAW_TEMPLATE").toVariable(Seq("some content")), - spec("FILE_TEMPLATE_AGENT_DESTINATION_PATH").toVariable(Seq("/tmp/other-destination.txt")), - spec("FILE_TEMPLATE_TEMPLATE_TYPE").toVariable(Seq("mustache")), - spec("FILE_TEMPLATE_OWNER").toVariable(Seq("root")), - spec("FILE_TEMPLATE_GROUP_OWNER").toVariable(Seq("root")), - spec("FILE_TEMPLATE_PERMISSIONS").toVariable(Seq("777")), - spec("FILE_TEMPLATE_PERSISTENT_POST_HOOK").toVariable(Seq("true")), - spec("FILE_TEMPLATE_TEMPLATE_POST_HOOK_COMMAND").toVariable(Seq("/bin/true")) - ).map(v => (v.spec.name, v)).toMap + getVariablesFromSpec( + fileTemplateTechnique, + List( + ("FILE_TEMPLATE_RAW_OR_NOT", Seq("Raw")), + ("FILE_TEMPLATE_TEMPLATE", Seq("")), + ("FILE_TEMPLATE_RAW_TEMPLATE", Seq("some content")), + ("FILE_TEMPLATE_AGENT_DESTINATION_PATH", Seq("/tmp/other-destination.txt")), + ("FILE_TEMPLATE_TEMPLATE_TYPE", Seq("mustache")), + ("FILE_TEMPLATE_OWNER", Seq("root")), + ("FILE_TEMPLATE_GROUP_OWNER", Seq("root")), + ("FILE_TEMPLATE_PERMISSIONS", Seq("777")), + ("FILE_TEMPLATE_PERSISTENT_POST_HOOK", Seq("true")), + ("FILE_TEMPLATE_TEMPLATE_POST_HOOK_COMMAND", Seq("/bin/true")) + ) + ) } lazy val fileTemplate2: BoundPolicyDraft = { val id = PolicyId( @@ -1273,7 +1292,7 @@ class TestNodeConfiguration( ruleName = "60-rule-technique-std-lib", directiveName = "20-File template 2", technique = fileTemplateTechnique, - variableMap = fileTemplateVariables2.map(a => (ComponentId(a._1, Nil, None), a._2)), + variableMap = fileTemplateVariables2, system = false, policyMode = Some(PolicyMode.Enforce) ) @@ -1291,7 +1310,7 @@ class TestNodeConfiguration( ruleName = "99-rule-technique-std-lib", directiveName = "20-File template 2", technique = fileTemplateTechnique, - variableMap = fileTemplateVariables2.map(a => (ComponentId(a._1, Nil, None), a._2)), + variableMap = fileTemplateVariables2, system = false, policyMode = Some(PolicyMode.Enforce) ) @@ -1299,12 +1318,15 @@ class TestNodeConfiguration( val ncf1Technique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("Create_file"), TechniqueVersionHelper("1.0"))) val ncf1Variables = { - val spec = ncf1Technique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("expectedReportKey Directory create").toVariable(Seq("directory_create_/tmp/foo")), - spec("expectedReportKey File create").toVariable(Seq("file_create_/tmp/foo/bar")), - spec("1AAACD71-C2D5-482C-BCFF-5EEE6F8DA9C2").toVariable(Seq("\"foo")) - ).map(v => (v.spec.name, v)).toMap + getVariablesFromSpec( + ncf1Technique, + List( + ("expectedReportKey Directory create", Seq("directory_create_/tmp/foo")), + ("expectedReportKey File create", Seq("file_create_/tmp/foo/bar")), + ("1AAACD71-C2D5-482C-BCFF-5EEE6F8DA9C2", Seq("\"foo")) + // here we don't define 3021FC4F-DA33-4D84-8991-C42EBAB2335F to check that the default " " is well used + ) + ) } val ncf1: BoundPolicyDraft = { val id = PolicyId( @@ -1317,7 +1339,7 @@ class TestNodeConfiguration( ruleName = "50-rule-technique-ncf", directiveName = "Create a file", technique = ncf1Technique, - variableMap = ncf1Variables.map(a => (ComponentId(a._1, Nil, Some(s"reportId_${a._1}")), a._2)), + variableMap = ncf1Variables, system = false, policyMode = Some(PolicyMode.Enforce) ) @@ -1346,24 +1368,26 @@ class TestNodeConfiguration( lazy val copyGitFileTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("copyGitFile"), TechniqueVersionHelper("2.3"))) def copyGitFileVariable(i: Int) = { - val spec = copyGitFileTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("COPYFILE_NAME").toVariable(Seq("file_name_" + i + ".json")), - spec("COPYFILE_EXCLUDE_INCLUDE_OPTION").toVariable(Seq("none")), - spec("COPYFILE_EXCLUDE_INCLUDE").toVariable(Seq("")), - spec("COPYFILE_DESTINATION").toVariable(Seq("/tmp/destination_" + i + ".json")), - spec("COPYFILE_RECURSION").toVariable(Seq(s"${i % 2}")), - spec("COPYFILE_PURGE").toVariable(Seq("false")), - spec("COPYFILE_COMPARE_METHOD").toVariable(Seq("mtime")), - spec("COPYFILE_OWNER").toVariable(Seq("root")), - spec("COPYFILE_GROUP").toVariable(Seq("root")), - spec("COPYFILE_PERM").toVariable(Seq("644")), - spec("COPYFILE_SUID").toVariable(Seq("false")), - spec("COPYFILE_SGID").toVariable(Seq("false")), - spec("COPYFILE_STICKY_FOLDER").toVariable(Seq("false")), - spec("COPYFILE_POST_HOOK_RUN").toVariable(Seq("true")), - spec("COPYFILE_POST_HOOK_COMMAND").toVariable(Seq("/bin/echo Value_" + i + ".json")) - ).map(v => (v.spec.name, v)).toMap + getVariablesFromSpec( + copyGitFileTechnique, + List( + ("COPYFILE_NAME", Seq("file_name_" + i + ".json")), + ("COPYFILE_EXCLUDE_INCLUDE_OPTION", Seq("none")), + ("COPYFILE_EXCLUDE_INCLUDE", Seq("")), + ("COPYFILE_DESTINATION", Seq("/tmp/destination_" + i + ".json")), + ("COPYFILE_RECURSION", Seq(s"${i % 2}")), + ("COPYFILE_PURGE", Seq("false")), + ("COPYFILE_COMPARE_METHOD", Seq("mtime")), + ("COPYFILE_OWNER", Seq("root")), + ("COPYFILE_GROUP", Seq("root")), + ("COPYFILE_PERM", Seq("644")), + ("COPYFILE_SUID", Seq("false")), + ("COPYFILE_SGID", Seq("false")), + ("COPYFILE_STICKY_FOLDER", Seq("false")), + ("COPYFILE_POST_HOOK_RUN", Seq("true")), + ("COPYFILE_POST_HOOK_COMMAND", Seq("/bin/echo Value_" + i + ".json")) + ) + ) } def copyGitFileDirectives(i: Int): BoundPolicyDraft = { @@ -1377,7 +1401,7 @@ class TestNodeConfiguration( ruleName = "90-copy-git-file", directiveName = "Copy git file", technique = copyGitFileTechnique, - variableMap = copyGitFileVariable(i).map(a => (ComponentId(a._1, Nil, None), a._2)), + variableMap = copyGitFileVariable(i), system = false, policyMode = Some(PolicyMode.Enforce) ) @@ -1431,11 +1455,13 @@ class TestNodeConfiguration( lazy val gvdTechnique = techniqueRepository.unsafeGet(TechniqueId(TechniqueName("genericVariableDefinition"), TechniqueVersionHelper("2.0"))) lazy val gvdVariables1 = { - val spec = gvdTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("GENERIC_VARIABLE_NAME").toVariable(Seq("var1")), - spec("GENERIC_VARIABLE_CONTENT").toVariable(Seq("value from gvd #1 should be first")) // the one to override - ).map(v => (v.spec.name, v)).toMap + getVariablesFromSpec( + gvdTechnique, + List( + ("GENERIC_VARIABLE_NAME", Seq("var1")), + ("GENERIC_VARIABLE_CONTENT", Seq("value from gvd #1 should be first")) // the one to override + ) + ) } lazy val gvd1: BoundPolicyDraft = { val id = PolicyId(RuleId("rule1"), DirectiveId(DirectiveUid("directive1")), TechniqueVersionHelper("1.0")) @@ -1444,7 +1470,7 @@ class TestNodeConfiguration( ruleName = "10. Global configuration for all nodes", directiveName = "99. Generic Variable Def #1", technique = gvdTechnique, - variableMap = gvdVariables1.map(a => (ComponentId(a._1, Nil, None), a._2)), + variableMap = gvdVariables1, system = false, policyMode = Some(PolicyMode.Enforce) ).copy( @@ -1452,11 +1478,13 @@ class TestNodeConfiguration( ) } lazy val gvdVariables2 = { - val spec = gvdTechnique.getAllVariableSpecs.map(s => (s.name, s)).toMap - Seq( - spec("GENERIC_VARIABLE_NAME").toVariable(Seq("var1")), - spec("GENERIC_VARIABLE_CONTENT").toVariable(Seq("value from gvd #2 should be last")) // the one to use for override - ).map(v => (v.spec.name, v)).toMap + getVariablesFromSpec( + gvdTechnique, + List( + ("GENERIC_VARIABLE_NAME", Seq("var1")), + ("GENERIC_VARIABLE_CONTENT", Seq("value from gvd #2 should be last")) // the one to use for override + ) + ) } lazy val gvd2: BoundPolicyDraft = { val id = PolicyId(RuleId("rule1"), DirectiveId(DirectiveUid("directive2")), TechniqueVersionHelper("1.0")) @@ -1466,7 +1494,7 @@ class TestNodeConfiguration( directiveName = "00. Generic Variable Def #2", // sort name comes before sort name of directive 1 technique = gvdTechnique, - variableMap = gvdVariables2.map(a => (ComponentId(a._1, Nil, None), a._2)), + variableMap = gvdVariables2, system = false, policyMode = Some(PolicyMode.Enforce) ).copy( diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/TestBuildNodeConfiguration.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/TestBuildNodeConfiguration.scala index b540f9d51a7..ecaa50541fd 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/TestBuildNodeConfiguration.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/TestBuildNodeConfiguration.scala @@ -99,9 +99,9 @@ class TestBuildNodeConfiguration extends Specification { def newNode(i: Int): CoreNodeFact = fact1.modify(_.id).setTo(NodeId("node" + i)).modify(_.fqdn).setTo(s"node$i.localhost") - val allNodes: MapView[NodeId, CoreNodeFact] = ((1 to 1000).map(newNode) :+ factRoot).map(n => (n.id, n)).toMap.view + val allNodes: MapView[NodeId, CoreNodeFact] = ((1 to 100).map(newNode) :+ factRoot).map(n => (n.id, n)).toMap.view // only one group with all nodes - val group: NodeGroup = { + val group: NodeGroup = { NodeGroup( NodeGroupId(NodeGroupUid("allnodes")), name = "allnodes", @@ -113,7 +113,7 @@ class TestBuildNodeConfiguration extends Specification { _isEnabled = true ) } - val groupLib: FullNodeGroupCategory = FullNodeGroupCategory( + val groupLib: FullNodeGroupCategory = FullNodeGroupCategory( NodeGroupCategoryId("test_root"), "", "", @@ -128,9 +128,13 @@ class TestBuildNodeConfiguration extends Specification { ) ) ) - val directives: Map[DirectiveId, Directive] = - (1 to 1000).map(i => data.rpmDirective("rpm" + i, "somepkg" + i)).map(d => (d.id, d)).toMap - val directiveLib: FullActiveTechniqueCategory = FullActiveTechniqueCategory( + val directives: Map[DirectiveId, Directive] = { + ((1 to 100).map(i => data.rpmDirective("rpm" + i, "somepkg" + i)) :+ + // add a directive with a node property with a default value of " " - it should work, see https://issues.rudder.io/issues/25557 + data.rpmDirective("blank_default_node", """${node.properties[udp_open_ports] | default=" "}""")).map(d => (d.id, d)) + }.toMap + + val directiveLib: FullActiveTechniqueCategory = FullActiveTechniqueCategory( ActiveTechniqueCategoryId("root"), name = "root", description = "", @@ -194,26 +198,26 @@ class TestBuildNodeConfiguration extends Specification { logger.trace(s"init : ${System.currentTimeMillis - t0} ms") - for (i <- 0 until 10) { - logger.trace("\n--------------------------------") - val t1 = System.currentTimeMillis() - val ruleVal = - ruleValService.buildRuleVal(rule, directiveLib, groupLib, allNodes.mapValues(_.rudderSettings.isPolicyServer)) - val ruleVals = Seq(ruleVal.getOrElse(throw new RuntimeException("oups"))) - val t2 = System.currentTimeMillis() - val nodeContexts = buildContext - .getNodeContexts( - allNodes.keySet.toSet, - allNodes, - groupLib, - Nil, - data.globalAgentRun, - data.globalComplianceMode, - globalPolicyMode - ) - .getOrElse(throw new RuntimeException("oups")) - val t3 = System.currentTimeMillis() - BuildNodeConfiguration.buildNodeConfigurations( + logger.trace("\n--------------------------------") + val t1 = System.currentTimeMillis() + val ruleVal = + ruleValService.buildRuleVal(rule, directiveLib, groupLib, allNodes.mapValues(_.rudderSettings.isPolicyServer)) + val ruleVals = Seq(ruleVal.getOrElse(throw new RuntimeException("oups"))) + val t2 = System.currentTimeMillis() + val nodeContexts = buildContext + .getNodeContexts( + allNodes.keySet.toSet, + allNodes, + groupLib, + Nil, + data.globalAgentRun, + data.globalComplianceMode, + globalPolicyMode + ) + .getOrElse(throw new RuntimeException("oups")) + val t3 = System.currentTimeMillis() + val res = BuildNodeConfiguration + .buildNodeConfigurations( activeNodeIds, ruleVals, nodeContexts.ok, @@ -225,48 +229,13 @@ class TestBuildNodeConfiguration extends Specification { jsTimeout, generationContinueOnError ) - val t4 = System.currentTimeMillis() + .openOrThrowException(throw new RuntimeException(s"Error: node configuration failed")) + val t4 = System.currentTimeMillis() - logger.trace(s"ruleval: ${t2 - t1} ms") - logger.trace(s"context: ${t3 - t2} ms") - logger.trace(s"config : ${t4 - t3} ms") - } - true === true + logger.trace(s"ruleval: ${t2 - t1} ms") + logger.trace(s"context: ${t3 - t2} ms") + logger.trace(s"config : ${t4 - t3} ms") + + res.errors == Nil } } - -//object FOO { -// -// import zio._ -// import zio.syntax._ -// import com.normation.zio._ -// import com.normation.errors._ -// -// def main(args: Array[String]): Unit = { -// -// def nano = ZIO.succeed(System.nanoTime) -// def log(s: String, t1: Long, t2: Long) = ZIO.succeed(logger.trace(s + s"${(t2-t1)/1000} µs")) -// -// val count = 0 until 1 -// val prog = -// ZIO.foreachParN(8)(0 until 10) { j => -// for { -// ref <- Ref.make(0L) -// t1 <- nano -// loop <- ZIO.foreach(count) { i => // yes only one element -// for { -// t2 <- nano -// res <- i.succeed -// t3 <- nano -// _ <- ref.update(t => t + t3 - t2) -// } yield (i) -// } -// t4 <- nano -// _ <- log(s"external $j : ", t1, t4) -// _ <- ref.get.flatMap(t => ZIO.succeed(logger.trace(s"inner sum $j: ${t/1000} µs"))) -// } yield () -// } -// ZioRuntime.unsafeRun(prog) -// -// } -//} diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/write/PolicyAgregationTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/write/PolicyAgregationTest.scala index bcec8b035e5..1d58552eaed 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/write/PolicyAgregationTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/policies/write/PolicyAgregationTest.scala @@ -52,6 +52,7 @@ import com.normation.rudder.services.policies.ComponentId import com.normation.rudder.services.policies.MergePolicyService import com.normation.rudder.services.policies.NodeConfigData import com.normation.rudder.services.policies.PolicyId +import com.softwaremill.quicklens.* import org.joda.time.DateTime import org.junit.runner.RunWith import org.specs2.matcher.MatchResult @@ -142,10 +143,10 @@ class PolicyAgregationTest extends Specification { id, ruleName = "rule name", directiveName = "directive name", - technique, + technique.modify(_.rootSection.children).setTo(List(v.spec)), acceptationDate = DateTime.now, - expandedVars = Map(ComponentId(v.spec.name, Nil, None) -> v), - originalVars = Map(ComponentId(v.spec.name, Nil, None) -> v), + expandedVars = Map(ComponentId(v.spec.name, List("root"), None) -> v), + originalVars = Map(ComponentId(v.spec.name, List("root"), None) -> v), trackerVariable, priority = 5, isSystem = false, diff --git a/webapp/sources/rudder/rudder-rest/src/test/resources/api/api_directives.yml b/webapp/sources/rudder/rudder-rest/src/test/resources/api/api_directives.yml index 58163e33762..f4d81fe3b15 100644 --- a/webapp/sources/rudder/rudder-rest/src/test/resources/api/api_directives.yml +++ b/webapp/sources/rudder/rudder-rest/src/test/resources/api/api_directives.yml @@ -251,7 +251,7 @@ response: "longDescription":"", "techniqueName":"Create_file", "techniqueVersion":"1.0", - "parameters":{"section":{"name":"sections","sections":[{"section":{"name":"Directory create","vars":[{"var":{"name":"expectedReportKey Directory create","value":"directory_create_/tmp/foo"}}]}},{"section":{"name":"File create","vars":[{"var":{"name":"expectedReportKey File create","value":"file_create_/tmp/foo/bar"}}]}},{"section":{"name":"Technique parameters","vars":[{"var":{"name":"1AAACD71-C2D5-482C-BCFF-5EEE6F8DA9C2","value":"\"foo"}}]}}]}}, + "parameters":{"section":{"name":"sections","sections":[{"section":{"name":"Directory create","vars":[{"var":{"name":"expectedReportKey Directory create","value":"directory_create_/tmp/foo"}}]}},{"section":{"name":"File create","vars":[{"var":{"name":"expectedReportKey File create","value":"file_create_/tmp/foo/bar"}}]}},{"section":{"name":"Technique parameters","vars":[{"var":{"name":"1AAACD71-C2D5-482C-BCFF-5EEE6F8DA9C2","value":"\"foo"}},{"var":{"name":"3021FC4F-DA33-4D84-8991-C42EBAB2335F","value":" "}}]}}]}}, "priority":5, "enabled":false, "system":false, @@ -2015,6 +2015,12 @@ response: "name": "1AAACD71-C2D5-482C-BCFF-5EEE6F8DA9C2", "value": "\"foo" } + }, + { + "var": { + "name": "3021FC4F-DA33-4D84-8991-C42EBAB2335F", + "value":" " + } } ] }