Skip to content

Commit 0921e00

Browse files
joekrjoekr
authored
fix: add more nil checks and error handling (#491)
In order to prevent more nil pointer dereferencing this will do some basic nil checks. * Add StringEquals function to ptr package and RequeueAfter to requeue work * try to fix kustomize to fix e2e
1 parent 6b03d7d commit 0921e00

14 files changed

+116
-20
lines changed

‎argo/capoci-e2e-workflow.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,11 @@ spec:
268268
docker version
269269
270270
# Ensure binaries installed by the Makefile are on PATH (kustomize, ginkgo, etc.)
271-
export PATH="/workspace/bin:${PATH}"
271+
export PATH="/workspace/hack/tools/bin:/workspace/bin:${PATH}"
272272
# Also provide kustomize in a standard location for tools that exec 'kustomize' by name.
273-
ln -sf /workspace/bin/kustomize /usr/local/bin/kustomize || true
273+
ln -sf /workspace/hack/tools/bin/kustomize /usr/local/bin/kustomize || true
274274
ln -sf /workspace/bin/ginkgo /usr/local/bin/ginkgo || true
275-
ln -sf /workspace/bin/kubectl /usr/local/bin/kubectl || true
275+
ln -sf /workspace/hack/tools/bin/kubectl /usr/local/bin/kubectl || true
276276
command -v kustomize || true
277277
kustomize version || true
278278
command -v kubectl || true

‎argo/capoci-e2e-workflowtemplate.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,11 @@ spec:
309309
fi
310310
311311
# Ensure binaries installed by the Makefile are on PATH (kustomize, ginkgo, etc.)
312-
export PATH="/workspace/bin:${PATH}"
312+
export PATH="/workspace/hack/tools/bin:/workspace/bin:${PATH}"
313313
# Also provide kustomize in a standard location for tools that exec 'kustomize' by name.
314-
ln -sf /workspace/bin/kustomize /usr/local/bin/kustomize || true
314+
ln -sf /workspace/hack/tools/bin/kustomize /usr/local/bin/kustomize || true
315315
ln -sf /workspace/bin/ginkgo /usr/local/bin/ginkgo || true
316-
ln -sf /workspace/bin/kubectl /usr/local/bin/kubectl || true
316+
ln -sf /workspace/hack/tools/bin/kubectl /usr/local/bin/kubectl || true
317317
command -v kustomize || true
318318
kustomize version || true
319319
command -v kubectl || true

‎cloud/ociutil/ptr/from_ptr.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ func ToString(p *string) (v string) {
1313
return *p
1414
}
1515

16+
// StringEquals safely checks if a string pointer is non-nil and equals the given value.
17+
// Returns false if the pointer is nil.
18+
func StringEquals(p *string, value string) bool {
19+
if p == nil {
20+
return false
21+
}
22+
return *p == value
23+
}
24+
1625
// ToBool returns bool value dereferenced if the passed
1726
// in pointer was not nil. Returns a bool zero value (false) if the
1827
// pointer was nil.

‎cloud/ociutil/ptr/from_ptr_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,48 @@ func TestToString(t *testing.T) {
3333
}
3434
}
3535

