diff --git a/rest-api/AGENTS.md b/rest-api/AGENTS.md index 8605a76f34..27b8b21a27 100644 --- a/rest-api/AGENTS.md +++ b/rest-api/AGENTS.md @@ -202,6 +202,11 @@ verification expectations. send its ID in the list request, and require each returned row to match that exact tenant ID. Test a dual-role caller whose response also contains provider-owned and other-tenant resources. +- When a site-wide TUI resource picker must ignore a narrower active scope, + clear that scope and invalidate filtered caches before fetching, then restore + the scope and invalidate filtered caches again on every return path so + site-wide entries cannot be reused under the restored scope. Test the scoped + case. - Tests that need a database use a PostgreSQL container (testcontainers-go or the Makefile-managed container). - Organize tests by the production function or method under test, not by individual diff --git a/rest-api/cli/tui/commands.go b/rest-api/cli/tui/commands.go index eae37aaa38..94ec9e6338 100644 --- a/rest-api/cli/tui/commands.go +++ b/rest-api/cli/tui/commands.go @@ -40,6 +40,7 @@ func AllCommands() []Command { {Name: "vpc update", Description: "Update a VPC", Run: cmdVPCUpdate}, {Name: "vpc virtualization update", Description: "Update VPC virtualization", Run: cmdVPCVirtualizationUpdate}, {Name: "vpc delete", Description: "Delete a VPC", Run: cmdVPCDelete}, + {Name: "vpc-peering create", Description: "Create VPC peerings", Run: cmdVPCPeeringCreate}, {Name: "subnet list", Description: "List all subnets", Run: cmdSubnetList}, {Name: "subnet get", Description: "Get subnet details", Run: cmdSubnetGet}, diff --git a/rest-api/cli/tui/generated_body_form_test.go b/rest-api/cli/tui/generated_body_form_test.go index 174950b0b3..15d325f692 100644 --- a/rest-api/cli/tui/generated_body_form_test.go +++ b/rest-api/cli/tui/generated_body_form_test.go @@ -315,7 +315,11 @@ func TestGeneratedTUICommandGuidedBodyResolvesNamesToIDs(t *testing.T) { "", ) _, err := withStdin(t, "\ny\n", func() (string, error) { - return "", requireTUICommand(t, "vpc-peering create").Run(session, nil) + return "", runGeneratedTUICommand( + session, + generatedCommandInfoByName(t, "vpc-peering create"), + nil, + ) }) require.NoError(t, err) diff --git a/rest-api/cli/tui/repl_pty_test.go b/rest-api/cli/tui/repl_pty_test.go index 861a65bf09..19557b5302 100644 --- a/rest-api/cli/tui/repl_pty_test.go +++ b/rest-api/cli/tui/repl_pty_test.go @@ -324,20 +324,36 @@ func TestCLIRegression_RealTerminalAndNonInteractive(t *testing.T) { terminal.send(t, "n\r") terminal.waitFor(t, "nico:acme") - // Guided request bodies preload site/VPC names, resolve two body IDs in - // order, and execute only after confirmation. + // Choosing exactly two VPCs preserves the original guided workflow. terminal.send(t, "vpc-peering create\r") - terminal.waitFor(t, "Request body input") - terminal.send(t, "\r") - terminal.waitFor(t, "Site id:") + terminal.waitFor(t, "VPC peering creation requires a site") + terminal.waitFor(t, "Site:") terminal.send(t, "site-one\r") - terminal.waitFor(t, "Vpc1id:") + terminal.waitFor(t, "VPC selection") + terminal.send(t, "Choose VPCs\r") + terminal.waitFor(t, "VPC:") terminal.send(t, "vpc-one\r") - terminal.waitFor(t, "Vpc2id:") + terminal.waitFor(t, "VPC:") terminal.send(t, "vpc-two\r") - terminal.waitFor(t, "Run vpc-peering create (POST)?") + terminal.waitFor(t, "Add another VPC (selected 2)?") + terminal.send(t, "n\r") + terminal.waitFor(t, "Selected VPCs (2)") + terminal.waitFor(t, "Peerings to create (1)") + terminal.waitFor(t, "Create 1 VPC peering(s)?") terminal.send(t, "y\r") - terminal.waitFor(t, `"id": "peering-1"`) + terminal.waitFor(t, "Summary: created 1, skipped 0, failed 0") + + // Selecting all same-site VPCs previews every unique pair and skips the + // peering created by the preceding two-VPC workflow. + terminal.send(t, "vpc-peering create\r") + terminal.waitFor(t, "VPC selection") + terminal.send(t, "Select all\r") + terminal.waitFor(t, "Selected VPCs (3)") + terminal.waitFor(t, "Peerings to create (2)") + terminal.waitFor(t, "Existing peerings to skip (1)") + terminal.waitFor(t, "Create 2 VPC peering(s)?") + terminal.send(t, "y\r") + terminal.waitFor(t, "Summary: created 2, skipped 1, failed 0") // Generated enum and secret fields use the guided form. Optional // free-form fields can be skipped, and terminal password input is not @@ -422,12 +438,16 @@ func TestCLIRegression_RealTerminalAndNonInteractive(t *testing.T) { http.MethodPost, "/v2/org/acme/nico/vpc-peering", ) - require.Len(t, peeringRequests, 1, "cancelled mutation must not reach the API") - assert.JSONEq( - t, + require.Len(t, peeringRequests, 3, "cancelled and existing peerings must not reach the API") + peeringBodies := make([]string, len(peeringRequests)) + for i, request := range peeringRequests { + peeringBodies[i] = request.Body + } + assert.ElementsMatch(t, []string{ `{"siteId":"site-1","vpc1Id":"vpc-1","vpc2Id":"vpc-2"}`, - peeringRequests[0].Body, - ) + `{"siteId":"site-1","vpc1Id":"vpc-1","vpc2Id":"vpc-flat"}`, + `{"siteId":"site-1","vpc1Id":"vpc-2","vpc2Id":"vpc-flat"}`, + }, peeringBodies) prefixRequests := recorder.matching( http.MethodPost, @@ -829,6 +849,28 @@ func newInteractiveRegressionHandler(recorder *cliRegressionRecorder) http.Handl request.URL.Path == "/v2/org/acme/nico/vpc-peering": w.WriteHeader(http.StatusCreated) _, _ = io.WriteString(w, `{"id":"peering-1","status":"Ready"}`) + case request.Method == http.MethodGet && + request.URL.Path == "/v2/org/acme/nico/vpc-peering": + peerings := make([]map[string]string, 0) + for i, peeringRequest := range recorder.matching( + http.MethodPost, + "/v2/org/acme/nico/vpc-peering", + ) { + var peering map[string]string + if err := json.Unmarshal([]byte(peeringRequest.Body), &peering); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + peerings = append(peerings, map[string]string{ + "id": fmt.Sprintf("peering-%d", i+1), + "siteId": peering["siteId"], + "vpc1Id": peering["vpc1Id"], + "vpc2Id": peering["vpc2Id"], + }) + } + if err := json.NewEncoder(w).Encode(peerings); err != nil { + return + } case request.Method == http.MethodPut && request.URL.Path == "/v2/org/acme/nico/credential/bmc": w.WriteHeader(http.StatusAccepted) diff --git a/rest-api/cli/tui/vpc_peering_create.go b/rest-api/cli/tui/vpc_peering_create.go new file mode 100644 index 0000000000..7ebb86c213 --- /dev/null +++ b/rest-api/cli/tui/vpc_peering_create.go @@ -0,0 +1,269 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tui + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "strings" +) + +const vpcPeeringCreateCommand = "vpc-peering create" + +type vpcPeeringPair struct { + first NamedItem + second NamedItem +} + +func (p vpcPeeringPair) label() string { + return fmt.Sprintf("%s (%s) <-> %s (%s)", p.first.Name, p.first.ID, p.second.Name, p.second.ID) +} + +type vpcPeeringPairKey struct { + first string + second string +} + +func newVPCPeeringPairKey(first, second string) vpcPeeringPairKey { + first = strings.TrimSpace(first) + second = strings.TrimSpace(second) + if first > second { + first, second = second, first + } + return vpcPeeringPairKey{first: first, second: second} +} + +type vpcPeeringPlan struct { + create []vpcPeeringPair + skip []vpcPeeringPair +} + +func buildVPCPeeringPlan(selected, existing []NamedItem) vpcPeeringPlan { + unique := make([]NamedItem, 0, len(selected)) + seen := make(map[string]struct{}, len(selected)) + for _, vpc := range selected { + vpc.ID = strings.TrimSpace(vpc.ID) + if vpc.ID == "" { + continue + } + if _, duplicate := seen[vpc.ID]; duplicate { + continue + } + seen[vpc.ID] = struct{}{} + unique = append(unique, vpc) + } + + existingPairs := make(map[vpcPeeringPairKey]struct{}, len(existing)) + for _, peering := range existing { + first := strings.TrimSpace(peering.Extra["vpc1Id"]) + second := strings.TrimSpace(peering.Extra["vpc2Id"]) + if first == "" || second == "" || first == second { + continue + } + existingPairs[newVPCPeeringPairKey(first, second)] = struct{}{} + } + + plan := vpcPeeringPlan{} + for i, first := range unique { + for _, second := range unique[i+1:] { + pair := vpcPeeringPair{first: first, second: second} + if _, exists := existingPairs[newVPCPeeringPairKey(first.ID, second.ID)]; exists { + plan.skip = append(plan.skip, pair) + continue + } + plan.create = append(plan.create, pair) + } + } + return plan +} + +func cmdVPCPeeringCreate(s *Session, args []string) error { + if len(args) > 0 { + info, ok := generatedAutocompleteInfo(vpcPeeringCreateCommand) + if !ok { + return fmt.Errorf("generated command %q is unavailable", vpcPeeringCreateCommand) + } + return runGeneratedTUICommand(s, info, args) + } + + siteID, err := requireSiteScope(s, "VPC peering creation requires a site. Select a site.") + if err != nil { + return err + } + selected, err := promptVPCPeeringVPCs(s, siteID) + if err != nil { + return err + } + existing, err := fetchVPCPeeringsForSite(s, siteID) + if err != nil { + return fmt.Errorf("listing existing VPC peerings: %w", err) + } + + plan := buildVPCPeeringPlan(selected, existing) + printVPCPeeringPlan(os.Stdout, selected, plan) + if len(plan.create) > 0 { + confirmed, confirmErr := PromptConfirm(fmt.Sprintf("Create %d VPC peering(s)?", len(plan.create))) + if confirmErr != nil { + return confirmErr + } + if !confirmed { + return nil + } + } + return executeVPCPeeringPlan(s, siteID, plan, os.Stdout) +} + +func promptVPCPeeringVPCs(s *Session, siteID string) ([]NamedItem, error) { + savedVpcID, savedVpcName := s.Scope.VpcID, s.Scope.VpcName + s.Scope.VpcID, s.Scope.VpcName = "", "" + s.Cache.InvalidateFiltered() + defer func() { + s.Scope.VpcID, s.Scope.VpcName = savedVpcID, savedVpcName + s.Cache.InvalidateFiltered() + }() + + vpcs, err := s.Resolver.Fetch(context.Background(), "vpc") + if err != nil { + return nil, fmt.Errorf("listing VPCs: %w", err) + } + siteVPCs := readyVPCsForSite(vpcs, siteID) + if len(siteVPCs) < 2 { + return nil, fmt.Errorf("site requires at least two Ready VPCs to create peerings") + } + + mode, err := PromptChoice( + "VPC selection", + []string{"Choose VPCs", "Select all"}, + "Choose VPCs", + ) + if err != nil { + return nil, err + } + if mode == "Select all" { + return siteVPCs, nil + } + + selected := make([]NamedItem, 0, len(siteVPCs)) + selectedIDs := make(map[string]struct{}, len(siteVPCs)) + for len(selected) < len(siteVPCs) { + available := make([]NamedItem, 0, len(siteVPCs)-len(selected)) + for _, vpc := range siteVPCs { + if _, picked := selectedIDs[vpc.ID]; !picked { + available = append(available, vpc) + } + } + vpc, selectErr := s.Resolver.SelectFromItems("VPC", available) + if selectErr != nil { + return nil, selectErr + } + selected = append(selected, *vpc) + selectedIDs[vpc.ID] = struct{}{} + if len(selected) < 2 { + continue + } + if len(selected) == len(siteVPCs) { + break + } + more, promptErr := PromptConfirm(fmt.Sprintf("Add another VPC (selected %d)?", len(selected))) + if promptErr != nil { + return nil, promptErr + } + if !more { + break + } + } + return selected, nil +} + +func readyVPCsForSite(vpcs []NamedItem, siteID string) []NamedItem { + ready := make([]NamedItem, 0, len(vpcs)) + for _, vpc := range vpcs { + if strings.TrimSpace(vpc.Extra["siteId"]) == siteID && + strings.EqualFold(strings.TrimSpace(vpc.Status), "Ready") { + ready = append(ready, vpc) + } + } + return ready +} + +func fetchVPCPeeringsForSite(s *Session, siteID string) ([]NamedItem, error) { + items, err := s.fetchAll(apiPath(s, "vpc-peering"), map[string]string{"siteId": siteID}) + if err != nil { + return nil, err + } + peerings := make([]NamedItem, 0, len(items)) + for _, item := range items { + peerings = append(peerings, NamedItem{Extra: map[string]string{ + "vpc1Id": str(item, "vpc1Id"), + "vpc2Id": str(item, "vpc2Id"), + }}) + } + return peerings, nil +} + +func printVPCPeeringPlan(w io.Writer, selected []NamedItem, plan vpcPeeringPlan) { + fmt.Fprintf(w, "Selected VPCs (%d):\n", len(selected)) + for _, vpc := range selected { + fmt.Fprintf(w, " - %s (%s)\n", vpc.Name, vpc.ID) + } + fmt.Fprintf(w, "Peerings to create (%d):\n", len(plan.create)) + for _, pair := range plan.create { + fmt.Fprintf(w, " - %s\n", pair.label()) + } + fmt.Fprintf(w, "Existing peerings to skip (%d):\n", len(plan.skip)) + for _, pair := range plan.skip { + fmt.Fprintf(w, " - %s\n", pair.label()) + } +} + +type vpcPeeringFailure struct { + pair vpcPeeringPair + err error +} + +func executeVPCPeeringPlan(s *Session, siteID string, plan vpcPeeringPlan, w io.Writer) error { + created := make([]vpcPeeringPair, 0, len(plan.create)) + failed := make([]vpcPeeringFailure, 0) + for _, pair := range plan.create { + body, _ := json.Marshal(map[string]string{ + "siteId": siteID, + "vpc1Id": pair.first.ID, + "vpc2Id": pair.second.ID, + }) + _, _, err := s.Client.Do("POST", apiPath(s, "vpc-peering"), nil, nil, body) + if err != nil { + failed = append(failed, vpcPeeringFailure{pair: pair, err: err}) + continue + } + created = append(created, pair) + } + if len(plan.create) > 0 && s.Cache != nil { + s.Cache.Invalidate("vpc-peering") + } + + fmt.Fprintln(w, "VPC peering results:") + for _, pair := range created { + fmt.Fprintf(w, " CREATED %s\n", pair.label()) + } + for _, pair := range plan.skip { + fmt.Fprintf(w, " SKIPPED %s (already exists)\n", pair.label()) + } + for _, failure := range failed { + fmt.Fprintf(w, " FAILED %s: %v\n", failure.pair.label(), failure.err) + } + fmt.Fprintf( + w, + "Summary: created %d, skipped %d, failed %d\n", + len(created), + len(plan.skip), + len(failed), + ) + if len(failed) > 0 { + return fmt.Errorf("%d VPC peering(s) failed", len(failed)) + } + return nil +} diff --git a/rest-api/cli/tui/vpc_peering_create_test.go b/rest-api/cli/tui/vpc_peering_create_test.go new file mode 100644 index 0000000000..f7beaf46fc --- /dev/null +++ b/rest-api/cli/tui/vpc_peering_create_test.go @@ -0,0 +1,185 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tui + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + appcli "github.com/NVIDIA/infra-controller/rest-api/cli/pkg" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildVPCPeeringPlan(t *testing.T) { + tests := []struct { + name string + selected []NamedItem + existing []NamedItem + wantCreate int + wantSkip int + }{ + { + name: "three VPCs produce three peerings", + selected: testVPCPeeringItems(3), + wantCreate: 3, + }, + { + name: "five VPCs produce ten peerings", + selected: testVPCPeeringItems(5), + wantCreate: 10, + }, + { + name: "existing peering is skipped regardless of order", + selected: testVPCPeeringItems(3), + existing: []NamedItem{{Extra: map[string]string{ + "vpc1Id": "vpc-3", + "vpc2Id": "vpc-1", + }}}, + wantCreate: 2, + wantSkip: 1, + }, + { + name: "duplicate selection does not create duplicate or self peering", + selected: append(testVPCPeeringItems(3), testVPCPeeringItems(1)...), + wantCreate: 3, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + plan := buildVPCPeeringPlan(test.selected, test.existing) + assert.Len(t, plan.create, test.wantCreate) + assert.Len(t, plan.skip, test.wantSkip) + for _, pair := range append(plan.create, plan.skip...) { + assert.NotEqual(t, pair.first.ID, pair.second.ID) + } + }) + } +} + +func TestExecuteVPCPeeringPlan(t *testing.T) { + var requests []map[string]string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.Equal(t, "/v2/org/acme/nico/vpc-peering", r.URL.Path) + var body map[string]string + decodeErr := json.NewDecoder(r.Body).Decode(&body) + require.NoError(t, decodeErr) + requests = append(requests, body) + if body["vpc1Id"] == "vpc-1" && body["vpc2Id"] == "vpc-3" { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message":"peering failed"}`)) + return + } + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":"peering-1"}`)) + })) + defer server.Close() + + selected := testVPCPeeringItems(3) + plan := buildVPCPeeringPlan(selected, []NamedItem{{Extra: map[string]string{ + "vpc1Id": "vpc-1", + "vpc2Id": "vpc-2", + }}}) + session := NewSession( + appcli.NewClient(server.URL, "acme", "token", nil, false), + "acme", + "", + ) + var output bytes.Buffer + err := executeVPCPeeringPlan(session, "site-1", plan, &output) + require.Error(t, err) + assert.Len(t, requests, 2, "later pairs must run after a failure") + assert.Contains(t, output.String(), "SKIPPED VPC 1 (vpc-1) <-> VPC 2 (vpc-2)") + assert.Contains(t, output.String(), "FAILED VPC 1 (vpc-1) <-> VPC 3 (vpc-3)") + assert.Contains(t, output.String(), "Summary: created 1, skipped 1, failed 1") +} + +func TestFetchVPCPeeringsForSite(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.Equal(t, "/v2/org/acme/nico/vpc-peering", r.URL.Path) + assert.Equal(t, "site-1", r.URL.Query().Get("siteId")) + assert.Empty(t, r.URL.Query().Get("vpcId")) + _, _ = w.Write([]byte(`[{ + "id":"peering-1", + "siteId":"site-1", + "vpc1Id":"vpc-1", + "vpc2Id":"vpc-2" + }]`)) + })) + defer server.Close() + + session := NewSession( + appcli.NewClient(server.URL, "acme", "token", nil, false), + "acme", + "", + ) + session.Scope.VpcID = "vpc-1" + peerings, err := fetchVPCPeeringsForSite(session, "site-1") + require.NoError(t, err) + require.Len(t, peerings, 1) + assert.Equal(t, "vpc-1", peerings[0].Extra["vpc1Id"]) + assert.Equal(t, "vpc-2", peerings[0].Extra["vpc2Id"]) +} + +func TestPromptVPCPeeringVPCs(t *testing.T) { + session := NewSession(nil, "acme", "") + session.Scope.VpcID = "vpc-1" + session.Scope.VpcName = "VPC 1" + session.Cache.Set("vpc", []NamedItem{{ID: "vpc-1"}}) + session.Resolver.RegisterFetcher("vpc", func(context.Context) ([]NamedItem, error) { + assert.Empty(t, session.Scope.VpcID) + assert.Empty(t, session.Scope.VpcName) + return nil, assert.AnError + }) + + _, err := promptVPCPeeringVPCs(session, "site-1") + require.ErrorIs(t, err, assert.AnError) + assert.Equal(t, "vpc-1", session.Scope.VpcID) + assert.Equal(t, "VPC 1", session.Scope.VpcName) + assert.Nil(t, session.Cache.Get("vpc")) +} + +func TestReadyVPCsForSite(t *testing.T) { + vpcs := []NamedItem{ + {ID: "ready", Status: "Ready", Extra: map[string]string{"siteId": "site-1"}}, + {ID: "pending", Status: "Pending", Extra: map[string]string{"siteId": "site-1"}}, + {ID: "other-site", Status: "Ready", Extra: map[string]string{"siteId": "site-2"}}, + } + + assert.Equal(t, []NamedItem{vpcs[0]}, readyVPCsForSite(vpcs, "site-1")) +} + +func testVPCPeeringItems(count int) []NamedItem { + items := make([]NamedItem, count) + for i := range count { + id := i + 1 + items[i] = NamedItem{ + Name: fmt.Sprintf("VPC %d", id), + ID: fmt.Sprintf("vpc-%d", id), + Extra: map[string]string{"siteId": "site-1"}, + } + } + return items +} + +func TestPrintVPCPeeringPlan(t *testing.T) { + selected := testVPCPeeringItems(3) + plan := buildVPCPeeringPlan(selected, nil) + var output bytes.Buffer + printVPCPeeringPlan(&output, selected, plan) + + assert.Contains(t, output.String(), "Selected VPCs (3)") + assert.Contains(t, output.String(), "Peerings to create (3)") + assert.Contains(t, output.String(), "Existing peerings to skip (0)") + assert.Equal(t, 3, strings.Count(output.String(), "<->")) +}