motdgen, issuegen: write staged changes to a tempfile

[rfairley]

For the same reason as in #39
(copying the phrasing below):

With the staged file shared, there would be potential for two or
more processes executing motdgen to write to it resulting in
corrupted output, or the error in the cat command due to missing
file reported in #35 (comment). Currently, this is not a problem with
motdgen, but could be if motdgen were invoked by something like the
udev rules that invoke issuegen.

Instead, write the intermediate output to a variable before writing
to the final output file. This ensures only valid output is written
to the issue file shown to the terminal.

Additionally, perform code tidyups similar to those done for
issuegen in #40.

[cgwalters]

It's too bad bash doesn't expose O_TMPFILE, it's exactly what we want here.

But still though aren't there going to be races since we're still truncating and replacing files that the main issuegen process will re-read?

In other words don't we want to do something like "write to a file ending in .tmp, rename() into place", and ensure that issuegen ignores files ending in ".tmp" or so? (Which sadly we still need to do even with O_TMPFILE, see also https://lore.kernel.org/linux-fsdevel/[email protected]/ )

[rfairley]

But still though aren't there going to be races since we're still truncating and replacing files that the main issuegen process will re-read?

I think you're right - it's still possible for the writes to the intermediate snippets going into issuegen/motdgen to overstep each other. I'll take a look at mktemp for this - having different .abcdef.tmp files for each process before renaming would fix this. Then mv to the main issuegen file can be done by processes simultaneously.

Also having doubts with the current change in this PR - redirection echo "${generated_string}" > "${generated_file}" between two or more processes may interleave in execution. Will check if this is a problem. If generalizing the tmpfiles logic above though, it'd be easy to replace this redirection with a similar mktemp and mv.

[rfairley]

Updated to use mktemp and mv for file writes, and did the same for issuegen while at it. Interdiff from previous changes: https://github.com/coreos/console-login-helper-messages/compare/6afc0a4b0ee634e9507a90fa3be01c86d2159160..8fedbd386d8e456e0ec2e28d4b55f3600ca0a2d5.

Working on a make target to allow more easily testing this - should be up soon.

[rfairley]

Now ready for review!

The Makefile and sync_to_vm.sh script in #57 can be used to test, if anyone would like to test the changes locally.

[sohankunkerkar]

LGTM
I have also test run these changes locally using https://github.com/coreos/console-login-helper-messages/blob/master/README-devel.md#testing-changes-in-a-virtual-machine and #44 (comment)

[cgwalters]

(somewhat superficial since I don't know this code) LGTM

[dustymabe]

LGTM

[dustymabe]

I found #64 while looking into this change, but this particular change doesn't help or hurt that issue. I say let's go with it.