Upgrade dependencies #233
Upgrade dependencies #233
Conversation
Submodule: github.com/googleapis/googleapis d6e96e2b2ec3f23fc9e3f84a194a56b5f5442274 Direct: github.com/lestrrat-go/jwx v1.2.29 github.com/pion/dtls/v2 v2.2.8-0.20240327211025-8244c4570c01 github.com/plgd-dev/device/v2 v2.4.5-0.20240403073803-48efa094e7b0 github.com/plgd-dev/go-coap/v3 v3.3.4-0.20240403064319-6ed2ef2c4664 github.com/plgd-dev/hub/v2 v2.17.4-0.20240403125943-fbad2f31e1ed google.golang.org/api v0.172.0 google.golang.org/grpc v1.63.0 Indirect: cloud.google.com/go v0.112.1 cloud.google.com/go/compute v1.25.1 github.com/bufbuild/protocompile v0.9.0 github.com/cenkalti/backoff/v4 v4.3.0 github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 github.com/go-co-op/gocron/v2 v2.2.9 github.com/googleapis/gax-go/v2 v2.12.3 github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0 github.com/lestrrat-go/httprc v1.0.5 github.com/lestrrat-go/jwx/v2 v2.0.21 github.com/nats-io/nats.go v1.34.1 github.com/panjf2000/ants/v2 v2.9.1 github.com/pion/transport/v3 v3.0.2 golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8 golang.org/x/mod v0.16.0 golang.org/x/net v0.23.0 golang.org/x/tools v0.19.0 google.golang.org/genproto v0.0.0-20240227224415-6ceb2ff114de google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda
Chizkiyahu/delete-untagged-ghcr-action: v3 -> v4
WalkthroughThis comprehensive update spans multiple areas, focusing on enhancing code quality, security, and maintainability. It includes updates to dependencies, the introduction of new linting rules, and improvements in error handling and code consistency. The changes aim to streamline development practices, enforce better coding standards, and ensure the project's components are up-to-date with the latest versions. Changes
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 56
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum
Files selected for processing (45)
- .github/actions/cleanup-stale/action.yaml (1 hunks)
- .golangci.yml (3 hunks)
- dependency/googleapis (1 hunks)
- pb/get_configuration.go (2 hunks)
- service/config/config.go (3 hunks)
- service/device/authenticationPreSharedKey.go (3 hunks)
- service/device/authenticationX509.go (4 hunks)
- service/device/service.go (4 hunks)
- service/grpc/clearCache.go (1 hunks)
- service/grpc/clearCache_test.go (5 hunks)
- service/grpc/createDeleteResource_test.go (2 hunks)
- service/grpc/disownDevice.go (1 hunks)
- service/grpc/getConfiguration.go (1 hunks)
- service/grpc/getDeviceResourceLinks_test.go (1 hunks)
- service/grpc/getDevice_test.go (4 hunks)
- service/grpc/getDevices.go (7 hunks)
- service/grpc/getDevices_test.go (4 hunks)
- service/grpc/getIdentityCertificate.go (1 hunks)
- service/grpc/getJSONWebKeys.go (2 hunks)
- service/grpc/getResource_test.go (3 hunks)
- service/grpc/jsonWebKeyCache.go (2 hunks)
- service/grpc/onboardDevice.go (2 hunks)
- service/grpc/ownDevice.go (3 hunks)
- service/grpc/ownDevice_test.go (2 hunks)
- service/grpc/reset.go (1 hunks)
- service/grpc/server.go (1 hunks)
- service/grpc/service.go (2 hunks)
- service/grpc/updateJSONWebKeys.go (2 hunks)
- service/grpc/updateResource_test.go (4 hunks)
- service/http/clearCache_test.go (5 hunks)
- service/http/createDeleteResource_test.go (2 hunks)
- service/http/getDeviceResourceLinks_test.go (1 hunks)
- service/http/getDevice_test.go (2 hunks)
- service/http/getDevices_test.go (1 hunks)
- service/http/getJSONWebKeys_test.go (1 hunks)
- service/http/getResource_test.go (3 hunks)
- service/http/onboardDevice_test.go (4 hunks)
- service/http/ownDevice_test.go (5 hunks)
- service/http/pskInitialize_test.go (3 hunks)
- service/http/remoteInitialize_test.go (9 hunks)
- service/http/reset_test.go (1 hunks)
- service/http/service.go (2 hunks)
- service/http/updateResource_test.go (4 hunks)
- service/service.go (2 hunks)
- test/test.go (1 hunks)
Files skipped from review due to trivial changes (3)
- .github/actions/cleanup-stale/action.yaml
- dependency/googleapis
- test/test.go
Additional comments not posted (65)
service/grpc/disownDevice.go (1)
43-43: The change to useGetOwnershipStatus()method instead of direct field access is a good practice for encapsulation and ensures that any necessary logic associated with getting the ownership status is encapsulated within the method.service/grpc/getConfiguration.go (1)
26-42: The use of getter methods and checks fornilvalues before accessing fields in theGetConfigurationmethod enhances the robustness and maintainability of the code. This approach prevents potential nil pointer dereferences and encapsulates the logic for accessing fields within their respective getter methods.service/http/getJSONWebKeys_test.go (1)
41-52: The addition of deferred closure statements to ensure the response body is closed after HTTP requests is a good practice. It helps prevent resource leaks and is particularly important in a testing environment where many requests might be made.service/grpc/createDeleteResource_test.go (1)
47-47: The replacement ofdev.Idwithdev.GetId()enhances encapsulation by using getter methods.Also applies to: 53-53, 58-58, 67-67, 73-73, 78-78
service/http/reset_test.go (2)
40-42: Properly closing response bodies using deferred closures is a good practice for resource management.Also applies to: 47-49
57-59: The updated assertions provide clearer validation of the expected state after a reset operation.service/grpc/getDevice_test.go (1)
51-51: The replacement ofdev.Idwithdev.GetId()enhances encapsulation by using getter methods.Also applies to: 57-57, 63-63, 72-72, 81-81, 90-90
service/http/createDeleteResource_test.go (1)
43-43: The replacement ofdev.Idwithdev.GetId()in HTTP requests is consistent with best practices for encapsulation.Also applies to: 53-53, 60-60, 67-67, 73-73
service/grpc/getDeviceResourceLinks_test.go (1)
55-55: The replacement ofdev.Idwithdev.GetId()for theDeviceIdfield in request and response structs is a good practice for encapsulation.Also applies to: 59-59
service/grpc/ownDevice_test.go (1)
44-44: The replacement ofdev.Idwithdev.GetId()for theDeviceIdfield in request structs is consistent with best practices for encapsulation.Also applies to: 49-49, 54-54, 76-76, 81-81, 86-86
service/grpc/updateJSONWebKeys.go (1)
51-53: The error handling for retrieving the owner from a token has been improved for clarity and specificity.service/device/authenticationPreSharedKey.go (4)
71-71: Consider adding a comment explaining the rationale behind supporting only 16-byte PSKs, as this could be important for future maintainers or users of the code.
80-80: Ensure that returningnilanderrPreSharedKeyAuthenticationinDialTLSis the intended behavior, as this effectively disables TLS dialing for pre-shared key authentication.
97-97: Add a comment explaining whyGetIdentityCSRreturnsnilanderrPreSharedKeyAuthentication, to clarify that CSR generation is not supported for PSK authentication.
101-101: Similarly, add a comment forSetIdentityCertificateto explain the decision to returnerrPreSharedKeyAuthentication, indicating that setting identity certificates is not supported for PSK authentication.service/grpc/getDevices_test.go (3)
46-46: Replacerequire.NotEmpty(t, srv.Devices)with a more specific assertion that checks for the expected number of devices, enhancing the test's precision.
57-57: Consider adding error messages to therequire.NoError(t, err)assertions to provide more context in case of test failures.
76-76: Validate the parsed URL fromdevice.GetEndpoints()[0]to ensure it is well-formed and meets expectations.service/http/clearCache_test.go (1)
64-64: Ensure that the HTTP status code is checked before reading the response body to avoid processing responses from failed requests.service/grpc/updateResource_test.go (4)
59-59: Ensure that the test case "doxm update" includes assertions to verify the expected behavior of theUpdateResourcecall.
80-80: For the "device resource - fail" test case, consider adding a comment explaining why the permission is denied, enhancing the test's readability.
114-114: In the "unknown href" test case, add assertions to validate that the error message or status code matches the expected outcome for an unknown resource.
131-131: For the "permission denied - cannot establish TLS connection" test case, it would be helpful to include a setup step that ensures TLS connection establishment fails, to make the test more self-contained and understandable.service/http/updateResource_test.go (4)
63-63: Add assertions to the "doxm update" test case to verify the response content and status code match the expected outcomes.
81-81: In the "device resource" test case, provide more context in the test name or add a comment explaining why the operation is expected to be forbidden.
103-103: For the "unknown href" test case, ensure that the test validates the specific error returned for accessing an unknown resource.
114-114: In the "forbidden - cannot establish TLS connection" test case, clarify the setup conditions that lead to the inability to establish a TLS connection.service/service.go (2)
55-56: Confirm that the removal of thectxparameter fromnewGrpcServicedoes not impact any asynchronous operations or cleanup tasks that might have relied on it.
118-118: Ensure that all calls tonewGrpcServicehave been updated to reflect the change in its function signature.Verification successful
The verification process has confirmed that all calls to
newGrpcServicewithin the codebase have been updated to reflect the change in its function signature. There are no outdated calls or discrepancies found in the provided context.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any outdated calls to newGrpcService with a context parameter. rg --type go 'newGrpcService\('Length of output: 384
pb/get_configuration.go (2)
38-38: Ensure that the getter methodGetCsrChallengeStateExpirationis implemented and correctly returns the expected value.
105-105: Similarly, verify thatGetRemoteProvisioningcorrectly retrieves theRemoteProvisioninginstance, especially in cases where it might benil.service/http/onboardDevice_test.go (6)
65-65: Ensure that theGetId()method is implemented and correctly returns the device ID.
83-83: Verify thatGetId()correctly retrieves the device ID and does not introduce any side effects.
143-147: Confirm thatGetId()is used consistently throughout the codebase for accessing device IDs.
152-152: Double-check thatGetId()provides the expected device ID in this context.
155-155: Ensure thatGetId()method accurately fetches the device ID for the offboarding process.
162-162: Confirm the correctness and consistency of usingGetId()for device identification in disowning operations.service/http/service.go (1)
129-136: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [140-140]
Be cautious when crafting HTTP redirects with user input to avoid open redirect vulnerabilities. Ensure that the redirect URL is validated against an allowlist or that user input is sanitized.
service/http/ownDevice_test.go (6)
57-57: Ensure that theGetId()method is implemented and correctly returns the device ID.
169-169: Verify thatGetId()correctly retrieves the device ID and does not introduce any side effects.
183-183: Confirm thatGetId()is used consistently throughout the codebase for accessing device IDs.
194-194: Double-check thatGetId()provides the expected device ID in this context.
204-204: Ensure thatGetId()method accurately fetches the device ID for the offboarding process.
212-212: Confirm the correctness and consistency of usingGetId()for device identification in disowning operations..golangci.yml (4)
9-12: Addinggovetsettings fornilnessandshadowenhances code quality by catching potential issues early.
55-129: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-96]
Adding new linters and modifying existing ones broadens the scope of code quality checks, contributing to a more robust and error-free codebase.
138-147: Customizing excluded directories and rules for linters is a thoughtful approach to managing linting noise and focusing efforts where they are most beneficial.
150-150: Updating the Go version to "1.20" aligns with the objective of leveraging the latest language features and improvements.service/http/remoteInitialize_test.go (12)
65-67: Using defer statements for closing response bodies is a best practice that helps prevent resource leaks.
81-83: Properly closing response bodies using defer statements enhances resource management and prevents leaks.
98-100: Employing defer statements to ensure response bodies are closed is crucial for effective resource management.
114-116: The addition of defer statements for closing response bodies is a good practice for managing resources efficiently.
121-123: Utilizing defer statements to close response bodies after HTTP requests is a commendable practice for resource conservation.
199-201: Incorporating defer statements for closing response bodies in tests is a best practice that aids in preventing resource leaks.
215-217: The use of defer statements to close response bodies after HTTP requests is a good practice for efficient resource management.
232-234: Adding defer statements to ensure response bodies are closed is crucial for effective resource management in tests.
243-245: Employing defer statements for closing response bodies in test functions is commendable for managing resources efficiently.
254-256: Utilizing defer statements to close response bodies after HTTP requests in tests is a good practice for resource conservation.
261-263: The addition of defer statements for closing response bodies in test functions is a best practice for resource management.
272-274: Incorporating defer statements to ensure response bodies are closed in tests aligns with best practices for preventing resource leaks.service/device/service.go (2)
90-90: Replacingfmt.Errorfwitherrors.Newis appropriate for simple error messages without formatting directives.
200-211: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [186-208]
The replacement of
fmt.Errorfwitherrors.Newis correctly applied for simple error messages. Ensure consistency in error handling practices across the project.service/grpc/getDevices.go (3)
175-175: Replacingfmt.Errorfwitherrors.Newis appropriate for simple error messages without formatting directives.
260-260: The replacement offmt.Errorfwitherrors.Newfor simple error messages is correctly applied.
563-563: The use of getter methods (GetUseCache,GetUseMulticast,GetUseEndpoints) instead of direct field access is a good practice for encapsulation and null safety.Also applies to: 606-606
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
fmt.Errorftoerrors.Newfor more concise error handling.Chores