Skip to content

Commit b7ff8a5

Browse files
committed
Added Copilot review suggestions
1 parent 74462ee commit b7ff8a5

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

pkg/cloud/isolated_network.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ func (c *client) CreateIsolatedNetwork(fd *infrav1.CloudStackFailureDomain, isoN
155155
// OpenFirewallRules opens a CloudStack egress firewall for an isolated network.
156156
func (c *client) OpenFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) {
157157
// Early return if VPC is present
158+
// Firewall rules are not opened for isolated networks within a VPC because VPCs have their own mechanisms for managing firewall rules.
158159
if isoNet.Spec.VPC != nil && isoNet.Spec.VPC.ID != "" {
159160
return nil
160161
}
@@ -283,12 +284,19 @@ func (c *client) createFirewallRule(isoNet *infrav1.CloudStackIsolatedNetwork, p
283284
return nil
284285
}
285286

287+
// Named constants for ignorable error substrings
288+
const (
289+
ErrAlreadyExists = "there is already"
290+
ErrRuleConflict = "conflicts with rule"
291+
ErrNewRuleConflict = "new rule conflicts with existing rule"
292+
)
293+
286294
// Helper function to check if an error is ignorable
287295
func (c *client) isIgnorableError(err error) bool {
288296
errorMsg := strings.ToLower(err.Error())
289-
return strings.Contains(errorMsg, "there is already") ||
290-
strings.Contains(errorMsg, "conflicts with rule") ||
291-
strings.Contains(errorMsg, "new rule conflicts with existing rule")
297+
return strings.Contains(errorMsg, ErrAlreadyExists) ||
298+
strings.Contains(errorMsg, ErrRuleConflict) ||
299+
strings.Contains(errorMsg, ErrNewRuleConflict)
292300
}
293301

294302
// GetPublicIP gets a public IP with ID for cluster endpoint.

pkg/cloud/tags_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ var _ = ginkgo.Describe("Tag Unit Tests", func() {
156156
createdByCAPCResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
157157
rtlp := &csapi.ListTagsParams{}
158158
ctp := &csapi.CreateTagsParams{}
159+
// Expecting NewListTagsParams to be called twice:
160+
// Once for verifying existing tags and once for creating new tags.
159161
rs.EXPECT().NewListTagsParams().Return(rtlp).Times(2)
160162
rs.EXPECT().ListTags(rtlp).Return(createdByCAPCResponse, nil).Times(2)
161163
rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()).Return(ctp)

0 commit comments

Comments
 (0)