-
Notifications
You must be signed in to change notification settings - Fork 26
Create gRPC service to be used by the CLI #855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
371a5e8
to
5f50f59
Compare
5f50f59
to
06df31f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #855 +/- ##
==========================================
- Coverage 85.50% 85.50% -0.01%
==========================================
Files 82 82
Lines 5707 5690 -17
Branches 109 109
==========================================
- Hits 4880 4865 -15
+ Misses 772 770 -2
Partials 55 55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions that I think will make the changes better (I didn't add the same comment on every single crate of the module, but some apply to all three)
9ddf755
to
0fdb6bc
Compare
Converting to draft until I fixed the tests |
8c14998
to
4b6c6fb
Compare
e2cf75f
to
aaf77bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Nice work! Once @3v1n0 is also happy, feel free to merge.
61b1d8d
to
69dc5ec
Compare
Thanks @adombeck for the awesome work. |
Can we get this merged soon? :) |
Any update @adombeck? |
@shiv-tyagi Sorry, we had other priorities the last two weeks. I hope we will get this merged next week! |
No worries. Looking forward :-) |
There was a problem hiding this 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 migrates CLI-specific gRPC methods from the NSS service into a dedicated UserService. The key changes include renaming and refactoring of service packages and method calls, removal of the old NSS service implementation, and updates to gRPC proto definitions and client registrations.
Reviewed Changes
Copilot reviewed 36 out of 46 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
internal/services/user/permissions.go | Renamed package from "nss" to "user" to reflect the new service context. |
internal/services/permissions.go | Updated global access check to use userService instead of nssService. |
internal/services/nss/nss.go | Removed the entire NSS service implementation that is no longer needed. |
internal/services/manager_test.go | Updated tests to work with the new UserService client instead of the deprecated NSS client. |
internal/services/manager.go | Replaced references to nssService with userService, including gRPC registration changes. |
internal/proto/authd/authd_grpc.pb.go | Updated gRPC service definitions and client implementations from NSS to UserService. |
internal/proto/authd/authd.proto | Removed the NSS service definitions and added UserService definitions with updated message types. |
Files not reviewed (10)
- internal/services/nss/testdata/golden/TestGetGroupEntries/Return_all_groups: Language not supported
- internal/services/nss/testdata/golden/TestGetGroupEntries/Return_no_groups: Language not supported
- internal/services/nss/testdata/golden/TestGetShadowByName/Return_existing_user: Language not supported
- internal/services/nss/testdata/golden/TestGetShadowEntries/Return_all_users: Language not supported
- internal/services/nss/testdata/golden/TestGetShadowEntries/Return_no_users: Language not supported
- internal/services/testdata/TestRegisterGRPCServices/golden: Language not supported
- internal/services/testdata/golden/TestRegisterGRPCServices: Language not supported
- internal/services/user/testdata/golden/TestGetGroupByID/Return_existing_group: Language not supported
- internal/services/user/testdata/golden/TestGetGroupByName/Return_existing_group: Language not supported
- internal/services/user/testdata/golden/TestGetUserByID/Return_existing_user: Language not supported
f472873
to
82a3b47
Compare
The NSS service duplicated functionality from the user service.
I rebased on main and resolved the conflicts |
@3v1n0 anything still missing for your approval? Do you want to take another look? |
82a3b47
to
62ffae0
Compare
The current state of #782 adds the gRPC methods used by the CLI to the NSS service. They don't belong there, so this PR adds a new gRPC service which provides methods intended for use by the CLI.
UDENG-6492