Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #4444: don't duplicate allowusers - rudder-code part #457

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -122,11 +122,13 @@ class SystemVariableServiceImpl(
var varManagedNodes = systemVariableSpecService.get("MANAGED_NODES_NAME").toVariable()
var varManagedNodesId = systemVariableSpecService.get("MANAGED_NODES_ID").toVariable()
var varAllowedNetworks = systemVariableSpecService.get("AUTHORIZED_NETWORKS").toVariable()
var varManagedNodesAdmin = systemVariableSpecService.get("MANAGED_NODES_ADMIN").toVariable()

val allowConnect = collection.mutable.Set[String]()

val clientList = collection.mutable.Set[String]()


// If we are facing a policy server, we have to allow each child to connect, plus the policy parent,
// else it's only the policy server
if(nodeInfo.isPolicyServer) {
Expand Down Expand Up @@ -165,6 +167,23 @@ class SystemVariableServiceImpl(
val children = allNodeInfos.filter(_.policyServerId == nodeInfo.id).toSeq
varManagedNodes = varManagedNodes.copyWithSavedValues(children.map(_.hostname))
varManagedNodesId = varManagedNodesId.copyWithSavedValues(children.map(_.id.value))

val varNameForAdminUsers = "${rudder.hasPolicyServer-" + nodeInfo.id.value + ".target.admin}"
val allowedUserVarSpec = SystemVariableSpec(name = varNameForAdminUsers, description = "", multivalued = true)
val allowedUserVar = SystemVariable(allowedUserVarSpec, Seq()).copyWithSavedValues(Seq(varNameForAdminUsers))

parameterizedValueLookupService.lookupRuleParameterization(Seq(allowedUserVar), allNodeInfos, groupLib, directiveLib, allRules) match {
case Full(variable) =>
varManagedNodesAdmin = varManagedNodesAdmin.copyWithSavedValues(variable.flatMap(x => x.values).distinct)
case Empty =>
logger.warn(s"No variable parametrized found for ${varNameForAdminUsers}")
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 that log should be at warn level. What an user of Rudder can do about it ?
And if he is looking for something strange, it seems likely that he will be in debug mode.

Copy link
Member

Choose a reason for hiding this comment

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

Moreover, the spec file say that the variable may be empty, so I don't see any reason to have that info above debug level.

Copy link
Member Author

Choose a reason for hiding this comment

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

The variable may be empty, but it SHOULD be there
if it is not, then it means the rule for the policy server doesn't exist, and it is really bad.

Copy link
Member Author

Choose a reason for hiding this comment

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

hum, the lookupRuleParameterization can return either Full or Failure, so I guess this case is never used

Copy link
Member

Choose a reason for hiding this comment

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

OK, so could you please explain to the user what he can do to correct the problem, because here, he just get a warning message without any way to change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

unfortunately, user cannot do much about this. This is a system Rule and Group, so he cannot recreate it manually easily.

Copy link
Member

Choose a reason for hiding this comment

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

OK, so at least could you explain what he should do? Look for ? Debug it ? Because if I'm seing a WARN message, I want to check what is happening, and correct it. Here, I (as an user) can't just do anything but be stressed by the message.

Copy link
Member

Choose a reason for hiding this comment

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

And it's likelly that I won't even understand what the message is about, so it's just garbage for me (as an user)

case f: Failure =>
val e = f ?~! "Failure when fetching the list of administrators of managed nodes"
logger.error(e.messageChain)
return e
}


}

allNodeInfos.find( _.id == nodeInfo.policyServerId) match {
Expand Down Expand Up @@ -212,6 +231,7 @@ class SystemVariableServiceImpl(
, (varManagedNodesId.spec.name, varManagedNodesId)
, (varManagedNodes.spec.name, varManagedNodes)
, (varAllowedNetworks.spec.name, varAllowedNetworks)
, (varManagedNodesAdmin.spec.name, varManagedNodesAdmin)
, (varWebdavUser.spec.name, varWebdavUser)
, (varWebdavPassword.spec.name, varWebdavPassword)
, (syslogPortConfig.spec.name, syslogPortConfig)
Expand Down