Fixes #14041: Replace all RudderUniqueID in paths for inputs list and rudder.json, and correctly generate paths based on agent type #2103
Conversation
… rudder.json, and correctly generate paths based on agent type
|
must be merged after Normation/ldap-inventory#143 |
There was a problem hiding this comment.
It seems that we are not doing the replace of tag at the correct place. It should be possible to have only one place where we build the destination (make unique, and replace tag in path), and use that to both build the correct bundle variables and the correct out path.
Moreover, can you please add a test in dsc agent (or point me to it if I missed ii in an other PR), because the change reach is not trivial and we must keep example of working metadata/templates and corresponding expected generated files.
| */ | ||
| private[this] def parseResource(techniqueId: TechniqueId, xml: Node, isTemplate:Boolean): (TechniqueResourceId, String) = { | ||
| private[this] def parseResource(techniqueId: TechniqueId, xml: Node, isTemplate:Boolean, agentType:Option[AgentType]): (TechniqueResourceId, String) = { |
There was a problem hiding this comment.
Here, it seems that the problem is that isTemplate is not sufficiently precise, but having both that parameter and agentType is redondant. Can you change (isTemplate, agentType) by templateForAgent:Option[AgentType]
So that we encode the whole information in just that parameter?
There was a problem hiding this comment.
i'm not sure about this one - what if we add a new agent, that will need isTemplate & agentType at the same time ?
| * Replace the Rudder Unique Id also in the path for input list | ||
| */ | ||
| def replaceRudderUniqueId(path: String, p: Policy): String = { | ||
| path.replace(TAG_OF_RUDDER_MULTI_POLICY, p.id.getRudderUniqueId) |
There was a problem hiding this comment.
it should be a replaceAll to avoid problems in a case where someone put several tag in the path for whatever reason
| @@ -378,6 +378,13 @@ final object Policy { | |||
| path.replaceFirst(subPath, s"${p.technique.id.name.value}/${p.id.getRudderUniqueId}") | |||
There was a problem hiding this comment.
Is their a case where we want to make a unique dest without replacing the tag? I think not (and it would avoid to walk the path several times)
There was a problem hiding this comment.
I don't know - that's why i didn't replaceAll in replaceRudderUniqueId - i was wondering if there was an unknown reason i was missing
|
Tests in DSC are here: https://github.com/Normation/rudder-agent-windows/pull/292 |
|
OK, I'm merging like that, but please open an other ticket for me to try to normalize things |
|
OK, merging this PR |
https://issues.rudder.io/issues/14041