RDKB-62988 [GitHub Coverity] Enable Coverity Scan for cable-modem-agent using Native Build Integration#24
RDKB-62988 [GitHub Coverity] Enable Coverity Scan for cable-modem-agent using Native Build Integration#24Deepak-Rathn wants to merge 10 commits intodevelopfrom
Conversation
…nt using Native Build Integration Reason for change: Native build and Github Action Workflow Changes for Enabling Coverity Scan for github cable-modem-agent Test Procedure: verify able-modem-agent dependency and build native build and using Github workflow Risks: Low Priority: P1 Signed-off-by: Deepak.M <deepak_m@comcast.com>
Signed-off-by: dm097 <deepak_m@comcast.com>
test native-build.yml
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This pull request adds native build integration and GitHub Actions workflow to enable Coverity scan for the cable-modem-agent component. It introduces a generic, reusable build system that uses JSON configuration to manage dependencies and build settings, making it easier to set up Coverity analysis in CI/CD pipelines.
Changes:
- Generic bash build scripts for dependency setup, component building, and orchestration
- JSON-based configuration file defining all dependencies and build settings
- GitHub Actions workflow for automated native builds
- Comprehensive documentation explaining the build system usage
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| cov_docker_script/setup_dependencies.sh | Main script that clones dependencies, copies headers, and builds required libraries |
| cov_docker_script/common_external_build.sh | Orchestration script that runs the complete 2-step build pipeline |
| cov_docker_script/common_build_utils.sh | Shared utility library with functions for building, cloning, and patching |
| cov_docker_script/build_native.sh | Script for building the native component after dependencies are set up |
| cov_docker_script/component_config.json | JSON configuration defining dependencies, headers, patches, and build settings |
| cov_docker_script/README.md | Comprehensive documentation for the build system with usage examples |
| .github/workflows/native-build.yml | GitHub Actions workflow for automated native builds in CI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| { | ||
| "name": "rdkb-halif-dhcp", | ||
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", |
There was a problem hiding this comment.
The repository URL "https://github.com/rdkcentral/rdkb-halif-vlan.git" doesn't match the name "rdkb-halif-dhcp". This should likely be "https://github.com/rdkcentral/rdkb-halif-dhcp.git" to match the intended dependency.
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", | |
| "repo": "https://github.com/rdkcentral/rdkb-halif-dhcp.git", |
| { | ||
| "name": "rdkb-halif-dhcp", | ||
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", | ||
| "branch": "main", |
There was a problem hiding this comment.
Inconsistent indentation using tab characters instead of spaces. The rest of the JSON file uses spaces for indentation (2 spaces). This line uses a tab character which can cause parsing issues and inconsistency.
| mkdir -p "$build_dir" | ||
|
|
||
| step "Running cmake" | ||
| if ! eval "cmake -S . -B $build_dir $cmake_flags"; then |
There was a problem hiding this comment.
Using eval on cmake_flags read from JSON without sanitization is a security risk. If an attacker can modify the config file, they can inject malicious commands. Consider implementing input validation or using safer alternatives to eval.
| "name": "rdkb-halif-vlan", | ||
| "repo": "https://github.com/rdkcentral/rdkb-halif-cm.git", |
There was a problem hiding this comment.
The repository name "rdkb-halif-vlan" doesn't match the actual repository URL which points to "rdkb-halif-cm". This appears to be a copy-paste error and will cause the wrong dependency to be cloned.
cov_docker_script/build_native.sh
Outdated
| log "copying python files generic..." | ||
| copy_python_files_generic |
There was a problem hiding this comment.
The function "copy_python_files_generic" is called but is not defined anywhere in this script or in the sourced common_build_utils.sh. This will cause the script to fail at runtime with a "command not found" error.
cov_docker_script/build_native.sh
Outdated
|
|
||
| # Find and copy all library files (shared objects, static, libtool archives) | ||
| find "$COMPONENT_DIR" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$LIB_PATH/" \; 2>/dev/null || true |
There was a problem hiding this comment.
The find command searches the entire component directory recursively for library files, which could be inefficient for large codebases. Consider limiting the search depth or specifying known build output directories (e.g., ".libs", "build", ".") to improve performance.
| # Find and copy all library files (shared objects, static, libtool archives) | |
| find "$COMPONENT_DIR" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$LIB_PATH/" \; 2>/dev/null || true | |
| # Prefer searching known build output directories to avoid scanning the entire | |
| # component tree, which can be expensive on large codebases. | |
| local search_dirs=() | |
| local maxdepth_args=() | |
| # Use configured build directory if available | |
| local build_dir | |
| build_dir=$(jq -r '.native_component.build.build_dir // empty' "$CONFIG_FILE") | |
| if [[ -n "$build_dir" && -d "$COMPONENT_DIR/$build_dir" ]]; then | |
| search_dirs+=("$COMPONENT_DIR/$build_dir") | |
| fi | |
| # Common native build output directories | |
| if [[ -d "$COMPONENT_DIR/.libs" ]]; then | |
| search_dirs+=("$COMPONENT_DIR/.libs") | |
| fi | |
| if [[ -d "$COMPONENT_DIR/build" && "$COMPONENT_DIR/build" != "${search_dirs[0]:-}" ]]; then | |
| search_dirs+=("$COMPONENT_DIR/build") | |
| fi | |
| # Fallback: shallow search from component root if no known output dirs exist | |
| if ((${#search_dirs[@]} == 0)); then | |
| search_dirs+=("$COMPONENT_DIR") | |
| maxdepth_args=(-maxdepth 3) | |
| fi | |
| # Find and copy all library files (shared objects, static, libtool archives) | |
| find "${search_dirs[@]}" "${maxdepth_args[@]}" \ | |
| \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \ | |
| \( -type f -o -type l \) -exec cp -Pv {} "$LIB_PATH/" \; 2>/dev/null || true |
cov_docker_script/build_native.sh
Outdated
| # Expand variables safely | ||
| command=$(eval echo "$command") | ||
|
|
||
| log " [$((i+1))/$cmd_count] $description" | ||
| if eval "$command"; then |
There was a problem hiding this comment.
Using eval on user-provided configuration data is a security risk. The command is read from the JSON config file and evaluated directly without sanitization. If an attacker can modify the config file, they can execute arbitrary commands. Consider using safer alternatives or implementing input validation.
| # Expand variables safely | |
| command=$(eval echo "$command") | |
| log " [$((i+1))/$cmd_count] $description" | |
| if eval "$command"; then | |
| log " [$((i+1))/$cmd_count] $description" | |
| if bash -c "$command"; then |
cov_docker_script/README.md
Outdated
| ### Example: Migrating from Utopia to CcspPandM | ||
|
|
||
| ```bash | ||
| # 1. Copy scripts to CcspPandM | ||
| cp utopia/cov_docker_script/*.sh ccsp-p-and-m/cov_docker_script/ | ||
|
|
||
| # 2. Create ccsp-p-and-m/cov_docker_script/component_config.json | ||
| # Update: component name, dependencies, build settings | ||
|
|
||
| # 3. Run build | ||
| cd ccsp-p-and-m/cov_docker_script |
There was a problem hiding this comment.
The example shows migrating scripts from "utopia" to "ccsp-p-and-m", but this PR is for "cable-modem-agent". While the example is generic, it would be more consistent with the PR context to use "cable-modem-agent" as the example, or at least clarify that these are generic examples.
| ### Example: Migrating from Utopia to CcspPandM | |
| ```bash | |
| # 1. Copy scripts to CcspPandM | |
| cp utopia/cov_docker_script/*.sh ccsp-p-and-m/cov_docker_script/ | |
| # 2. Create ccsp-p-and-m/cov_docker_script/component_config.json | |
| # Update: component name, dependencies, build settings | |
| # 3. Run build | |
| cd ccsp-p-and-m/cov_docker_script | |
| ### Example: Migrating from Utopia to cable-modem-agent | |
| ```bash | |
| # 1. Copy scripts to cable-modem-agent | |
| cp utopia/cov_docker_script/*.sh cable-modem-agent/cov_docker_script/ | |
| # 2. Create cable-modem-agent/cov_docker_script/component_config.json | |
| # Update: component name, dependencies, build settings | |
| # 3. Run build | |
| cd cable-modem-agent/cov_docker_script |
| dst_dir=$(expand_path "$dst_dir") | ||
| mkdir -p "$dst_dir" | ||
|
|
||
| log "Copying libraries to $dst_dir" | ||
| find "$src_dir" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$dst_dir/" \; 2>/dev/null || true |
There was a problem hiding this comment.
The find command searches the entire src_dir recursively for library files, which could be inefficient. Consider limiting the search depth or specifying known build output directories to improve performance, especially when src_dir is a large repository root.
| dst_dir=$(expand_path "$dst_dir") | |
| mkdir -p "$dst_dir" | |
| log "Copying libraries to $dst_dir" | |
| find "$src_dir" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$dst_dir/" \; 2>/dev/null || true | |
| src_dir=$(expand_path "$src_dir") | |
| dst_dir=$(expand_path "$dst_dir") | |
| mkdir -p "$dst_dir" | |
| # Prefer common build output subdirectories when present to avoid scanning entire trees | |
| local search_root="$src_dir" | |
| if [ -d "$src_dir/lib" ]; then | |
| search_root="$src_dir/lib" | |
| elif [ -d "$src_dir/build" ]; then | |
| search_root="$src_dir/build" | |
| elif [ -d "$src_dir/out" ]; then | |
| search_root="$src_dir/out" | |
| fi | |
| log "Copying libraries from $search_root to $dst_dir" | |
| find "$search_root" -maxdepth 5 \( -type f -o -type l \) \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) -exec cp -Pv {} "$dst_dir/" \; 2>/dev/null || true |
| clone_repo() { | ||
| local name="$1" repo="$2" branch="$3" dest="$4" | ||
|
|
||
| if [[ -d "$dest" ]]; then | ||
| warn "$name already exists, skipping clone" | ||
| return 0 | ||
| fi | ||
|
|
||
| log "Cloning $name (branch: $branch)" | ||
| if ! git clone --branch "$branch" "$repo" "$dest" --depth 1; then |
There was a problem hiding this comment.
The clone_repo helper clones dependency repositories using a mutable branch name from component_config.json (e.g., git clone --branch "$branch" "$repo" ...) without pinning to an immutable commit or verifying integrity. When these dependencies point to third-party repositories, a compromise of the remote repo or branch (or a forced-push/tag-move) can silently change the code executed in your build/analysis pipeline, enabling supply-chain attacks with access to the build environment. To harden this, require dependencies to be pinned to specific commits or verified tags (and ideally validate commit hashes or signatures) instead of using mutable branches alone.
…nt using Native Build Integration Reason for change: Standardized the native build with wrapper script architecture and Github Action Workflow Changes for Enabling Coverity Scan for github cable-modem-agent Test Procedure: verify able-modem-agent dependency and build native build and using Github workflow Risks: Low Priority: P1 Signed-off-by: dm097 <deepak_m@comcast.com>
…nt using Native Build Integration Reason for change: Standardized the native build with wrapper script architecture and Github Action Workflow Changes for Enabling Coverity Scan for github cable-modem-agent Test Procedure: verify able-modem-agent dependency and build native build and using Github workflow Risks: Low Priority: P1 Signed-off-by: dm097 <deepak_m@comcast.com>
| name: Build Cable Modem Agent component in github rdkcentral | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ghcr.io/rdkcentral/docker-rdk-ci:latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: native build | ||
| run: | | ||
| chmod +x cov_docker_script/run_setup_dependencies.sh | ||
| ./cov_docker_script/run_setup_dependencies.sh | ||
| chmod +x cov_docker_script/run_native_build.sh | ||
| ./cov_docker_script/run_native_build.sh | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }} |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
In general, to fix this issue you should explicitly declare a permissions block either at the workflow root (applies to all jobs) or under the specific job, granting only the scopes actually needed (often just contents: read as a minimum). This constrains the automatic GITHUB_TOKEN created by GitHub Actions, even if your steps never reference it directly, and documents the minimal required permissions.
For this workflow, the simplest change that does not affect existing behavior is to add a minimal permissions block at the job level for build-cable-modem-agent-on-pr. The steps only check out the repository and run local scripts; there is no evidence they need write access to the repo or other resources via GITHUB_TOKEN. Therefore, adding:
permissions:
contents: readunder the job definition (aligned with runs-on:) is sufficient and safest. No additional imports, methods, or definitions are required, since this is purely a YAML configuration change in .github/workflows/native-build.yml.
| @@ -10,6 +10,8 @@ | ||
| build-cable-modem-agent-on-pr: | ||
| name: Build Cable Modem Agent component in github rdkcentral | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| container: | ||
| image: ghcr.io/rdkcentral/docker-rdk-ci:latest | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "echo \"${PWD}\"", | ||
| "cd source/cm", | ||
| "autoreconf -i", | ||
| "./configure", | ||
| "make CFLAGS=\"-Os -pipe -g -feliminate-unused-debug-types -I${HOME}/usr/include/rdkb -I${HOME}/build/rdkb-halif-cm/include\" LDFLAGS=\"-Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed\"" |
There was a problem hiding this comment.
Inconsistent indentation detected. Lines 87-91 use tabs for indentation while the rest of the file uses spaces. While this doesn't break JSON parsing, it makes the file harder to maintain. Consider using consistent indentation (spaces) throughout the file to match the established pattern.
| }, | ||
| { | ||
| "name": "hal-cm-generic", | ||
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , |
There was a problem hiding this comment.
Trailing whitespace detected after the URL on line 79. This should be removed for cleaner code formatting.
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , | |
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal", |
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
Using actions/checkout@v3 which may be outdated. Consider using actions/checkout@v4 for improved performance and features, unless there's a specific reason to use v3.
| uses: actions/checkout@v3 | |
| uses: actions/checkout@v4 |
| # Verify setup_dependencies.sh exists before running | ||
| if [[ ! -f "$BUILD_TOOLS_DIR/cov_docker_script/setup_dependencies.sh" ]]; then | ||
| err "setup_dependencies.sh not found in build_tools_workflows" | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
Redundant verification: setup_dependencies.sh is already included in the REQUIRED_SCRIPTS array (line 14) and checked in the loop (lines 43-50). The additional check on lines 54-57 is unnecessary and can be removed to avoid duplication.
| # Verify setup_dependencies.sh exists before running | |
| if [[ ! -f "$BUILD_TOOLS_DIR/cov_docker_script/setup_dependencies.sh" ]]; then | |
| err "setup_dependencies.sh not found in build_tools_workflows" | |
| exit 1 | |
| fi |
| "build_dir": "build", | ||
| "configure_options_file": "cov_docker_script/configure_options.conf", | ||
| "configure_options": [ | ||
| "SRC_DIRS=$HOME/build/cable-model-agent/source/CMAgentSsp/" |
There was a problem hiding this comment.
Inconsistent indentation detected. Line 122 uses tabs for indentation while the rest of the file uses spaces. While this doesn't break JSON parsing, it makes the file harder to maintain. Consider using consistent indentation (spaces) throughout the file to match the established pattern.
| "SRC_DIRS=$HOME/build/cable-model-agent/source/CMAgentSsp/" | |
| "SRC_DIRS=$HOME/build/cable-model-agent/source/CMAgentSsp/" |
.github/workflows/native-build.yml
Outdated
|
|
||
| on: | ||
| push: | ||
| branches: [ main, 'sprint/**', 'release/**', 'feature/**', develop ] |
There was a problem hiding this comment.
Branch pattern inconsistency: The 'topic/RDK*' pattern is included in pull_request triggers (line 7) but not in push triggers (line 5). If topic/RDK* branches should trigger builds, consider adding this pattern to the push branches list for consistency. If this is intentional (e.g., topic branches should only build on PRs), this is acceptable.
| branches: [ main, 'sprint/**', 'release/**', 'feature/**', develop ] | |
| branches: [ main, 'sprint/**', 'release/**', 'feature/**', topic/RDK*, develop ] |
| -L$HOME/usr/local/lib/ | ||
| -Wl,--allow-shlib-undefined | ||
| -Wl,--unresolved-symbols=ignore-all | ||
| -ldbus-1 |
There was a problem hiding this comment.
Trailing whitespace detected after '-ldbus-1'. While this may not cause issues in most cases, it's best practice to remove trailing whitespace for cleaner code formatting.
| -ldbus-1 | |
| -ldbus-1 |
| "build_dir": "build", | ||
| "configure_options_file": "cov_docker_script/configure_options.conf", | ||
| "configure_options": [ | ||
| "SRC_DIRS=$HOME/build/cable-model-agent/source/CMAgentSsp/" |
There was a problem hiding this comment.
Typo in directory path: "cable-model-agent" should be "cable-modem-agent" to match the component name defined on line 100. This path inconsistency will cause the build to fail as it won't find the correct source directory.
| "SRC_DIRS=$HOME/build/cable-model-agent/source/CMAgentSsp/" | |
| "SRC_DIRS=$HOME/build/cable-modem-agent/source/CMAgentSsp/" |
| "name": "hal-cm-generic", | ||
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , | ||
| "branch": "rdk-next", | ||
| "repo_dir": "hal-cm-generic/source/cm", | ||
| "include_path": "$HOME/usr/include/rdkb/", | ||
| "lib_output_path": "$HOME/usr/local/lib", | ||
| "build": { | ||
| "type": "commands", | ||
| "commands": [ | ||
| "echo \"${PWD}\"", | ||
| "cd source/cm", | ||
| "autoreconf -i", | ||
| "./configure", | ||
| "make CFLAGS=\"-Os -pipe -g -feliminate-unused-debug-types -I${HOME}/usr/include/rdkb -I${HOME}/build/rdkb-halif-cm/include\" LDFLAGS=\"-Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed\"" |
There was a problem hiding this comment.
Inconsistent indentation detected. The file uses tabs for indentation on lines 78, 81, 82, 87-88, 90-91, and 122 while the rest of the file uses spaces. While this doesn't break JSON parsing, it makes the file harder to maintain. Consider using consistent indentation (spaces) throughout the file to match the established pattern.
| "name": "hal-cm-generic", | |
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , | |
| "branch": "rdk-next", | |
| "repo_dir": "hal-cm-generic/source/cm", | |
| "include_path": "$HOME/usr/include/rdkb/", | |
| "lib_output_path": "$HOME/usr/local/lib", | |
| "build": { | |
| "type": "commands", | |
| "commands": [ | |
| "echo \"${PWD}\"", | |
| "cd source/cm", | |
| "autoreconf -i", | |
| "./configure", | |
| "make CFLAGS=\"-Os -pipe -g -feliminate-unused-debug-types -I${HOME}/usr/include/rdkb -I${HOME}/build/rdkb-halif-cm/include\" LDFLAGS=\"-Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed\"" | |
| "name": "hal-cm-generic", | |
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , | |
| "branch": "rdk-next", | |
| "repo_dir": "hal-cm-generic/source/cm", | |
| "include_path": "$HOME/usr/include/rdkb/", | |
| "lib_output_path": "$HOME/usr/local/lib", | |
| "build": { | |
| "type": "commands", | |
| "commands": [ | |
| "echo \"${PWD}\"", | |
| "cd source/cm", | |
| "autoreconf -i", | |
| "./configure", | |
| "make CFLAGS=\"-Os -pipe -g -feliminate-unused-debug-types -I${HOME}/usr/include/rdkb -I${HOME}/build/rdkb-halif-cm/include\" LDFLAGS=\"-Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed\"" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| chmod +x cov_docker_script/run_setup_dependencies.sh | ||
| ./cov_docker_script/run_setup_dependencies.sh | ||
| chmod +x cov_docker_script/run_native_build.sh | ||
| ./cov_docker_script/run_native_build.sh | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }} |
There was a problem hiding this comment.
The environment variable GITHUB_TOKEN is set to a secret named RDKCM_RDKE, but GITHUB_TOKEN is a reserved name used by GitHub Actions. If this is meant to be a custom token for accessing private repositories, it should use a different name (e.g., CUSTOM_GITHUB_TOKEN or ACCESS_TOKEN) to avoid confusion with the automatic GITHUB_TOKEN provided by Actions.
| chmod +x cov_docker_script/run_setup_dependencies.sh | |
| ./cov_docker_script/run_setup_dependencies.sh | |
| chmod +x cov_docker_script/run_native_build.sh | |
| ./cov_docker_script/run_native_build.sh | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }} | |
| export GITHUB_TOKEN="$CUSTOM_GITHUB_TOKEN" | |
| chmod +x cov_docker_script/run_setup_dependencies.sh | |
| ./cov_docker_script/run_setup_dependencies.sh | |
| chmod +x cov_docker_script/run_native_build.sh | |
| ./cov_docker_script/run_native_build.sh | |
| env: | |
| CUSTOM_GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }} |
| }, | ||
| { | ||
| "name": "hal-cm-generic", | ||
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , |
There was a problem hiding this comment.
The repo URL uses 'https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal' which is a Gerrit server URL. Git clone operations typically require '.git' extension or proper Gerrit-specific clone commands. Verify this URL works correctly with standard git clone commands.
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal" , | |
| "repo": "https://code.rdkcentral.com/r/rdkb/components/opensource/ccsp/hal.git", |
|
|
||
| # MoCA specific | ||
| -DMOCA_HOME_ISOLATION | ||
| -DMOCA_DIAGONISTIC |
There was a problem hiding this comment.
The define appears to have a typo: "MOCA_DIAGONISTIC" should likely be "MOCA_DIAGNOSTIC".
| -DMOCA_DIAGONISTIC | |
| -DMOCA_DIAGNOSTIC |
…nt using Native Build Integration Reason for change: addressed co-pilot review comments Test Procedure: verify cable-modem-agent dependency and build native build and using Github workflow Risks: Low Priority: P1 Signed-off-by: dm097 <deepak_m@comcast.com>
…nt using Native Build Integration Reason for change: undoing yml script test changes Test Procedure: verify cable-modem-agent dependency and build native build and using Github workflow Risks: Low Priority: P1 Signed-off-by: dm097 <deepak_m@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "command": "python3 $HOME/usr/include/rdkb/dm_pack_code_gen.py config-arm/TR181-CM.XML source/CMAgentSsp/dm_pack_datamodel.c" | ||
| } | ||
| ], | ||
| "header_sources": [ |
There was a problem hiding this comment.
The pre_build_command references a Python script (dm_pack_code_gen.py) that should exist in the Utopia dependency. However, the script expects it to be available at $HOME/usr/include/rdkb/dm_pack_code_gen.py before the pre_build_commands execute. Verify that the Utopia or Utopia-headers dependency includes this Python script in its header_paths configuration, or add it explicitly. If the script is not provided by any dependency, this pre_build_command will fail.
| "header_sources": [ | |
| "header_sources": [ | |
| { "source": "dm_pack_code_gen.py", "destination": "$HOME/usr/include/rdkb" }, |
| "type": "commands", | ||
| "commands": [ | ||
| "echo \"${PWD}\"", | ||
| "cd source/cm", |
There was a problem hiding this comment.
The hal-cm-generic dependency has a "repo_dir" field set to "hal-cm-generic/source/cm" (line 81), but the build commands then reference "source/cm" (line 88). This appears to navigate into source/cm twice. Either the repo_dir should be just "hal-cm-generic" or the cd command should be adjusted. Verify the correct directory structure and ensure the build commands will execute in the intended location.
| "cd source/cm", |
| "type": "autotools", | ||
| "build_dir": "build", | ||
| "configure_options_file": "cov_docker_script/configure_options.conf", | ||
| "configure_options": [ | ||
| "SRC_DIRS=$HOME/build/cable-modem-agent/source/CMAgentSsp/" | ||
| ] |
There was a problem hiding this comment.
The native_component build configuration has inconsistent indentation (lines 117-124). The "build" object and its contents use mixed spacing. Standardize to consistent 2 or 4-space indentation throughout for better readability.
| "type": "autotools", | |
| "build_dir": "build", | |
| "configure_options_file": "cov_docker_script/configure_options.conf", | |
| "configure_options": [ | |
| "SRC_DIRS=$HOME/build/cable-modem-agent/source/CMAgentSsp/" | |
| ] | |
| "type": "autotools", | |
| "build_dir": "build", | |
| "configure_options_file": "cov_docker_script/configure_options.conf", | |
| "configure_options": [ | |
| "SRC_DIRS=$HOME/build/cable-modem-agent/source/CMAgentSsp/" | |
| ] |
| -D_ANSC_USE_OPENSSL_ | ||
| -D_ANSC_AES_USED_ | ||
| -D_NO_ANSC_ZLIB_ | ||
| -U_ANSC_IPV6_COMPATIBLE_ |
There was a problem hiding this comment.
The CPPFLAGS section includes both -D_ANSC_IPV6_COMPATIBLE_ undefined (line 31 with -U) and no corresponding definition. If IPv6 compatibility is not supported, this is correct. However, if any of the WiFi or network features enabled in this configuration (lines 71-104) require IPv6 support, this could cause runtime issues. Verify that disabling IPv6 compatibility is intentional and compatible with all enabled features.
|
|
||
| jobs: | ||
| build-cable-modem-agent-on-pr: | ||
| name: Build Cable Modem Agent component in github rdkcentral |
There was a problem hiding this comment.
The workflow does not include fork protection (if: ${{ ! github.event.pull_request.head.repo.fork }}) that is present in the fossid workflow (line 13 of fossid_integration_stateless_diffscan_target_repo.yml). Since this workflow uses the RDKCM_RDKE secret to access potentially private repositories, it should not run for PRs from forks where secrets are not available. Add a condition to skip fork PRs to prevent workflow failures.
| name: Build Cable Modem Agent component in github rdkcentral | |
| name: Build Cable Modem Agent component in github rdkcentral | |
| if: ${{ github.event_name != 'pull_request' || ! github.event.pull_request.head.repo.fork }} |
| "header_paths": [ | ||
| { "source": "source/include/sysevent", "destination": "$HOME/usr/include/rdkb/sysevent" }, | ||
| { "source": "source/include/syscfg", "destination": "$HOME/usr/include/rdkb/syscfg" }, | ||
| { "source": "source/include/utctx", "destination": "$HOME/usr/include/rdkb/utctx" }, |
There was a problem hiding this comment.
The Utopia-headers configuration includes a source path "source/include/utctx" (line 44), but the corresponding path in the Utopia dependency (line 24) uses just "source/include" with destination containing "/utctx". This inconsistency suggests line 44 may be incorrect - if "source/include/utctx" doesn't exist in the Utopia repository structure, this header copy will fail. Verify the correct source path structure in the Utopia repository and align both entries.
| "name": "Utopia-headers", | ||
| "repo": "https://github.com/rdkcentral/utopia.git", | ||
| "branch": "main", | ||
| "header_paths": [ | ||
| { "source": "source/include/sysevent", "destination": "$HOME/usr/include/rdkb/sysevent" }, | ||
| { "source": "source/include/syscfg", "destination": "$HOME/usr/include/rdkb/syscfg" }, | ||
| { "source": "source/include/utctx", "destination": "$HOME/usr/include/rdkb/utctx" }, | ||
| { "source": "source/include", "destination": "$HOME/usr/include/rdkb" }, | ||
| { "source": "source/utapi/lib", "destination": "$HOME/usr/include/rdkb/utapi" }, | ||
| { "source": "source/include/ulog", "destination": "$HOME/usr/include/rdkb/ulog" }, | ||
| { "source": "source/util/utils", "destination": "$HOME/usr/include/rdkb" }, | ||
| { "source": "source/sysevent/lib", "destination": "$HOME/usr/include/rdkb/sysevent" }, | ||
| { "source": "source/util/print_uptime", "destination": "$HOME/usr/include/rdkb" } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "rdkb-halif-vlan", | ||
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", |
There was a problem hiding this comment.
The Utopia dependency (lines 18-36) and Utopia-headers dependency (lines 38-52) have identical header_paths configurations but different branch names (feature/cov_native_build vs main). The first one includes a build configuration while the second does not. This appears redundant and could lead to header conflicts or confusion. Consider consolidating these into a single Utopia dependency entry, or clearly document why both are needed.
| "name": "Utopia-headers", | |
| "repo": "https://github.com/rdkcentral/utopia.git", | |
| "branch": "main", | |
| "header_paths": [ | |
| { "source": "source/include/sysevent", "destination": "$HOME/usr/include/rdkb/sysevent" }, | |
| { "source": "source/include/syscfg", "destination": "$HOME/usr/include/rdkb/syscfg" }, | |
| { "source": "source/include/utctx", "destination": "$HOME/usr/include/rdkb/utctx" }, | |
| { "source": "source/include", "destination": "$HOME/usr/include/rdkb" }, | |
| { "source": "source/utapi/lib", "destination": "$HOME/usr/include/rdkb/utapi" }, | |
| { "source": "source/include/ulog", "destination": "$HOME/usr/include/rdkb/ulog" }, | |
| { "source": "source/util/utils", "destination": "$HOME/usr/include/rdkb" }, | |
| { "source": "source/sysevent/lib", "destination": "$HOME/usr/include/rdkb/sysevent" }, | |
| { "source": "source/util/print_uptime", "destination": "$HOME/usr/include/rdkb" } | |
| ] | |
| }, | |
| { | |
| "name": "rdkb-halif-vlan", | |
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", | |
| "name": "rdkb-halif-vlan", | |
| "repo": "https://github.com/rdkcentral/rdkb-halif-vlan.git", | |
| "branch": "main", | |
| "header_paths": [ | |
| { "source": "include", "destination": "$HOME/usr/include" } | |
| ] | |
| }, | |
| { | |
| "name": "rdkb-halif-wifi", | |
| "repo": "https://github.com/rdkcentral/rdkb-halif-wifi.git", | |
| "branch": "develop", | |
| "header_paths": [ | |
| { "source": "include", "destination": "$HOME/usr/include" } | |
| ] | |
| }, | |
| { | |
| "name": "rdkb-halif-cm", | |
| "repo": "https://github.com/rdkcentral/rdkb-halif-cm.git", |
| [LDFLAGS] | ||
| -L$HOME/usr/local/lib/ | ||
| -Wl,--allow-shlib-undefined | ||
| -Wl,--unresolved-symbols=ignore-all |
There was a problem hiding this comment.
The LDFLAGS section includes --unresolved-symbols=ignore-all (line 140) which suppresses linker errors for undefined symbols. This is risky for a production build as it can hide missing dependencies or typos in symbol names that would cause runtime failures. While this may be necessary for Coverity analysis builds where stubs are used, consider using --warn-unresolved-symbols instead, or document why this aggressive suppression is required.
| -Wl,--unresolved-symbols=ignore-all | |
| -Wl,--warn-unresolved-symbols |
| branches: [ main, 'sprint/**', 'release/**', develop ] | ||
| pull_request: | ||
| branches: [ main, 'sprint/**', 'release/**', topic/RDK*, develop ] | ||
|
|
There was a problem hiding this comment.
The workflow does not declare permissions, unlike the fossid workflow which explicitly declares "contents: read" and "pull-requests: read" permissions. Following security best practices and the pattern established in other workflows in this repository, add explicit permissions to follow the principle of least privilege.
| permissions: | |
| contents: read | |
| pull-requests: read |
| ### Important: Add to .gitignore | ||
|
|
||
| Add the following to your component's `.gitignore` to exclude temporary build artifacts: | ||
|
|
||
| ```gitignore | ||
| # Build tools (downloaded by wrapper scripts) | ||
| build_tools_workflows/ | ||
|
|
||
| # Dependency build artifacts | ||
| build/ | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The README.md recommends adding build_tools_workflows/ and build/ to .gitignore, but these entries are not present in the actual .gitignore file. These directories will be created during the build process and should be excluded from version control.
| ### Important: Add to .gitignore | |
| Add the following to your component's `.gitignore` to exclude temporary build artifacts: | |
| ```gitignore | |
| # Build tools (downloaded by wrapper scripts) | |
| build_tools_workflows/ | |
| # Dependency build artifacts | |
| build/ | |
| ``` | |
| ### Important: Ignore build artifacts in your component repo | |
| To prevent temporary build artifacts from being committed, configure your component's version-control ignore rules (for example, `.gitignore`) to exclude the directories used by this native build workflow: | |
| - `build_tools_workflows/` – build tools downloaded by the wrapper scripts | |
| - `build/` – dependency and intermediate build artifacts |
2 participants
Reason for change: Native build and Github Action Workflow Changes for Enabling Coverity Scan for github cable-modem-agent
Test Procedure: verify able-modem-agent dependency and build native build and using Github workflow
Risks: Low
Priority: P1
Signed-off-by: Deepak.M deepak_m@comcast.com