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 #26243: Improve license errors handling in webapp #6138

Open
wants to merge 6 commits into
base: branches/rudder/8.3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ object RudderPackagePlugin {
trait RudderPackageService {
import RudderPackageService.*

def update(): IOResult[Option[CredentialError]]
def update(): IOResult[Option[PluginSettingsError]]

def listAllPlugins(): IOResult[Chunk[RudderPackagePlugin]]

Expand All @@ -667,18 +667,27 @@ trait RudderPackageService {

object RudderPackageService {

val ERROR_CODE: Int = 1

// see PluginSettings : the url and credentials configuration could cause errors :
final case class CredentialError(msg: String) extends RudderError

object CredentialError {
private val regex = "^.*ERROR.* (Received an HTTP 401 Unauthorized error.*)$".r

def fromResult(cmdResult: CmdResult): Option[CredentialError] = {
(cmdResult.code, cmdResult.stderr.strip) match { // do not forget to strip stderr
case (ERROR_CODE, regex(err)) => Some(CredentialError(err))
case _ => None
// PluginSettings configuration could cause errors with known codes and colored stderr in rudder-package
sealed abstract class PluginSettingsError extends RudderError
object PluginSettingsError {
final case class InvalidCredentials(override val msg: String) extends PluginSettingsError
final case class Unauthorized(override val msg: String) extends PluginSettingsError

private val colorRegex = "\u001b\\[[0-9;]*m".r
private val uncolor = (str: String) => colorRegex.replaceAllIn(str, "")

private val sanitizeCmdResult = (res: CmdResult) => res.transform(uncolor.compose(_.strip))

/**
* Maps known errors codes from rudder package and adapt the error message to have no color
*/
def fromResult(cmdResult: CmdResult): PureResult[Option[PluginSettingsError]] = {
val result = sanitizeCmdResult(cmdResult)
result.code match {
case 0 => Right(None)
case 2 => Right(Some(PluginSettingsError.InvalidCredentials(result.stderr)))
case 3 => Right(Some(PluginSettingsError.Unauthorized(result.stderr)))
case _ => Left(Inconsistency(result.debugString()))
}
}
}
Expand All @@ -695,17 +704,16 @@ class RudderPackageCmdService(configCmdLine: String) extends RudderPackageServic
case h :: tail => Right((h, tail))
}

override def update(): IOResult[Option[CredentialError]] = {
override def update(): IOResult[Option[PluginSettingsError]] = {
// In case of error we need to check the result
for {
res <- runCmd("update" :: Nil)
(cmd, result) = res
err = CredentialError.fromResult(result)
_ <- ZIO.when(result.code != 0 && err.isEmpty) {
Inconsistency(
s"An error occurred while updating plugins list with '${cmd.display}':\n code: ${result.code}\n stderr: ${result.stderr}\n stdout: ${result.stdout}"
).fail
}
err <-
PluginSettingsError
.fromResult(result)
.toIO
.chainError(s"An error occurred while updating plugins list with '${cmd.display}'")
} yield {
err
}
Expand Down Expand Up @@ -762,10 +770,7 @@ class RudderPackageCmdService(configCmdLine: String) extends RudderPackageServic
}
private def runCmdOrFail(params: List[String])(errorMsg: String): IOResult[CmdResult] = {
runCmd(params).reject {
case (cmd, result) if result.code != 0 =>
Inconsistency(
s"${errorMsg} with '${cmd.display}':\n code: ${result.code}\n stderr: ${result.stderr}\n stdout: ${result.stdout}"
)
case (cmd, result) if result.code != 0 => Inconsistency(s"${errorMsg} with '${cmd.display}': ${result.debugString()}")
}.map(_._2)
}
}
Expand All @@ -775,7 +780,7 @@ class RudderPackageCmdService(configCmdLine: String) extends RudderPackageServic
*/
trait PluginSystemService {

def updateIndex(): IOResult[Option[CredentialError]]
def updateIndex(): IOResult[Option[PluginSettingsError]]
def list(): IOResult[Chunk[JsonPluginSystemDetails]]
def install(plugins: Chunk[PluginId]): IOResult[Unit]
def remove(plugins: Chunk[PluginId]): IOResult[Unit]
Expand All @@ -787,7 +792,7 @@ trait PluginSystemService {
* Implementation for tests, will do any operation without any error
*/
class InMemoryPluginSystemService(ref: Ref[Map[PluginId, JsonPluginSystemDetails]]) extends PluginSystemService {
override def updateIndex(): IOResult[Option[CredentialError]] = {
override def updateIndex(): IOResult[Option[PluginSettingsError]] = {
ZIO.none
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,24 @@ import zio.syntax.*
final case class Cmd(cmdPath: String, parameters: List[String], environment: Map[String, String], cwdPath: Option[String]) {
def display: String = s"${cmdPath} ${parameters.mkString(" ")}"
}
final case class CmdResult(code: Int, stdout: String, stderr: String)
final case class CmdResult(code: Int, stdout: String, stderr: String) {

/**
* Display the attributes of this result, by default each on a new line with indentation
*/
def debugString(sep: String = "\n "): String = s"code: ${code}\n stderr: ${stderr}\n stdout: ${stdout}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will apply this in places where it should


/**
* Strips the stdout and stderr of the result, may be useful when matching with a regex or for display purpose
*/
def strip: CmdResult = transform(_.strip)

/**
* Apply some transformation (e.g. sanitizing on stdout and stderr)
*/
def transform(f: String => String): CmdResult = copy(stdout = f(stdout), stderr = f(stderr))

}

object RunNuCommand {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
*************************************************************************************
*/
package com.normation.plugins

import com.normation.errors.*
import com.normation.rudder.hooks.CmdResult
import org.junit.runner.RunWith
import org.specs2.mutable.Specification
Expand All @@ -44,19 +46,29 @@ import org.specs2.runner.JUnitRunner
class TestRudderPackageService extends Specification {

"RudderPackageService" should {
"handle credentials error" in {
"handle zero error code and message" in {
RudderPackageService.PluginSettingsError.fromResult(CmdResult(0, "", "OK")) must beRight(beNone)
}
"handle non-zero error code and message" in {
val res = CmdResult(
1,
2,
"",
"ERROR Received an HTTP 401 Unauthorized error when trying to get 8.2/rpkg.index. Please check your credentials in the configuration."
"\u001b[31mERROR\u001b[0m Invalid credentials, please check your credentials in the configuration. (received HTTP 401)\n"
)
RudderPackageService.CredentialError.fromResult(res).aka("CredentialError from cmd result") must beSome(
beEqualTo(
RudderPackageService.CredentialError(
"Received an HTTP 401 Unauthorized error when trying to get 8.2/rpkg.index. Please check your credentials in the configuration."
RudderPackageService.PluginSettingsError.fromResult(res).aka("PluginSettingsError from cmd result") must beRight(
beSome(
beEqualTo(
RudderPackageService.PluginSettingsError.InvalidCredentials(
"ERROR Invalid credentials, please check your credentials in the configuration. (received HTTP 401)"
)
)
)
)
}
"handle unknown error code and message" in {
RudderPackageService.PluginSettingsError.fromResult(
CmdResult(12345, "", "\u001b[31mERROR\u001b[0m Unknown error")
) must beLeft(beLike[RudderError] { case err: Inconsistency => err.msg must contain("ERROR Unknown error") })
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,19 @@ object RudderJsonResponse {
def fromSchema(schema: EndpointSchema): ResponseSchema = ResponseSchema(schema.name, schema.dataContainer)
}

sealed trait ResponseError
sealed trait ResponseError {
def errorMsg: Option[String]
def toLiftErrorResponse(id: Option[String], schema: ResponseSchema)(implicit
prettify: Boolean
): LiftResponse = this match {
case UnauthorizedError(errorMsg) => unauthorizedError(id, schema, errorMsg)
case ForbiddenError(errorMsg) => forbiddenError(id, schema, errorMsg)
case NotFoundError(errorMsg) => notFoundError(id, schema, errorMsg)
}
}
final case class UnauthorizedError(errorMsg: Option[String]) extends ResponseError
final case class ForbiddenError(errorMsg: Option[String]) extends ResponseError
final case class NotFoundError(errorMsg: Option[String]) extends ResponseError

//////////////////////////// utility methods to build responses ////////////////////////////

Expand Down Expand Up @@ -224,12 +235,12 @@ object RudderJsonResponse {
): LiftJsonResponse[JsonRudderApiResponse[Unit]] = {
generic.unauthorizedError(JsonRudderApiResponse.error(id, schema, errorMsg))
}
def notFoundError(id: Option[String], schema: ResponseSchema, errorMsg: String)(implicit
def notFoundError(id: Option[String], schema: ResponseSchema, errorMsg: Option[String])(implicit
prettify: Boolean
): LiftJsonResponse[JsonRudderApiResponse[Unit]] = {
generic.notFoundError(JsonRudderApiResponse.error(id, schema, errorMsg))
}
def forbiddenError(id: Option[String], schema: ResponseSchema, errorMsg: String)(implicit
def forbiddenError(id: Option[String], schema: ResponseSchema, errorMsg: Option[String])(implicit
prettify: Boolean
): LiftJsonResponse[JsonRudderApiResponse[Unit]] = {
generic.forbiddenError(JsonRudderApiResponse.error(id, schema, errorMsg))
Expand Down Expand Up @@ -332,12 +343,8 @@ object RudderJsonResponse {
},
either => {
ev.apply(either) match {
case Left(e) =>
e match {
case UnauthorizedError(errorMsg) => unauthorizedError(id.error, schema, errorMsg)
}
case Right(b) =>
successOne[B](schema, b, id.success(either))
case Left(e) => e.toLiftErrorResponse(id.error, schema)
case Right(b) => successOne[B](schema, b, id.success(either))
}
}
)
Expand All @@ -354,8 +361,7 @@ object RudderJsonResponse {
toLiftResponseOneEither(params, ResponseSchema.fromSchema(schema), SuccessIdTrace(id))(JsonEncoder[B], ev)
}

// create a response from specific API errors modeled as an Either[ResponseError, Any] (the "zero" is for : "there is nothing on the right")
private def toLiftResponseZeroEither(
def toLiftResponseZeroEither(
params: DefaultParams,
schema: ResponseSchema,
id: IdTrace
Expand All @@ -369,12 +375,8 @@ object RudderJsonResponse {
},
either => {
ev.apply(either) match {
case Left(e) =>
e match {
case UnauthorizedError(errorMsg) => unauthorizedError(id.error, schema, errorMsg)
}
case Right(_) =>
successZero(schema)
case Left(e) => e.toLiftErrorResponse(id.error, schema)
case Right(_) => successZero(schema)
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import com.normation.plugins.PluginId
import com.normation.plugins.PluginSettings
import com.normation.plugins.PluginSystemService
import com.normation.plugins.PluginSystemStatus
import com.normation.plugins.RudderPackageService.PluginSettingsError
import com.normation.rudder.api.ApiVersion
import com.normation.rudder.domain.logger.ApplicationLoggerPure
import com.normation.rudder.rest.ApiModuleProvider
Expand Down Expand Up @@ -87,7 +88,11 @@ class PluginInternalApi(
.updateIndex()
.chainError("Could not update plugins index")
.tapError(err => ApplicationLoggerPure.Plugin.error(err.fullMsg))
.map(_.map(err => RudderJsonResponse.UnauthorizedError(Some(err.msg))).toLeft(()))
.map(_.map {
// the corresponding HTTP errors are the following ones
case PluginSettingsError.InvalidCredentials(msg) => RudderJsonResponse.UnauthorizedError(Some(msg))
case PluginSettingsError.Unauthorized(msg) => RudderJsonResponse.ForbiddenError(Some(msg))
}.toLeft(()))
.toLiftResponseZeroEither(params, schema, None)
}

Expand Down
Loading