36+
func TestStringEquals(t *testing.T) {
37+
tests := []struct {
38+
name string
39+
input *string
40+
value string
41+
want bool
42+
}{
43+
{
44+
name: "nil pointer",
45+
input: nil,
46+
value: "test",
47+
want: false,
48+
},
49+
{
50+
name: "non-nil pointer equals value",
51+
input: func() *string { s := "test"; return &s }(),
52+
value: "test",
53+
want: true,
54+
},
55+
{
56+
name: "non-nil pointer does not equal value",
57+
input: func() *string { s := "test"; return &s }(),
58+
value: "different",
59+
want: false,
60+
},
61+
{
62+
name: "non-nil pointer with empty string equals empty value",
63+
input: func() *string { s := ""; return &s }(),
64+
value: "",
65+
want: true,
66+
},
67+
}
68+
69+
for _, tt := range tests {
70+
t.Run(tt.name, func(t *testing.T) {
71+
if got := StringEquals(tt.input, tt.value); got != tt.want {
72+
t.Errorf("StringEquals() = %v, want %v", got, tt.want)
73+
}
74+
})
75+
}
76+
}
77+
3678
func TestToBool(t *testing.T) {
3779
tests := []struct {
3880
name string

‎cloud/scope/cluster.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/go-logr/logr"
2626
infrastructurev1beta2 "github.com/oracle/cluster-api-provider-oci/api/v1beta2"
2727
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil"
28+
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr"
2829
identityClient "github.com/oracle/cluster-api-provider-oci/cloud/services/identity"
2930
lb "github.com/oracle/cluster-api-provider-oci/cloud/services/loadbalancer"
3031
nlb "github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer"
@@ -309,7 +310,7 @@ func GetRegionCodeFromRegion(ctx context.Context, identityClient identityClient.
309310
return "", errors.Wrap(err, "failed to list oci regions")
310311
}
311312
for _, regionCode := range regionCodes.Items {
312-
if *regionCode.Name == region {
313+
if ptr.StringEquals(regionCode.Name, region) {
313314
return *regionCode.Key, nil
314315
}
315316
}

‎cloud/scope/drg_reconciler.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222

2323
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil"
24+
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr"
2425
"github.com/oracle/oci-go-sdk/v65/common"
2526
"github.com/oracle/oci-go-sdk/v65/core"
2627
"github.com/pkg/errors"
@@ -92,7 +93,7 @@ func (s *ClusterScope) GetDRG(ctx context.Context) (*core.Drg, error) {
9293
return nil, err
9394
}
9495
for _, drg := range response.Items {
95-
if *drg.DisplayName == s.getDRG().Name {
96+
if ptr.StringEquals(drg.DisplayName, s.getDRG().Name) {
9697
if s.IsResourceCreatedByClusterAPI(drg.FreeformTags) {
9798
return &drg, nil
9899
} else {

‎cloud/scope/drg_rpc_attachment_reconciler.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"time"
2222

2323
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil"
24+
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr"
2425
vcn "github.com/oracle/cluster-api-provider-oci/cloud/services/vcn"
2526
"github.com/oracle/oci-go-sdk/v65/common"
2627
"github.com/oracle/oci-go-sdk/v65/core"
@@ -183,7 +184,7 @@ func (s *ClusterScope) lookupRPC(ctx context.Context, drgId *string, rpcId *stri
183184
return nil, err
184185
}
185186
for _, rpc := range response.Items {
186-
if *rpc.DisplayName == s.OCIClusterAccessor.GetName() {
187+
if ptr.StringEquals(rpc.DisplayName, s.OCIClusterAccessor.GetName()) {
187188
if s.IsResourceCreatedByClusterAPI(rpc.FreeformTags) {
188189
rpcs = append(rpcs, rpc)
189190
} else {

‎cloud/scope/machine.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ func (m *MachineScope) ReconcileDeleteInstanceOnLB(ctx context.Context) error {
805805

806806
func (m *MachineScope) containsNLBBackend(backendSet networkloadbalancer.BackendSet, backendName string) bool {
807807
for _, backend := range backendSet.Backends {
808-
if *backend.Name == backendName {
808+
if ptr.StringEquals(backend.Name, backendName) {
809809
m.Logger.Info("Instance present in the backend")
810810
return true
811811
}
@@ -815,7 +815,7 @@ func (m *MachineScope) containsNLBBackend(backendSet networkloadbalancer.Backend
815815

816816
func (m *MachineScope) containsLBBackend(backendSet loadbalancer.BackendSet, backendName string) bool {
817817
for _, backend := range backendSet.Backends {
818-
if *backend.Name == backendName {
818+
if ptr.StringEquals(backend.Name, backendName) {
819819
m.Logger.Info("Instance present in the backend")
820820
return true
821821
}

‎cloud/scope/managed_control_plane.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func (s *ManagedControlPlaneScope) GetOrCreateControlPlane(ctx context.Context)
262262

263263
var clusterId *string
264264
for _, resource := range wrResponse.Resources {
265-
if *resource.EntityType == "cluster" {
265+
if ptr.StringEquals(resource.EntityType, "cluster") {
266266
clusterId = resource.Identifier
267267
}
268268
}
@@ -1079,7 +1079,7 @@ func getActualAddonConfigurations(addonConfigurations []oke.AddonConfiguration)
10791079

10801080
func getAddon(addons []infrastructurev1beta2.Addon, name string) *infrastructurev1beta2.Addon {
10811081
for i, addon := range addons {
1082-
if *addon.Name == name {
1082+
if ptr.StringEquals(addon.Name, name) {
10831083
return &addons[i]
10841084
}
10851085
}

‎cloud/scope/service_gateway_reconciler.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"strings"
2222

2323
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil"
24+
"github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr"
2425
"github.com/oracle/oci-go-sdk/v65/common"
2526
"github.com/oracle/oci-go-sdk/v65/core"
2627
"github.com/pkg/errors"
@@ -139,7 +140,7 @@ func (s *ClusterScope) GetServiceGateway(ctx context.Context) (*core.ServiceGate
139140
return nil, errors.Wrap(err, "failed to list Service gateways")
140141
}
141142
for _, sgw := range sgws.Items {
142-
if *sgw.DisplayName == ServiceGatewayName {
143+
if ptr.StringEquals(sgw.DisplayName, ServiceGatewayName) {
143144
if s.IsResourceCreatedByClusterAPI(sgw.FreeformTags) {
144145
return &sgw, nil
145146
}

0 commit comments

Comments
 (0)