Repository: Azure/aad-pod-identity
Pull request #555 (2020-04-08 22:25:49)

add nil check during conversion
<!-- Thank you for helping AAD Pod Identity with a pull request! -->

**Reason for Change**:
<!-- What does this PR improve or fix in AAD Pod Identity? Why is it needed? -->
- Checks if the `AzureIdentityRef` and `AzureBindingRef` exists before conversion to prevent accessing nil pointer.

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 1234, add "Fixes #1234" to the next line. -->

**Notes for Reviewers**:



Patch:
diff --git a/cmd/nmi/main.go b/cmd/nmi/main.go
index 04d16ee2..4b9c2eb7 100644
--- a/cmd/nmi/main.go
+++ b/cmd/nmi/main.go
@@ -83,7 +83,7 @@ func main() {
 	exit := make(<-chan struct{})
 	client.Start(exit)
 	*forceNamespaced = *forceNamespaced || "true" == os.Getenv("FORCENAMESPACED")
-	klog.Infof("Running NMI in namespaced mode: %v", forceNamespaced)
+	klog.Infof("Running NMI in namespaced mode: %v", *forceNamespaced)
 
 	s := server.NewServer(*micNamespace, *blockInstanceMetadata, *metadataHeaderRequired)
 	s.KubeClient = client
diff --git a/pkg/apis/aadpodidentity/v1/conversion.go b/pkg/apis/aadpodidentity/v1/conversion.go
index dbd65909..e253db61 100644
--- a/pkg/apis/aadpodidentity/v1/conversion.go
+++ b/pkg/apis/aadpodidentity/v1/conversion.go
@@ -41,8 +41,14 @@ func ConvertV1IdentityToInternalIdentity(identity AzureIdentity) (resIdentity aa
 }
 
 func ConvertV1AssignedIdentityToInternalAssignedIdentity(assignedIdentity AzureAssignedIdentity) (resAssignedIdentity aadpodid.AzureAssignedIdentity) {
-	retIdentity := ConvertV1IdentityToInternalIdentity(*assignedIdentity.Spec.AzureIdentityRef)
-	retBinding := ConvertV1BindingToInternalBinding(*assignedIdentity.Spec.AzureBindingRef)
+	var retIdentity aadpodid.AzureIdentity
+	var retBinding aadpodid.AzureIdentityBinding
+	if assignedIdentity.Spec.AzureIdentityRef != nil {
+		retIdentity = ConvertV1IdentityToInternalIdentity(*assignedIdentity.Spec.AzureIdentityRef)
+	}
+	if assignedIdentity.Spec.AzureBindingRef != nil {
+		retBinding = ConvertV1BindingToInternalBinding(*assignedIdentity.Spec.AzureBindingRef)
+	}
 
 	return aadpodid.AzureAssignedIdentity{
 		TypeMeta:   assignedIdentity.TypeMeta,


Test patch:
diff --git a/pkg/apis/aadpodidentity/v1/conversion_test.go b/pkg/apis/aadpodidentity/v1/conversion_test.go
index 8afe61c3..e72e9f84 100644
--- a/pkg/apis/aadpodidentity/v1/conversion_test.go
+++ b/pkg/apis/aadpodidentity/v1/conversion_test.go
@@ -200,6 +200,19 @@ func TestConvertV1AssignedIdentityToInternalAssignedIdentity(t *testing.T) {
 	if !cmp.Equal(assignedIDInternal, convertedAssignedIDInternal) {
 		t.Errorf("Failed to convert from v1 to internal AzureAssignedIdentity")
 	}
+
+	// test no panics when azure identity or binding ref is nil
+	assignedIDV1.Spec.AzureIdentityRef = nil
+	assignedIDV1.Spec.AzureBindingRef = nil
+
+	convertedAssignedIDInternal = ConvertV1AssignedIdentityToInternalAssignedIdentity(assignedIDV1)
+	assignedIDInternal = CreateInternalAssignedIdentity()
+	assignedIDInternal.Spec.AzureIdentityRef = &aadpodid.AzureIdentity{}
+	assignedIDInternal.Spec.AzureBindingRef = &aadpodid.AzureIdentityBinding{}
+
+	if !cmp.Equal(assignedIDInternal, convertedAssignedIDInternal) {
+		t.Errorf("Failed to convert from v1 to internal AzureAssignedIdentity")
+	}
 }
 
 func TestConvertInternalBindingToV1Binding(t *testing.T) {