Skip to content

refactor: move async runtime and signatures fetcher to oci client [NR-520796]#2218

Merged
sigilioso merged 3 commits intofreeze-developfrom
feat/sync-oci-client
Feb 18, 2026
Merged

refactor: move async runtime and signatures fetcher to oci client [NR-520796]#2218
sigilioso merged 3 commits intofreeze-developfrom
feat/sync-oci-client

Conversation

@sigilioso
Copy link
Contributor

Summary

  • Move the tokio runtime from OCIArtifactDownloader into oci::Client, making Client a synchronous-facing wrapper that internally block_ons all async OCI operations. This removes the need for callers to manage and pass the runtime separately.
  • Integrate PublicKeyFetcher into oci::Client so that signature verification (verify_signature) fetches public keys from a JWKS URL directly, replacing the previous API that required callers to supply pre-fetched PublicKey slices.
  • Replace pull_blob with pull_blob_to_file, encapsulating file creation and sync_data inside the client instead of requiring callers to manage AsyncWrite streams.

Notes for reviewers and next steps

  • Moving the runtime to the oci-client seems counter intuitive: we need to block the current thread and wait for more tokio tasks mor often. However it is consistent with the rest of the Agent Control code and allow us to reuse already blocking components such as the public-keys fetcher.
  • We need to use the client's verify_signature method in the downloader (this will come in a follow up PR also including the changes in feat(test): add signatures to oci integration tests [NR-520796] #2212.

@sigilioso sigilioso requested a review from a team as a code owner February 18, 2026 10:37
}
impl Client {
/// Helper to build the [PublicKeyFetcher] corresponding to the client.
pub(super) fn try_build_public_key_fetcher(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are also building a fetcher for the config verifier that checks signatures:

let public_key_fetcher = PublicKeyFetcher::new(http_client);

We could consider building it outside and inject it in both (it could even be a shared reference). But I'd leave that refactor out of the scope of this PR

@sigilioso sigilioso changed the title refactor: move async runtime and signatures fetcher to oci client refactor: move async runtime and signatures fetcher to oci client [NR-520796] Feb 18, 2026
@sigilioso sigilioso force-pushed the feat/sync-oci-client branch from 63710a6 to 9c4ed9f Compare February 18, 2026 10:45
Copy link
Contributor

@vjripoll vjripoll left a comment

Choose a reason for hiding this comment

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

i think appart of this doubt everything is ok for me :)

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe here we can check here if the reference.digest() is already there and avoid one request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense! We missed that in the previous PR, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check added here: Thanks! 2d2db5e

Comment on lines 139 to 146
Copy link
Contributor

Choose a reason for hiding this comment

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

Binding to this expression seems to work without relying on an "uninitialized" separate_signer:

Suggested change
let separate_signer;
let kp_signer = match signer_position {
Some(pos) => &jwks_key_pairs[pos],
None => {
separate_signer = TestKeyPair::new(num_jwks_keys + 1);
&separate_signer
}
};
let kp_signer = match signer_position {
Some(pos) => &jwks_key_pairs[pos],
None => &TestKeyPair::new(num_jwks_keys + 1),
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified here: 2d2db5e. Thank you!

@sigilioso sigilioso requested review from a team, DavSanchez and vjripoll February 18, 2026 14:39

pub async fn pull_blob<T: AsyncWrite>(
/// Pulls the specified blob through [oci_client::Client::pull_blob] and stores it in the specified file path.
pub fn pull_blob_to_file(
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the naming and type improvements! impl AsRef<Path> 💯

/// Obtains public keys from the provided `public_key_url` and performs signature verification of the provided
/// `reference`. If verification succeeds, it returns the `reference` (identified by digest) that has been
/// verified.
pub fn verify_signature(
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

@sigilioso sigilioso merged commit 193ef66 into freeze-develop Feb 18, 2026
30 checks passed
@sigilioso sigilioso deleted the feat/sync-oci-client branch February 18, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments