Prevent resource registration from overwriting unknown properties at random points in the future. (#2176)

This commit is contained in:
CyrusNajmabadi 2018-11-07 20:24:16 -08:00 committed by GitHub
parent 4ae39b8bec
commit 08fc305b7f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 31 additions and 33 deletions

View file

@ -144,25 +144,24 @@ export function resolveProperties(
} }
// Otherwise, unmarshal the value, and store it on the resource object. // Otherwise, unmarshal the value, and store it on the resource object.
let resolve = resolvers[k]; const resolve = resolvers[k];
if (resolve === undefined) { if (resolve === undefined) {
let resolveValue: (v: any) => void; // engine returned a property that was not in our initial property-map. This can happen
let resolveIsKnown: (v: boolean) => void; // for outputs that were registered through direct calls to 'registerOutputs'. We do
// *not* want to do anything with these returned properties. First, the component
resolve = (v, isKnown) => { // resources that were calling 'registerOutputs' will have already assigned these fields
resolveValue(v); // directly on them themselves. Second, if we were to try to assign here we would have
resolveIsKnown(isKnown); // an incredibly bad race condition for two reasons:
}; //
// 1. This call to 'resolveProperties' happens asynchronously at some point far after
// If there is no property yet, zero initialize it. This ensures unexpected properties // the resource was constructed. So the user will have been able to observe the
// are still made available on the object. This isn't ideal, because any code running // initial value up until we get to this point.
// prior to the actual resource CRUD operation can't hang computations off of it, but //
// it's better than tossing it. // 2. The component resource will have often assigned a value of some arbitrary type
(res as any)[k] = new Output( // (say, a 'string'). If we overwrite this with an `Output<string>` we'll be changing
res, // the type at some non-deterministic point in the future.
debuggablePromise(new Promise<any>(r => resolveValue = r)), continue;
debuggablePromise(new Promise<boolean>(r => resolveIsKnown = r)));
} }
try { try {

View file

@ -290,22 +290,21 @@ async def resolve_outputs(res: 'Resource', props: 'Inputs', outputs: struct_pb2.
log.debug(f"looking for resolver using translated name {key}") log.debug(f"looking for resolver using translated name {key}")
resolve = resolvers.get(key) resolve = resolvers.get(key)
if resolve is None: if resolve is None:
# If there is no property yet, zero initialize it. This ensures unexpected properties # engine returned a property that was not in our initial property-map. This can happen
# are still made available on the object. This isn't ideal, because any code running # for outputs that were registered through direct calls to 'registerOutputs'. We do
# prior to the actual resource CRUD operation can't hang computations off of it, but # *not* want to do anything with these returned properties. First, the component
# it's better than tossing it. # resources that were calling 'registerOutputs' will have already assigned these fields
resolve_value = asyncio.Future() # directly on them themselves. Second, if we were to try to assign here we would have
resolve_known = asyncio.Future() # an incredibly bad race condition for two reasons:
#
def do_resolve(resolve_fut, known_fut, value, known): # 1. This call to 'resolveProperties' happens asynchronously at some point far after
resolve_fut.set_result(value) # the resource was constructed. So the user will have been able to observe the
known_fut.set_result(known) # initial value up until we get to this point.
#
# Note: the functools.partial is REQUIRED here. It's not safe to capture variables # 2. The component resource will have often assigned a value of some arbitrary type
# by reference in a loop. We use functools.partial here to capture the value of # (say, a 'string'). If we overwrite this with an `Output<string>` we'll be changing
# resolve_value and resolve_known at the point of invocation and not by reference. # the type at some non-deterministic point in the future.
resolve = functools.partial(do_resolve, resolve_value, resolve_known) continue
res.__setattr__(key, known_types.new_output({res}, resolve_value, resolve_known))
# If either we are performing a real deployment, or this is a stable property value, we # If either we are performing a real deployment, or this is a stable property value, we
# can propagate its final value. Otherwise, it must be undefined, since we don't know # can propagate its final value. Otherwise, it must be undefined, since we don't know