[21266] Example refactor: Security #5006

Carlosespicur · 2024-06-28T10:01:27Z

security folder created in examples with a modified version of hello_world, supporting security plugins.

Description

Added plugin configuration in xml file to support security plugins. Changed CLI parser to allow selection of publisher interval. README updated.

Namespaces and headers updated in all examples

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
N/A Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
N/A Any new/modified methods have been properly documented using Doxygen.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
N/A Changes are backport compatible: they do NOT break ABI nor change library core behavior.
N/A Changes are API compatible.
New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
If this is a critical bug fix, backports to the critical-only supported branches have been requested.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

Mario-DL

Good starting point !
First thing would be rebasing the branch on top of master, solving the conflicts.
Leaving some suggestions to address.

examples/cpp/security/CLIParser.hpp

examples/cpp/security/CMakeLists.txt

examples/cpp/security/PublisherApp.hpp

examples/cpp/security/SubscriberApp.hpp

examples/cpp/security/README.md

examples/cpp/security/SubscriberApp.cpp

examples/cpp/security/hello_world_profile.xml

test/examples/security.compose.yml

versions.md

Mario-DL

We are getting closer !
As we internally commented, I attach some modified files. We should:

Remove hello_world_profile.xml and place the two provided profiles there instead.
Replace the security.compose.yml with the one provided
Update the README.md with the instructions for launching the example i.e defining the CERTS_PATH and FASTDDS_DEFAULT_PROFILES_FILE for each of the shells

Note: test the example in local and double check with wireshark that we see the SEC_PREFIX, SEC_BODY submsgs

files.zip

test/examples/test_security.py

examples/cpp/security/main.cpp

Mario-DL

Please remove the old DiscoveryServerExample folder in examples/cpp/dds

Mario-DL

Please, review all modified .cpp files since there are somre headers that are not needed. There should only be headers macros and header sorted by name changes

examples/cpp/discovery_server/ClientSubscriberApp.cpp

examples/cpp/discovery_server/ClientPublisherApp.cpp

Mario-DL

Please review the rest of the examples in which any .cpp and .hpp were modified. I left the modifications just for some of them, we should do something similar on the remaining ones.

examples/cpp/security/README.md

examples/cpp/configuration/PublisherApp.cpp

examples/cpp/configuration/PublisherApp.hpp

examples/cpp/configuration/PublisherApp.cpp

examples/cpp/configuration/SubscriberApp.cpp

examples/cpp/custom_payload_pool/PublisherApp.hpp

examples/cpp/custom_payload_pool/SubscriberApp.cpp

examples/cpp/custom_payload_pool/SubscriberApp.hpp

Mario-DL

Leaving another round of suggestions, most of them are nitpicky

examples/cpp/custom_payload_pool/Application.hpp

examples/cpp/content_filter/CLIParser.hpp

examples/cpp/content_filter/CustomContentFilterFactory.hpp

examples/cpp/content_filter/PublisherApp.hpp

examples/cpp/dds/DynamicHelloWorldExample/HelloWorldPublisher.h

examples/cpp/dds/WriterLoansExample/LoanableHelloWorldPublisher.h

examples/cpp/dds/WriterLoansExample/LoanableHelloWorldSubscriber.h

examples/cpp/dds/ZeroCopyExample/LoanableHelloWorldSubscriber.h

examples/cpp/delivery_mechanisms/PubSubApp.cpp

Mario-DL

Three suggestions only

examples/cpp/dds/SecureHelloWorldExample/HelloWorldPublisher.cpp

examples/CMakeLists.txt

examples/cpp/hello_world/Application.cpp

Mario-DL

Pls, rebase on top of master to avoid conflicts.
We also need to figure out a way in the CMakeLists.txt here to add the security test only if SECURITY is ON

examples/cpp/dds/CMakeLists.txt

Mario-DL · 2024-07-11T14:48:47Z

As an idea for addressing my former comment would be adding list(FILTER examples_python_tests EXCLUDE REGEX "*security*") if security is OFF

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…MakeLists Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Mario-DL

Good job here !
Lets ask for ci run

LGTM with Green CI
Linter pending

After github ci we should check in local that building without security and launching the example tests, correctly performs.

Mario-DL

3 small changes that should make the test and uncrustify pass.
The write() method returns a retcode that is 0 on success.

I am going to check in local that test.security is not run neither built if we compile with SECURITY=OFF and EXAMPLE_TESTS=ON

examples/cpp/security/PublisherApp.cpp

examples/cpp/configuration/Application.hpp

examples/cpp/configuration/PublisherApp.hpp

test/examples/CMakeLists.txt

…sts.txt Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Mario-DL

LGTM with green CI !

Tested also that test.security is not run neither built if we compile with SECURITY=OFF, EXAMPLE_TESTS=ON and COMPILE_EXAMPLES=ON

Mario-DL

LGTM

Carlosespicur requested a review from EduPonz June 28, 2024 10:10

Carlosespicur force-pushed the feature/security_example branch 3 times, most recently from 83da20c to fa91cb1 Compare July 2, 2024 05:52

EduPonz added this to the v3.0.0 milestone Jul 3, 2024

