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 pkcs11 support to get_security_files #66

Merged
merged 6 commits into from Apr 8, 2024

Conversation

MiguelCompany
Copy link
Contributor

@MiguelCompany MiguelCompany commented Jan 10, 2023

This implements ros2/design#332

@ruffsl
Copy link
Member

ruffsl commented Feb 14, 2023

Ping @clalancette or @mikaelarguedas ? Could you review?

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-02-16/29927/1

@SubaruArai
Copy link

@MiguelCompany any update on this pr?

Or should we wait REP-2015 to be approved first?

@MiguelCompany
Copy link
Contributor Author

@SubaruArai I don't know what the process is. I guess we should wait for REP-2015, but I am just an external contributor, and I suppose that someone in the ROS 2 team should answer that.

Maybe @clalancette or @gbiggs can clarify?

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

This PR is stuck for a while, are we planning to merge this before feature freeze? @clalancette ?

I'm missing some tests in the test_security.cpp file. Do you mind to add them?

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Copy link
Contributor Author

@ahcorde I just rebased this. Will try to add tests in the test_security.cpp file

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Copy link
Contributor Author

@ahcorde I updated the tests in test_security.cpp

rmw_dds_common/test/test_security.cpp Show resolved Hide resolved
Comment on lines 90 to 93
std::vector<std::pair<std::string, security_file_processor>>;


// Key: the security attribute
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::vector<std::pair<std::string, security_file_processor>>;
// Key: the security attribute
std::vector<std::pair<std::string, security_file_processor>>;
// Key: the security attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rmw_dds_common/src/security.cpp Show resolved Hide resolved
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@clalancette
Copy link
Contributor

clalancette commented Apr 5, 2024

Here is CI for this:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

clalancette commented Apr 8, 2024

A couple more pieces of CI, then I think this is good to go:

  • RHEL Build Status
  • Clang Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor

All of the warnings here were also in the nightlies, so going ahead and merging this one in.

@clalancette clalancette merged commit d82d23a into ros2:rolling Apr 8, 2024
3 checks passed
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.

None yet

6 participants