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 delegated identity API support to spire-api package #43

Conversation

EItanya
Copy link
Collaborator

@EItanya EItanya commented Aug 14, 2023

No description provided.

@EItanya EItanya marked this pull request as ready for review August 14, 2023 15:58
Signed-off-by: Eitan Yarmush <[email protected]>
@maxlambrecht maxlambrecht self-requested a review August 14, 2023 16:29
Copy link
Owner

@maxlambrecht maxlambrecht left a comment

Choose a reason for hiding this comment

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

Thanks for this work, @EItanya!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@@ -15,10 +15,25 @@ categories = ["cryptography"]
keywords = ["SPIFFE", "SPIRE"]

[dependencies]
spiffe = { version = "0.3.1", path = "../spiffe" }
bytes = { version = "1", features = ["serde"] }
spiffe = { path = "../spiffe" }
Copy link
Owner

Choose a reason for hiding this comment

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

Considering that the crates in this repository will be versioned independently, it would be prudent to pin the spiffe dependency to a specific version when publishing the spire-api crate. This ensures that anyone using a published version of the spire-api crate will receive a consistent and known-good version of the spiffe crate. By explicitly specifying the version, we maintain control over the compatibility of the dependencies.

spire-api/Cargo.toml Outdated Show resolved Hide resolved
spire-api/src/agent/delegated_identity.rs Outdated Show resolved Hide resolved
/// * An error occurs while setting up the stream.
///
/// Individual stream items might also be errors if there's an issue processing the response for a specific update.
pub async fn stream_x509_svids(
Copy link
Owner

Choose a reason for hiding this comment

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

I like this name for the streaming method. 👌

spire-api/src/selectors.rs Show resolved Hide resolved
spire-api/src/selectors.rs Outdated Show resolved Hide resolved
spire-api/src/selectors.rs Show resolved Hide resolved
spire-api/src/selectors.rs Outdated Show resolved Hide resolved
&spiffe::spiffe_id::TrustDomain::new("example.org".as_ref())
.expect("Failed to parse trust domain ="),
)
.expect("Failed to get bundle");
}
Copy link
Owner

Choose a reason for hiding this comment

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

There should be tests exercising the streaming methods.

@EItanya EItanya force-pushed the feature/delegated-identity-spire-api branch 2 times, most recently from 55456ec to e2e4e50 Compare August 15, 2023 13:24
@EItanya EItanya force-pushed the feature/delegated-identity-spire-api branch from e2e4e50 to 274781a Compare August 15, 2023 13:25
Copy link
Owner

@maxlambrecht maxlambrecht left a comment

Choose a reason for hiding this comment

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

🎉

@maxlambrecht maxlambrecht merged commit efeac42 into maxlambrecht:spire-api Aug 15, 2023
4 checks passed
maxlambrecht added a commit that referenced this pull request Aug 21, 2023
* Refactor repository: new `spiffe` folder, new `spire-api` crate, and `spire-api-sdk` submodule (#42)

Signed-off-by: Max Lambrecht <[email protected]>

* Add delegated identity API support to spire-api package (#43)

Signed-off-by: Eitan Yarmush <[email protected]>

* Refactoring `spiffe` crate: Introducing `spiffe-types` and `workload-api` features (#44)

Signed-off-by: Max Lambrecht <[email protected]>

* Add Releasing Process Documentation for Crates (#45)

Signed-off-by: Max Lambrecht <[email protected]>

---------

Signed-off-by: Max Lambrecht <[email protected]>
Co-authored-by: Eitan Yarmush <[email protected]>
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.

2 participants