Skip to content

Conversation

@xiaoyuyao
Copy link
Contributor

What changes were proposed in this pull request?

  • Change the TokenVerifier interface to check the command type and the block id.
  • Token verification based on token encode in the command done inside HddsDispatcher.
  • Remove the Grpc Client/Server CredentialInterceptor as it cannot fit into Ratis commands.
  • Added more unit test coverage on the Tokenverifier.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2321

How was this patch tested?

Added Unit test testBlockTokenVerifier()
Update Unit test in TestSecureContainerServer.java
ozone secure smoke test.

@xiaoyuyao
Copy link
Contributor Author

/retest

Copy link
Contributor

@anuengineer anuengineer left a comment

Choose a reason for hiding this comment

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

I am +1, on this patch. Some minor comments in on this version.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

This fixes ozonesecure acceptance test, which has been consistently failing on master recently. Big thanks @xiaoyuyao for that.

Note that there are some CI issues introduced by this change:

@xiaoyuyao
Copy link
Contributor Author

Thanks @adoroszlai and @anuengineer for the review. The unit test and checkstyle issues are fixed.

@xiaoyuyao
Copy link
Contributor Author

/retest

1 similar comment
@adoroszlai
Copy link
Contributor

/retest

@adoroszlai
Copy link
Contributor

Thanks @xiaoyuyao for the update.

Acceptance test failure is unrelated, it happens on master, too. Details: #11 (comment)

Integration test failure is also unrelated, already tracked in Jira (HDDS-2367 and HDDS-2392).

@xiaoyuyao
Copy link
Contributor Author

Thanks @adoroszlai for the confirmation. I will merge the PR shortly.

@xiaoyuyao xiaoyuyao closed this Nov 5, 2019
ptlrs pushed a commit to ptlrs/ozone that referenced this pull request Mar 8, 2025
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