Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for terminating active SOL sessions #156

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

zevweiss
Copy link
Contributor

@zevweiss zevweiss commented Jan 9, 2024

Description

These patches add a DeactivateSOL request to the grpc API, which terminates an active SOL session for a given server using the method of the same name recently added to bmclib.

Why is this needed

Active SOL sessions may at times need to be forcefully ended so as to enforce proper access control.

How Has This Been Tested?

I started an an SOL session through a local BMC via ipmitool sol activate and a pbnj service via go run main.go server. Upon using evans to call the DeactivateSOL method, I observed that the SOL session was successfully terminated.

How are existing users impacted? What migration steps/scripts do we need?

There should be no impact to existing users, as no existing behavior is changed.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

chrisdoherty4
chrisdoherty4 previously approved these changes Jan 23, 2024
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (5660359) 81.87% compared to head (ed39ec9) 82.29%.

Files Patch % Lines
grpc/rpc/bmc.go 91.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #156      +/-   ##
==========================================
+ Coverage   81.87%   82.29%   +0.42%     
==========================================
  Files           9        9              
  Lines         491      514      +23     
==========================================
+ Hits          402      423      +21     
- Misses         75       76       +1     
- Partials       14       15       +1     

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

joelrebel
joelrebel previously approved these changes Jan 25, 2024
@chrisdoherty4 chrisdoherty4 added the ready-to-merge Signal to Mergify to merge the PR. label Jan 29, 2024
Thanks to Claudia for dealing with the go tooling problems for me...

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Note that this includes some unrelated changes in the generated code
due, I gather, to my versions of the protoc tools differing from the
ones that generated the existing versions.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
@zevweiss
Copy link
Contributor Author

Updated to use new 2.2.1 tag of bmclib instead of a commit hash.

@joelrebel joelrebel merged commit 1dd6f1e into tinkerbell:main Jan 31, 2024
6 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants