Add grpc-ssl-target option to CLI to override SSL target name for gRPC connections
#121
Conversation
WalkthroughA new CLI option Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Scanner/Components
participant ScanossGrpc
User->>CLI: Run command with --grpc-ssl-target
CLI->>Scanner/Components: Instantiate with grpc_ssl_target
Scanner/Components->>ScanossGrpc: Pass grpc_ssl_target to constructor
ScanossGrpc->>ScanossGrpc: Create gRPC channel (override target if set)
ScanossGrpc->>gRPC Server: Establish connection with (possibly overridden) SSL target
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…RPC connections Fix lint errors Fix lint errors
matiasdaloia
force-pushed
the
fix/mdaloia/support-override-grpc-domain-name
branch
from
309d7a1 to
c7a07fb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/scanoss/components.py (2)
53-53: Use consistent type annotation for optional parameters.For consistency with other optional parameters in this constructor, use
Optional[str] = Noneinstead ofstr = None.- grpc_ssl_target: str = None, + grpc_ssl_target: Optional[str] = None,
57-70: Update docstring to document the new parameter.The
grpc_ssl_targetparameter is missing from the docstring documentation.:param grpc_proxy: Specific gRPC proxy (optional) :param ca_cert: TLS client certificate (optional) + :param grpc_ssl_target: SSL target name override for gRPC connections (optional) :param pac: Proxy Auto-Config file (optional)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(2 hunks)src/scanoss/__init__.py(1 hunks)src/scanoss/cli.py(7 hunks)src/scanoss/components.py(2 hunks)src/scanoss/scanner.py(15 hunks)src/scanoss/scanners/container_scanner.py(1 hunks)src/scanoss/scanners/scanner_config.py(2 hunks)src/scanoss/scanossgrpc.py(5 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~13-~13: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...hanges... ## [1.25.0] - 2025-06-04 ### Added - Add grpc-ssl-target option to CLI to over...
(REPEATED_VERBS)
🪛 Pylint (3.3.7)
src/scanoss/scanner.py
[error] 32-32: Unable to import 'importlib_resources'
(E0401)
[error] 33-33: Unable to import 'progress.bar'
(E0401)
[error] 34-34: Unable to import 'progress.spinner'
(E0401)
[error] 35-35: Unable to import 'pypac.parser'
(E0401)
[warning] 43-43: Unused ScanossSettings imported from scanoss_settings
(W0611)
[refactor] 72-72: Too many arguments (38/5)
(R0913)
[refactor] 72-72: Too many positional arguments (38/5)
(R0917)
[refactor] 72-72: Too many local variables (41/15)
(R0914)
[refactor] 289-289: Too many arguments (7/5)
(R0913)
[refactor] 289-289: Too many positional arguments (7/5)
(R0917)
[refactor] 337-337: Too many local variables (19/15)
(R0914)
[refactor] 337-337: Too many branches (20/12)
(R0912)
[refactor] 337-337: Too many statements (69/50)
(R0915)
[convention] 405-405: Line too long (157/100)
(C0301)
[refactor] 537-537: Too many arguments (7/5)
(R0913)
[refactor] 537-537: Too many positional arguments (7/5)
(R0917)
[refactor] 608-608: Too many local variables (18/15)
(R0914)
[refactor] 608-608: Too many branches (19/12)
(R0912)
[refactor] 608-608: Too many statements (65/50)
(R0915)
[convention] 662-662: Line too long (151/100)
(C0301)
[convention] 671-671: Line too long (157/100)
(C0301)
[convention] 680-680: Line too long (120/100)
(C0301)
[refactor] 758-758: Too many local variables (26/15)
(R0914)
[refactor] 758-758: Too many branches (31/12)
(R0912)
[refactor] 758-758: Too many statements (85/50)
(R0915)
src/scanoss/scanossgrpc.py
[convention] 181-181: Line too long (106/100)
(C0301)
[convention] 183-183: Line too long (106/100)
(C0301)
[convention] 185-185: Line too long (119/100)
(C0301)
[convention] 186-186: Line too long (116/100)
(C0301)
[convention] 190-190: Line too long (112/100)
(C0301)
[convention] 191-191: Line too long (117/100)
(C0301)
[convention] 195-195: Line too long (114/100)
(C0301)
🔇 Additional comments (20)
src/scanoss/__init__.py (1)
25-25: LGTM! Version bump is appropriate.The minor version increment correctly reflects the addition of the new
grpc-ssl-targetfeature.CHANGELOG.md (2)
12-15: LGTM! Changelog entry properly documents the new feature.The entry follows the established format and clearly describes the new
grpc-ssl-targetCLI option functionality.🧰 Tools
🪛 LanguageTool
[grammar] ~13-~13: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...hanges... ## [1.25.0] - 2025-06-04 ### Added - Addgrpc-ssl-targetoption to CLI to over...(REPEATED_VERBS)
529-530: Version comparison links correctly updated.The changelog comparison links have been properly updated to include the new 1.25.0 version.
src/scanoss/components.py (1)
81-81: Parameter correctly passed to ScanossGrpc constructor.The
grpc_ssl_targetparameter is properly propagated to the underlying gRPC client.src/scanoss/scanners/scanner_config.py (2)
54-54: LGTM! Field properly typed and positioned.The
grpc_ssl_targetfield uses the correctOptional[str]type annotation and is logically positioned with other gRPC-related configuration fields.
73-73: Proper field initialization following established pattern.The initialization using
getattr(args, 'grpc_ssl_target', None)correctly follows the same pattern as other optional configuration fields.src/scanoss/scanners/container_scanner.py (1)
231-231:✅ Verification successful
Verify ContainerScannerConfig has grpc_ssl_target field.
The code is accessing
config.grpc_ssl_targetbut theContainerScannerConfigdataclass doesn't appear to include this field in the provided code. This could cause an AttributeError at runtime.
🏁 Script executed:
#!/bin/bash # Description: Check if ContainerScannerConfig dataclass has grpc_ssl_target field # Expected: The field should be defined in the dataclass # Search for ContainerScannerConfig definition and grpc_ssl_target field ast-grep --pattern $'@dataclass class ContainerScannerConfig: $$$ grpc_ssl_target: $_ = $_ $$$' # Also search for any grpc_ssl_target references in container scanner rg -A 5 -B 5 "grpc_ssl_target" src/scanoss/scanners/Length of output: 2444
No issues:
grpc_ssl_targetis defined in the config dataclass
TheScannerConfigdataclass insrc/scanoss/scanners/scanner_config.pydeclaresgrpc_ssl_target: Optional[str] = Noneso using
config.grpc_ssl_targetincontainer_scanner.pyis valid.src/scanoss/scanner.py (2)
99-99: LGTM! Parameter addition and propagation looks correct.The
grpc_ssl_targetparameter is properly added to the constructor and correctly passed to theScanossGrpcinstance. This enables the SSL target override functionality for the scanner.Also applies to: 173-173
25-51: Import reorganization improves code organization.The import statements have been reorganized for better clarity and consistency.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 32-32: Unable to import 'importlib_resources'
(E0401)
[error] 33-33: Unable to import 'progress.bar'
(E0401)
[error] 34-34: Unable to import 'progress.spinner'
(E0401)
[error] 35-35: Unable to import 'pypac.parser'
(E0401)
[warning] 43-43: Unused ScanossSettings imported from scanoss_settings
(W0611)
src/scanoss/scanossgrpc.py (4)
106-106: LGTM! Parameter properly added and stored.The
grpc_ssl_targetparameter is correctly added to the constructor and stored as an instance variable.Also applies to: 136-136
176-179: LGTM! Correct implementation of SSL target override.The logic properly sets the gRPC channel option
grpc.ssl_target_name_overrideonly whengrpc_ssl_targetis provided. This is the correct gRPC option for overriding SSL target names.
185-195: LGTM! All secure channels properly configured.All
grpc.secure_channelcalls have been consistently updated to include theoptions=channel_optionsparameter, ensuring the SSL target override is applied to all gRPC stubs.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 185-185: Line too long (119/100)
(C0301)
[convention] 186-186: Line too long (116/100)
(C0301)
[convention] 190-190: Line too long (112/100)
(C0301)
[convention] 191-191: Line too long (117/100)
(C0301)
[convention] 195-195: Line too long (114/100)
(C0301)
708-708: LGTM! Configuration dataclass and factory properly updated.The
GrpcConfigdataclass andcreate_grpc_config_from_argsfactory function are correctly updated to include the newgrpc_ssl_targetparameter.Also applies to: 725-725
src/scanoss/cli.py (7)
764-769: Proper registration of--grpc-ssl-targetCLI option
The new--grpc-ssl-targetflag is correctly added for all scan/grpc sub-commands and adheres to the existing argument style.
1147-1148: Propagate SSL target override to Scanner
Passinggrpc_ssl_target=args.grpc_ssl_targetinto theScannerconstructor ensures the override flows into gRPC channel creation.
1627-1628: Propagate SSL target override to Components (vulns)
TheComponentsconstructor now correctly acceptsgrpc_ssl_target, enabling SSL target override in the vulnerability lookup.
1663-1664: Propagate SSL target override to Components (semgrep)
The semgrep sub-command correctly forwardsgrpc_ssl_target=args.grpc_ssl_target.
1702-1703: Propagate SSL target override to Components (search)
The component search sub-command now includes the SSL target override parameter.
1748-1749: Propagate SSL target override to Components (versions)
The versions sub-command correctly passesgrpc_ssl_targetinto theComponentsclient.
1784-1785: Propagate SSL target override to Components (provenance)
The provenance sub-command now forwardsgrpc_ssl_targetto the underlying gRPC client.
|
|
||
| FAST_WINNOWING = False | ||
| try: | ||
| from scanoss_winnowing.winnowing import Winnowing | ||
|
|
||
| FAST_WINNOWING = True | ||
| except ModuleNotFoundError or ImportError: | ||
| except ModuleNotFoundError or ImportError: # noqa: PLW0711 |
There was a problem hiding this comment.
Fix incorrect exception syntax.
The exception handling syntax is incorrect and will cause a runtime error. The or operator should not be used in exception handling.
-except ModuleNotFoundError or ImportError: # noqa: PLW0711
+except (ModuleNotFoundError, ImportError): # noqa: PLW0711📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except ModuleNotFoundError or ImportError: # noqa: PLW0711 | |
| except (ModuleNotFoundError, ImportError): # noqa: PLW0711 |
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 58-60: Exception to catch is the result of a binary "or" operation
(W0711)
🤖 Prompt for AI Agents
In src/scanoss/scanner.py at line 58, the exception handling uses incorrect
syntax with 'or' between exceptions. Replace 'except ModuleNotFoundError or
ImportError:' with 'except (ModuleNotFoundError, ImportError):' to correctly
catch both exceptions.
Summary by CodeRabbit
New Features
Documentation
Style