updog and migrator: signed migrations #930

webern · 2020-05-20T19:18:30Z

Issue number:

Closes #91
Closes #905

Description of changes:

Implements all changes necessary for migrator to load a locally cached tuf repo and obtain migrations from there. See #905 for a detailed explanation of the aigned migrations design.

Additionally, we decided to maintain compatibility so that instances can be upgraded into signed migrations. To facilitate this, in the current version, upog writes both signed and unsigned migrations. migrator checks the from version. If it's less than 0.3.5, migrator executes unsigned migrations, otherwise it executes signed migrations.

In a future version, we can create a breaking change in which unsigned migrations are no-longer read. At that point, versions less than 0.3.5 must pass through a version that supports both signed and unsigned migrations (such as this one) in order to upgrade into signed migrations.

There is one behavioral change about signed migrations. Previously we used a sort of the migration names to provide stability to their order of execution (when more than one migration existed for a single version pair). With signed migrations, this is no longer necessary and the migrations will run in the order that they are listed in the manifest.

Testing done:

The testing procedure is extensive. Note: a trace was added so that migrator announces in the system journal whether it is running signed or unsigned migrations. Also note, v0.3.1 was used as a starting point because a migration exists for v0.3.2 (which changes the host container version). Thus upgrading from v0.3.1 presents an actual unsigned migration.

Test Setup

Create an AMI of v0.3.1, but with my non-production root.json.
Create matching image binaries for v0.3.1 with my non-production root.json and add them to a TUF repo (replacing the prod images).
Create a version of the code in this PR, naming the version 0.99.0, create an AMI of this and add the update binaries to the TUF repo/manifest.
Create a version of the code in this PR, along with a migration that adds a foo setting and sets its value to bar. Call this v0.99.1 and add it to the TUF repo/manifest.

Test Execution

Starting from the v0.3.1 AMI, perform the following sequence of upgrades and downgrades.

Start at v0.3.1 via AMI
Upgrade v0.3.1 -> v0.99.0
Upgrade v0.99.0 -> v0.99.1
Downgrade v0.99.1 -> v0.99.0
Downgrade v0.99.0 -> v0.3.1

At each step along the way.

Ensure that Kubernetes is working by running a busybox pod.
Copy the system journal and Bottlerocket API settings to local storage.

At the end of the cycle, diff the system journals to observe that migrator annouced:

Upgrade v0.3.1 -> v0.99.0 'running unsigned migrations'
Upgrade v0.99.0 -> v0.99.1 'running signed migrations'
Downgrade v0.99.1 -> v0.99.0 'running signed migrations'
Downgrade v0.99.0 -> v0.3.1: older version of migrator made no announcement

Diff the API settings to observe the following setting changes:

Upgrade v0.3.1 -> v0.99.0: host container changed from v0.4.0 to v0.5.0
Upgrade v0.99.0 -> v0.99.1: "foo": "bar" was added
Downgrade v0.99.1 -> v0.99.0: "foo": "bar" was removed
Downgrade v0.99.0 -> v0.3.1: host container changed from v0.5.0 to v0.4.0

Additional Testing

Additional testing has been performed with v0.99.0 AMI -> v0.99.1 -> v0.99.0, though this has not been repeated at every testing cycle.

TODO

Use pentacle
Use tough::RepoEditor to create the migrator unit test repo on the fly. (pushed)
Use a single migration script for testing instead of two different migration scripts, and compress the migration script on the fly instead of checking in a compressed version of it. (pushed)
Set repo expiration to 1970-01-01 to make it obvious that it is expired. Also make a note about this in the test. (pushed)

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

sources/api/migration/migrator/src/main.rs

sources/updater/update_metadata/src/lib.rs

sources/api/migration/migrator/Cargo.toml

...itory/targets/e823375105cd38d5e0476a4e277bc61eef3c15bd0d8831e2f78bc9394b4f1335.manifest.json

sources/api/migration/migrator/tests/data/repository/metadata/timestamp.json

sources/updater/update_metadata/src/lib.rs

sources/api/migration/migrator/src/args.rs

sources/api/migration/migrator/src/main.rs

webern · 2020-06-02T01:20:23Z

push to 3b57a35 addresses most of the comments, with the exception of using a tempdir in updog and migrator.

webern · 2020-06-02T22:25:25Z

push to 3f62781 uses tempfile::TempDIr in both updog and migrator for the tough datastore. Tested with the full test cycle.

sources/api/migration/migrator/Cargo.toml

sources/updater/updog/Cargo.toml

sources/api/migration/migrator/tests/data/repository/targets/a-second-migration.sh

sources/api/migration/migrator/tests/data/repository/metadata/timestamp.json

sources/api/migration/migrator/src/args.rs

sources/api/migration/migrator/src/error.rs

sources/api/migration/migrator/src/main.rs

sources/api/migration/migrator/tests/data/repository/metadata/1.root.json

sources/api/migration/migrator/src/args.rs

tjkirch

Hoping @iliana can confirm the tempdir for tough datastore is OK.

tjkirch · 2020-06-16T17:45:36Z

@webern confirmed for me that this force-push ^ was just a rebase.

webern · 2020-06-16T18:21:08Z

webern force-pushed the webern:pr/signed-migrations branch from 649b11a to ee995a6 now

This adds backward compatibility with unsigned migrations.

iliana · 2020-06-16T18:24:23Z

Hoping @iliana can confirm the tempdir for tough datastore is OK.

I know @webern and I discussed it, and I was confident that the tempdir was fine for this particular case. I'll try and write up some full details, and maybe those can turn into a doc comment on tough. :)

webern · 2020-06-16T18:36:31Z

webern force-pushed the webern:pr/signed-migrations branch from ee995a6 to f481a2d 3 minutes ago

This is a rebase from develop that bumps the version of tough.

webern · 2020-06-17T21:24:37Z

webern force-pushed the webern:pr/signed-migrations branch from f481a2d to 4443441

This push addresses:

Use tough::RepoEditor to create the migrator unit test repo on the fly.
migrator/src/args.rs review comment (unwrap_or_else messages).
migrator/src/error.rs review comment (PathUrl unclear).

webern · 2020-06-17T21:34:30Z

webern force-pushed the webern:pr/signed-migrations branch from 4443441 to dab3f59 41 seconds ago

Small: cleanup in migrator/srv/error.rs

webern · 2020-06-17T23:00:59Z

webern force-pushed the webern:pr/signed-migrations branch from dab3f59 to b3a4354 14 seconds ago

Small cleanups.

sources/api/migration/migrator/src/main.rs

sources/api/migration/migrator/tests/data/fake-key.pem

sources/api/migration/migrator/src/main.rs

bcressey

LGTM

sources/api/migration/migrator/src/args.rs

sources/updater/update_metadata/src/lib.rs

sources/api/migration/migrator/src/main.rs

webern · 2020-06-18T21:36:07Z

webern force-pushed the webern:pr/signed-migrations branch from b3a4354 to 50cb08d

Added pentacle tested with the full Bottlerocket test cycle. I did some acrobatics to preserve my migrator end-to-end "unit" test, but it works.

webern · 2020-06-19T01:52:05Z

webern force-pushed the webern:pr/signed-migrations branch from 50cb08d to f5a28e5

Addresses Ben's comments.

webern · 2020-06-19T02:00:56Z

webern force-pushed the webern:pr/signed-migrations branch from f5a28e5 to 00ce955

Addresses iliana's smaller comments.

webern · 2020-06-19T02:18:39Z

webern force-pushed the webern:pr/signed-migrations branch from 00ce955 to acd95cf 16 seconds ago

Fix a one word oops.

sources/api/migration/migrator/Cargo.toml

sources/api/migration/migrator/src/args.rs

sources/api/migration/migrator/src/error.rs

sources/api/migration/migrator/src/main.rs

webern · 2020-06-23T01:04:03Z

sources/api/migration/migrator/src/main.rs

+    // unsigned migrations code and return.
+    if !are_migrations_signed(&current_version) {
+        // note in the system journal that the unsigned code path ran.
+        eprintln!("migrator is running unsigned migrations");


@tjkirch left a comment here that, inexplicably, GitHub allowed me to delete when I was, in fact, trying to delete my own response. Fortunately I have both pasted to a text file:

@tjkirch wrote:

I don't think we need these messages in the logs/journal, we know which type of migrations ran based on the version. (Ties with my other comments about simplifying/removing the branching.)

And I replied:
I am using them during test cycles. I can delete them, but once I do I will no longer be able to verify correctness during the test cycles as I have been (see the test procedure in the PR comments).

I'd prefer to use a more structured method like running different migrations on each path. I'm hoping we can go back to the shell script method so having test migrations is trivial...

I meant when I run Bottlerocket and check the logs to ensure that migrator did what I want. However, I can remove these print statements as the last push.

Yeah, I was suggesting that we use different test migrations (which can print different output) to get the same effect, and even check it in the test, rather than impacting the actual codebase here.

removed these in this push https://github.com/bottlerocket-os/bottlerocket/compare/b6fe3cd824df6df149da51efc14b01b222f46787..7b6b6c27a0abbeedd77535ba948aa0c7d220bb1c

webern · 2020-06-23T17:04:39Z

webern force-pushed the webern:pr/signed-migrations branch from acd95cf to e864eba 1 minute ago

Quickfixes for some of @tjkirch's comments. Not done yet...

webern · 2020-06-23T17:47:24Z

webern force-pushed the webern:pr/signed-migrations branch from e864eba to 1f69203 now

Uses url::Url instead of format!("file:/{}", path)

webern · 2020-06-23T19:10:39Z

webern force-pushed the webern:pr/signed-migrations branch from 1f69203 to afc4b5d 13 seconds ago

I removed the large end-to-end migrator 'unit' test. I will try to introduce it in a future PR where we can spend more time refining it.

webern · 2020-06-23T19:38:33Z

webern force-pushed the webern:pr/signed-migrations branch from afc4b5d to 4f34544 now

Addresses most of @tjkirch's remaining comments.

webern · 2020-06-23T19:48:30Z

webern force-pushed the webern:pr/signed-migrations branch from 4f34544 to b6fe3cd now

git rebase develop

sources/api/migration/migrator/src/main.rs

tjkirch

No more blockers from me, as long as testing is still good.

webern · 2020-06-23T21:18:05Z

Just did a successful test cycle with b6fe3cd

webern · 2020-06-23T21:27:06Z

webern force-pushed the webern:pr/signed-migrations branch from b6fe3cd to 7b6b6c2 1 minute ago

Last cleanup: restores the original unit test (which will be deleted once unsigned migrations are gone), and stops printing to the system journal per @tjkirch's request.

It's now ready to merge.

webern changed the title ~~update and migrator: signed migrations~~ updog and migrator: signed migrations May 20, 2020

webern commented May 20, 2020

View reviewed changes

sources/api/migration/migrator/src/main.rs Outdated Show resolved Hide resolved

sources/api/migration/migrator/src/main.rs Outdated Show resolved Hide resolved

sources/updater/update_metadata/src/lib.rs Outdated Show resolved Hide resolved

webern commented May 21, 2020

View reviewed changes

sources/api/migration/migrator/Cargo.toml Outdated Show resolved Hide resolved

webern marked this pull request as draft May 21, 2020 17:20

webern requested review from tjkirch, iliana and sam-aws May 21, 2020 17:20

tjkirch reviewed May 22, 2020

View reviewed changes

webern commented Jun 3, 2020

View reviewed changes

sources/api/migration/migrator/Cargo.toml Outdated Show resolved Hide resolved

sources/updater/updog/Cargo.toml Show resolved Hide resolved

tjkirch reviewed Jun 4, 2020

View reviewed changes

tjkirch reviewed Jun 5, 2020

View reviewed changes

webern marked this pull request as ready for review June 16, 2020 18:58

iliana approved these changes Jun 17, 2020

View reviewed changes

iliana reviewed Jun 17, 2020

View reviewed changes

sources/api/migration/migrator/src/main.rs Outdated Show resolved Hide resolved

bcressey approved these changes Jun 18, 2020

View reviewed changes

webern mentioned this pull request Jun 19, 2020

tuftool: feature to dump stats awslabs/tough#156

Open

iliana approved these changes Jun 19, 2020

View reviewed changes

iliana added this to the v0.4.0 milestone Jun 22, 2020

bcressey approved these changes Jun 22, 2020

View reviewed changes

tjkirch reviewed Jun 22, 2020

View reviewed changes

bottlerocket-os deleted a comment from tjkirch Jun 23, 2020

webern commented Jun 23, 2020

View reviewed changes

sources/api/migration/migrator/src/main.rs Show resolved Hide resolved

webern commented Jun 23, 2020

View reviewed changes

iliana mentioned this pull request Jun 23, 2020

Remove deprecated migration handling code #954

Closed

webern commented Jun 23, 2020

View reviewed changes

sources/api/migration/migrator/src/main.rs Outdated Show resolved Hide resolved

tjkirch approved these changes Jun 23, 2020

View reviewed changes

iliana approved these changes Jun 23, 2020

View reviewed changes

updog and migrator: signed migrations

7b6b6c2

webern merged commit 45152e8 into bottlerocket-os:develop Jun 23, 2020

webern mentioned this pull request Jun 23, 2020

migrator: end-to-end test #956

Merged

tjkirch mentioned this pull request Jun 30, 2020

v0.4.1 / v0.5.0 repo pivot migration #961

Closed

webern deleted the pr/signed-migrations branch January 13, 2021 19:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updog and migrator: signed migrations #930

updog and migrator: signed migrations #930

webern commented May 20, 2020 •

edited

Loading

webern commented Jun 2, 2020 •

edited

Loading

webern commented Jun 2, 2020

tjkirch left a comment

tjkirch commented Jun 16, 2020

webern commented Jun 16, 2020

iliana commented Jun 16, 2020

webern commented Jun 16, 2020

webern commented Jun 17, 2020

webern commented Jun 17, 2020

webern commented Jun 17, 2020

bcressey left a comment

webern commented Jun 18, 2020

webern commented Jun 19, 2020

webern commented Jun 19, 2020

webern commented Jun 19, 2020

webern Jun 23, 2020

tjkirch Jun 23, 2020

webern Jun 23, 2020

tjkirch Jun 23, 2020

webern Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

tjkirch left a comment

webern commented Jun 23, 2020

webern commented Jun 23, 2020

updog and migrator: signed migrations #930

updog and migrator: signed migrations #930

Conversation

webern commented May 20, 2020 • edited Loading

Issue number:

Description of changes:

Testing done:

Test Setup

Test Execution

Additional Testing

TODO

Terms of contribution:

webern commented Jun 2, 2020 • edited Loading

webern commented Jun 2, 2020

tjkirch left a comment

Choose a reason for hiding this comment

tjkirch commented Jun 16, 2020

webern commented Jun 16, 2020

iliana commented Jun 16, 2020

webern commented Jun 16, 2020

webern commented Jun 17, 2020

webern commented Jun 17, 2020

webern commented Jun 17, 2020

bcressey left a comment

Choose a reason for hiding this comment

webern commented Jun 18, 2020

webern commented Jun 19, 2020

webern commented Jun 19, 2020

webern commented Jun 19, 2020

webern Jun 23, 2020

Choose a reason for hiding this comment

tjkirch Jun 23, 2020

Choose a reason for hiding this comment

webern Jun 23, 2020

Choose a reason for hiding this comment

tjkirch Jun 23, 2020

Choose a reason for hiding this comment

webern Jun 23, 2020

Choose a reason for hiding this comment

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented Jun 23, 2020

tjkirch left a comment

Choose a reason for hiding this comment

webern commented Jun 23, 2020

webern commented Jun 23, 2020

webern commented May 20, 2020 •

edited

Loading

webern commented Jun 2, 2020 •

edited

Loading