From 420b55698b36844873d72111ddffcc5c3e617e7f Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Mon, 19 Dec 2022 10:27:46 +0100 Subject: [PATCH] OCPBUGS-4549: destroy: azure: handle `nil` responses from msgraph sdk It seems the SDK can return `nil` as a response even if no errors occurred. If we don't check for that, we get the following error on Gov cloud: ``` 12-19 13:21:52.860 level=debug msg=deleting application registrations 12-19 13:21:52.860 E1219 05:21:48.969224 743 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference) 12-19 13:21:52.860 goroutine 1 [running]: 12-19 13:21:52.860 k8s.io/apimachinery/pkg/util/runtime.logPanic({0x6132160?, 0x22371b20}) 12-19 13:21:52.860 /go/src/github.com/openshift/installer/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:75 +0x99 12-19 13:21:52.860 k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x0?}) 12-19 13:21:52.860 /go/src/github.com/openshift/installer/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:49 +0x75 12-19 13:21:52.861 panic({0x6132160, 0x22371b20}) 12-19 13:21:52.861 /usr/lib/golang/src/runtime/panic.go:884 +0x212 12-19 13:21:52.861 github.com/openshift/installer/pkg/destroy/azure.getServicePrincipalsByTag({0x1d9580b0, 0xc0008443c0}, 0xc000cec420, {0xc000792570, 0x29}, {0xc000dcc650, 0xd}) 12-19 13:21:52.861 /go/src/github.com/openshift/installer/pkg/destroy/azure/azure.go:632 +0x322 12-19 13:21:52.861 github.com/openshift/installer/pkg/destroy/azure.deleteApplicationRegistrations({0x1d9580b0, 0xc0008443c0}, 0xc000cec420, {0x1d98a0d0, 0xc0001da000}, {0xc000dcc650, 0xd}) ``` --- pkg/destroy/azure/azure.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/destroy/azure/azure.go b/pkg/destroy/azure/azure.go index 7dc6c097c8d..1c112df441b 100644 --- a/pkg/destroy/azure/azure.go +++ b/pkg/destroy/azure/azure.go @@ -578,14 +578,18 @@ func extractODataError(err error) error { } func deleteApplicationRegistrations(ctx context.Context, graphClient *msgraphsdk.GraphServiceClient, logger logrus.FieldLogger, infraID string) error { - var errorList []error - tag := fmt.Sprintf("kubernetes.io_cluster.%s=owned", infraID) servicePrincipals, err := getServicePrincipalsByTag(ctx, graphClient, tag, infraID) if err != nil { return errors.Wrap(err, "failed to gather list of Service Principals by tag") } + // msgraphsdk can return a `nil` response even if no errors occurred + if servicePrincipals == nil { + logger.Debug("Empty response from API when listing Service Principals by tag") + return nil + } + var errorList []error for _, sp := range servicePrincipals { appID := *sp.GetAppId() logger := logger.WithField("appID", appID) @@ -602,6 +606,11 @@ func deleteApplicationRegistrations(ctx context.Context, graphClient *msgraphsdk errorList = append(errorList, err) continue } + // msgraphsdk can return a `nil` response even if no errors occurred + if resp == nil { + logger.Debugf("Empty response getting Application from Service Principal %s", *sp.GetDisplayName()) + continue + } apps := resp.GetValue() if len(apps) != 1 { err = fmt.Errorf("should have received only a single matching AppID, received %d instead", len(apps)) @@ -626,7 +635,7 @@ func getServicePrincipalsByTag(ctx context.Context, graphClient *msgraphsdk.Grap }, } resp, err := graphClient.ServicePrincipals().Get(ctx, &listQuery) - if err != nil { + if err != nil || resp == nil { return nil, err } return resp.GetValue(), nil