Skip to content

OSAC-1533: Fix --network-attachment ignored with --catalog-item #705

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.

Sign up for GitHub

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

Open
obochan-rh wants to merge 2 commits into
base: main
Choose a base branch
from
+32 −0
Open

OSAC-1533: Fix --network-attachment ignored with --catalog-item #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e4d620
OSAC-1533: Fix --network-attachment ignored with --catalog-item
obochan-rh Jun 15, 2026
6f07494
OSAC-1533: Add unit tests for buildSpecFromCatalogItem networking
obochan-rh Jun 15, 2026
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
3 changes: 3 additions & 0 deletions internal/cmd/cli/create/computeinstance/create_compute_instance_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,9 @@ func (c *runnerContext) buildSpecFromCatalogItem(catalogItemID string) (*publicv
if c.args.userData != "" {
spec.UserData = new(c.args.userData)
}
if err := c.applyNetworkingFlags(&spec); err != nil {
return nil, err
}
return spec.Build(), nil
}

Expand Down
29 changes: 29 additions & 0 deletions internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,35 @@ var _ = Describe("buildSpec", func() {
})
})

var _ = Describe("buildSpecFromCatalogItem", func() {
It("should populate attachments when network-attachment flags are set", func() {
c := &runnerContext{}
c.args.networkAttachments = []string{"n1", "subnet=n2,security-groups=g1"}
spec, err := c.buildSpecFromCatalogItem("cat-001")
Expect(err).NotTo(HaveOccurred())

want := publicv1.ComputeInstanceSpec_builder{
CatalogItem: "cat-001",
NetworkAttachments: []*publicv1.NetworkAttachment{
publicv1.NetworkAttachment_builder{Subnet: "n1"}.Build(),
publicv1.NetworkAttachment_builder{Subnet: "n2", SecurityGroups: []string{"g1"}}.Build(),
},
}.Build()
Expect(proto.Equal(spec, want)).To(BeTrue(), "spec should equal expected spec")
})

It("should return spec without attachments when no network flags are set", func() {
c := &runnerContext{}
spec, err := c.buildSpecFromCatalogItem("cat-002")
Expect(err).NotTo(HaveOccurred())

want := publicv1.ComputeInstanceSpec_builder{
CatalogItem: "cat-002",
}.Build()
Expect(proto.Equal(spec, want)).To(BeTrue(), "spec should equal expected spec")
})
})
Comment on lines +76 to +103

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a catalog-item error-path test for invalid network attachment input.

This suite validates success/empty cases but not parse failure propagation; add one case where networkAttachments contains an invalid value and assert buildSpecFromCatalogItem returns an error, so future regressions don’t silently drop invalid networking input.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go`
around lines 76 - 103, Add a new test case (It block) to the
buildSpecFromCatalogItem describe suite that validates error handling for
invalid input. Create a runnerContext with invalid or malformed values in the
networkAttachments field (such as a string that cannot be parsed correctly),
then call buildSpecFromCatalogItem and assert that it returns an error using
Expect(err).To(HaveOccurred()). This ensures the function properly rejects and
reports invalid network attachment input rather than silently ignoring it.


var _ = Describe("Create computeinstance flag registration", func() {
It("should register --catalog-item flag", func() {
cmd := Cmd()
Expand Down
Loading