[WIP] issuegen: detect util-linux RPM version, set output directory appropriately #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
2 times, most recently
from
23a8497 to
c2eb4b8
Compare
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Apr 20, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
from
c2eb4b8 to
2b5da38
Compare
|
will review this one more thoroughly when we get #39 merged and this is rebased |
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
from
2b5da38 to
3f4e49a
Compare
rfairley
commented
Apr 20, 2020
| ;; | ||
| esac | ||
|
|
||
| # TODO: it would be nice to have /run/issue.d be an official directory, | ||
| # see https://github.com/karelzak/util-linux/commit/1fc82a1360305f696dc1be6105c9c56a9ea03f52#commitcomment-27949895 | ||
| # until then, $GENERATED_ISSUE writes to the privately scoped directory (not in /run/issue.d) |
There was a problem hiding this comment.
In particular, #7 is tracking this comment - no longer need to mention in the source.
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
from
3f4e49a to
81458f2
Compare
rfairley
commented
Apr 20, 2020
| ISSUE_SNIPPETS_PATH=${PKG_NAME}/issue.d | ||
| ETC_SNIPPETS="/etc/${ISSUE_SNIPPETS_PATH}" | ||
| RUN_SNIPPETS="/run/${ISSUE_SNIPPETS_PATH}" | ||
| USR_LIB_SNIPPETS="/usr/lib/${ISSUE_SNIPPETS_PATH}" |
There was a problem hiding this comment.
I'm thinking the variable names should not need to distinguish the private/public path, at least for now, since for issuegen all snippets are written into the "private" directories. Previously, ISSUE_DIR_PUBLIC was unused. The *_SNIPPETS variables will just refer to the snippet paths that will get read in order to create ${generated_file}.
For #7, we may allow a choice of using the public or private locations for the snippet locations by invoking e.g. issuegen or issuegen --legacy respectively from the console-login-helper-messages-issuegen.service unit (this choice would be needed for systems that have util-linux version under 2.35 - e.g. RHCOS). We would probably try to split the ssh, udev, and generated issue parts up and have SSH_KEY_OUTDIR and UDEV_IF_OUTDIR set to either /run/issue.d (public) or /run/console-login-helper-messages/issue.d (private - i.e. the RUN_SNIPPETS variable). The private locations would be used for the *_OUTDIR variables if --legacy was passed. However more discussion can be done on this in #7. In any case, the *_SNIPPETS locations should be consumed only when creating ${generated_file} - the *_SNIPPETS variables shouldn't need to change after this.
There was a problem hiding this comment.
we could either use a config file (like a file in /etc/console-login-helper-messages/foo.conf) to set the paths to output to or we could detect the version of util-linux and switch based on that.
There was a problem hiding this comment.
though would we still want to use a private directory for all the snippets and then just write out generated_file to the public directory like we do for motdgen in #44
There was a problem hiding this comment.
👍 a config file would be good at some point. A specfile would make the choice on the output paths set in /etc/console-login-helper-messages/foo.conf, based on the util-linux version.
I'm thinking if a config file is added, we'd want to support an extensible config file format - which would allow on/off configuration for each piece of information (udev interfaces, ssh keys, etc) (#15), and whether the snippets are output in the issue, motd, or other. I've opened this to discuss #45.
For now, going with having issuegen check the util-linux version and set SSH_KEY_OUTDIR, UDEV_IF_OUTDIR, and generated_file appropriately seems simplest - this could be default behaviour if a config path was not specified if we added a config file. Will update this PR with the check on the util-linux version and setting the path shortly.
There was a problem hiding this comment.
Now updated with the version check - kept so that the check would only make the switch automatically for RPM-based distributions for now.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Apr 20, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Apr 20, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
from
81458f2 to
b6196b8
Compare
rfairley
changed the title
Clean up code by minimizing the number of variables and shortening the variable names. Remove unneeded comments, and add a few more clarifying comments. Separate code into clearer blocks for different operations (generating SSH key information, udev data, and the final output issue file).
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
May 18, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
If the util-linux version installed is greater than or equal to 2.35, when support for /run/issue.d was added, set the the output paths so that /run/issue.d is utilized. The version check for util-linux is done by using `rpm` to check the version of an installed util-linux RPM package. For non-RPM-based systems, the output paths will take a default value of the private directory location /run/console-login-helper-messages/issue.d. `rpm` is chosen to perform the check as the only kown consumers of console-login-helper-messages are RPM-based distributions, however support could always later be added for Debian-based or other Linux distributions.
rfairley
force-pushed
the
rfairley-issuegen-tidy
branch
from
b6196b8 to
abfced0
Compare
rfairley
changed the title
|
For now, putting this PR on hold and making the PR about the util-linux version check rather than just the issuegen code tidyups, due to #7 (comment). Bringing forward the same issuegen tidyups in #43 - let's do the review there |
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
May 22, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 1, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 1, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 2, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 2, 2020
For the same reason as in coreos#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 coreos#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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 3, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 3, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 5, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 5, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 6, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 6, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Jul 8, 2020
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 coreos#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`. A bug reported in `issuegen` for this reason is: coreos#35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. 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 coreos#40.
|
The work in this PR is continued in #76. I created the new PR since the repo changed quite a bit since, and it would be quite a pain to resolve all the conflicts. Thanks Robert for the helpful comments. |
|
Closing for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #36, with more general tidyups to the issuegen code. This is prep for more general work for #7 .