veyron2,runtimes/google/rt: Resolve todo and remove other stale todos.
Change-Id: I6f507a4ffa21bbfe9dfed300f51fdea8fd7245eb
diff --git a/runtimes/google/rt/runtime.go b/runtimes/google/rt/runtime.go
index 8820293..3614268 100644
--- a/runtimes/google/rt/runtime.go
+++ b/runtimes/google/rt/runtime.go
@@ -21,6 +21,7 @@
"v.io/core/veyron2/vtrace"
"v.io/core/veyron/lib/flags"
+ "v.io/core/veyron/lib/stats"
_ "v.io/core/veyron/lib/stats/sysstats"
iipc "v.io/core/veyron/runtimes/google/ipc"
imanager "v.io/core/veyron/runtimes/google/ipc/stream/manager"
@@ -60,7 +61,6 @@
opts []ipc.ServerOpt
}
-// TODO(mattr,suharshs): Decide if Options would be better than this.
func Init(ctx *context.T, appCycle veyron2.AppCycle, protocols []string, listenSpec *ipc.ListenSpec, flags flags.RuntimeFlags,
reservedDispatcher ipc.Dispatcher, dispatcherOpts ...ipc.ServerOpt) (*Runtime, *context.T, veyron2.Shutdown, error) {
r := &Runtime{deps: dependency.NewGraph()}
@@ -139,7 +139,7 @@
if err != nil {
return nil, nil, nil, err
}
- ctx = context.WithValue(ctx, principalKey, principal)
+ ctx = r.setPrincipal(ctx, principal)
// Set up secure client.
ctx, _, err = r.SetNewClient(ctx)
@@ -149,9 +149,6 @@
ctx = r.SetBackgroundContext(ctx)
- // TODO(suharshs,mattr): Go through the rt.Cleanup function and make sure everything
- // gets cleaned up.
-
return r, ctx, r.shutdown, nil
}
@@ -281,16 +278,20 @@
return cl
}
+func (*Runtime) setPrincipal(ctx *context.T, principal security.Principal) *context.T {
+ // We uniquely identity a principal with "security/principal/<publicKey>"
+ principalName := "security/principal/" + principal.PublicKey().String()
+ stats.NewStringFunc(principalName+"/blessingstore", principal.BlessingStore().DebugString)
+ stats.NewStringFunc(principalName+"/blessingroots", principal.Roots().DebugString)
+ return context.WithValue(ctx, principalKey, principal)
+}
+
func (r *Runtime) SetPrincipal(ctx *context.T, principal security.Principal) (*context.T, error) {
var err error
newctx := ctx
- newctx = context.WithValue(newctx, principalKey, principal)
+ newctx = r.setPrincipal(ctx, principal)
- // TODO(mattr, suharshs): The stream manager holds a cache of vifs
- // which were negotiated with the principal, so we replace it here when the
- // principal changes. However we should negotiate the vif with a
- // random principal and then we needn't replace this here.
if newctx, _, err = r.setNewStreamManager(newctx); err != nil {
return ctx, err
}
@@ -312,10 +313,6 @@
func (r *Runtime) SetNewClient(ctx *context.T, opts ...ipc.ClientOpt) (*context.T, ipc.Client, error) {
otherOpts := append([]ipc.ClientOpt{}, opts...)
- // TODO(mattr, suharshs): Currently there are a lot of things that can come in as opts.
- // Some of them will be removed as opts and simply be pulled from the context instead
- // these are:
- // stream.Manager, Namespace, LocalPrincipal, preferred protocols.
sm, _ := ctx.Value(streamManagerKey).(stream.Manager)
ns, _ := ctx.Value(namespaceKey).(naming.Namespace)
p, _ := ctx.Value(principalKey).(security.Principal)
diff --git a/runtimes/google/rt/security.go b/runtimes/google/rt/security.go
index b4a3aef..f06f9ed 100644
--- a/runtimes/google/rt/security.go
+++ b/runtimes/google/rt/security.go
@@ -13,7 +13,6 @@
"v.io/core/veyron2/security"
"v.io/core/veyron/lib/exec"
- "v.io/core/veyron/lib/stats"
vsecurity "v.io/core/veyron/security"
"v.io/core/veyron/security/agent"
)
@@ -24,10 +23,6 @@
return nil, err
}
- // TODO(suharshs,mattr): Move this code to SetNewPrincipal and determine what their string should be.
- stats.NewString("security/principal/key").Set(principal.PublicKey().String())
- stats.NewStringFunc("security/principal/blessingstore", principal.BlessingStore().DebugString)
- stats.NewStringFunc("security/principal/blessingroots", principal.Roots().DebugString)
return principal, nil
}
diff --git a/tools/mgmt/test.sh b/tools/mgmt/test.sh
index 6441eb7..0cf9890 100755
--- a/tools/mgmt/test.sh
+++ b/tools/mgmt/test.sh
@@ -156,8 +156,8 @@
fi
# Verify the device's default blessing is as expected.
- shell_test::assert_eq "$("${DEBUG_BIN}" stats read "${DM_NAME}/__debug/stats/security/principal/blessingstore" | head -1 | sed -e 's/^.*Default blessings: '//)" \
- "alice/myworkstation" "${LINENO}"
+ shell_test::assert_contains "$("${DEBUG_BIN}" stats read "${DM_NAME}/__debug/stats/security/principal/*/blessingstore" | head -1)" \
+ "Default blessings: alice/myworkstation" "${LINENO}"
# Get the device's profile.
local -r DEVICE_PROFILE=$("${DEVICE_BIN}" describe "${DM_NAME}/device" | sed -e 's/{Profiles:map\[\(.*\):{}]}/\1/')
@@ -212,8 +212,8 @@
# TODO(rjkroege): Verify that the app is actually running as ${SUID_USER}
# Verify the app's default blessing.
- shell_test::assert_eq "$("${DEBUG_BIN}" stats read "${INSTANCE_NAME}/stats/security/principal/blessingstore" | head -1 | sed -e 's/^.*Default blessings: '//)" \
- "alice/myapp/BINARYD" "${LINENO}"
+ shell_test::assert_contains "$("${DEBUG_BIN}" stats read "${INSTANCE_NAME}/stats/security/principal/*/blessingstore" | head -1)" \
+ "Default blessings: alice/myapp/BINARYD" "${LINENO}"
# Stop the instance.
"${DEVICE_BIN}" stop "${INSTANCE_NAME}"
