Fix TransferLifeCycle SciTag boundary and FQAN fallback mapping

[ShawnMcKee]

Summary

This PR fixes three firefly marker eligibility issues in TransferLifeCycle.

1. Primary SciTag boundary bug:

• Before: valid transfer tag value 64 was rejected by <= 64 check.
• After: range validation is 64..65535 inclusive.

2. Fallback FQAN normalization bug:

• Before: fallback lookup used raw vo.getGroup() values (for example /atlas/usatlas) against plain VO keys (atlas).
• After: fallback normalizes group path by stripping a leading slash and taking the first path component.

3. Excludes semantics bug:

• Before: excludes were applied when only the source endpoint matched an excluded subnet.
• After: excludes are applied only when both source and destination endpoints match excluded subnets.

Code Changes

• modules/dcache/src/main/java/org/dcache/pool/movers/TransferLifeCycle.java
  • Fix SciTag range validation.
  • Normalize FQAN group for fallback VO mapping.
  • Require both endpoints to match pool.firefly.excludes before suppressing a marker.
• modules/dcache/src/test/java/org/dcache/pool/movers/TransferLifeCycleTest.java
  • Add regression tests for SciTag boundary and FQAN fallback behavior.
  • Add regression tests for excludes behavior (both excluded vs source-only/destination-only).

Tests

Executed:

• mvn -pl modules/dcache -am -Dtest=org.dcache.pool.movers.TransferLifeCycleTest -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false test

Result:

• Tests run: 6
• Failures: 0
• Errors: 0
• Skipped: 0

UMFS19 Runtime Verification

Verified on deployed 11.2.1 RPM runtime after replacing dcache-core jar and restarting pool domains:

• dcache@umfs19Domain_1.service active
• dcache@umfs19Domain_2.service active

Applied full excludes configuration:

• pool.firefly.excludes=10.10.0.0/16,192.41.230.0/23,192.41.236.0/23,2001:48a8:68f7:1::/50,2001:48a8:68f7:8001::/50

Installed-runtime matrix check:

• neither excluded => marker sent
• destination-only excluded => marker sent
• source-only excluded => marker sent
• both excluded => marker suppressed

Live capture verification (udp dst port 10514, 3 minutes):

• udp_packets_total=3
• non_excluded_network_payloads=3
• both_endpoints_excluded_payloads=0
• sample non-excluded src=2001:4118:1a:2c10:7ec2:55ff:fe6b:9705

This confirms markers are still emitted for transfers involving non-excluded networks while suppressing excluded-to-excluded transfers.

Operational Impact

• No mandatory end-user configuration changes are required after deploying this fix.
• Sites can keep standard pool.firefly.vo-mapping entries (temporary slash-prefixed workaround entries can be removed after upgrade).

[DmitryLitvintsev]

Think this comment here on this line will be confusing (as we never get here if scitag is present). Suggest:

comment just afert line

String scitag = request.getHeader(HEADER_SCITAG)

somethig like

// if defined SciTag is always returned (or takes precedence) 

I possible impovement (to avoi repetition) would be:

 private static final String[] SCITAG_HEADERS = { “SciTag”, “TransferHeaderSciTag};
... 

 private String readTransferTag(HttpServletRequest request) {
    for (String header: SCITAG_HEADERS) { 
            String tag = request.getHeader(header); 
            if (tag != null && !tag.isBlank()) {
                String trimmed = tag.trim();
                if (LOGGER.isDebugEnabled()) {
                    LOGGER.debug(“{} header found: {} (from client={})",
                          header, trimmed, request.getRemoteAddr());
                }
                return trimmed;
            }
  }

[kofemann]

Hi @ShawnMcKee. Thanks for the contribution. Please sigh your commits with the Signed-off-by tag as described in the CONTRIBUTING.md

[ShawnMcKee]

Thanks for the detailed review. I have addressed the requested updates:

• Refactored SciTag parsing to reduce repetition and made precedence explicit (SciTag is checked first).
• Replaced transfer-tag magic numbers with named constants.
• Updated fallback VO extraction to use Splitter-based parsing.
• Added Signed-off-by to all commits to satisfy contributor requirements (history was rewritten and force-pushed).

The sync check is now passing.
Please take another look when you have a moment.

[DmitryLitvintsev]

may be something along these lines:

  String groupPath = vo.getGroup();
  if (groupPath == null || groupPath.isBlank()) { 
     return OptionalInt.empty();
  }
  groupPath = groupPath.toLowerCase();
  String voName =  FQAN_GROUP_SPLITTER.splitToList(groupPath).get(0);

to avoid too much fussing with checking arrays. It is my understanding that

FQAN_GROUP_SPLITTER.splitToList(groupPath).get(0)

will never fail as long as there is non-empty string as input.

also, just curious, what is the significance of lowering the case? These strings are not supposed to be ever capitalized? (IOW there is RFC for that?)

[ShawnMcKee]

Hi Dmitry, We lower-cased the strings to fix some issues we had early on with some implementations using capital letters. We don't really have use for mixed-case values. I am happy to change as you noted. It is a bit more logical in how it is processed. Thanks,
Shawn

[DmitryLitvintsev]

Looks good to me.

[ShawnMcKee]

OK, our target for this functionality would be 11.2.2 if possible. Also I am working on another PR, also targeting 11.2.2. The problem is that fireflies, when enabled, are intended to be sent to the same destination as the flow they are identifying. This allows any network along the path to discover the data flow owner and purpose. For ease of accounting we also want to support sending an optional firefly to a regional or global collector. This is not yet implemented in 11.2 but I will make a PR to do so.

[kofemann]

Looks good to me. Shall I merge?