Skip to content
This repository has been archived by the owner on Feb 7, 2019. It is now read-only.

Fixes #10879: Adapt inventory processor so it can read agent certificate #110

Merged
merged 1 commit into from
Jun 9, 2017
Merged

Fixes #10879: Adapt inventory processor so it can read agent certificate #110

merged 1 commit into from
Jun 9, 2017

Conversation

VinceMacBuche
Copy link
Member

@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 99e8f5c to 044615a Compare June 9, 2017 10:11
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 044615a to 6502565 Compare June 9, 2017 10:34
VinceMacBuche
VinceMacBuche commented Jun 9, 2017
View reviewed changes
inventory-repository/src/main/scala/com/normation/inventory/ldap/core/InventoryMapper.scala Outdated
@@ -720,7 +711,7 @@ class InventoryMapper(
root.setOpt(server.inventoryDate, A_INVENTORY_DATE, { x: DateTime => GeneralizedTime(x).toString })
root.setOpt(server.receiveDate, A_RECEIVE_DATE, { x: DateTime => GeneralizedTime(x).toString })
root +=! (A_AGENTS_NAME, server.agents.map(x => x.toJsonString):_*)
root +=! (A_PKEYS, server.publicKeys.map(x => x.key):_*)
root +=! (A_PKEYS, server.agents.map(_.securityToken).collect{case p: PublicKey => p.key}:_*)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should remove that since we store in in agent now

@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 6502565 to 8025c49 Compare June 9, 2017 10:37
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 8025c49 to 18f2749 Compare June 9, 2017 12:42
fanf
fanf reviewed Jun 9, 2017
View reviewed changes
inventory-api/src/main/scala/com/normation/inventory/domain/AgentTypes.scala Outdated
override def toRulesPath() = "/dsc"
final case object Dsc extends AgentType with HashcodeCaching {
override def toString = A_DSC_AGENT
override def toRulesPath = "/dsc"
override val inventorySoftwareName = "Rudder agent"
override def toAgentVersionName(softwareVersionName: String) = softwareVersionName+" (dsc)"
}

object AgentType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to change the name of agent type as above, please put the declaration inside object AgentType so that the common way to address them is AgentType.Name.

Also, please use: CfeCommunity vs Community, and CfeEnterprise vs Nova.

inventory-api/src/main/scala/com/normation/inventory/domain/AgentTypes.scala Outdated
override def toString() = A_NOVA_AGENT
override def toRulesPath() = "/cfengine-nova"
final case object Nova extends AgentType with HashcodeCaching {
override def toString = A_NOVA_AGENT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure is it valid to override toString() with toString. Is it?

inventory-api/src/main/scala/com/normation/inventory/domain/DataTypes.scala
value
} else {
s"""-----BEGIN CERTIFICATE-----
|${value.grouped(80).mkString("\n")}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we really in PEM serialisation format? I think x509 is "-----BEGIN x509 CERTIFICATE-----" but that must be validated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that the format of the certificate we generate is PEM which is "-----BEGIN CERTIFICATE-----" and "-----END CERTIFICATE-----" for a x509 certificate

(https://en.wikipedia.org/wiki/X.509#Certificate_filename_extensions and read from Boundy Castle PEMParser )

or a SO answer here : https://stackoverflow.com/questions/3313020/write-x509-certificate-into-pem-formatted-string-in-java

...y-fusion/src/main/scala/com/normation/inventory/provisioning/fusion/RudderParsingRules.scala
}
}

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to remove that, but now that security token are really mandatory, we should check for them, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's checked with AgentsName now!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, does AgentName also check for the (now mandatory) security token ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you can look a few line below ! (Just added an error case on DSC which cannot be managed with it)

@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 18f2749 to 46619ec Compare June 9, 2017 14:02
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 46619ec to 70d954b Compare June 9, 2017 15:35
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 70d954b to 12965b6 Compare June 9, 2017 15:38
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the ust_10879/adapt_inventory_processor_so_it_can_read_agent_certificate branch from 12965b6 to 0f40d6a Compare June 9, 2017 15:43
@Normation-Quality-Assistant
Copy link

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 0f40d6a into Normation:prototype/windows-dsc Jun 9, 2017
ncharles
ncharles reviewed Jun 22, 2017
View reviewed changes
inventory-api/src/main/scala/com/normation/inventory/domain/AgentTypes.scala
case invalidJson =>
tokenDefault match {
case Some(default) => Full(PublicKey(default))
case None => Failure(s"Invalid value for security token, ${compactRender(invalidJson)}, and no public key were stored")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot compactRender an invalid json

ncharles
ncharles reviewed Jun 22, 2017
View reviewed changes
inventory-api/src/main/scala/com/normation/inventory/domain/AgentTypes.scala
}
token <- json \ "securityToken" match {
case JObject(json) => parseSecurityToken(agentType, json, optToken)
case _ => parseSecurityToken(agentType, JNothing, optToken)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since tokenJson \ "type" will be invalid in this case, this will always return Failure. And fail

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fanf fanf fanf left review comments

@ncharles ncharles ncharles left review comments

Assignees
No one assigned
Labels
Ready for merge
Projects
None yet
Milestone
No milestone
4 participants
@VinceMacBuche @Normation-Quality-Assistant @fanf @ncharles