server: check old policy path for bundle ownership #4847
Conversation
Before, we'd only check if the NEW policy path was owned by a bundle. Now, we'll also check if the to-be-updated policy is owned by a bundle. If so, return an error. Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
| path: "/policies/someid", | ||
| body: `package other.path`, | ||
| code: http.StatusBadRequest, | ||
| resp: `{"code": "invalid_parameter", "message": "path x/y/z is owned by bundle \"test-bundle\""}`, |
There was a problem hiding this comment.
So, there is a policy with id someid, it contains the package x.y.z, and we're attempting to replace this policy with one containing only the other.path package; effectively removing the x.y.z package. Am I understanding this correctly?
It might not be immediately apparent for the user that they're affecting packages not present in the pushed policy. Is there a sensible way of refining this error message? Like explicitly saying that the action would modify/remove the package.
There was a problem hiding this comment.
If the new policy was to replace an existing policy not owned by a bundle, so that the new package other.path is strictly additive, this action would be allowed? If so, then perhaps the error should state that the someid policy is protected. And this error would be reserved for when an update of an unprotected policy would infringe on a protected policy with e.g. a package overlap.
There was a problem hiding this comment.
Let's revisit the error messages for the next release. I'd rather get this issue out of the way quick. 🌻
…#4847) Before, we'd only check if the NEW policy path was owned by a bundle. Now, we'll also check if the to-be-updated policy is owned by a bundle. If so, return an error. Fixes open-policy-agent#4846 Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
…#4847) Before, we'd only check if the NEW policy path was owned by a bundle. Now, we'll also check if the to-be-updated policy is owned by a bundle. If so, return an error. Fixes open-policy-agent#4846 Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com> (cherry picked from commit b2bf19f)
Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.
Fixes #4846.
The problematic call that succeeds in #4846 now properly yields