Code review comment for ~larsks/cloud-init:lp/1644064

Revision history for this message
Scott Moser (smoser) wrote :

This looks good, but there is one issue, in that you "fixed" /etc/ssh/sshd_config's permissions
by changing the behavior of write_file(filename, "foo").

That means all callers that do not pass 'mode' will now have (possibly) changed behavior.

The other issue here is that there is no way for the caller to say "use existing, but if new, use 0o755".

we could change the signature to include a 'reuse_existing_mode' (with open bikeshedding for a better name) or some other way indicate that the perms on the existing file should be used if it exists.

Lastly... atomic_helper.write_file currently has the same signature as util.write_file and i'd like to keep that, do lets plan on updating it as well.

review: Needs Fixing

« Back to merge proposal