Conversation
Precondition work so the rust toolchain gates can pass at all — the workspace was written before it was ever run through rustfmt, and three clippy lints fire on the current toolchain: - cargo fmt --all (mechanical, no semantic change) - studio-compose: implement std::iter::FromIterator for SkillsLibrary instead of an inherent from_iter (should_implement_trait) - studio-cp: derive Default for FleetRuntime (derivable_impls); use `?` in role_identity (question_mark); crate-level allow too_many_arguments for the flat dispatch-arg provision/observe API — same class of allowance already used on this call surface's callers. Same normalization the still-open openabdev#163 branch had to land for the same gates; no behavior change intended. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The create-vs-redeploy half of openabdev#111 already landed via openabdev#116: a missing stored manifest routes provision_from_library/provision_agent through build_default_manifest into the same apply_manifests path deploy_apply uses. What never landed is a check on the piece that was written blind — the assembled OABServiceManifest has zero coverage (its author notes a manual field-by-field check against validate() in lieu of compiling). Split the pure manifest assembly out of build_default_manifest into pub default_service_manifest (the wrapper keeps the AWS-only work: EC2 networking discovery + the ACP Secrets Manager write) and add tests/first_provision_manifest.rs pinning: - the produced document passes OABServiceManifest::validate() — the gate validate_apply_request enforces on every apply — and round-trips through serde_yaml + oabctl::studio_api::parse_manifests, the exact parser deploy_apply's apply_deployment entry point calls; - the create.rs wizard defaults (256/512, FARGATE/X86_64, private subnets, no public IP, no ingress, generation 0, unset bundle_from until the caller wires the uploaded bundle's prefix); - the acp_enabled <-> OPENAB_ACP_AUTH_KEY secrets pairing the Spec contract requires. No behavior change — build_default_manifest produces the identical manifest. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.
Refs #111
Summary
#111's create-vs-redeploy seam already landed via #116: when the compose
deploy_provision/deploy_provision_agentpath finds no stored manifest,build_default_manifestassembles a freshOABServiceManifest(create.rswizard defaults) and it is persisted+applied through the same
apply_manifestspathdeploy_applyuses; an existing manifest still takesredeploy's patch path. The k8s counterpart landed with it.What never landed is a check on the piece that was written blind — the
assembled manifest had zero coverage (a manual field-by-field check against
validate()stood in for compiling). This PR makes that contract testableand pins it:
default_service_manifest— the pure manifest-assembly half ofbuild_default_manifest, split out and madepub. The async wrapperkeeps the AWS-only work (
default_networkingEC2 discovery, the ACPSecrets Manager write) and delegates; the produced manifest is identical.
tests/first_provision_manifest.rs— asserts the produced documentpasses
OABServiceManifest::validate()(the gatevalidate_apply_requestenforces on every apply) and round-trips through
serde_yaml+oabctl::studio_api::parse_manifests— the exact parserdeploy_apply'sapply_deploymentcalls — so drift between the producer and the applysurface fails in CI instead of at first real deploy. Also pins the
create.rs-derived defaults (256/512, FARGATE/X86_64, private subnets / no
public IP, no ingress, generation 0,
bundle_fromleft for the caller towire to the uploaded bundle's prefix) and the
acp_enabled⇔OPENAB_ACP_AUTH_KEYsecrets pairingSpec.acp_enabledrequires.Gate-driven cleanups (no behavior change)
cargo fmt --allnormalization — the workspace was hand-formatted anddrifts from stable rustfmt; the enforced profile requires
cargo fmt --all -- --checkgreen. Isolated in the first commit(identical normalization to the one the fix(observe): key every ECS/logs query on the full oab-{ns}-{name} name #163/feat(console): provider-aware fleet onboarding for k8s #164 branches landed).
FromIteratorforSkillsLibrary,#[derive(Default)]onFleetRuntime,?inrole_identity, crate-leveltoo_many_argumentsallow — satisfy
cargo clippy --workspace --all-targets -- -D warningson stable 1.98.
Test plan
cargo test --workspace— newfirst_provision_manifestsuite (3tests) green; 224 existing tests still green
no default_service_manifest in the root(exit 101)cargo fmt --all -- --checkclean;cargo clippy --workspace --all-targets -- -D warningsclean+ New fleetfirst-create — unchanged code path; out ofscope for this test-only seam (covered by the same wrapper as before)
Generated with Devin