Skip to content

feat: enables support for redirection over CIRA#826

Merged
rsdmike merged 3 commits intomainfrom
ciraRedirection
Mar 9, 2026
Merged

feat: enables support for redirection over CIRA#826
rsdmike merged 3 commits intomainfrom
ciraRedirection

Conversation

@rsdmike
Copy link
Member

@rsdmike rsdmike commented Mar 6, 2026

requires go-wsman-messages v2.37+

device-management-toolkit/go-wsman-messages#649

Copilot AI review requested due to automatic review settings March 6, 2026 22:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR encapsulates the global CIRA connections map behind thread-safe accessor functions and adds support for redirection (KVM/SOL) over CIRA tunnels. Previously, the Connections map was exported and accessed directly with inconsistent locking; now it's unexported with dedicated GetConnectionEntry, SetConnectionEntry, RemoveConnection, and HasConnections functions protected by a sync.RWMutex. The new WriteToConnection method and the CIRA redirection path in Redirector.SetupWsmanClient enable routing redirection sessions through existing APF tunnels.

Changes:

  • Encapsulated the global connections map with thread-safe accessor functions and upgraded from sync.Mutex to sync.RWMutex
  • Added CIRA redirection support in Redirector.SetupWsmanClient that routes through APF tunnels using wsman.NewCIRARedirectionMessages
  • Added WriteToConnection method and ensureAPFChannelStore using sync.Once for lazy, thread-safe initialization of the APF channel store

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
internal/usecase/devices/wsman/message.go Encapsulated connections map, added exported accessors, WriteToConnection, ensureAPFChannelStore with sync.Once, and changed mutex to RWMutex
internal/usecase/devices/redirection.go Added early return path for CIRA redirection using NewCIRARedirectionMessages
internal/controller/tcp/cira/tunnel.go Migrated from direct map access to using exported accessor functions, removed local mutex
internal/controller/tcp/cira/tunnel_test.go Updated tests to use new exported accessor functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 39.79%. Comparing base (c133123) to head (4ebace2).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/controller/tcp/cira/tunnel.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #826      +/-   ##
==========================================
+ Coverage   39.76%   39.79%   +0.02%     
==========================================
  Files         114      114              
  Lines       10768    10769       +1     
==========================================
+ Hits         4282     4285       +3     
+ Misses       6087     6085       -2     
  Partials      399      399              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

management methods to handle concurrent CIRA connections
@rsdmike rsdmike enabled auto-merge (rebase) March 9, 2026 18:26
@rsdmike rsdmike force-pushed the ciraRedirection branch 2 times, most recently from c5a6058 to fa2c4a8 Compare March 9, 2026 19:43
@rsdmike rsdmike merged commit a8bbd23 into main Mar 9, 2026
17 checks passed
@rsdmike rsdmike deleted the ciraRedirection branch March 9, 2026 19:58
@RosieAMT
Copy link

RosieAMT commented Mar 9, 2026

🎉 This PR is included in version 1.22.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants