[None][infra] Adjust PLC nightly to handle release scanning#13245
Merged
yuanjingx87 merged 20 commits intoNVIDIA:mainfrom May 4, 2026
Merged
[None][infra] Adjust PLC nightly to handle release scanning#13245yuanjingx87 merged 20 commits intoNVIDIA:mainfrom
yuanjingx87 merged 20 commits intoNVIDIA:mainfrom
Conversation
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThe pipeline and scanning workflow are refactored to support both branch names and commit SHAs via a new Changes
Sequence Diagram(s)sequenceDiagram
participant Jenkins as Jenkins Pipeline
participant Scan as Scanning Script<br/>(main.py)
participant ES as Elasticsearch
participant Kibana as Kibana Dashboard
participant Slack as Slack Webhook
Jenkins->>Scan: Execute with ref, scan_mode=monitor
Scan->>ES: Submit scan with build_metadata.ref
ES-->>Scan: Scan ID
Scan->>ES: Lookup last scan result
ES-->>Scan: Previous scan data
Scan->>Scan: Diff & detect new risks
alt New Risks Detected (monitor mode)
Scan->>ES: Generate dashboard URL
ES-->>Scan: Dashboard URL
Scan->>Slack: Post vulnerability alert<br/>+ dashboard link
Slack-->>Scan: Success
Scan-->>Jenkins: {"status": "unstable", "risks": [...]}
Jenkins->>Jenkins: Set build to UNSTABLE
else No New Risks
Scan-->>Jenkins: {"status": "success"}
Jenkins->>Jenkins: Build succeeds
end
sequenceDiagram
participant Jenkins as Jenkins Pipeline
participant Scan as Scanning Script<br/>(main.py)
participant ES as Elasticsearch
Jenkins->>Scan: Execute with ref, scan_mode=release
Scan->>ES: Submit scan with build_metadata.ref
ES-->>Scan: Scan ID
Scan->>ES: Lookup last scan result
ES-->>Scan: Previous scan data
Scan->>Scan: Diff & detect new risks<br/>(only_report_new_risk=true)
Scan-->>Jenkins: {"status": "success"}<br/>(no Slack post in release mode)
Jenkins->>Jenkins: Build succeeds
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jenkins/scripts/get_image_key_to_tag.py (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate the NVIDIA copyright year in this modified file.
This file is modified in this PR, but the SPDX copyright range still ends at 2024.
As per coding guidelines, "Add NVIDIA copyright header on ALL new files and update year on modified files" and "All TensorRT-LLM source files must contain an NVIDIA copyright header with the year of latest meaningful modification."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/get_image_key_to_tag.py` at line 2, The SPDX copyright header in get_image_key_to_tag.py still ends at 2024; update the copyright year range in the SPDX header line (the existing "# SPDX-FileCopyrightText: Copyright (c) 2022-2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved.") to reflect the latest modification year (e.g., 2022-2026) so the header matches the file's current modification; locate the header at the top of get_image_key_to_tag.py and replace the year range accordingly.
🧹 Nitpick comments (6)
jenkins/scripts/pulse_in_pipeline_scanning/main.py (2)
56-57:RISKY_DEPENDENCIESshould usesnake_casesince it's a mutable local variable.Per coding guidelines, UPPER_SNAKE_CASE is reserved for constants. This is a mutable list that gets appended to, so it should use lowercase naming.
♻️ Proposed naming fix
def process_result(): - RISKY_DEPENDENCIES = [] + risky_dependencies = []And update all references accordingly (lines 66, 75, 94, 113-114, 117-118, 124).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/main.py` around lines 56 - 57, In process_result(), rename the mutable local list RISKY_DEPENDENCIES to snake_case (e.g., risky_dependencies) and update every usage within the function (all appends, reads, and references) to the new name so the variable follows the coding guideline that UPPER_SNAKE_CASE is reserved for constants; ensure no global/constants are accidentally renamed and run tests/lint to verify no remaining references to RISKY_DEPENDENCIES.
56-128: Add return type annotation toprocess_result.The function returns a dictionary with a defined structure. Adding a return type improves clarity.
📝 Suggested annotation
-def process_result(): +def process_result() -> dict[str, str | list[str]]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/main.py` around lines 56 - 128, Add an explicit return type annotation to the function process_result to reflect the dict structure it returns (keys: "status", optional "detail", "risks", "dashboard_url" for unstable, or just "status" for success); update the signature of process_result to return an appropriate typing such as Dict[str, Any] or a more specific TypedDict/Union type that captures the two shapes, and import typing symbols (e.g., Dict, Any, TypedDict, Union) as needed so callers know the exact return contract.jenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py (1)
1-16: Missing NVIDIA copyright header.Per coding guidelines, all TensorRT-LLM source files must contain an NVIDIA copyright header with the year of latest meaningful modification. This file appears to be missing the required header.
📝 Proposed fix to add copyright header
+# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import os🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py` around lines 1 - 16, Add the required NVIDIA copyright header (with the year of latest meaningful modification) to the top of this source file; update the header in jenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py so the file beginning (above imports) includes the standard NVIDIA/TensorRT-LLM copyright block. Ensure the header precedes the imports and apply the same header format used across the repo so functions like post_slack_msg and constants like TRTLLM_PLC_WEBHOOK remain unchanged.jenkins/scripts/pulse_in_pipeline_scanning/utils/report.py (1)
44-50: Consider adding type annotations.The new
get_preapproved_deps_mapfunction would benefit from type annotations for clarity.📝 Suggested annotations
-def get_preapproved_deps_map(scan_type): +def get_preapproved_deps_map(scan_type: str) -> dict[str, bool]: preapproved_deps = get_latest_license_preapproved_container_deps(scan_type) - map_preapproved_deps = {} + map_preapproved_deps: dict[str, bool] = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/report.py` around lines 44 - 50, Add type annotations to get_preapproved_deps_map by annotating the parameter scan_type (e.g., str or specific union) and the return type as Dict[str, bool]; annotate the intermediate variable preapproved_deps as a List[Dict[str, Any]] (or a more specific TypedDict if available) and item as Dict[str, Any]; import typing names (Dict, List, Any) or define a TypedDict for the preapproved item returned by get_latest_license_preapproved_container_deps to make types explicit for get_preapproved_deps_map, map_preapproved_deps, and the item["s_package_name"] lookup.jenkins/scripts/pulse_in_pipeline_scanning/utils/es.py (2)
139-149: Add type annotations for consistency.The new
get_dashboard_urlfunction lacks type annotations. Adding them improves readability and IDE support.📝 Suggested type annotations
-def get_dashboard_url(build_number, branch): +def get_dashboard_url(build_number: str, branch: str) -> str:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/es.py` around lines 139 - 149, Add type annotations to the get_dashboard_url function signature and its return type for consistency and IDE support: annotate the parameters (build_number and branch) and the return as a string (e.g., build_number as int or str depending on callers, branch: str, return: str). Update the def get_dashboard_url(...) signature accordingly and adjust any callers if needed to match the chosen build_number type.
122-128: Consider simplifyingshouldtofilter/mustfor a single required condition.Using
"should"with"minimum_should_match": 1for a single clause is functionally correct but unconventional. A"filter"or"must"clause is more idiomatic for an exact match requirement.♻️ Suggested simplification
"query": { "bool": { - "should": [ - {"match": {"s_scan_type": scan_type}}, - ], - "minimum_should_match": 1, + "filter": [ + {"term": {"s_scan_type": scan_type}}, + ], } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/es.py` around lines 122 - 128, The bool query currently uses a "should" array with {"match": {"s_scan_type": scan_type}} and "minimum_should_match": 1; replace that with a single required clause (either "must" or "filter") under the same "query" -> "bool" object so the match on "s_scan_type" becomes a required condition (e.g., "must": [{"match": {"s_scan_type": scan_type}}]) and remove "minimum_should_match": 1 to simplify and make the intent idiomatic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jenkins/scripts/get_image_key_to_tag.py`:
- Around line 93-97: The current parsing of build_number in
get_image_key_to_tag.py uses int(sys.argv[2]) but allows 0 and negatives; update
the try/except block around build_number (the variable assigned from
int(sys.argv[2])) to validate that the parsed integer is a positive value (> 0)
and exit with a clear stderr message if not; keep the existing ValueError
handling and add an additional check after conversion that prints e.g. "Error:
build number must be a positive integer, got '<value>'" to stderr and calls
sys.exit(1) when build_number <= 0.
- Around line 74-87: The loop currently backs up on any non-200 and can scan
unboundedly; change it so only a 404 (artifact missing) triggers
decrement/backtrack (e.g., check if status == 404), and for any other non-200
(network/server/transient) print the error and exit non-zero immediately; also
introduce a bounded retry/backtrack limit (e.g., MAX_BACKTRACK or max_backtracks
variable) and stop after that many backtracks rather than going down to build 1;
update the logic around build_number, artifact_url, fetch_url and sys.exit calls
to implement these checks and bounds while still writing data.decode() and
exiting on successful status == 200 with data.
In `@jenkins/scripts/pulse_in_pipeline_scanning/submit_report.py`:
- Around line 217-219: The code is fetching preapproved dependencies for the
wrong scan type — get_preapproved_deps_map is called with "source_code_license"
while the current flow is handling container licenses (see trtllm_deps from
diff_licenses("container_license") and documents using s_type
"container_license"); update the get_preapproved_deps_map call to use
"container_license" so map_preapproved aligns with the container scan, ensuring
downstream filtering and document creation use the same scan type (refer to
get_preapproved_deps_map, trtllm_deps, and map_preapproved).
In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/es.py`:
- Around line 116-136: The function
get_latest_license_preapproved_container_deps currently assumes the ES response
has at least one hit and directly uses data["hits"]["hits"][0]["_source"], which
can raise IndexError; update the function to defensively check data.get("hits",
{}).get("hits", []) is non-empty before indexing, return [] if no hits, and also
guard that the retrieved data_source contains "_source" and
"nested_preapproved_deps" (return [] if missing) while keeping references to
ES_CLIENT and ES_INDEX_PREAPPROVED_BASE intact.
In `@jenkins/TensorRT_LLM_PLC.groovy`:
- Around line 86-102: The validateRef method risks shell injection by
interpolating params.ref into the sh command; to fix it, add a Groovy-side
validation (e.g., a new isValidBranchName function) that only allows a safe
character set for branch names (letters, digits, dot, underscore, hyphen, slash)
and rejects any value containing quotes or other metacharacters, use
isCommitId(ref) OR isValidBranchName(ref) before calling container and git
check-ref-format, and if the ref fails the validation call error("Invalid branch
name: '${ref}'") without invoking the shell; reference the existing isCommitId
and validateRef functions and the git check-ref-format call to locate where to
add the check.
- Around line 309-315: The postMergeArgs variable and its two builder branches
(checking params.postMergePipelineName and params.postMergeBuildNumber) are dead
code since the Python invocation of main.py doesn't use them and main.py doesn't
accept those flags; remove the postMergeArgs declaration and the two if blocks
that append to it (references to postMergeArgs, params.postMergePipelineName,
params.postMergeBuildNumber) so no unused variables remain, or alternatively if
you intend to propagate these flags, add corresponding CLI options in main.py
and include postMergeArgs in the Python invocation—prefer removing the unused
postMergeArgs code in this PR.
---
Outside diff comments:
In `@jenkins/scripts/get_image_key_to_tag.py`:
- Line 2: The SPDX copyright header in get_image_key_to_tag.py still ends at
2024; update the copyright year range in the SPDX header line (the existing "#
SPDX-FileCopyrightText: Copyright (c) 2022-2024 NVIDIA CORPORATION & AFFILIATES.
All rights reserved.") to reflect the latest modification year (e.g., 2022-2026)
so the header matches the file's current modification; locate the header at the
top of get_image_key_to_tag.py and replace the year range accordingly.
---
Nitpick comments:
In `@jenkins/scripts/pulse_in_pipeline_scanning/main.py`:
- Around line 56-57: In process_result(), rename the mutable local list
RISKY_DEPENDENCIES to snake_case (e.g., risky_dependencies) and update every
usage within the function (all appends, reads, and references) to the new name
so the variable follows the coding guideline that UPPER_SNAKE_CASE is reserved
for constants; ensure no global/constants are accidentally renamed and run
tests/lint to verify no remaining references to RISKY_DEPENDENCIES.
- Around line 56-128: Add an explicit return type annotation to the function
process_result to reflect the dict structure it returns (keys: "status",
optional "detail", "risks", "dashboard_url" for unstable, or just "status" for
success); update the signature of process_result to return an appropriate typing
such as Dict[str, Any] or a more specific TypedDict/Union type that captures the
two shapes, and import typing symbols (e.g., Dict, Any, TypedDict, Union) as
needed so callers know the exact return contract.
In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/es.py`:
- Around line 139-149: Add type annotations to the get_dashboard_url function
signature and its return type for consistency and IDE support: annotate the
parameters (build_number and branch) and the return as a string (e.g.,
build_number as int or str depending on callers, branch: str, return: str).
Update the def get_dashboard_url(...) signature accordingly and adjust any
callers if needed to match the chosen build_number type.
- Around line 122-128: The bool query currently uses a "should" array with
{"match": {"s_scan_type": scan_type}} and "minimum_should_match": 1; replace
that with a single required clause (either "must" or "filter") under the same
"query" -> "bool" object so the match on "s_scan_type" becomes a required
condition (e.g., "must": [{"match": {"s_scan_type": scan_type}}]) and remove
"minimum_should_match": 1 to simplify and make the intent idiomatic.
In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/report.py`:
- Around line 44-50: Add type annotations to get_preapproved_deps_map by
annotating the parameter scan_type (e.g., str or specific union) and the return
type as Dict[str, bool]; annotate the intermediate variable preapproved_deps as
a List[Dict[str, Any]] (or a more specific TypedDict if available) and item as
Dict[str, Any]; import typing names (Dict, List, Any) or define a TypedDict for
the preapproved item returned by get_latest_license_preapproved_container_deps
to make types explicit for get_preapproved_deps_map, map_preapproved_deps, and
the item["s_package_name"] lookup.
In `@jenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py`:
- Around line 1-16: Add the required NVIDIA copyright header (with the year of
latest meaningful modification) to the top of this source file; update the
header in jenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py so the file
beginning (above imports) includes the standard NVIDIA/TensorRT-LLM copyright
block. Ensure the header precedes the imports and apply the same header format
used across the repo so functions like post_slack_msg and constants like
TRTLLM_PLC_WEBHOOK remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3380ddec-6179-4f6e-8153-4354b2ff64ab
📒 Files selected for processing (8)
jenkins/TensorRT_LLM_PLC.groovyjenkins/scripts/get_image_key_to_tag.pyjenkins/scripts/pulse_in_pipeline_scanning/main.pyjenkins/scripts/pulse_in_pipeline_scanning/submit_preapproved.pyjenkins/scripts/pulse_in_pipeline_scanning/submit_report.pyjenkins/scripts/pulse_in_pipeline_scanning/utils/es.pyjenkins/scripts/pulse_in_pipeline_scanning/utils/report.pyjenkins/scripts/pulse_in_pipeline_scanning/utils/slack.py
💤 Files with no reviewable changes (1)
- jenkins/scripts/pulse_in_pipeline_scanning/submit_preapproved.py
tburt-nv
reviewed
Apr 24, 2026
Instead check if any of the licenses is non-permisive, checking if all licenses are permissive. Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
yuanjingx87
force-pushed
the
user/yuanjingx/update_for_release_pipeline
branch
3 times, most recently
from
cbadf44 to
130d0e8
Compare
yuanjingx87
force-pushed
the
user/yuanjingx/update_for_release_pipeline
branch
from
130d0e8 to
e1d2d90
Compare
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
yuanjingx87
force-pushed
the
user/yuanjingx/update_for_release_pipeline
branch
from
f7aebe7 to
46d82eb
Compare
tburt-nv
reviewed
May 1, 2026
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
reduce call to nspect license check api avoiding expose token in jenkins log Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
yuanjingx87
force-pushed
the
user/yuanjingx/update_for_release_pipeline
branch
from
39c4d5b to
4f61be4
Compare
tburt-nv
approved these changes
May 4, 2026
Collaborator
Author
|
/bot skip --comment "no need to run CI" |
Collaborator
|
PR_Github #46687 [ skip ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #46687 [ skip ] completed with state |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.