Skip to content
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.

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

Update dependencies to fix vulnerabilities #139

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

erikbosch
Copy link
Contributor

@erikbosch erikbosch commented Jul 15, 2024

Updating dependencies to address vulnerabilities, if merged and released it may also fix some vulnerabilities in https://github.com/eclipse-velocitas/vehicle-app-python-template as that repo depends on this repo. Doing necessary refactoring.

All examples tested by trying to build and start-up them, but not by actually sending messages on MQTT, gRPC and similar. Some problems detected and fixes exist in #141 but no problems found related to the changes in this repository.

After a discussion in ETAS dev team proposing to change to fixed versions also in *.in/setup.py. I updated all *.in/setup.py files to what was used after the --upgrade I did with pip-compile before testing.

Background

Some time ago we added in #119 a fix to eclipse-velocitas/vehicle-app-python-template#225 by putting an upper limit on used paho-version, due to a backward incompatible change in paho-mqtt. Now that has partially changed in paho-mqtt 2.1, see:

So now CallbackAPIVersion.VERSION1 is default.

@erikbosch erikbosch marked this pull request as ready for review July 15, 2024 08:30
@erikbosch erikbosch requested a review from MP91 July 15, 2024 08:31
@erikbosch
Copy link
Contributor Author

FYI: @lukasmittag

@MP91
Copy link
Contributor

MP91 commented Jul 16, 2024

Hey @erikbosch,
you can test your SDK by setting the the sdkGitRepo and the sdkGitRef variables in your velocitas.json. We should at least verify that the apps in the template still work with MQTT

@erikbosch
Copy link
Contributor Author

Thanks for the input @MP91 - I tested like below, found one regression that is fixed in a new commit.

Tested with https://github.com/eclipse-velocitas/vehicle-app-python-template/blob/main/app/src/main.py

Changinging .velocitas.json like

{
    "packages": {
        ...
    },
    "components": [
        ... ,
        "sdk-installer"
    ],
    "variables": {
        ... ,
        "sdkGitRepo": "https://github.com/SoftwareDefinedVehicle/vehicle-app-python-sdk",
        "sdkGitRefOld": "6c090886690376a023d29c4352133f20ef0d5f94",
        "sdkGitRef": "erik_dep"
    },
    "cliVersion": "v0.10.1"
}

Two observations - even if the variable is called sdkGitRef it does not seem to support a Git ref but requires a branch. Secondly, the sdk-installer printout will list the versio, not the branch in the printout which better should be fixed

Installing package version '0.14.1' from 'https://github.com/SoftwareDefinedVehicle/vehicle-app-python-sdk'...

The code below from https://github.com/eclipse-velocitas/devenv-devcontainer-setup/blob/main/sdk-installer/src/run.py is to blame for that SHA does not work as it use -b but state that ref or tag is just fine to use


def force_clone_repo(
    git_url: str, git_ref: str, output_dir: str, verbose_logging: bool
) -> None:
    """Clones the given git repository, forcefully removing any previously
    existing directory structure at the given output directory.

    Args:
        git_url (str): The URL of the git repo to clone.
        git_ref (str): The git ref (branch, tag, SHA) to clone.
        output_dir (str): The output directory to which to output the cloned
            repository.
        verbose_logging (bool): Enable verbose logging.
    """

    if os.path.exists(output_dir):
        shutil.rmtree(output_dir)

    subprocess.check_call(
        ["git", "clone", "--depth", "1", "-b", git_ref, git_url, output_dir],
        stdout=subprocess.DEVNULL if not verbose_logging else None,
    )

    subprocess.check_call(
        ["git", "config", "--global", "--add", "safe.directory", output_dir],
        stdout=subprocess.DEVNULL if not verbose_logging else None,
    )

with this changed a regression was found - otelTraceSampled needs to be defined. It is boolean and I opted for True but that can likely be discussed.

Subscribe MQTT

sudo apt update
sudo apt mosquitto-clients
mosquitto_sub -h localhost -p 1883  -t "sampleapp/getSpeed/response"

Send MQTT

mosquitto_pub -h localhost -p 1883 -m "hej" -t "sampleapp/getSpeed"

Check App logs

 *  Executing task: velocitas exec runtime-local run-vehicle-app python3 /workspaces/vehicle-app-python-template/app/src/main.py 

2024-07-16 13:20:43,806 DEBUG [asyncio] [selector_events.py:54] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - Using selector: EpollSelector
2024-07-16 13:20:43,807 INFO [__main__] [main.py:118] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - Starting SampleApp...
2024-07-16 13:20:43,808 DEBUG [grpc._cython.cygrpc] [_channel.py:365] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - Using AsyncIOEngine.POLLER as I/O engine
2024-07-16 13:20:43,810 DEBUG [velocitas_sdk.vehicle_app] [vehicle_app.py:72] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - VehicleApp instantiation successfully done
2024-07-16 13:20:43,810 INFO [velocitas_sdk.vdb.subscriptions] [subscriptions.py:72] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - Subscribing to SELECT Vehicle.Speed
2024-07-16 13:20:43,811 DEBUG [velocitas_sdk.native.mqtt] [mqtt.py:56] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - Mqtt native connection OK!
2024-07-16 13:33:53,378 DEBUG [__main__] [main.py:91] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - PubSub event for the Topic: sampleapp/getSpeed -> is received with the data: hej
2024-07-16 13:34:45,638 DEBUG [__main__] [main.py:91] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - PubSub event for the Topic: sampleapp/getSpeed -> is received with the data: hej
2024-07-16 13:35:22,816 DEBUG [__main__] [main.py:91] [trace_id=b42cffef72698996c2abd5a0f86b4cc5 span_id=2f28887c3c5ff44a resource.service.name=/workspaces/vehicle-app-python-template/app/src/main.py trace_sampled=True] - PubSub event for the Topic: sampleapp/getSpeed -> is received with the data: hej

Verify SUB output
{"result": {"status": 0, "message": "Current Speed = 53.2295036315918"}}

@erikbosch
Copy link
Contributor Author

Discussion: Use fixed version in setup.py

@erikbosch erikbosch force-pushed the erik_dep branch 4 times, most recently from 1bad60d to 13ed217 Compare July 30, 2024 06:36
Also changed:
- After discussion in ETAS dev team proposing to use fixed versions also in *.in files
- Fixing backward incompatible changes when updating Paho
- Fixing workflow so that we really test with example from current branch
@erikbosch
Copy link
Contributor Author

@doosuu @MP91 - changed PR to use fixed versions based on versions used when I tested. Also tried to align versions. If you are happy with it this one should be ready to merge

Copy link
Member

@doosuu doosuu left a comment

Choose a reason for hiding this comment

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

lgtm

@erikbosch erikbosch merged commit 7c021bb into eclipse-velocitas:main Aug 7, 2024
8 checks passed
@erikbosch erikbosch deleted the erik_dep branch August 7, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants