Skip to content

Cluster manager reverse tunnel#801

Merged
roosterfish merged 10 commits into
canonical:mainfrom
edlerd:websocket
Jun 25, 2026
Merged

Cluster manager reverse tunnel#801
roosterfish merged 10 commits into
canonical:mainfrom
edlerd:websocket

Conversation

@edlerd

@edlerd edlerd commented May 26, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Done

  • added reverse tunnel to cluster manager
  • proxy endpoint to expose the LXD API through the tunnel
  • expect an authorization / bearer header for auth, reply with the session that the client can cache
  • changes the CLI/config key naming from kebab-case to snake_case

Ready for review.

github-advanced-security[bot]
github-advanced-security AI found potential problems May 26, 2025
View reviewed changes
Comment thread cmd/microcloudd/cluster_manager_task.go Fixed
Comment thread cmd/microcloudd/cluster_manager_task.go Fixed
@edlerd edlerd force-pushed the websocket branch 7 times, most recently from 1d36035 to e4741bd Compare May 27, 2025 08:54
@roosterfish

roosterfish commented Jul 24, 2025

Copy link
Copy Markdown
Contributor

@edlerd can this PR be closed?

@edlerd

edlerd commented Jul 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@edlerd can this PR be closed?

We might want to use this approach at a later time, so I'd keep it open in draft state.

@edlerd edlerd force-pushed the websocket branch 3 times, most recently from 99939f7 to 0f73e50 Compare February 27, 2026 09:22
@github-actions github-actions Bot added the Documentation Documentation needs updating label Feb 27, 2026
@github-actions github-actions Bot removed the Documentation Documentation needs updating label Feb 27, 2026
@edlerd edlerd force-pushed the websocket branch 6 times, most recently from 03a8fa9 to 99d12b3 Compare March 2, 2026 17:59
github-advanced-security[bot]
github-advanced-security AI found potential problems Apr 29, 2026
View reviewed changes
Comment thread cmd/microcloudd/cluster_manager_task.go Fixed
Comment thread cmd/microcloudd/cluster_manager_task.go Fixed
@edlerd edlerd force-pushed the websocket branch 3 times, most recently from 8988de9 to f1f9f2f Compare May 28, 2026 07:46
@edlerd edlerd marked this pull request as ready for review May 28, 2026 07:47
@edlerd edlerd force-pushed the websocket branch 2 times, most recently from ff350a7 to a4c4569 Compare June 24, 2026 10:34
@edlerd

edlerd commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@roosterfish I updated microcluster v3 and adjusted to use the on stop hook for the background tasks. please give this changeset a pass at review.

roosterfish
roosterfish requested changes Jun 24, 2026
View reviewed changes

@roosterfish roosterfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for applying those changes so quickly. Just one more comment as I think we don't have to manually cancel the context inside the OnStop as this is already done by the daemon.

Comment thread cmd/microcloudd/main.go Outdated
Comment thread cmd/microcloudd/main.go Outdated
@roosterfish

roosterfish commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Did you test stopping the MicroCloud daemon to see it doesn't block for some reason we wouldn't expect? I am pretty sure we have some snap stop microcloud in the test suite but just to double check when the cluster manager routines are spawned.

@edlerd edlerd force-pushed the websocket branch 2 times, most recently from 39efd3e to c178f70 Compare June 24, 2026 11:43
@edlerd

edlerd commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Did simplifications as suggested above.

Did you test stopping the MicroCloud daemon to see it doesn't block for some reason we wouldn't expect?

Yes, a simple snap stop worked well. With and without cluster manager configured or with/without tunnel.

roosterfish
roosterfish previously approved these changes Jun 24, 2026
View reviewed changes

@roosterfish roosterfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Just a few comments for follow ups and about the flagged security issue where we might want to have a whitelist of LXD endpoints to be safe?

Comment thread cmd/microcloudd/cluster_manager_tunnel.go Outdated
Comment thread cmd/microcloudd/cluster_manager_tunnel.go Outdated
@edlerd

edlerd commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

few comments for follow ups

Addressed those just now.

about the flagged security issue where we might want to have a whitelist of LXD endpoints to be safe?

If I recall correctly, we discussed this previously, and @tomponline raised a valid concern that we would then have to maintain that list of allowed endpoints, which could become cumbersome or even blocking in the future. Hence my current approach to allow anything with /1.0 prefix and a concrete list of methods.

roosterfish
roosterfish requested changes Jun 24, 2026
View reviewed changes
Comment thread cmd/microcloudd/cluster_manager_task.go
roosterfish
roosterfish requested changes Jun 24, 2026
View reviewed changes
Comment thread api/cluster_manager.go Outdated
Comment thread cmd/microcloudd/cluster_manager_tunnel.go
edlerd added 10 commits June 24, 2026 16:57
@edlerd
api/types: Add cluster manager tunnel request/response types
5047839 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
database/cluster_manager: Refactor configuration loading and add helpers
2ee7752 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
client: Add websocket tunnel support to cluster manager client
f53111d 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
cmd/microcloudd/cluster_manager_task: Use new database helpers and im…
c75be13 
…prove error handling

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
api/cluster_manager: Update handlers for tunnel and interval configur…
164370c 
…ation

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
cmd/microcloud: Update cluster manager CLI for new configuration keys
efa3926 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
test/suites: Update cluster manager tests for new functionality
fbbe782 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
chore(deps) update microcluster
4cd0092 
Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
cmd/microcloudd/cluster_manager_tunnel: Add cluster manager reverse t…
53ba8f4 
…unnel implementation

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd
cmd/microcloudd: Register cluster manager tunnel reconciliation
540797a 
Signed-off-by: David Edler <david.edler@canonical.com>
roosterfish
roosterfish approved these changes Jun 24, 2026
View reviewed changes

@roosterfish roosterfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@roosterfish roosterfish merged commit b106576 into canonical:main Jun 25, 2026
53 of 56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@roosterfish roosterfish roosterfish approved these changes
+1 more reviewer
@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@edlerd @roosterfish @github-advanced-security