pubsys: update to AWS SDK Rust #2415

jpculp · 2022-09-12T22:54:54Z

Issue number:

#1968

Description of changes:

Replaces rusoto with aws-sdk-for-rust in pubsys.

Testing done:

publish ami to us-west-2
publish multiple amis with chained assumed roles (2 different accounts, 3 regions)
grant ami access
revoke ami access
publish ssm parameters
promote ssm parameters

TUF testing done:
Thanks, @etungsten!

use KMS key when creating TUF repo
check TUF repo expirations
validate TUF repo
refresh TUF repo

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.

tools/pubsys/src/aws/client.rs

jpculp · 2022-09-14T20:50:58Z

Fixed the result unwrapping issue while also improving how and where the profile gets loaded.

stmcginnis

This looks good to me - with the caveat that I am not very familiar with this code. But updated code looks logical, and maybe most importantly, all tests are passing.

zmrow

As an aside, if you're not already doing so it'd probably be good to run down the entire set of pubsys commands in your own account, making sure that the grant/revoke and SSM params all work as expected. Those are super important.

tools/pubsys/src/aws/client.rs

zmrow · 2022-09-16T21:33:47Z

tools/pubsys/src/aws/client.rs

-    }
+    // Load a chained credential config if role is specified in aws.region.REGION.role.
+    let maybe_regional_role = aws.region.get(&region).and_then(|r| r.role.clone());
+    let config = if maybe_regional_role.is_some() {


I'm confused about this let config... are we replacing the config we just built?

Correct, but I can rename it to make it more clear.

jpculp · 2022-09-16T22:43:51Z

Fixed missing region from chained configuration.
Removed the extraneous else's.

etungsten

Can you please also test cargo make check-repo-expirations, cargo make validate-repo, and cargo make refresh-repo?

You can find examples in the PR descriptions here:
#2303
#1575

tools/pubsys/src/aws/ami/mod.rs

tools/pubsys/src/repo.rs

tools/pubsys/src/main.rs

tools/pubsys/src/aws/ssm/ssm.rs

tools/pubsys/src/aws/publish_ami/mod.rs

tools/pubsys/src/aws/client.rs

jpculp · 2022-09-19T18:20:43Z

Removed extraneous clones and references.
Set default log level for aws_config and aws_smithy to Warn.
Improved error handling and logging.

etungsten · 2022-09-19T19:08:05Z

Tested check-repo-expirations:

$ cargo make  -e BUILDSYS_VARIANT=aws-k8s-1.23 -e BUILDSYS_ARCH=x86_64 check-repo-expirations
[cargo-make] INFO - cargo make 0.35.15
[cargo-make] INFO - Build File: Makefile.toml
[cargo-make] INFO - Task: check-repo-expirations
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: setup
[cargo-make] INFO - Running Task: fetch-sources
[cargo-make] INFO - Running Task: tuftool
[cargo-make] INFO - Running Task: publish-setup-tools
[cargo-make] INFO - Running Task: publish-setup
19:01:24 [INFO] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[cargo-make] INFO - Running Task: setup
[cargo-make] INFO - Running Task: fetch-sources
[cargo-make] INFO - Running Task: publish-tools
[cargo-make] INFO - Running Task: check-repo-expirations
[2022-09-19T19:01:26Z INFO  pubsys_config] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Loaded TUF repo:   file:///home/ec2-user/bottlerocket/build/repos/default/bottlerocket-1.9.2-deea371c/aws-k8s-1.23/x86_64
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Root expiration:   2023-09-18 18:58:32 UTC
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Snapshot expiration:       2022-10-03 19:01:12.156262570 UTC
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Targets expiration:        2022-10-03 19:01:12.156262570 UTC
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Timestamp expiration:      2022-09-26 19:01:12.156262570 UTC
[2022-09-19T19:01:26Z INFO  pubsys::repo::check_expirations] Looking for metadata expirations happening from now to 2022-09-22 19:01:26.973914183 UTC
[cargo-make] INFO - Build Done in 4.72 seconds.

Tested validate-repo:

$ cargo make  -e BUILDSYS_VARIANT=aws-k8s-1.23 -e BUILDSYS_ARCH=x86_64 validate-repo
[cargo-make] INFO - cargo make 0.35.15
[cargo-make] INFO - Build File: Makefile.toml
[cargo-make] INFO - Task: validate-repo
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: setup
[cargo-make] INFO - Running Task: fetch-sources
[cargo-make] INFO - Running Task: tuftool
[cargo-make] INFO - Running Task: publish-setup-tools
[cargo-make] INFO - Running Task: publish-setup
19:01:45 [INFO] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[cargo-make] INFO - Running Task: setup
[cargo-make] INFO - Running Task: fetch-sources
[cargo-make] INFO - Running Task: publish-tools
[cargo-make] INFO - Running Task: validate-repo
[2022-09-19T19:01:47Z INFO  pubsys_config] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Loaded TUF repo: file:///home/ec2-user/bottlerocket/build/repos/default/bottlerocket-1.9.2-deea371c/aws-k8s-1.23/x86_64
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: migrate_v1.10.0_dns-settings.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-v1.9.2.img.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: manifest.json
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-1.9.2-deea371c-root.ext4.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-1.9.2-deea371c-boot.ext4.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-1.9.2-deea371c.img.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: aws-k8s-1.23-x86_64-kmod-kit-v1.9.2.tar.xz
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-1.9.2-deea371c-data.img.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-v1.9.2-data.img.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: bottlerocket-aws-k8s-1.23-x86_64-1.9.2-deea371c-root.verity.lz4
[2022-09-19T19:01:47Z INFO  pubsys::repo::validate_repo] Downloading target: migrate_v1.10.0_dns-settings-metadata.lz4
[cargo-make] INFO - Build Done in 4.25 seconds.

Tested refresh-repo with KMS key:

$ cargo make  -e BUILDSYS_VARIANT=aws-k8s-1.23 -e BUILDSYS_ARCH=x86_64 refresh-repo        
[cargo-make] INFO - cargo make 0.35.15
[cargo-make] INFO - Build File: Makefile.toml
[cargo-make] INFO - Task: refresh-repo
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: setup
[cargo-make] INFO - Running Task: fetch-sources
[cargo-make] INFO - Running Task: tuftool
[cargo-make] INFO - Running Task: publish-setup-tools
[cargo-make] INFO - Running Task: publish-setup
19:05:28 [INFO] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[cargo-make] INFO - Running Task: publish-tools
[cargo-make] INFO - Running Task: refresh-repo
[2022-09-19T19:05:28Z INFO  pubsys_config] Found infra config at path: /home/ec2-user/bottlerocket/Infra.toml
[2022-09-19T19:05:28Z INFO  pubsys::repo::refresh_repo] Using repo expiration policy from path: /home/ec2-user/bottlerocket/tools/pubsys/policies/repo-expiration/2w-2w-1w.toml
[2022-09-19T19:05:28Z INFO  pubsys::repo::refresh_repo] Loaded TUF repo: file:///home/ec2-user/bottlerocket/build/repos/default/bottlerocket-1.9.2-deea371c-bak/aws-k8s-1.23/x86_64
[2022-09-19T19:05:28Z INFO  pubsys::repo] Setting non-root metadata expiration times:
        snapshot:  2022-10-03 19:05:28.522312124 UTC
        targets:   2022-10-03 19:05:28.522312124 UTC
        timestamp: 2022-09-26 19:05:28.522312124 UTC
[2022-09-19T19:05:28Z INFO  pubsys::repo::refresh_repo] Writing repo metadata to: /home/ec2-user/bottlerocket/build/repos/default/bottlerocket-1.9.2-deea371c/aws-k8s-1.23/x86_64
[cargo-make] INFO - Build Done in 2.40 seconds.

tools/deny.toml

tools/pubsys/src/aws/ami/wait.rs

tools/pubsys/src/aws/client.rs

tools/pubsys/src/aws/promote_ssm/mod.rs

tools/pubsys/src/aws/ami/wait.rs

tools/pubsys/src/repo.rs

tools/pubsys/src/aws/publish_ami/mod.rs

ecpullen · 2022-09-20T00:28:15Z

tools/pubsys/src/aws/ami/register.rs

+        .set_device_name(Some(ROOT_DEVICE_NAME.to_string()))
+        .set_ebs(Some(
+            EbsBlockDevice::builder()
+                .set_delete_on_termination(Some(true))
+                .set_snapshot_id(Some(root_snapshot.clone()))
+                .set_volume_type(Some(VolumeType::from(VOLUME_TYPE)))
+                .set_volume_size(ami_args.root_volume_size)
+                .build(),


The sdk allows not including Some if you set the values without set_. (This happens in a lot of places so might not be in the scope of this pr)

Suggested change

.set_device_name(Some(ROOT_DEVICE_NAME.to_string()))

.set_ebs(Some(

EbsBlockDevice::builder()

.set_delete_on_termination(Some(true))

.set_snapshot_id(Some(root_snapshot.clone()))

.set_volume_type(Some(VolumeType::from(VOLUME_TYPE)))

.set_volume_size(ami_args.root_volume_size)

.build(),

.device_name(ROOT_DEVICE_NAME.to_string())

.ebs(EbsBlockDevice::builder()

.delete_on_termination(true)

.snapshot_id(root_snapshot.clone())

.volume_type(VolumeType::from(VOLUME_TYPE))

.set_volume_size(ami_args.root_volume_size)

.build(),

This definitely works, but my understanding was the setters are more idiomatic for the SDK.

Where did you see the perspective on idiomatic use? Not disagreeing, and it's not a blocker here in any case, but for me the code that @ecpullen wrote seems preferable since it's fewer characters and we don't have to wrap everything in an Option.

It was just an impression we got from the SDK team that the non-setter way might eventually be deprecated.

jpculp · 2022-09-20T22:07:49Z

Brought back stronger-typed regions and region_from_string().
Renamed aws: AwsConfig to pubsys_aws_config: PubsysAwsConfig to reduce confusion.
Kept arch as a stronger-type throughout.
Replaced ImageState ensure statement with match.
Replaced thread::spawn with tokio::runtime::Runtime::new().

bcressey · 2022-09-21T00:24:52Z

tools/pubsys/src/aws/mod.rs

+fn region_from_string(name: &str) -> Result<Region> {
+    Ok(Region::new(name.to_owned()))


If this is infallible now we should probably just return a Region. However, I like it this way for the purposes of the SDK migration, to minimize the diff. Maybe add a FIXME to rework later.

This is a pretty quick change. Rather than a FIXME, should I just make it the final change once we think this PR hits all the edge cases?

tools/pubsys/src/aws/client.rs

jpculp · 2022-09-21T01:31:06Z

Changed credentials logic will run even if only a single role is specified.
Credential expirations can now be None.
Removed endpoints.

zmrow

🦺

jpculp · 2022-09-22T19:49:56Z

Rewrote client.rs to return providers instead of credentials.
Removed returning unneccessary result-types (and their associated error contexts).

jpculp · 2022-09-22T20:10:49Z

Suppressed noisy [2022-09-22T20:01:26Z INFO tracing::span] assume_role; log.

zmrow

🔬

jpculp requested review from zmrow, ecpullen and etungsten September 12, 2022 22:54

jpculp commented Sep 12, 2022

View reviewed changes

tools/pubsys/src/aws/client.rs Outdated Show resolved Hide resolved

jpculp force-pushed the aws-sdk-rust-pubsys branch from efcacc8 to dbf6609 Compare September 14, 2022 20:48

kdaula requested review from arnaldo2792, rpkelly, bcressey and stmcginnis September 16, 2022 21:56

stmcginnis reviewed Sep 16, 2022

View reviewed changes

zmrow reviewed Sep 16, 2022

View reviewed changes

jpculp force-pushed the aws-sdk-rust-pubsys branch from dbf6609 to 7946314 Compare September 16, 2022 22:42

etungsten suggested changes Sep 17, 2022

View reviewed changes

jpculp force-pushed the aws-sdk-rust-pubsys branch from 7946314 to deea371 Compare September 19, 2022 18:18

jpculp requested a review from etungsten September 19, 2022 19:57

jpculp marked this pull request as ready for review September 19, 2022 19:58

etungsten approved these changes Sep 19, 2022

View reviewed changes

bcressey requested changes Sep 20, 2022

View reviewed changes

ecpullen reviewed Sep 20, 2022

View reviewed changes

jpculp force-pushed the aws-sdk-rust-pubsys branch from deea371 to 1ec0fc4 Compare September 20, 2022 22:01

bcressey reviewed Sep 21, 2022

View reviewed changes

jpculp force-pushed the aws-sdk-rust-pubsys branch from 1ec0fc4 to 347308c Compare September 21, 2022 01:28

bcressey approved these changes Sep 21, 2022

View reviewed changes

etungsten approved these changes Sep 21, 2022

View reviewed changes

zmrow approved these changes Sep 21, 2022

View reviewed changes

jpculp force-pushed the aws-sdk-rust-pubsys branch from 347308c to 6204440 Compare September 22, 2022 19:47

jpculp force-pushed the aws-sdk-rust-pubsys branch from 6204440 to 1707005 Compare September 22, 2022 20:08

jpculp requested review from stmcginnis, ecpullen, bcressey, zmrow and etungsten September 22, 2022 20:11

ecpullen approved these changes Sep 22, 2022

View reviewed changes

etungsten approved these changes Sep 22, 2022

View reviewed changes

zmrow approved these changes Sep 22, 2022

View reviewed changes

pubsys: update to AWS SDK Rust

1175053

jpculp force-pushed the aws-sdk-rust-pubsys branch from 1707005 to 1175053 Compare September 22, 2022 23:40

bcressey approved these changes Sep 22, 2022

View reviewed changes

ecpullen approved these changes Sep 23, 2022

View reviewed changes

jpculp merged commit 0938f2b into bottlerocket-os:develop Sep 23, 2022

jpculp deleted the aws-sdk-rust-pubsys branch September 23, 2022 00:14

jpculp mentioned this pull request Sep 26, 2022

pubsys: switch back to simplelog #2454

Merged

jpculp mentioned this pull request Oct 5, 2022

infrasys: update to AWS SDK Rust #2414

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pubsys: update to AWS SDK Rust #2415

pubsys: update to AWS SDK Rust #2415

jpculp commented Sep 12, 2022 •

edited

jpculp commented Sep 14, 2022

stmcginnis left a comment

zmrow left a comment

zmrow Sep 16, 2022

jpculp Sep 16, 2022

jpculp commented Sep 16, 2022

etungsten left a comment

jpculp commented Sep 19, 2022

etungsten commented Sep 19, 2022

ecpullen Sep 20, 2022 •

edited

jpculp Sep 20, 2022

bcressey Sep 21, 2022

jpculp Sep 22, 2022

jpculp commented Sep 20, 2022

bcressey Sep 21, 2022

jpculp Sep 21, 2022

jpculp commented Sep 21, 2022

zmrow left a comment

jpculp commented Sep 22, 2022

jpculp commented Sep 22, 2022

zmrow left a comment

		fn region_from_string(name: &str) -> Result<Region> {
		Ok(Region::new(name.to_owned()))

pubsys: update to AWS SDK Rust #2415

pubsys: update to AWS SDK Rust #2415

Conversation

jpculp commented Sep 12, 2022 • edited

jpculp commented Sep 14, 2022

stmcginnis left a comment

Choose a reason for hiding this comment

zmrow left a comment

Choose a reason for hiding this comment

zmrow Sep 16, 2022

Choose a reason for hiding this comment

jpculp Sep 16, 2022

Choose a reason for hiding this comment

jpculp commented Sep 16, 2022

etungsten left a comment

Choose a reason for hiding this comment

jpculp commented Sep 19, 2022

etungsten commented Sep 19, 2022

ecpullen Sep 20, 2022 • edited

Choose a reason for hiding this comment

jpculp Sep 20, 2022

Choose a reason for hiding this comment

bcressey Sep 21, 2022

Choose a reason for hiding this comment

jpculp Sep 22, 2022

Choose a reason for hiding this comment

jpculp commented Sep 20, 2022

bcressey Sep 21, 2022

Choose a reason for hiding this comment

jpculp Sep 21, 2022

Choose a reason for hiding this comment

jpculp commented Sep 21, 2022

zmrow left a comment

Choose a reason for hiding this comment

jpculp commented Sep 22, 2022

jpculp commented Sep 22, 2022

zmrow left a comment

Choose a reason for hiding this comment

jpculp commented Sep 12, 2022 •

edited

ecpullen Sep 20, 2022 •

edited