Carlosespicur force-pushed the feature/security_example branch from 95a6922 to d0a1327 Compare July 3, 2024 12:06

Mario-DL requested changes Jul 3, 2024

View reviewed changes

Mario-DL changed the title ~~[21266] Add secured hello world example~~ [21266] Add security example Jul 3, 2024

Mario-DL changed the title ~~[21266] Add security example~~ [21266] Example refactor: Security Jul 3, 2024

Carlosespicur force-pushed the feature/security_example branch 3 times, most recently from 321b87a to 4d79da4 Compare July 8, 2024 09:30

Mario-DL requested changes Jul 9, 2024

View reviewed changes

test/examples/test_security.py Outdated Show resolved Hide resolved

test/examples/test_security.py Outdated Show resolved Hide resolved

examples/cpp/security/main.cpp Outdated Show resolved Hide resolved

Mario-DL requested changes Jul 9, 2024

View reviewed changes

examples/cpp/discovery_server/ClientSubscriberApp.cpp Show resolved Hide resolved

examples/cpp/discovery_server/ClientPublisherApp.cpp Outdated Show resolved Hide resolved

Mario-DL requested changes Jul 10, 2024

View reviewed changes

Carlosespicur force-pushed the feature/security_example branch from 74fa09c to 26a88d6 Compare July 10, 2024 08:09

Mario-DL requested changes Jul 11, 2024

View reviewed changes

Carlosespicur force-pushed the feature/security_example branch 2 times, most recently from e9ef466 to 0076d34 Compare July 11, 2024 13:45

Mario-DL requested changes Jul 11, 2024

View reviewed changes

examples/cpp/dds/SecureHelloWorldExample/HelloWorldPublisher.cpp Outdated Show resolved Hide resolved

examples/CMakeLists.txt Outdated Show resolved Hide resolved

examples/cpp/hello_world/Application.cpp Outdated Show resolved Hide resolved

Mario-DL requested changes Jul 11, 2024

View reviewed changes

examples/cpp/dds/CMakeLists.txt Outdated Show resolved Hide resolved

Carlosespicur added 7 commits July 11, 2024 16:58

Refs #21266 Add secured hello world example

7c3f4b8

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266 Modify versions.md

649fcb6

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Delete Waitset subscribers

e8d93cf

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266 Change namespaces, README and regenerate types

a5ecca2

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Update fastdds examples guards and namespaces

d89c448

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Delete namespace used in delivery_mechanism example

1ecda05

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Add corrections

f7bb785

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur added 4 commits July 11, 2024 16:58

Refs #21266: Change XML profiles and modify README

51375d3

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Delete unnecesary headers and old DiscoveryServerExample

e4f58e3

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Fix header position and README

eb4dc8b

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #21266: Remove old SecureHelloWorldExample and modify examples C…

1f74591

…MakeLists Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur force-pushed the feature/security_example branch from 3ccaaba to 1f74591 Compare July 11, 2024 15:03

Mario-DL approved these changes Jul 11, 2024

View reviewed changes

Mario-DL self-requested a review July 11, 2024 15:10

github-actions bot added the ci-pending PR which CI is running label Jul 11, 2024

Mario-DL requested changes Jul 12, 2024

View reviewed changes

examples/cpp/security/PublisherApp.cpp Outdated Show resolved Hide resolved

examples/cpp/configuration/Application.hpp Outdated Show resolved Hide resolved

examples/cpp/configuration/PublisherApp.hpp Outdated Show resolved Hide resolved

Mario-DL requested changes Jul 12, 2024

View reviewed changes

test/examples/CMakeLists.txt Outdated Show resolved Hide resolved

Refs #21266: Add changes for uncrustify and fix test/examples/CMakeLi…

6999b95

…sts.txt Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Mario-DL approved these changes Jul 12, 2024

View reviewed changes

Mario-DL self-requested a review July 12, 2024 07:16

Mario-DL added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Jul 12, 2024

Mario-DL approved these changes Jul 12, 2024

View reviewed changes

EduPonz merged commit 4e8dd38 into master Jul 12, 2024
15 of 17 checks passed

EduPonz deleted the feature/security_example branch July 12, 2024 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[21266] Example refactor: Security #5006

[21266] Example refactor: Security #5006

Carlosespicur commented Jun 28, 2024 •

edited by Mario-DL

Loading

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL left a comment

Mario-DL commented Jul 11, 2024 •

edited

Loading

Mario-DL left a comment •

edited

Loading

Mario-DL left a comment

Mario-DL left a comment •

edited

Loading

Mario-DL left a comment

[21266] Example refactor: Security #5006

[21266] Example refactor: Security #5006

Conversation

Carlosespicur commented Jun 28, 2024 • edited by Mario-DL Loading

Description

Contributor Checklist

Reviewer Checklist

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL commented Jul 11, 2024 • edited Loading

Mario-DL left a comment • edited Loading

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment • edited Loading

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Carlosespicur commented Jun 28, 2024 •

edited by Mario-DL

Loading

Mario-DL commented Jul 11, 2024 •

edited

Loading

Mario-DL left a comment •

edited

Loading

Mario-DL left a comment •

edited

Loading