Skip to content

Create a cli package for Services#1377

Merged
prasadtalasila merged 127 commits intoINTO-CPS-Association:feature/distributed-demofrom
8ohamed:feature/distributed-demo
Jan 9, 2026
Merged

Create a cli package for Services#1377
prasadtalasila merged 127 commits intoINTO-CPS-Association:feature/distributed-demofrom
8ohamed:feature/distributed-demo

Conversation

@8ohamed
Copy link
Contributor

@8ohamed 8ohamed commented Dec 5, 2025

Create a cli poetry package for managing services

Type of Change

  • New feature

Description

This PR solves the #1375 issue, by creating a poetry cli package, so the user can manage the platform services by entring command lines.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 52 changed files in this pull request and generated no new comments.

@prasadtalasila
Copy link
Contributor

@8ohamed please skip the updating of code outside of deploy/services/cli. Let us focus on getting this PR done.

@8ohamed
Copy link
Contributor Author

8ohamed commented Jan 4, 2026

@8ohamed please skip the updating of code outside of deploy/services/cli. Let us focus on getting this PR done.

@prasadtalasila Should I revert the changes in the top cli, and wait for you to merge this PR, then in a different PR I commit the changes for the top cli.

@prasadtalasila
Copy link
Contributor

@8ohamed please refer the changes in the top-level cli. Thanks.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated no new comments.

@8ohamed
Copy link
Contributor Author

8ohamed commented Jan 5, 2026

Hi @prasadtalasila I updated the stop command it should fix the status command. if not I will look into it again in the next PR.

@prasadtalasila
Copy link
Contributor

@8ohamed are the qlty issues fixed as well?

@8ohamed
Copy link
Contributor Author

8ohamed commented Jan 5, 2026

@prasadtalasila yes in "a646ea0"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated 1 comment.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated 1 comment.

@@ -0,0 +1,432 @@
# pylint: disable=redefined-outer-name
"""Tests for Service class and Docker operations"""
import os
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

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

Import of 'os' is not used.

Copilot uses AI. Check for mistakes.
@prasadtalasila
Copy link
Contributor

@8ohamed There are still eleven qlty issues in the deploy/services/cli. Are you still working on them? Thanks for clarification.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated no new comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated no new comments.

@8ohamed
Copy link
Contributor Author

8ohamed commented Jan 8, 2026

Hi @prasadtalasila I fixed the qlty issues, if you inspect qlty.sh on the repo it might still say that some files has 1 or 2 issues, but if you enter the file it doesn't show any issues, I think it takes more time to update the values.

And here's the output of the commands on the server

commands_on_server

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 40 out of 51 changed files in this pull request and generated 1 comment.

ensure you run the setup command with appropriate privileges:

```bash
sudo -E env PATH="$PATH" dtaas-services setup
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The recommended command sudo -E env PATH="$PATH" dtaas-services setup preserves the unprivileged user’s PATH when escalating to root, which can allow execution of attacker-controlled binaries as root if a writable directory (e.g. $HOME/bin) appears early in PATH. An attacker who can modify a directory on the user’s PATH could place a malicious docker or other executable that the CLI or its dependencies invoke, leading to local privilege escalation and full compromise of the host. To mitigate this, avoid preserving the full user PATH for privileged runs and rely on a minimal, root-controlled PATH when invoking dtaas-services with elevated privileges.

Suggested change
sudo -E env PATH="$PATH" dtaas-services setup
sudo dtaas-services setup

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants