Unverified Commit bf678fd7 authored by Luca Palmieri's avatar Luca Palmieri Committed by GitHub
Browse files

[Release workflow] Workaround for the ownership bug (#2318) 

* First publish everything, then try to correct ownership.

* Avoid trying to remove teams, since we know it won't succeed.

* Fix warning and update dependencies.

* Avoid allocation unless necessary.

* Get full error representation for retryable failures.

* We want to hash the file content, not the file path.
parent a3075ea5

tools/ci-build/publisher/Cargo.lock

+253 −200

File changed.

Preview size limit exceeded, changes collapsed.

tools/ci-build/publisher/src/retry.rs

+1 −1
Original line number Diff line number Diff line
@@ -51,7 +51,7 @@ where 
                    }

                    ErrorClass::Retry => {

                        info!(

                            "{} failed on attempt {} with retryable error: {}. Will retry after {:?}",

                            "{} failed on attempt {} with retryable error: {:?}. Will retry after {:?}",

                            what, attempt, err.into(), backoff

                        );

                    }

tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs

+3 −6
Original line number Diff line number Diff line
@@ -197,7 +197,7 @@ fn hash_models(projection: &SmithyBuildProjection) -> Result<String> { 
    // Must match `hashModels` in `CrateVersioner.kt`

    let mut hashes = String::new();

    for import in &projection.imports {

        hashes.push_str(&sha256::digest_file(import).context("hash model")?);

        hashes.push_str(&sha256::try_digest(import.as_path())?);

        hashes.push('\n');

    }

    Ok(sha256::digest(hashes))
@@ -217,7 +217,7 @@ impl SmithyBuildRoot { 


#[derive(Debug, Deserialize)]

struct SmithyBuildProjection {

    imports: Vec<String>,

    imports: Vec<PathBuf>,

}



#[cfg(test)]
@@ -348,10 +348,7 @@ mod tests { 
        fs::write(&model1b, "bar").unwrap();



        let hash = hash_models(&SmithyBuildProjection {

            imports: vec![

                model1a.to_str().unwrap().to_string(),

                model1b.to_str().unwrap().to_string(),

            ],

            imports: vec![model1a, model1b],

        })

        .unwrap();

tools/ci-build/publisher/src/subcommand/publish.rs

+26 −11
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ pub async fn subcommand_publish( 
    // Don't proceed unless the user confirms the plan

    confirm_plan(&batches, stats, *skip_confirmation)?;



    for batch in batches {

    for batch in &batches {

        let mut any_published = false;

        for package in batch {

            // Only publish if it hasn't been published yet.
@@ -65,13 +65,12 @@ pub async fn subcommand_publish( 
                // Sometimes it takes a little bit of time for the new package version

                // to become available after publish. If we proceed too quickly, then

                // the next package publish can fail if it depends on this package.

                wait_for_eventual_consistency(&package).await?;

                info!("Successfully published `{}`", package.handle);

                wait_for_eventual_consistency(package).await?;

                info!("Successfully published `{}`", &package.handle);

                any_published = true;

            } else {

                info!("`{}` was already published", package.handle);

                info!("`{}` was already published", &package.handle);

            }

            correct_owner(&package.handle, &package.category).await?;

        }

        if any_published {

            info!("Sleeping 30 seconds after completion of the batch");
@@ -81,6 +80,12 @@ pub async fn subcommand_publish( 
        }

    }



    for batch in &batches {

        for package in batch {

            correct_owner(&package.handle, &package.category).await?;

        }

    }



    Ok(())

}
@@ -142,6 +147,11 @@ async fn wait_for_eventual_consistency(package: &Package) -> Result<()> { 


/// Corrects the crate ownership.

pub async fn correct_owner(handle: &PackageHandle, category: &PackageCategory) -> Result<()> {

    // https://github.com/orgs/awslabs/teams/smithy-rs-server

    const SMITHY_RS_SERVER_OWNER: &str = "github:awslabs:smithy-rs-server";

    // https://github.com/orgs/awslabs/teams/rust-sdk-owners

    const RUST_SDK_OWNER: &str = "github:awslabs:rust-sdk-owners";



    run_with_retry(

        &format!("Correcting ownership of `{}`", handle.name),

        3,
@@ -151,7 +161,7 @@ pub async fn correct_owner(handle: &PackageHandle, category: &PackageCategory) - 
            let expected_owners = expected_package_owners(category, &handle.name);



            let owners_to_be_added = expected_owners.difference(&actual_owners);

            let incorrect_owners = actual_owners.difference(&expected_owners);

            let owners_to_be_removed = actual_owners.difference(&expected_owners);



            let mut added_individual = false;

            for crate_owner in owners_to_be_added {
@@ -162,21 +172,26 @@ pub async fn correct_owner(handle: &PackageHandle, category: &PackageCategory) - 
                // Teams in crates.io start with `github:` while individuals are just the GitHub user name

                added_individual |= !crate_owner.starts_with("github:");

            }

            for incorrect_owner in incorrect_owners {

            for crate_owner in owners_to_be_removed {

                // Trying to remove them will result in an error due to a bug in crates.io

                // Upstream tracking issue: https://github.com/rust-lang/crates.io/issues/2736

                if crate_owner == SMITHY_RS_SERVER_OWNER || crate_owner == RUST_SDK_OWNER {

                    continue;

                }

                // Adding an individual owner requires accepting an invite, so don't attempt to remove

                // anyone if an owner was added, as removing the last individual owner may break.

                // The next publish run will remove the incorrect owner.

                if !added_individual {

                    cargo::RemoveOwner::new(&handle.name, incorrect_owner)

                    cargo::RemoveOwner::new(&handle.name, crate_owner)

                        .spawn()

                        .await

                        .context(format!("remove incorrect owner `{}` from crate `{}`", incorrect_owner, handle))?;

                        .with_context(|| format!("remove incorrect owner `{}` from crate `{}`", crate_owner, handle))?;

                    info!(

                        "Removed incorrect owner `{}` from crate `{}`",

                        incorrect_owner, handle

                        crate_owner, handle

                    );

                } else {

                    info!("Skipping removal of incorrect owner `{}` from crate `{}` due to new owners", incorrect_owner, handle);

                    info!("Skipping removal of incorrect owner `{}` from crate `{}` due to new owners", crate_owner, handle);

                }

            }

            Result::<_, BoxError>::Ok(())