Skip to content

Add test support infrastructure for identity#227

Merged
hawkw merged 8 commits intomasterfrom
eliza/the-test-identity
Apr 2, 2019
Merged

Add test support infrastructure for identity#227
hawkw merged 8 commits intomasterfrom
eliza/the-test-identity

Conversation

@hawkw
Copy link
Contributor

@hawkw hawkw commented Mar 28, 2019

This branch adds a mock Identity service implementation to the proxy's
test-support module, and wires up the rest of the test support code to
allow mocking the Identity service in tests. The existing tests for the
l5d-server-id header have been updated to work with the identity
service.

Note that this branch does not add any new tests for the proxy's
behaviour with the Identity service. I was planning to add these in a
follow-up PR.

See also linkerd/linkerd2#2505

Signed-off-by: Eliza Weisma [email protected]

@hawkw hawkw self-assigned this Mar 28, 2019
@hawkw hawkw requested a review from seanmonstar March 28, 2019 20:40
)
-> Box<Future<Item = grpc::Response<pb::CertifyResponse>, Error = grpc::Status> + Send>
+ Send,
>;
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 want to note here that I did not choose this formatting, this is what rustfmt thought would look good.

Copy link
Contributor

Choose a reason for hiding this comment

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

wat

@hawkw hawkw requested a review from olix0r March 28, 2019 22:50
/// An implementation of `Strings` that reads the values from environment variables.
pub struct Env;

#[derive(Clone)]
Copy link
Member

Choose a reason for hiding this comment

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

just curious.... can this be #[cfg(test)]?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so, since the tests are compiled as the binary in test mode with the proxy as just a regular dependency.

Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

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

looks great. would love a review from @seanmonstar, too, since he's most familiar with test infra

thanks @kleimkuhler for noticing :)
Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw merged commit c83aaa0 into master Apr 2, 2019
hawkw added a commit that referenced this pull request Apr 2, 2019
Depends on #227 

This branch adds a proxy integration test for the `/ready` endpoint. The
test asserts that when a proxy has not yet had its identity certified by
the Identity service, it does not consider itself "ready", and that a it
becomes ready after its identity is certified.

See also linkerd/linkerd2#2505

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Apr 2, 2019
Depends on #227 and #228.

This branch adds a new integration test that asserts that a proxy will
refresh its identity before the specified expiration time.

I also added a `trace` log in the `identity` module that logs the actual
duration we will wait before attempting to refresh. This was useful when
debugging why this test was failing (it was because I neglected to set
the LINKERD2_PROXY_IDENTITY_MIN_REFRESH env var, and it defaulted to 10
seconds). I felt like it was useful to log the actual duration we will
wait as well as the `SystemTime` when the cert expires, but I'm happy to
back that commit off if we don't actually want that log line.

Closes linkerd/linkerd2#2505

Signed-off-by: Eliza Weisman <[email protected]>
@olix0r olix0r deleted the eliza/the-test-identity branch August 17, 2019 01:35
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.

4 participants