[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r195050604
##
File path: core/controller/src/main/scala/whisk/core/controller/Actions.scala
##
@@ -187,7 +187,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with
PostActionActivation with
override def create(user: Identity, entityName:
FullyQualifiedEntityName)(implicit transid: TransactionId) = {
parameter('overwrite ? false) { overwrite =>
entity(as[WhiskActionPut]) { content =>
-val request = content.resolve(user.namespace)
+val request = content.resolve(user.namespace.name)
Review comment:
Addressed in latest commit
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r195042158
##
File path: common/scala/src/main/scala/whisk/core/entity/Identity.scala
##
@@ -38,13 +37,17 @@ object UserLimits extends DefaultJsonProtocol {
implicit val serdes = jsonFormat3(UserLimits.apply)
}
+protected[core] case class Namespace(name: EntityName, uuid: UUID)
+
+protected[core] object Namespace extends DefaultJsonProtocol {
+ implicit val serdes = jsonFormat2(Namespace.apply)
+}
+
protected[core] case class Identity(subject: Subject,
-namespace: EntityName,
+namespace: Namespace,
authkey: AuthKey,
rights: Set[Privilege],
-limits: UserLimits = UserLimits()) {
- def uuid = authkey.uuid
Review comment:
I was asked by Markus to remove that fuction, to make more explicit that the
uuid is retrieved from the namespace.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r195042158
##
File path: common/scala/src/main/scala/whisk/core/entity/Identity.scala
##
@@ -38,13 +37,17 @@ object UserLimits extends DefaultJsonProtocol {
implicit val serdes = jsonFormat3(UserLimits.apply)
}
+protected[core] case class Namespace(name: EntityName, uuid: UUID)
+
+protected[core] object Namespace extends DefaultJsonProtocol {
+ implicit val serdes = jsonFormat2(Namespace.apply)
+}
+
protected[core] case class Identity(subject: Subject,
-namespace: EntityName,
+namespace: Namespace,
authkey: AuthKey,
rights: Set[Privilege],
-limits: UserLimits = UserLimits()) {
- def uuid = authkey.uuid
Review comment:
I was asked by Markus to remove that function, to make more explicit that
the uuid is retrieved from the namespace.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r194765182
##
File path: tests/src/test/scala/whisk/core/limits/ActionLimitsTests.scala
##
@@ -245,7 +245,7 @@ class ActionLimitsTests extends TestHelpers with
WskTestHelpers {
val allowedSize = ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT.toBytes
// Needs some bytes grace since activation message is not only the payload.
-val args = Map("p" -> ("a" * (allowedSize - 700).toInt).toJson)
+val args = Map("p" -> ("a" * (allowedSize - 750).toInt).toJson)
Review comment:
Not really. Serializing the uuid as an additional part of the namespace has
added some crucial bytes to the message so that the "grace bytes" were not
enough to get the message passed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r194765182
##
File path: tests/src/test/scala/whisk/core/limits/ActionLimitsTests.scala
##
@@ -245,7 +245,7 @@ class ActionLimitsTests extends TestHelpers with
WskTestHelpers {
val allowedSize = ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT.toBytes
// Needs some bytes grace since activation message is not only the payload.
-val args = Map("p" -> ("a" * (allowedSize - 700).toInt).toJson)
+val args = Map("p" -> ("a" * (allowedSize - 750).toInt).toJson)
Review comment:
Not really. Serializing the uuid as an additional part of the namespace has
added some crucial bytes to the message so that the "grace bytes" where not
enough to get the message passed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r194703110
##
File path: common/scala/src/main/scala/whisk/core/entity/Identity.scala
##
@@ -38,13 +37,17 @@ object UserLimits extends DefaultJsonProtocol {
implicit val serdes = jsonFormat3(UserLimits.apply)
}
+protected[core] case class Namespace(name: EntityName, uuid: UUID)
Review comment:
please see my comment above
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mhenke1 commented on a change in pull request #3752: Move uuid into namespace to decouple it from authkey
mhenke1 commented on a change in pull request #3752: Move uuid into namespace
to decouple it from authkey
URL:
https://github.com/apache/incubator-openwhisk/pull/3752#discussion_r194703021
##
File path: common/scala/src/main/scala/whisk/core/entity/Identity.scala
##
@@ -38,13 +37,17 @@ object UserLimits extends DefaultJsonProtocol {
implicit val serdes = jsonFormat3(UserLimits.apply)
}
+protected[core] case class Namespace(name: EntityName, uuid: UUID)
+
+protected[core] object Namespace extends DefaultJsonProtocol {
+ implicit val serdes = jsonFormat2(Namespace.apply)
+}
+
protected[core] case class Identity(subject: Subject,
-namespace: EntityName,
+namespace: Namespace,
Review comment:
The PR has to two motivations.
The first one is to make the (mis-)usage of the uuid as unique namespace
alias explicit.
The change makes unrelated parts of the system (like for example
authentication and user metrics) also unrelated on the code level. This could
of course be done by providing new access functions to the Identity object.
My second - not fully outspoken - motivation for the change is to pave the
path
to make the authentication extensible and/or exchangeable. For both cases it
is imaginable
the another authentication implementation might not depend (fully) on the
subject db but (additional) on an external identity and access management
system that will not provide a uuid.
I plan to discuss a proposal for extending authentication and authorization
on the dev list in the very near future. Since I saw this change more as driven
by hygienic reasons (see my first motivation) I put it out separately.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
