Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions cli/docs/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ azd app run --environment production
| `reqs` | Check and verify required tools and optionally auto-generate requirements | [→ Full Spec](commands/reqs.md) |
| `deps` | Install dependencies for detected projects | [→ Full Spec](commands/deps.md) |
| `add` | Add a well-known container service to azure.yaml | [→ Full Spec](commands/add.md) |
| `remove` | Remove a service from azure.yaml | |
| `run` | Run the development environment with service orchestration and lifecycle hooks | [→ Full Spec](commands/run.md) |
| `test` | Run tests for all services with coverage aggregation | [→ Full Spec](commands/test.md) |
| `start` | Start stopped services | [→ Full Spec](commands/start.md) |
Expand Down Expand Up @@ -307,6 +308,34 @@ azd app add redis --output json

---

## `azd app remove`

Remove a service from the services section of azure.yaml. This is the inverse of `azd app add`. It deletes the named service entry and leaves every other service in the file untouched.

### Usage

```bash
azd app remove [service]
```

### Examples

```bash
# Remove the redis service
azd app remove redis

# JSON output
azd app remove redis --output json
```

### Behavior

- Removing a service that is not present fails and lists the current service names.
- The rest of azure.yaml is preserved; only the named service entry is deleted.
- Supports the global `--output json` flag for scripting.

---

## `azd app run`

Starts your development environment based on project type with support for multi-service orchestration.
Expand Down
170 changes: 170 additions & 0 deletions cli/src/cmd/app/commands/remove.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package commands

import (
"fmt"
"os"
"path/filepath"
"sort"
"strings"

"github.com/jongio/azd-core/cliout"

"github.com/spf13/cobra"
"gopkg.in/yaml.v3"
)

// NewRemoveCommand creates the remove command.
func NewRemoveCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "remove [service]",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[F-005] The command requires exactly one service argument (enforced by MaximumNArgs(1) plus the manual zero-arg check in runRemove), but the usage string and docs present it as optional ([service]). Use angle brackets to signal a required positional argument, matching CLI convention:

Suggested change
Use: "remove [service]",
Use: "remove <service>",

Short: "Remove a service from azure.yaml",
Long: `Remove a service from the services section of azure.yaml.

This is the inverse of "azd app add". It deletes the named service entry and
leaves every other service in the file untouched. Use it to undo an add or to
drop a service you no longer run.

Examples:
# Remove the redis service
azd app remove redis

# JSON output
azd app remove redis --output json`,
SilenceUsage: true,
Args: cobra.MaximumNArgs(1),
RunE: runRemove,
ValidArgsFunction: completeServiceArgs,
}

return cmd
}

// RemoveResult represents the result of removing a service, mirroring AddResult.
type RemoveResult struct {
Service string `json:"service"`
Removed bool `json:"removed"`
Message string `json:"message,omitempty"`
}

func runRemove(_ *cobra.Command, args []string) error {
if len(args) == 0 {
return fmt.Errorf("specify a service name to remove")
}

serviceName := args[0]

azureYamlPath, err := findAzureYamlForAdd()
if err != nil {
return fmt.Errorf("find azure.yaml: %w", err)
}

cliout.CommandHeader("remove", fmt.Sprintf("Remove %s", serviceName))

removed, remaining, err := removeServiceFromYaml(azureYamlPath, serviceName)
if err != nil {
return fmt.Errorf("failed to remove service: %w", err)
}

if !removed {
return fmt.Errorf("%s", serviceNotFoundMessage(serviceName, remaining))
}
Comment on lines +68 to +70

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[F-001] --output json produces no JSON on the not-found path. runRemove returns a plain error here, which cobra writes to stderr with a non-zero exit — so azd app remove <svc> --output json prints nothing on stdout when the service is absent. The sibling add command emits a structured result for its analogous no-op case (AddResult{Added:false, ...}) on stdout, and RemoveResult.Removed bool exists for exactly this state but is never set to false. Emit JSON in JSON mode to keep the two commands' contracts symmetric:

Suggested change
if !removed {
return fmt.Errorf("%s", serviceNotFoundMessage(serviceName, remaining))
}
if !removed {
if cliout.IsJSON() {
return cliout.PrintJSON(RemoveResult{
Service: serviceName,
Removed: false,
Message: serviceNotFoundMessage(serviceName, remaining),
})
}
return fmt.Errorf("%s", serviceNotFoundMessage(serviceName, remaining))
}


if cliout.IsJSON() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[F-004] runRemove's success path is untested. Only the zero-arg guard (TestRunRemoveNoArgs) exercises runRemove; the file-discovery step, the cliout.IsJSON() to PrintJSON(RemoveResult{...}) branch (this block), the human Success branch, and the not-found error return have no coverage. The JSON-contract gap in F-001 would have been caught by a JSON-mode test. Add a test that chdirs into a t.TempDir() containing an azure.yaml, sets cliout.SetFormat("json"), captures stdout, and asserts a valid RemoveResult for both the removed and not-found cases.

return cliout.PrintJSON(RemoveResult{
Service: serviceName,
Removed: true,
Message: fmt.Sprintf("Removed %s from azure.yaml", serviceName),
})
}

