Doesn't a KeyFunc handle this already with meta.namespacekeyfunc

Sent from my iPhone

On Mar 19, 2015, at 7:42 PM, Prashanth B notifications@github.com wrote:

Currently its hard to surface a mismatch context in an update via generic etcd. If I do something like:

obj.Namespace = "valid"
update(api.WithNamespace(ctx, "invalid"), obj)
it will fail in the get with whatever go-etcd error, because the path /invalid/objname doesn't exist; and if i do:

obj.Namespace = "valid"
update(api.WithNamespace(ctx, "valid_different"), obj)
it will fail with a resource version mismatch (so NewConflict), because the get will return the object in /valid_different/objname, but its resource version is different.

Ideally both these cases should fail with a NewBadRequest. This would happen if the validation flow reaches BeforeUpdate, but it fails due to the mentioned checks earlier in the callstack. To catch both cases we probably need to refactor the namespace checking out of BeforeCreate and BeforeUpdate into a common location and call it from generic/etcd.update.

—
Reply to this email directly or view it on GitHub.

It doesn't take the namespace from the objectmeta, it just takes strings and the context (https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/registry/generic/etcd/etcd.go#L117).

So essentially it grabs the namespace from the ctx, the name from the object (via ObjectNameFunc) and constructs the path to the resource using the prefix specified in the constructor (eg: /registry/pods). In my second example, it would construct something like /registry/pods/valid_different/objname, and use that for the GET to retrieve the existing object.

I need it to barf saying the ctx.namespace != obj.Namespace upfront. This happens automatically later in the call graph but that code will never really execute because something else will give up.

Unless I misunderstood what you meant by meta.namespacekeyfunc, do you mean define one?

Sorry. Was referring to here:

https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/registry/generic/etcd/etcd.go#L119

Sent from my iPhone

On Mar 19, 2015, at 9:13 PM, Prashanth B notifications@github.com wrote:

It doesn't take the namespace from the objectmeta, it just takes strings and the context (https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/registry/generic/etcd/etcd.go#L117).

So essentially it grabs the namespace from the ctx, the name from the object (via ObjectNameFunc) and constructs the path to the resource using the prefix specified in the constructor (eg: /registry/pods). In my second example, it would construct something like /registry/pods/valid_different/objname, and use that for the GET to retrieve the existing object.

I need it to barf saying the ctx.namespace != obj.Namespace upfront. This happens automatically later in the call graph but that code will never really execute because something else will give up.

Unless I misunderstood what you meant by meta.namespacekeyfunc, do you mean define one?

—
Reply to this email directly or view it on GitHub.

Ah, yes that's the ns on the context, i'd like to compare that against the ns on the objectmeta of the update payload.

Seems like we could do something in validation.go in the object meta update possibly.

Sent from my iPhone

On Mar 20, 2015, at 12:35 AM, Prashanth B notifications@github.com wrote:

Ah, yes that's the ns on the context, i'd like to compare that against the ns on the objectmeta of the update payload.

—
Reply to this email directly or view it on GitHub.

Yeah, BeforeUpdate (https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/registry/generic/etcd/etcd.go#L288) would be the perfect spot, but it won't get there. It'll fail before in 1 of 2 ways:

1. The object under the bad ns doesn't exist: it'll fail with whatever error go-etcd returns (https://github.com/GoogleCloudPlatform/kubernetes/blob/1291401c2ec48be26cf28a15778d5b3c2248d3ea/pkg/tools/etcd_helper.go#L163)
2. The object under the bad ns exists, we'll get it and compare resource versions with the update payload, and complain about bad rv (https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/registry/generic/etcd/etcd.go#L284).

I would expect a BadRequest instead that complains about the ns mismatch. 2 is easily fixable, as I can just re-order those 2 checks and force BeforeUpdate to happen first, but for 1 I'll need to parse out the ns from the objectmeta before the Get. Currently etcd/generic tries to stay hermetic and only access things on api objects via pre-defined functions/a strategy, so I'm thinking something along those lines.