-
Notifications
You must be signed in to change notification settings - Fork 73
Info file parameter support #1896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
74b1d46
0d11399
6cc2517
8d4679f
ae97e49
10a9a8f
d761a98
b476c61
feb8ef7
cf47628
c3e5920
e35af4e
0138767
c693a25
4960315
67abcfc
bde6563
74f7c33
2a78da5
e0184bd
4dc697b
5348b50
1be61eb
69d7351
58e31c5
7d4bfd8
1d02e6f
661afa1
85bde8d
45e2daf
e831ed9
56fdc9e
ada9f71
6523fce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| name: CI - ISO definition | ||
|
|
||
| on: | ||
| push: | ||
| paths: | ||
| # NOTE: GitHub Actions do not allow using YAML references, the same path | ||
| # list is used below for the pull request event. Keep both lists in sync!! | ||
|
|
||
| # this file as well | ||
| - .github/workflows/ci-live.yml | ||
| # any change in the service subfolder | ||
| - live/** | ||
|
|
||
| pull_request: | ||
| paths: | ||
| # NOTE: GitHub Actions do not allow using YAML references, the same path | ||
| # list is used above for the push event. Keep both lists in sync!! | ||
|
|
||
| # this file as well | ||
| - .github/workflows/ci-live.yml | ||
| # any change in the service subfolder | ||
| - live/** | ||
|
|
||
| # allow running manually | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| ruby_tests: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| COVERAGE: 1 | ||
|
|
||
| defaults: | ||
| run: | ||
| working-directory: ./live | ||
|
|
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| distro: [ "tumbleweed" ] | ||
|
|
||
| container: | ||
| image: registry.opensuse.org/yast/head/containers_${{matrix.distro}}/yast-ruby | ||
|
|
||
| steps: | ||
|
|
||
| - name: Git Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Configure and refresh repositories | ||
| # disable unused repositories to have faster refresh | ||
| run: zypper modifyrepo -d repo-non-oss repo-openh264 repo-update && zypper ref | ||
|
|
||
| - name: Install Ruby development files | ||
| run: zypper --non-interactive install | ||
| make | ||
|
|
||
| - name: Run the tests | ||
| run: make check |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| [Unit] | ||
| Description=Agama kernel cmdline processing | ||
|
|
||
| # have to be after network to be able to download info files | ||
| # TODO: what to do in air gap scenario where we still need process cmdline? | ||
| After=network-online.target | ||
|
|
||
| # before starting the Agama servers so they read configuration parsed | ||
| Before=agama-web-server.service | ||
| Before=agama.service | ||
| Before=x11-autologin.service | ||
|
|
||
| [Service] | ||
| Type=oneshot | ||
| Environment=TERM=linux | ||
| ExecStart=agama-kernel-cmdline.sh | ||
| StandardInput=tty | ||
| TimeoutSec=0 | ||
|
|
||
| [Install] | ||
| WantedBy=default.target | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| #! /bin/sh | ||
|
|
||
| kernel-cmdline-conf.sh | ||
| info-cmdline-conf.sh |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #! /bin/sh | ||
|
|
||
| # Script that expand agama.info parameter by downloading its file and appending it to agama.conf | ||
| # the info content is stored in info.conf | ||
|
|
||
| set -e | ||
|
|
||
| TARGET="${1:-/run/agama/cmdline.d/agama.conf}" | ||
| INFO_CONTENT="${2:-/run/agama/cmdline.d/info.conf}" | ||
|
|
||
| expand_info_arg() { | ||
| INFO_URL=$(sed -n 's/\(.*[[:space:]]\|^\)agama\.info=\([^[:space:]]\+\).*/\2/p' "$TARGET") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This picks up only one info file. Linuxrc supports multiple info arguments. Then it downloads and merges all info files. This might be pretty useful. You can have a generic info file and an optional debugging one which additionally enables some debug features. This avoids duplicating the common parts between the info files. Just use the common one and if needed easily add the debugging one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And linuxrc supports nested info files, you can use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know about both of those linuxrc features and it is possible to implement it, I just do not see much usage for it. What I see so far from bug reports is that there is just single info file param that contain required parameters. In the end there is not so much parameters you need to use and usually it is more like debug.info and if needed some production.info file.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also allow different locations not sure if the same supported by curl... so something to document too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeap, also true |
||
| if [ -z "${INFO_URL}" ]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # TODO: should we use also --location-trusted if info file url contain user and password? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another item for a follow-up card.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done as #2024 |
||
| # if so check with security team | ||
| curl --location --silent "${INFO_URL}" > "${INFO_CONTENT}" | ||
| # remove info param | ||
| sed -in 's/\([[:space:]]\|^\)agama\.info=[^[:space:]]\+//' "${TARGET}" | ||
| # and add content of info file | ||
| cat "${INFO_CONTENT}" >> "${TARGET}" | ||
|
|
||
| return 0 | ||
| } | ||
|
|
||
| expand_info_arg | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #! /bin/sh | ||
|
|
||
| # Script to clean kernel command line from agama specific parameters. Result is later used for bootloader proposal. | ||
|
|
||
| SOURCE="${1:-/proc/cmdline}" | ||
jreidinger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| TARGET="${2:-/run/agama/cmdline.d/kernel.conf}" | ||
|
|
||
| write_kernel_args() { | ||
| DIR=$(dirname "${TARGET}") | ||
| mkdir -p "$DIR" | ||
| # ensure that kernel cmdline line is created to avoid reading agama params | ||
| # if there is no kernel params | ||
| touch "${TARGET}" | ||
|
|
||
| for _i in $(cat "${SOURCE}"); do | ||
| case ${_i} in | ||
| # remove all agama kernel params | ||
| # Add here also all linuxrc supported parameters | ||
| LIBSTORAGE_* | YAST_* | agama* | Y2* | ZYPP_* | autoyast* ) | ||
| _found=1 | ||
| ;; | ||
| esac | ||
|
|
||
| if [ -z "$_found" ]; then | ||
| echo "Non-Agama parameter found ($_i)" | ||
| echo -n " $_i" >>"${TARGET}" | ||
| fi | ||
| unset _found | ||
| done | ||
|
|
||
| return 0 | ||
| } | ||
|
|
||
| write_kernel_args | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is still and issue, we should record it somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeap, we need to test it. It is not just here, but also all password services is after network up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done as #2023