Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "skopeo", | ||
| "copy", | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token else []), | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and image.startswith("ghcr.io/") else []), |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, the fix is to avoid treating the entire image string as a URL-like blob and instead extract the registry/host portion using a proper parser or a dedicated image-reference parsing helper. Once we have the registry part (e.g., ghcr.io), we compare that exact value against the allowlisted host, rather than using startswith("ghcr.io/") on the whole string.
For this code, the most targeted change is to replace the image.startswith("ghcr.io/") checks with a helper that derives the registry from the image string in a Docker/OCI-compatible way. A simple and safe approach without changing behavior is:
- Split on the first
/to separate a potential registry from the remainder. - If the part before the
/contains a.or:or equalslocalhost, treat it as a registry (matching Docker’s reference rules). Otherwise, fall back to the default Docker registry, which we can treat as notghcr.iofor our purposes. - Compare that derived registry to
ghcr.io.
This keeps semantics consistent with how Docker interprets image references but removes the fragile prefix check. To keep the change minimal and localized, we’ll:
- Add a small internal function
_is_ghcr_image(image: str) -> boolnear the top of the file (after the globals), which implements registry extraction logic. - Replace both uses of
image.startswith("ghcr.io/")(in the*(...)expansion for Skopeo arguments and in theenv={...}expression) with_is_ghcr_image(image).
No external dependencies are needed; we can implement the parsing with basic string operations.
| @@ -44,6 +44,30 @@ | ||
| configuration = Configuration() | ||
|
|
||
|
|
||
| def _is_ghcr_image(image: str) -> bool: | ||
| """ | ||
| Return True if the given image reference is hosted on ghcr.io. | ||
|
|
||
| This parses the image reference in a Docker/OCI-compatible way: | ||
| - If the part before the first '/' contains a '.' or ':' or is 'localhost', | ||
| it is treated as the registry host. | ||
| - Otherwise, the default Docker registry is assumed, which is not ghcr.io. | ||
| """ | ||
| if not image: | ||
| return False | ||
| # Split "<registry>/<rest>" once | ||
| first, sep, rest = image.partition("/") | ||
| if not sep: | ||
| # No '/' at all; implicit default registry | ||
| return False | ||
| registry_candidate = first | ||
| # Docker treats components with '.' or ':' or equal to 'localhost' as registries | ||
| if "." in registry_candidate or ":" in registry_candidate or registry_candidate == "localhost": | ||
| return registry_candidate == "ghcr.io" | ||
| # Otherwise, this is a namespace on the default Docker registry | ||
| return False | ||
|
|
||
|
|
||
| @functools.cache | ||
| def detect_driver() -> typing.Literal["lima", "wsl"]: | ||
| has_lima = (importlib.resources.files("agentstack_cli") / "data" / "bin" / "limactl").is_file() or shutil.which("limactl") | ||
| @@ -778,12 +802,12 @@ | ||
| [ | ||
| "skopeo", | ||
| "copy", | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and image.startswith("ghcr.io/") else []), | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and _is_ghcr_image(image) else []), | ||
| f"docker://{image}", | ||
| f"containers-storage:{image}", | ||
| ], | ||
| f"Pulling image {image}", | ||
| env={"GITHUB_TOKEN": github_token} if github_token and image.startswith("ghcr.io/") else None, | ||
| env={"GITHUB_TOKEN": github_token} if github_token and _is_ghcr_image(image) else None, | ||
| ) | ||
|
|
||
| # --- Kagenti platform installation --- |
| ], | ||
| f"Pulling image {image}", | ||
| env={"GITHUB_TOKEN": github_token} if github_token else None, | ||
| env={"GITHUB_TOKEN": github_token} if github_token and image.startswith("ghcr.io/") else None, |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural shift by integrating Kagenti into the Agent Stack CLI. This integration simplifies agent management, enhances scalability, and provides a more streamlined local development experience. The changes involve removing custom Kubernetes management components and adopting Kagenti's deployment and discovery mechanisms. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring to integrate kagenti for agent lifecycle management, replacing the custom Kubernetes provider deployment and build system. The changes are extensive, touching many parts of the codebase from the CLI to the server-side services and database models. Key changes include removing the build command, simplifying the add and update agent commands, and introducing a new kagenti client for agent discovery. The provider model has been greatly simplified, and a new database migration reflects these changes. Overall, the changes are consistent with the goal of delegating agent management to kagenti. I've identified a few areas where maintainability could be improved by reducing hardcoded values in the platform setup scripts.
Note: Security Review did not run due to the size of the PR.
| await run_in_vm( | ||
| vm_name, | ||
| [ | ||
| "bash", | ||
| "-c", | ||
| textwrap.dedent("""\ | ||
| kubectl --kubeconfig=/kubeconfig apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.4.0/standard-install.yaml | ||
| kubectl --kubeconfig=/kubeconfig apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.17.2/cert-manager.yaml | ||
| kubectl --kubeconfig=/kubeconfig wait --for=condition=Available deployment -n cert-manager cert-manager-webhook --timeout=120s | ||
| """), | ||
| ], | ||
| "Installing kagenti prerequisites (Gateway API CRDs, cert-manager)", |
There was a problem hiding this comment.
The URLs for installing Gateway API CRDs and cert-manager are hardcoded with specific versions (v1.4.0 and v1.17.2 respectively). While this ensures reproducibility, it makes updates require code changes. To improve maintainability, consider defining these versions as constants at the top of the file.
| await run_in_vm( | ||
| vm_name, | ||
| [ | ||
| "bash", | ||
| "-c", | ||
| textwrap.dedent("""\ | ||
| ISTIO_VERSION=1.28.0 | ||
| ISTIO_REPO=https://istio-release.storage.googleapis.com/charts/ | ||
| helm repo add istio "$ISTIO_REPO" 2>/dev/null || true | ||
| helm repo update istio | ||
| kubectl --kubeconfig=/kubeconfig create namespace istio-system --dry-run=client -o yaml | kubectl --kubeconfig=/kubeconfig apply -f - | ||
| helm upgrade --install istio-base istio/base --version=$ISTIO_VERSION --namespace=istio-system --kubeconfig=/kubeconfig --wait --force-conflicts | ||
| helm upgrade --install istiod istio/istiod --version=$ISTIO_VERSION --namespace=istio-system --kubeconfig=/kubeconfig --wait --force-conflicts \ | ||
| --set pilot.resources.requests.cpu=50m \ | ||
| --set pilot.resources.requests.memory=256Mi | ||
| """), | ||
| ], | ||
| "Installing Istio (Gateway API controller)", |
There was a problem hiding this comment.
The Istio version (1.28.0) is hardcoded within this shell script block. This could lead to maintenance issues if kagenti's Istio dependency changes in the future. It would be more maintainable to define this version as a constant at the top of the file, making it easier to update.
| agent_namespace = agent.get("namespace", namespace) | ||
|
|
||
| # Construct service URL from k8s naming convention (service port 8080) | ||
| url = f"http://{name}.{agent_namespace}.svc.cluster.local:8080" |
There was a problem hiding this comment.
The agent service URL is constructed with a hardcoded port 8080. If kagenti allows agents to run on different ports, this could cause connection issues. If this is a fixed convention from kagenti, adding a comment to clarify this would be helpful. For more robustness, consider if the port can be discovered from the service definition rather than being hardcoded.
jezekra1
force-pushed
the
poc/kagenti-integration
branch
from
7de860d to
9c0fd63
Compare
jezekra1
force-pushed
the
poc/kagenti-integration
branch
from
9c0fd63 to
4ee4c89
Compare
jezekra1
force-pushed
the
poc/kagenti-integration
branch
3 times, most recently
from
3a6188c to
a95917a
Compare
- Replace Docker/registry-based providers with network-only providers - Add kagenti agent sync cron and provider health check refresh - Expose otel-collector via HTTPRoute for local agent telemetry - Upgrade Phoenix image to 12.31.2 for GraphQL API compatibility - Fix server traces reaching otel-collector (port 4318→8335) - Set default OTEL endpoint in Python SDK for local deployments - Simplify provider model: ProviderState (online/offline) replaces ProviderType/ProviderStatus/ProviderUnmanagedStatus - Remove Docker image labels, GitHub version resolving, provider builds - Fix checkbox selection UX in agent remove command Signed-off-by: Radek Ježek <radek.jezek@ibm.com>
jezekra1
force-pushed
the
poc/kagenti-integration
branch
from
a95917a to
898c888
Compare
1 participant
Signed-off-by: Radek Ježek radek.jezek@ibm.com
Summary
Refs #2304
Linked Issues
Documentation