-
Notifications
You must be signed in to change notification settings - Fork 6
feat(remove): add remove command to remove a service from azure.yaml #511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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]", | ||||||||||||||||||||||||||||
| 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [F-001]
Suggested change
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if cliout.IsJSON() { | ||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [F-004] |
||||||||||||||||||||||||||||
| 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:]...) | ||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| yamlOutput, err := yaml.Marshal(&doc) | ||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [F-002] A successful removal re-marshals the entire document via 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 { | ||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [F-003] |
||||||||||||||||||||||||||||
| 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, ", ")) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| 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()) | ||
| } | ||
| } |
There was a problem hiding this comment.
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 inrunRemove), but the usage string and docs present it as optional ([service]). Use angle brackets to signal a required positional argument, matching CLI convention: