Bug Report: Empty/truncated file considered valid when enforceTableACLConfig is set

[garfthoffman]

Overview of the Issue

When running vttablet with the flag --enforce-tableacl-config provided, it is possible to provide an empty or truncated file for the value of --table-acl-config which will successfully load an empty ACL config.

Reproduction Steps

Create an empty file, say ./empty-acl.json and run vttablet with the --table-acl-config ./empty-acl.json and --enforce-tableacl-config parameters.

Binary Version

vttablet `v18`

Operating System and Environment details

Ubuntu 20.04
Linux 5.15.0-125-generic
x86_64

Log Fragments

N/A

[arthurschreiber]

To give a bit more context: We have a tool that re-generates the ACL files used by vttablet when we start / restart vttablet processes.

This tool was not performing it's operations in an atomic fashion: the ACL file was opened and truncated, the ACL configuration was written, the file was closed (but critically, not fsynced), and then vttablet was started.

We've ran into one case where due to the missing fsync call, vttablet would start up and read the truncated tablet ACL file. In combination with the --queryserver-config-strict-table-acl flag, this lead to all query execution requests on this particular vttablet instance to fail.

Obviously, our tool was wrong (and has been fixed to perform these writes in a fully atomic fashion), but we were also surprised to learn that --enforce-tableacl-config prevents starting vttablet if the given ACL file does not exist, but does not prevent vttablet from starting if the given ACL file is 0 bytes long. This is because it's first attempted to be deserialized as protobuf, where a 0 byte input is treated as a noop.

@deepthi How do you feel about this change? 🙇‍♂️ Obviously, we can't prevent all misconfiguration cases, but this one might be something we want to prevent? 🤔