Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Fix namespaced multiple binding/identity handling and verbose logs #388

Merged
merged 2 commits into from
Sep 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 28 additions & 10 deletions pkg/mic/mic.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,15 +386,15 @@ func (c *Client) createDesiredAssignedIdentityList(
var matchedBindings []aadpodid.AzureIdentityBinding
for _, allBinding := range *listBindings {
if allBinding.Spec.Selector == crdPodLabelVal {
glog.V(5).Infof("Found binding match for pod %s/%s with binding %s", pod.Namespace, pod.Name, allBinding.Name)
glog.V(5).Infof("Found binding match for pod %s/%s with binding %s/%s", pod.Namespace, pod.Name, allBinding.Namespace, allBinding.Name)
matchedBindings = append(matchedBindings, allBinding)
nodeRefs[pod.Spec.NodeName] = true
}
}

for _, binding := range matchedBindings {
glog.V(5).Infof("Looking up id map: %v", binding.Spec.AzureIdentity)
if azureID, idPresent := idMap[binding.Spec.AzureIdentity]; idPresent {
if azureID, idPresent := idMap[getIDKey(binding.Namespace, binding.Spec.AzureIdentity)]; idPresent {
// working in Namespaced mode or this specific identity is namespaced
if c.IsNamespaced || aadpodid.IsNamespacedIdentity(&azureID) {
// They have to match all
Expand All @@ -404,8 +404,9 @@ func (c *Client) createDesiredAssignedIdentityList(
continue
}
}
glog.V(5).Infof("identity %s/%s assigned to %s/%s via %s/%s", azureID.Namespace, azureID.Name, pod.Namespace, pod.Name, binding.Namespace, binding.Namespace)
assignedID, err := c.makeAssignedIDs(&azureID, &binding, pod.Name, pod.Namespace, pod.Spec.NodeName)
glog.V(5).Infof("identity %s/%s assigned to %s/%s via %s/%s", azureID.Namespace, azureID.Name, pod.Namespace, pod.Name, binding.Namespace, binding.Name)

assignedID, err := c.makeAssignedIDs(azureID, binding, pod.Name, pod.Namespace, pod.Spec.NodeName)

if err != nil {
glog.Errorf("failed to create assignment for pod %s/%s with identity %s/%s with error %v", pod.Namespace, pod.Name, azureID.Namespace, azureID.Name, err.Error())
Expand Down Expand Up @@ -474,6 +475,15 @@ func (c *Client) matchAssignedID(x *aadpodid.AzureAssignedIdentity, y *aadpodid.
idX := x.Spec.AzureIdentityRef
idY := y.Spec.AzureIdentityRef

glog.V(6).Infof("assignedidX - %+v\n", x)
glog.V(6).Infof("assignedidY - %+v\n", y)

glog.V(6).Infof("bindingX - %+v\n", bindingX)
glog.V(6).Infof("bindingY - %+v\n", bindingY)

glog.V(6).Infof("idX - %+v\n", idX)
glog.V(6).Infof("idY - %+v\n", idY)

if bindingX.Name == bindingY.Name && bindingX.ResourceVersion == bindingY.ResourceVersion &&
idX.Name == idY.Name && idX.ResourceVersion == idY.ResourceVersion &&
x.Spec.Pod == y.Spec.Pod && x.Spec.PodNamespace == y.Spec.PodNamespace && x.Spec.NodeName == y.Spec.NodeName {
Expand Down Expand Up @@ -561,14 +571,17 @@ func (c *Client) getAzureAssignedIDsToDelete(old *[]aadpodid.AzureAssignedIdenti
return &delete, nil
}

func (c *Client) makeAssignedIDs(azID *aadpodid.AzureIdentity, azBinding *aadpodid.AzureIdentityBinding, podName, podNameSpace, nodeName string) (res *aadpodid.AzureAssignedIdentity, err error) {
func (c *Client) makeAssignedIDs(azID aadpodid.AzureIdentity, azBinding aadpodid.AzureIdentityBinding, podName, podNameSpace, nodeName string) (res *aadpodid.AzureAssignedIdentity, err error) {
binding := azBinding
id := azID

assignedID := &aadpodid.AzureAssignedIdentity{
ObjectMeta: v1.ObjectMeta{
Name: c.getAssignedIDName(podName, podNameSpace, azID.Name),
},
Spec: aadpodid.AzureAssignedIdentitySpec{
AzureIdentityRef: azID,
AzureBindingRef: azBinding,
AzureIdentityRef: &id,
AzureBindingRef: &binding,
Pod: podName,
PodNamespace: podNameSpace,
NodeName: nodeName,
Expand All @@ -578,7 +591,7 @@ func (c *Client) makeAssignedIDs(azID *aadpodid.AzureIdentity, azBinding *aadpod
},
}
// if we are in namespaced mode (or az identity is namespaced)
if c.IsNamespaced || aadpodid.IsNamespacedIdentity(azID) {
if c.IsNamespaced || aadpodid.IsNamespacedIdentity(&azID) {
assignedID.Namespace = azID.Namespace
} else {
// eventually this should be identity namespace
Expand All @@ -587,7 +600,8 @@ func (c *Client) makeAssignedIDs(azID *aadpodid.AzureIdentity, azBinding *aadpod
assignedID.Namespace = "default"
}

glog.V(5).Infof("Making assigned ID: %v", assignedID)
glog.V(6).Infof("Binding - %+v Identity - %+v", azBinding, azID)
glog.V(5).Infof("Making assigned ID: %+v", assignedID)
return assignedID, nil
}

Expand Down Expand Up @@ -646,10 +660,14 @@ func (c *Client) getUserMSIListForNode(nodeOrVMSSName string, isvmss bool) ([]st
return c.CloudClient.GetUserMSIs(nodeOrVMSSName, isvmss)
}

func getIDKey(ns, name string) string {
return strings.Join([]string{ns, name}, "/")
}

func (c *Client) convertIDListToMap(arr []aadpodid.AzureIdentity) (m map[string]aadpodid.AzureIdentity, err error) {
m = make(map[string]aadpodid.AzureIdentity, len(arr))
for _, element := range arr {
m[element.Name] = element
m[getIDKey(element.Namespace, element.Name)] = element
}
return m, nil
}
Expand Down
25 changes: 17 additions & 8 deletions pkg/mic/mic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,42 +598,51 @@ func TestMapMICClient(t *testing.T) {
idList := make([]aadpodid.AzureIdentity, 0)

id := new(aadpodid.AzureIdentity)
id.Namespace = "default"
id.Name = "test-azure-identity"

idList = append(idList, *id)

id.Namespace = "newns"
id.Name = "test-akssvcrg-id"

idList = append(idList, *id)

idMap, _ := micClient.convertIDListToMap(idList)

namespace := "default"
name := "test-azure-identity"
count := 3
if azureID, idPresent := idMap[name]; idPresent {
if azureID, idPresent := idMap[getIDKey(namespace, name)]; idPresent {
if azureID.Name != name {
t.Errorf("id map id value mismatch")
t.Fatalf("id map id value mismatch")
}
count = count - 1
} else {
t.Fatalf("id %s not found", name)
}

namespace = "newns"
name = "test-akssvcrg-id"
if azureID, idPresent := idMap[name]; idPresent {
if azureID, idPresent := idMap[getIDKey(namespace, name)]; idPresent {
if azureID.Name != name {
t.Errorf("id map id value mismatch")
t.Fatalf("id map id value mismatch")
}
count = count - 1
} else {
t.Fatalf("id %s not found", name)
}

namespace = "default"
name = "test not there"
if _, idPresent := idMap[name]; idPresent {
t.Errorf("not present found")
if _, idPresent := idMap[getIDKey(namespace, name)]; idPresent {
t.Fatalf("not present found")
} else {
count = count - 1
}
if count != 0 {
t.Errorf("Test count mismatch")
t.Fatalf("Test count mismatch - count %d", count)
}

}

func (c *TestMICClient) testRunSync() func(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions pkg/stats/stats_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package stats_test

import (
"fmt"
"reflect"
"sync"
"testing"
Expand Down Expand Up @@ -49,14 +48,14 @@ func TestConcurrency(t *testing.T) {
go func(c int) {
defer wg.Done()
startWg.Wait()
fmt.Printf("Updating %d \n", c)
//fmt.Printf("Updating %d \n", c)
stats.Update(stats.AssignedIDList, duration)
}(i)
wg.Add(1)
go func(c int) {
defer wg.Done()
startWg.Wait()
fmt.Printf("Getting %d\n", c)
//fmt.Printf("Getting %d\n", c)
stats.Get(stats.AssignedIDList)
}(i)
}
Expand Down