Accept v6 interfaces and interface names in whitelist [21531] #128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
juanlofer-eprosima
temporarily deployed
to
codecov
GitHub Actions
Inactive
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## integration/bump-3.0.0 #128 +/- ##
==========================================================
+ Coverage 37.79% 37.86% +0.06%
==========================================================
Files 153 153
Lines 6995 6986 -9
Branches 2806 2796 -10
==========================================================
+ Hits 2644 2645 +1
+ Misses 2922 2913 -9
+ Partials 1429 1428 -1 ☔ View full report in Codecov by Sentry. |
juanlofer-eprosima
changed the title
Signed-off-by: Juan Lopez Fernandez <[email protected]>
juanlofer-eprosima
force-pushed
the
feature/whitelist-improvements
branch
from
d6fe55c to
09cefd9
Compare
juanlofer-eprosima
temporarily deployed
to
codecov
GitHub Actions
Inactive
LuciaEchevarria99
approved these changes
Sep 6, 2024
Comment on lines
-221
to
-232
| if (types::Address::is_ipv4_correct(ip)) | ||
| { | ||
| udp_transport->interfaceWhiteList.emplace_back(ip); | ||
| EPROSIMA_LOG_INFO(DDSPIPE_COMMON_PARTICIPANT, | ||
| "Adding " << ip << " to UDP whitelist interfaces."); | ||
| } | ||
| else | ||
| { | ||
| // Invalid address, continue with next one | ||
| EPROSIMA_LOG_WARNING(DDSPIPE_COMMON_PARTICIPANT, | ||
| "Not valid IPv4. Discarding UDP whitelist interface " << ip << "."); | ||
| } |
There was a problem hiding this comment.
Future Suggestion:
Why not validate both IPv4 and IPv6 formats to ensure correctness?
-
Current behavior when an incorrect format is used:
The product continues running but doesn't function as expected, with the following FastDDS error:
[TRANSPORT_UDPV4 Error] All whitelist interfaces were filtered out. -
Expected behavior:
The product should terminate with a DDSPipe error indicating the invalid format.
There was a problem hiding this comment.
Because now that interface names are accepted, it cannot be (easily) checked from our layer whether an element is an invalid IPv4 address (due to a typo for example) or an interface name. These checks are internally performed by Fast-DDS library, and their choice is that warnings are displayed if they fail (instead of throwing an exception or similar as we would prefer to have).
LuciaEchevarria99
pushed a commit
that referenced
this pull request
Sep 10, 2024
Signed-off-by: Juan Lopez Fernandez <[email protected]>
rsanchez15
pushed a commit
that referenced
this pull request
Sep 10, 2024
* Update repository for FastDDS 3.0.0 compatibility Signed-off-by: Lucia Echevarria <[email protected]> * Fix tests and first approximation to change Dynamic Types Signed-off-by: Lucia Echevarria <[email protected]> * Fix DynTypesParticipant::on_type_discovery_ callback to register type_name-type_id tuple and some other minor changes Signed-off-by: Lucia Echevarria <[email protected]> * Change DynTypesParticipant to use RTPS Participant callbacks Signed-off-by: Lucia Echevarria <[email protected]> * Regenarate types Signed-off-by: Lucia Echevarria <[email protected]> * Add TypeIdentifiers to DdsTopic in order to pass the TypeInformation to RTPS Writer/Reader Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Regenerate types Signed-off-by: Lucia Echevarria <[email protected]> * Fix compilation after rebase to fastdds master Signed-off-by: Lucia Echevarria <[email protected]> * Continue rebasing to FastDDS master Signed-off-by: Lucia Echevarria <[email protected]> * Change .h headers to .hpp and regenerate types Signed-off-by: Lucia Echevarria <[email protected]> * Update repository with new changes in Fast DDS 3.x Signed-off-by: Lucia Echevarria <[email protected]> * Remove DynamicType to IDL serialization from DDSPipe Signed-off-by: Lucia Echevarria <[email protected]> * Delete public functions in PayloadPool inherited from IPayloadPool and add missing tests in CMake file Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Adjust to TopicDataType refactor and removal of WriterProxyData/ReaderProxyData from public API Signed-off-by: Lucia Echevarria <[email protected]> * Update after refactor in Participant discovery information Signed-off-by: Lucia Echevarria <[email protected]> * Update after TopicAttributes removal from public API Signed-off-by: Lucia Echevarria <[email protected]> * Regenerate types Signed-off-by: Lucia Echevarria <[email protected]> * Fix TopicDescription in CommonReader and CommonWriter Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Remove GuidPrefix attribute from DiscoveryServerConnectionAddress Signed-off-by: Lucia Echevarria <[email protected]> * Remove is_keyed_ method after Publication/SubscriptionBuiltinTopicData update in Fast DDS Signed-off-by: Lucia Echevarria <[email protected]> * Fix CacheChangePool after rebase Signed-off-by: Lucia Echevarria <[email protected]> * Fix tests Signed-off-by: Lucia Echevarria <[email protected]> * Fix uncrustify Signed-off-by: Lucia Echevarria <[email protected]> * Free allocated memory in PayloadPool::release_ Signed-off-by: Lucia Echevarria <[email protected]> * Fill TypeInformation in Topic Description just from complete TypeIdentifier in CommonWriter (already done in CommonReader) Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Make payload_owner=nullptr in PayloadPool::release_ before empty() to avoid assertion failure Signed-off-by: Lucia Echevarria <[email protected]> * Add assertion making sure TypeIdentifier is complete before calling add_schema Signed-off-by: Lucia Echevarria <[email protected]> * Apply suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Refactor rtps participants reckon_participant_attributes_ Signed-off-by: Lucia Echevarria <[email protected]> * Accept v6 interfaces and interface names in whitelist (#128) Signed-off-by: Juan Lopez Fernandez <[email protected]> * Make DiscoveryServer servers' guid prefix optional (#129) * Make DiscoveryServer servers' guid prefix optional Signed-off-by: Juan Lopez Fernandez <[email protected]> * Raise YAML reader version Signed-off-by: Juan Lopez Fernandez <[email protected]> * BONUS: improve participant discovery traces Signed-off-by: Juan Lopez Fernandez <[email protected]> --------- Signed-off-by: Juan Lopez Fernandez <[email protected]> * Add missing suggested changes Signed-off-by: Lucia Echevarria <[email protected]> * Correct type_kind_to_str output for TK_CHAR8 case Signed-off-by: Lucia Echevarria <[email protected]> * Remove unnecessary CacheChange include Signed-off-by: Lucia Echevarria <[email protected]> * Implement add_qos_properties_ in reckon_participant_qos_ Signed-off-by: Lucia Echevarria <[email protected]> * Remove Fast-DDS API ReturnCode explanation in PayloadPoolMediator::write Signed-off-by: Lucia Echevarria <[email protected]> * Change name ip_configuration -> initial_peers_configuration Signed-off-by: Lucia Echevarria <[email protected]> * Change name ds_configuration -> discovery_server_configuration Signed-off-by: Lucia Echevarria <[email protected]> * Use dynamic_pointer_cast instead of static_pointer_cast in reckon_participant_attributes_ Signed-off-by: Lucia Echevarria <[email protected]> * Change function descriptions in CommonParticipant Signed-off-by: Lucia Echevarria <[email protected]> * CHange char sequence message in tests Signed-off-by: Lucia Echevarria <[email protected]> * Add clarification comment in TopicDataType::compute_key(SerializedPayload_t, ...) Signed-off-by: Lucia Echevarria <[email protected]> * Fix YamlGetEntityGuidPrefixTest.get_guidprefix_explicitly test Signed-off-by: Lucia Echevarria <[email protected]> * Add comment clarifying the order of TypeIdentifiers (complete and minimal) in DdsTopic.type_identifiers Signed-off-by: Lucia Echevarria <[email protected]> * Fix Windows compilation Signed-off-by: Lucia Echevarria <[email protected]> * Uncrustify Signed-off-by: Lucia Echevarria <[email protected]> * Check if ParticipantConfiguration is nullptr after cast in reckon_participant_attributes_ Signed-off-by: Lucia Echevarria <[email protected]> * Fix uncrustify Signed-off-by: Lucia Echevarria <[email protected]> --------- Signed-off-by: Lucia Echevarria <[email protected]> Signed-off-by: Juan Lopez Fernandez <[email protected]> Co-authored-by: juanlofer-eprosima <[email protected]>
|
@Mergifyio backport 0.x |
❌ No backport have been created
Git reported the following error: |
|
@Mergifyio backport 0.x |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Sep 13, 2024
Signed-off-by: Juan Lopez Fernandez <[email protected]> (cherry picked from commit ac61a84) # Conflicts: # ddspipe_participants/src/cpp/participant/rtps/CommonParticipant.cpp
juanlofer-eprosima
added a commit
that referenced
this pull request
Sep 13, 2024
Signed-off-by: Juan Lopez Fernandez <[email protected]> (cherry picked from commit ac61a84) # Conflicts: # ddspipe_participants/src/cpp/participant/rtps/CommonParticipant.cpp
juanlofer-eprosima
added a commit
that referenced
this pull request
Sep 13, 2024
Signed-off-by: Juan Lopez Fernandez <[email protected]> (cherry picked from commit ac61a84) # Conflicts: # ddspipe_participants/src/cpp/participant/rtps/CommonParticipant.cpp Co-authored-by: juanlofer-eprosima <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the constraint of whitelist elements to be IPv4 addresses, accepting now IPv6 ones as well (for WAN and DiscoverServer participants only) and device names.