Skip to content
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

Add a basic noble migration check script #7334

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Nov 8, 2024

Status

Ready for review

Description of Changes

Perform a number of checks to ensure the system is ready for the noble migration. The results are written to a JSON file in /etc/ that other things like the JI and the upgrade script itself can read from.

The script is run hourly on a systemd timer but can also be run interactively for administrators who want slightly more details.

For ease of review, this is split into two commits: first set up all the configuration needed to ship Rust code in the securedrop-config package; and then second the actual check script, Rust dependencies, and systemd units.

Out of scope for this PR: OSSEC alerts, JI banner.

Refs #7322.

Testing

How should the reviewer test this PR?

  • staging CI passes
  • visual review

Deployment

Any special considerations for deployment? n/a

Checklist

@legoktm legoktm added the noble Ubuntu Noble related work label Nov 8, 2024
@legoktm legoktm requested a review from a team as a code owner November 8, 2024 21:30
@legoktm legoktm added this to the SecureDrop 2.11.0 milestone Nov 8, 2024
@legoktm
Copy link
Member Author

legoktm commented Nov 8, 2024

I'm still thinking of a few more checks to add, but I think this is ready for review + merge and other checks can be added incrementally.

@legoktm
Copy link
Member Author

legoktm commented Nov 13, 2024

I plan to port this to Rust based on #7332 (comment).

@legoktm legoktm marked this pull request as draft November 13, 2024 22:19
@legoktm
Copy link
Member Author

legoktm commented Nov 14, 2024

Now that this is in Rust, need to do a few more cargo vet reviews:

recommended audits for safe-to-deploy:
    Command                                     Publisher      Used By                            Audit Size
    cargo vet diff zerofrom-derive 0.1.3 0.1.4  robertbastian  zerofrom                           5 files changed, 49 insertions(+), 43 deletions(-)
    cargo vet diff idna 1.0.2 1.0.3             valenting      url and sequoia-openpgp            7 files changed, 207 insertions(+), 208 deletions(-)
    cargo vet diff url 2.5.1 2.5.3              valenting      noble-migration                    12 files changed, 386 insertions(+), 92 deletions(-)
    cargo vet inspect stable_deref_trait 1.2.0  Storyyeller    yoke and icu_provider              484 lines
    cargo vet inspect idna_adapter 1.2.0        hsivonen       idna                               612 lines
    cargo vet inspect smallvec 1.8.0            mbrubeck       idna, icu_normalizer, and 1 other  3879 lines
      NOTE: mozilla trusts Matt Brubeck (mbrubeck) - consider cargo vet trust smallvec mbrubeck

estimated audit backlog: 5960 lines

The main new dependency triggering reviews is url, which pulls in idna.

@legoktm legoktm force-pushed the stg-upgrade-check branch 2 times, most recently from ffbfd35 to 814c084 Compare November 20, 2024 20:41
@legoktm
Copy link
Member Author

legoktm commented Nov 20, 2024

In the end we only needed one audit for zerofrom-derive 0.1.3 -> 0.1.4 (done); I hadn't configured the new crate to use the safe-to-run standard. Went a bit out of the way to upgrade cargo-vet since a newer version was available.

@legoktm legoktm marked this pull request as ready for review November 21, 2024 19:07
@legoktm
Copy link
Member Author

legoktm commented Nov 21, 2024

Marking this as ready for review, I went through and added a bunch of docs and tried to make the error handling a bit more robust and obvious. Staging CI will fail until the new kernels are on apt-test.

Establish a folder where we can build Rust binaries that will
be shipped in the securedrop-config deb. That package is now
architecture-dependent and only built for amd64.

We are using Rust because a statically compiled binary is going
to be the most robust option during a system upgrade when Python
itself is being removed and installed (not to mention all the other
Rust benefits).
@legoktm legoktm force-pushed the stg-upgrade-check branch 2 times, most recently from ee7bf43 to 3f9ea29 Compare November 22, 2024 01:01
@legoktm
Copy link
Member Author

legoktm commented Nov 22, 2024

On staging we get:

{'ssh': False, 'ufw': True, 'free_space': False, 'apt': True, 'systemd': True}

Free space is understandable, I'll have it ignore that one. I'm confused by ssh, that should've been resolved. Will add some debug code 🤔

@legoktm legoktm force-pushed the stg-upgrade-check branch 3 times, most recently from fade6db to e471c94 Compare November 22, 2024 03:32
@legoktm
Copy link
Member Author

legoktm commented Nov 22, 2024

I had a bug in my getent output parser when I ported the logic from Python to Rust. I have paid penance by adding a unit/regression test.

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some comments. Haven't tested yet tho.

noble-migration/src/bin/check.rs Outdated Show resolved Hide resolved
noble-migration/src/bin/check.rs Outdated Show resolved Hide resolved
println!("All ready for migration!");
} else {
println!(
"Some errors were found that will block migration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to consider would be adding a "getting ready for Noble" page in the docs, that lists steps admins should take, and linking to that here as well/instead of the support link. Giving folks the tools to solve problems themselves drives down our support overhead and also helps admins who don't want to work with FPF for whatever reason.

(Just added freedomofpress/securedrop-docs#610)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will spend some time working on the documentation and agree that we should link it here, but don't think it should hold up review/approval of this since it'll be trivial to add in afterwards but before the release.

@legoktm legoktm force-pushed the stg-upgrade-check branch 2 times, most recently from 7645507 to 0024364 Compare November 22, 2024 19:02
@legoktm
Copy link
Member Author

legoktm commented Nov 22, 2024

Hm, and CI is failing now that I've removed the apt-test/apt-qa URLs... 🤔

@legoktm
Copy link
Member Author

legoktm commented Nov 22, 2024

They just yanked the url crate, so rust-audit is failing. Let me upgrade it...

Perform a number of checks to ensure the system is ready for the noble
migration. The results are written to a JSON file in /etc/ that other
things like the JI and the upgrade script itself can read from.

The script is run hourly on a systemd timer but can also be run
interactively for administrators who want slightly more details.

Refs #7322.
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on visual review and testing in staging.

@zenmonkeykstop zenmonkeykstop added this pull request to the merge queue Nov 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2024
@zenmonkeykstop zenmonkeykstop added this pull request to the merge queue Nov 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noble Ubuntu Noble related work
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

2 participants