Skip to content

RDKEMW-13945: Fix L2 Issue on RFC Component #182

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.

Sign up for GitHub

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

Open
madhubabutt wants to merge 1 commit into
base: develop
Choose a base branch
from
+29 −42
Open

RDKEMW-13945: Fix L2 Issue on RFC Component #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/workflows/L2-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,34 @@ jobs:
run: |
docker run -d --name native-platform --link mockxconf -v ${{ github.workspace }}:/mnt/L2_CONTAINER_SHARED_VOLUME ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest


- name: Build RFC and Run L2 inside Native Platform Container
run: |
docker exec -i native-platform /bin/bash -c "cd /mnt/L2_CONTAINER_SHARED_VOLUME/ && sh ./cov_build.sh && export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/lib/x86_64-linux-gnu:/lib/aarch64-linux-gnu:/usr/local/lib && sh run_l2.sh"

- name: Enable core dump
run: ulimit -c unlimited

Comment on lines +74 to +76
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ulimit -c unlimited is executed on the GitHub runner, but the RFC build/tests run inside the native-platform container. This ulimit won’t affect processes inside the container, so it won’t enable core dumps where you need them. Set the ulimit on docker run (e.g., --ulimit core=-1) or run ulimit via docker exec in the same container/session that launches parodus.

Copilot uses AI. Check for mistakes.
- name: Debug parodus binary
run: |
pwd
ls -l /usr/local/bin
file /usr/local/bin/parodus
ldd /usr/local/bin/parodus || true

- name: Run test
run: |
cd /usr/local/bin
./parodus '{"command":"SET","parameters":[{"name":"Device.DeviceInfo.X_RDKCENTRAL-COM_FirmwareToDownload","dataType":0,"value":"TESTIMAGE_DEV.bin"}]}'

Comment on lines +74 to +88
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new "Enable core dump", "Debug parodus binary", and "Run test" steps run on the GitHub Actions runner host, not inside the native-platform container where RFC/parodus is built/executed. This will likely fail (runner may not have /usr/local/bin/parodus) and ulimit -c unlimited in its own step won’t affect other steps or the container. Run these diagnostics via docker exec native-platform ... (and set ulimit in the same container-executed command where parodus runs).

Copilot uses AI. Check for mistakes.
- name: Print stacktrace
if: failure()
run: |
core=$(ls core* 2>/dev/null | head -n1)
if [ -n "$core" ]; then
gdb -batch -ex "bt full" /tmp/parodus $core
fi
Comment on lines +89 to +95
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stacktrace step looks for core* files on the GitHub runner and runs gdb ... /tmp/parodus $core, but /tmp/parodus is created inside the native-platform container (per run_l2.sh) and any core dump would also be inside that container’s filesystem. Additionally, this workflow doesn’t install gdb, so the step may fail even if a core exists. To make this work reliably, collect the core/binary from the container (e.g., docker cp) and either run gdb in the container or install gdb on the runner before invoking it.

Copilot uses AI. Check for mistakes.

- name: Copy unknown-accountid-xconf-rfc-response json to mockxconf service
run: |
docker cp ${{ github.workspace }}/test/test-artifacts/mockxconf/xconf-rfc-response-unknown-accountid.json mockxconf:/etc/xconf/xconf-rfc-response.json
Expand Down
41 changes: 2 additions & 39 deletions run_l2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,46 +25,9 @@ cp ./rfc.properties /opt/rfc.properties
cp /opt/certs/client.pem /etc/ssl/certs/client.pem
cp ./rfcMgr/gtest/mocks/tr181store.ini /opt/secure/RFC/tr181store.ini

rbuscli set Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Control.ConfigSetTime uint32 1763118860
ls -l /usr/local/bin/parodus
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ls -l /usr/local/bin/parodus is a debug-only step and doesn't contribute to setting up or running the tests. It adds noise to CI logs and will print an error if the file is missing (even though the script continues). Consider removing it or turning it into an explicit existence check that fails fast with a clear message.

Suggested change
ls -l /usr/local/bin/parodus
if [ ! -x /usr/local/bin/parodus ]; then
echo "Error: /usr/local/bin/parodus not found or not executable." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.

# Run L2 Test cases
pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_single_instance_run.json test/functional-tests/tests/test_rfc_single_instance_run.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_device_offline.json test/functional-tests/tests/test_rfc_device_offline_status.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_init_failure.json test/functional-tests/tests/test_rfc_initialization_failure.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_xconf_communication_success.json test/functional-tests/tests/test_rfc_xconf_communication.py


pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_setget_param.json test/functional-tests/tests/test_rfc_setget_param.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_tr181_setget_local_param.json test/functional-tests/tests/test_rfc_tr181_setget_local_param.py

# The cert selector test cases are commented for now. Once the code changes are moved to open source, it will be enabled.
#pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_dynamic_static_cert_selector.json test/functional-tests/tests/test_rfc_dynamic_static_cert_selector.py

#pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_static_cert_selector.json test/functional-tests/tests/test_rfc_static_cert_selector.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_xconf_rfc_data.json test/functional-tests/tests/test_rfc_xconf_rfc_data.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_xconf_request_params.json test/functional-tests/tests/test_rfc_xconf_request_params.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_valid_accountid.json test/functional-tests/tests/test_rfc_valid_accountid.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_factory_reset.json test/functional-tests/tests/test_rfc_factory_reset.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_trigger_reboot_unknown_accountid.json test/functional-tests/tests/test_rfc_trigger_reboot.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_feature_enable.json test/functional-tests/tests/test_rfc_feature_enable.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_configsethash_time.json test/functional-tests/tests/test_rfc_xconf_configsethash_time.py

echo "ENABLE_MAINTENANCE=true" >> /etc/device.properties

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_xconf_reboot.json test/functional-tests/tests/test_rfc_xconf_reboot.py

pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_override_rfc_prop.json test/functional-tests/tests/test_rfc_override_rfc_prop.py
cp /usr/local/bin/parodus /tmp/parodus
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copying the parodus binary directly to /tmp/parodus introduces a classic symlink attack risk if this script runs with elevated privileges. An unprivileged user can create /tmp/parodus as a symlink to a sensitive root-owned file, causing cp to overwrite that target when the script runs, leading to privilege escalation or corruption of critical system files. Use a secure temporary path (e.g., via mktemp) and ensure the destination is not a pre-existing symlink in a world-writable directory.

Suggested change
cp /usr/local/bin/parodus /tmp/parodus
PARODUS_TMP="$(mktemp /tmp/parodus.XXXXXX)"
cp /usr/local/bin/parodus "$PARODUS_TMP"
echo "parodus binary copied to temporary path: $PARODUS_TMP"

Copilot uses AI. Check for mistakes.

#pytest --json-report --json-report-summary --json-report-file $RESULT_DIR/rfc_rfc_webpa.json test/functional-tests/tests/test_rfc_webpa.py

6 changes: 3 additions & 3 deletions test/functional-tests/tests/test_rfc_webpa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

def test_WebPA_Set_ACC_Id():
print("Starting parodus mock process")
payload = '{"command":"SET","parameters":[{"name":"Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.AccountInfo.AccountID","dataType":0,"value":"412370664406228514"}]}'
command = ["/usr/local/bin/parodus", payload]
payload = '{"command":"SET","parameters":[{"name":"Device.DeviceInfo.X_RDKCENTRAL-COM_FirmwareToDownload","dataType":0,"value":"TESTIMAGE_DEV.bin"}]}'
command = ["/tmp/parodus", payload]
Comment on lines 22 to +25
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_WebPA_Set_ACC_Id no longer sets the AccountID parameter (it sets X_RDKCENTRAL-COM_FirmwareToDownload instead), but test_WebPA_Get_ACC_Id still asserts an AccountID value (412370664406228514) is returned/logged. As written, the GET test is no longer consistent/self-contained and is likely to fail depending on prior device state. Update the SET payload + expected value to match the GET assertion, or update the GET test/assertions (and test names) to validate the parameter you actually set here.

Copilot uses AI. Check for mistakes.

Comment on lines +24 to 26
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test now hard-codes the parodus binary path as /tmp/parodus, which makes it depend on external setup (e.g., run_l2.sh copying the binary first) and can break when this test is executed directly in environments where only /usr/local/bin/parodus exists. Consider resolving the binary via an environment variable with a default, or falling back to /usr/local/bin/parodus when /tmp/parodus is absent.

Copilot uses AI. Check for mistakes.
result = subprocess.run(command, capture_output=True, text=True)
assert result.returncode == 0, f"Command failed with error: {result.stderr}"
Expand All @@ -36,7 +36,7 @@ def test_WebPA_Set_ACC_Id():
def test_WebPA_Get_ACC_Id():
print("Starting parodus mock process")
payload ='{"command":"GET","names":["Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.AccountInfo.AccountID"]}'
command = ["/usr/local/bin/parodus", payload]
command = ["/tmp/parodus", payload]

Comment on lines 36 to 40
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coding the WebPA client path to /tmp/parodus makes this test dependent on external setup (e.g., run_l2.sh copying the binary). When running pytest directly, /tmp/parodus may not exist and the test will fail before exercising RFC/WebPA behavior. Consider using the canonical install location (or an env/configurable path) and/or have the test/fixture prepare the binary.

Copilot uses AI. Check for mistakes.
Comment on lines 24 to 40
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parodus binary path is hard-coded as /tmp/parodus in multiple places. Since this is an execution dependency, consider centralizing it (e.g., a constant in rfc_test_helper.py or a fixture/env var) so future path changes don’t require editing multiple tests.

Copilot uses AI. Check for mistakes.
result = subprocess.run(command, capture_output=True, text=True)
assert result.returncode == 0, f"Command failed with error: {result.stderr}"
Expand Down
Loading