feat(install): scaffold pup install <component> <platform> surface#568
Draft
rachelyangdog wants to merge 1 commit into
Draft
feat(install): scaffold pup install <component> <platform> surface#568rachelyangdog wants to merge 1 commit into
pup install <component> <platform> surface#568rachelyangdog wants to merge 1 commit into
Conversation
Adds the unified install command surface to pup, starting with `pup install ssi linux --host <X>` as the first concrete leaf. This is intentionally a scaffolding PR — the CLI surface and module structure land here so the pup team can review the shape; the actual russh client, OCI registry client, and `datadog-installer` invocation ship in follow-up PRs. See docs/INSTALL.md for the full design and the rollout plan. Motivation: dd-apm and other onboarding skills currently embed `bash -c "$(curl install_script_agent7.sh)"` in SKILL.md to install the Datadog Agent + SSI on remote hosts. Snyk correctly flags this as RCE-shaped, and the script lives outside any repo we control — no pinned version, no audit trail. Moving the install logic into pup turns the SKILL.md into a one-liner and shifts the install code into compiled Rust that already gets reviewed. What's in this PR: - `Commands::Install` variant + `InstallActions` / `InstallSsiActions` enums (gated on non-wasm targets, same pattern as Extensions). - `src/commands/install.rs` with the `ssi_linux` dispatcher. Today: `--dry-run` prints the planned install steps; without it, returns an actionable error pointing at docs/INSTALL.md. - `docs/INSTALL.md` with the design, security model, Linux SSI install flow, status, and open questions for the pup team. - Unit tests covering both the dry-run-prints-plan and not-yet-executable-bails-with-context paths. What's deferred to follow-up PRs: - russh client session (connect, exec, file transfer). - OCI registry manifest fetch + blob download + SHA-256 verify. - Datadog APT/YUM repo + signing keys setup on the remote host. - `datadog-installer setup` invocation (subcommand + flags need verification with the installer-binary owners). - Multi-distro coverage (Debian/Ubuntu/RHEL/CentOS/Amazon/Rocky/Alma). - Integration tests against a real host. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| ## Open questions for the pup team | ||
|
|
||
| 1. **Where should the install module live?** This PR puts everything in `src/commands/install.rs`. Once the implementation lands the install logic might want its own top-level `src/install/` module with submodules (`ssh.rs`, `oci.rs`, `linux_ssi.rs`). Happy to refactor at that point. |
Collaborator
There was a problem hiding this comment.
I would say yes, we should go with this approach. It will help keep this probably more sane to think about as long as they implement a trait
| ## Open questions for the pup team | ||
|
|
||
| 1. **Where should the install module live?** This PR puts everything in `src/commands/install.rs`. Once the implementation lands the install logic might want its own top-level `src/install/` module with submodules (`ssh.rs`, `oci.rs`, `linux_ssi.rs`). Happy to refactor at that point. | ||
| 2. **SSH transport.** `russh` is already an optional dep but currently used only as a server (`src/tunnel.rs`). Should the install module reuse `russh` as a client, or shell out to the system `ssh` binary? `russh` keeps pup self-contained (especially on Windows); shelling out is simpler but adds a runtime dependency. |
Collaborator
There was a problem hiding this comment.
I'm more inclined to say shelling out is probably more appropriate here as long as the failure cases are very clear for the user/agent on what the dependencies are if they are missing/mis-configured.
| 1. **Where should the install module live?** This PR puts everything in `src/commands/install.rs`. Once the implementation lands the install logic might want its own top-level `src/install/` module with submodules (`ssh.rs`, `oci.rs`, `linux_ssi.rs`). Happy to refactor at that point. | ||
| 2. **SSH transport.** `russh` is already an optional dep but currently used only as a server (`src/tunnel.rs`). Should the install module reuse `russh` as a client, or shell out to the system `ssh` binary? `russh` keeps pup self-contained (especially on Windows); shelling out is simpler but adds a runtime dependency. | ||
| 3. **`datadog-installer` CLI surface.** The exact subcommand and flags used by `install-ssi.sh` (`setup --flavor "APM SSI"` per one read of the script) need to be confirmed with whoever owns that binary as a public, stable contract. If the contract isn't stable, we need a different integration point. | ||
| 4. **OCI version pinning.** Who owns bumping the `datadog-installer` SHA-256 in pup when Datadog ships a new release? Manual on each agent release, or auto-bumped from a manifest? |
Collaborator
There was a problem hiding this comment.
do we actually want the pinning there? Or should this be an API pup can query for the latest version of each installer? (that api could literally be github release info too)
| 3. **`datadog-installer` CLI surface.** The exact subcommand and flags used by `install-ssi.sh` (`setup --flavor "APM SSI"` per one read of the script) need to be confirmed with whoever owns that binary as a public, stable contract. If the contract isn't stable, we need a different integration point. | ||
| 4. **OCI version pinning.** Who owns bumping the `datadog-installer` SHA-256 in pup when Datadog ships a new release? Manual on each agent release, or auto-bumped from a manifest? | ||
| 5. **k8s parity.** `pup install ssi k8s` is the natural next leaf. The mechanics are entirely different (helm + DatadogAgent CR + admission webhook) — does it deserve its own command tree, or stay under `install ssi`? | ||
| 6. **Auth integration.** Pup's existing `auth` config gives access to Datadog API. The install flow needs `DD_API_KEY` + `DD_SITE` to write `/etc/datadog-agent/datadog.yaml` — should the install command reuse `cfg.api_key` / `cfg.site`, or take its own flags? |
Collaborator
There was a problem hiding this comment.
This should be explicit and require these as flags. Considering the api_keys endpoints can be hooked up to pup, it could first ask the user to create a new key or to choose from an existing list if not explicitly specified
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the unified install command surface to pup, starting with
pup install ssi linux --host <X>as the first concrete leaf.This is intentionally a scaffolding PR — the CLI surface and module structure land here so the pup team can review the shape; the actual russh client, OCI registry client, and
datadog-installerinvocation ship in follow-up PRs. See docs/INSTALL.md for the full design and the rollout plan.Motivation: dd-apm and other onboarding skills currently embed
bash -c "$(curl install_script_agent7.sh)"in SKILL.md to install the Datadog Agent + SSI on remote hosts. Snyk correctly flags this as RCE-shaped, and the script lives outside any repo we control — no pinned version, no audit trail. Moving the install logic into pup turns the SKILL.md into a one-liner and shifts the install code into compiled Rust that already gets reviewed.What's in this PR:
Commands::Installvariant +InstallActions/InstallSsiActionsenums (gated on non-wasm targets, same pattern as Extensions).src/commands/install.rswith thessi_linuxdispatcher. Today:--dry-runprints the planned install steps; without it, returns an actionable error pointing at docs/INSTALL.md.docs/INSTALL.mdwith the design, security model, Linux SSI install flow, status, and open questions for the pup team.What's deferred to follow-up PRs:
datadog-installer setupinvocation (subcommand + flags need verification with the installer-binary owners).What does this PR do?
Motivation
Additional Notes
Checklist
Related Issues