cliout.Success("Removed %s from azure.yaml", serviceName)
return nil
}

// removeServiceFromYaml deletes serviceName from the services mapping in the
// azure.yaml at path. It returns whether the service was found and removed, plus
// the sorted names of the services that remain (used to build a helpful error
// when the service was not found). Every other part of the file is preserved by
// editing the parsed node tree in place.
func removeServiceFromYaml(path, serviceName string) (bool, []string, error) {
cleanPath := filepath.Clean(path)
data, err := os.ReadFile(cleanPath)
if err != nil {
return false, nil, err
}

var doc yaml.Node
if err = yaml.Unmarshal(data, &doc); err != nil {
return false, nil, err
}

if doc.Kind != yaml.DocumentNode || len(doc.Content) == 0 {
return false, nil, fmt.Errorf("invalid azure.yaml structure")
}

root := doc.Content[0]
if root.Kind != yaml.MappingNode {
return false, nil, fmt.Errorf("azure.yaml root must be a mapping")
}

// Find the services mapping.
var servicesNode *yaml.Node
for i := 0; i < len(root.Content)-1; i += 2 {
if root.Content[i].Value == "services" {
servicesNode = root.Content[i+1]
break
}
}
if servicesNode == nil || servicesNode.Kind != yaml.MappingNode {
return false, nil, nil
}

// Locate the service key/value pair.
removeIdx := -1
for j := 0; j < len(servicesNode.Content)-1; j += 2 {
if servicesNode.Content[j].Value == serviceName {
removeIdx = j
break
}
}

if removeIdx < 0 {
return false, serviceNamesFromMapping(servicesNode), nil
}

// Drop the key node and its value node (two consecutive entries).
servicesNode.Content = append(servicesNode.Content[:removeIdx], servicesNode.Content[removeIdx+2:]...)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💡 Improvement Opportunity (optional — not required to merge)

Benefit: Removing the last service leaves an empty services: {} mapping in azure.yaml rather than dropping the services: key. Deciding this intentionally (keep empty mapping vs. remove the key) and pinning it with a test prevents ambiguous output.
Effort: S
Optionality: Optional
Sketch: After the splice, if len(servicesNode.Content) == 0, optionally remove the services key/value pair from root.Content; add a test asserting the chosen behavior.


yamlOutput, err := yaml.Marshal(&doc)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[F-002] A successful removal re-marshals the entire document via yaml.Marshal(&doc), which re-emits at yaml.v3's default 4-space indent for the whole file — not just the services block. Any real remove therefore rewrites the user's entire azure.yaml formatting (e.g. 2-space to 4-space), producing a large, noisy diff even though only one service was deleted. The Long help and docs ('leaves every other service in the file untouched', 'The rest of azure.yaml is preserved') describe semantic content, not formatting, so they overstate the guarantee. This matches the existing add command, so it is a pre-existing repo trait rather than a regression. Pin the indent to reduce churn (apply to add too for consistency) and correct the docs wording:

	var buf bytes.Buffer
	enc := yaml.NewEncoder(&buf)
	enc.SetIndent(2)
	if err := enc.Encode(&doc); err != nil {
		return false, nil, err
	}
	_ = enc.Close()
	yamlOutput := buf.Bytes()

if err != nil {
return false, nil, err
}

// #nosec G306 -- azure.yaml needs to be readable
if err := os.WriteFile(path, yamlOutput, 0o644); err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[F-003] os.WriteFile truncates azure.yaml before writing the new content. If the write fails partway (disk full, I/O error) or the process is interrupted, the file is left empty or partial even though the command returns an error — destroying the user's project configuration on a routine remove. The repo already uses a write-then-rename pattern to avoid exactly this (see cli/src/internal/trust/workspace_trust.go). Write to a same-directory temp file, preserve permissions, then atomically rename over the target. This applies to the sibling add command's write as well.

return false, nil, err
}

return true, serviceNamesFromMapping(servicesNode), nil
}

// serviceNamesFromMapping returns the sorted service names in a services mapping
// node.
func serviceNamesFromMapping(servicesNode *yaml.Node) []string {
names := make([]string, 0, len(servicesNode.Content)/2)
for j := 0; j < len(servicesNode.Content)-1; j += 2 {
names = append(names, servicesNode.Content[j].Value)
}
sort.Strings(names)
return names
}

// serviceNotFoundMessage builds the error text shown when a service to remove is
// not present, listing the current service names when there are any.
func serviceNotFoundMessage(serviceName string, remaining []string) string {
if len(remaining) == 0 {
return fmt.Sprintf("service %q not found in azure.yaml", serviceName)
}
return fmt.Sprintf("service %q not found in azure.yaml. Current services: %s",
serviceName, strings.Join(remaining, ", "))
}
182 changes: 182 additions & 0 deletions cli/src/cmd/app/commands/remove_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
package commands

import (
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
)

func writeTempAzureYaml(t *testing.T, content string) string {
t.Helper()
dir := t.TempDir()
path := filepath.Join(dir, "azure.yaml")
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
t.Fatalf("failed to write azure.yaml: %v", err)
}
return path
}

func TestRemoveServiceFromYaml(t *testing.T) {
content := `name: test-app
services:
api:
language: python
project: ./api
redis:
host: containerapp
image: redis:7-alpine
ports:
- "6379:6379"
web:
language: js
project: ./web
`
path := writeTempAzureYaml(t, content)

removed, remaining, err := removeServiceFromYaml(path, "redis")
if err != nil {
t.Fatalf("removeServiceFromYaml() error: %v", err)
}
if !removed {
t.Fatal("removeServiceFromYaml() removed = false, want true")
}

wantRemaining := []string{"api", "web"}
if strings.Join(remaining, ",") != strings.Join(wantRemaining, ",") {
t.Errorf("remaining = %v, want %v", remaining, wantRemaining)
}

data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("failed to read azure.yaml: %v", err)
}
got := string(data)

if strings.Contains(got, "redis:") {
t.Errorf("azure.yaml still contains redis after removal:\n%s", got)
}
if strings.Contains(got, "6379:6379") {
t.Errorf("azure.yaml still contains redis ports after removal:\n%s", got)
}
// Siblings and top-level keys are preserved.
for _, want := range []string{"name: test-app", "api:", "web:"} {
if !strings.Contains(got, want) {
t.Errorf("azure.yaml missing %q after removal:\n%s", want, got)
}
}
}

func TestRemoveServiceFromYamlUnknown(t *testing.T) {
content := `name: test-app
services:
api:
language: python
web:
language: js
`
path := writeTempAzureYaml(t, content)

removed, remaining, err := removeServiceFromYaml(path, "redis")
if err != nil {
t.Fatalf("removeServiceFromYaml() error: %v", err)
}
if removed {
t.Error("removeServiceFromYaml() removed = true for unknown service, want false")
}

want := []string{"api", "web"}
if strings.Join(remaining, ",") != strings.Join(want, ",") {
t.Errorf("remaining = %v, want %v", remaining, want)
}

// The file must be untouched when nothing is removed.
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("failed to read azure.yaml: %v", err)
}
if string(data) != content {
t.Errorf("azure.yaml changed on a no-op removal:\n%s", string(data))
}
}

func TestRemoveServiceFromYamlNoServicesSection(t *testing.T) {
content := `name: test-app
`
path := writeTempAzureYaml(t, content)

removed, remaining, err := removeServiceFromYaml(path, "redis")
if err != nil {
t.Fatalf("removeServiceFromYaml() error: %v", err)
}
if removed {
t.Error("removeServiceFromYaml() removed = true with no services section, want false")
}
if len(remaining) != 0 {
t.Errorf("remaining = %v, want empty", remaining)
}
}

func TestServiceNotFoundMessage(t *testing.T) {
tests := []struct {
name string
service string
remaining []string
wantParts []string
}{
{
name: "with remaining services",
service: "redis",
remaining: []string{"api", "web"},
wantParts: []string{`"redis" not found`, "api, web"},
},
{
name: "no remaining services",
service: "redis",
remaining: nil,
wantParts: []string{`"redis" not found`},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
msg := serviceNotFoundMessage(tt.service, tt.remaining)
for _, part := range tt.wantParts {
if !strings.Contains(msg, part) {
t.Errorf("message %q missing %q", msg, part)
}
}
if tt.remaining == nil && strings.Contains(msg, "Current services") {
t.Errorf("message should not list services when none remain: %q", msg)
}
})
}
}

func TestRemoveResultJSON(t *testing.T) {
b, err := json.Marshal(RemoveResult{
Service: "redis",
Removed: true,
Message: "Removed redis from azure.yaml",
})
if err != nil {
t.Fatalf("marshal RemoveResult: %v", err)
}
got := string(b)
for _, want := range []string{`"service":"redis"`, `"removed":true`, `"message":"Removed redis from azure.yaml"`} {
if !strings.Contains(got, want) {
t.Errorf("RemoveResult JSON %s missing %q", got, want)
}
}
}

func TestRunRemoveNoArgs(t *testing.T) {
err := runRemove(nil, nil)
if err == nil {
t.Fatal("runRemove() with no args returned nil, want error")
}
if !strings.Contains(err.Error(), "specify a service name") {
t.Errorf("runRemove() error = %q, want it to mention specifying a service name", err.Error())
}
}
Loading
Loading