update immutable identity check + tests (#452)

Author: aramase

charts/aad-pod-identity/templates/mic-deployment.yaml

@@ -63,7 +63,7 @@ spec:
           - --clientQps={{ .Values.mic.clientQps }}
           {{- end }}          
           {{- if .Values.mic.immutableUserMSIs }}
-          - "--immutableUserMSIs={{- join "," .Values.mic.immutableUserMSIs}}"
+          - "--immutable-user-msis={{- join "," .Values.mic.immutableUserMSIs}}"
           {{- end }}
         env:
           - name: FORCENAMESPACED

cmd/mic/main.go

@@ -70,7 +70,7 @@ func main() {
 	flag.Float64Var(&clientQPS, "clientQps", 5, "Client QPS used for throttling of calls to kube-api server")
 
 	//Identities that should be never removed from Azure AD (used defined managed identities)
-	flag.StringVar(&immutableUserMSIs, "immutableUserMSIs", "", "prevent deletion of these IDs from the underlying VM/VMSS")
+	flag.StringVar(&immutableUserMSIs, "immutable-user-msis", "", "prevent deletion of these IDs from the underlying VM/VMSS")
 
 	flag.Parse()
 	if versionInfo {
@@ -112,7 +112,6 @@ func main() {
 	if immutableUserMSIs != "" {
 		immutableUserMSIsList = strings.Split(immutableUserMSIs, ",")
 	}
-	glog.Infof("immutable identities are %v", immutableUserMSIsList)
 
 	micClient, err := mic.NewMICClient(cloudconfig, config, forceNamespaced, syncRetryDuration, &leaderElectionCfg, enableScaleFeatures, createDeleteBatch, immutableUserMSIsList)
 	if err != nil {

docs/readmes/README.featureflags.md

@@ -46,5 +46,5 @@ their use cases.
 ## ImmutableUserMSIs flag
 > Available from 1.5.4 release
 
-Aad-pod-identity has a new flag `immutableUserMSIs` which can be used to prevent deletion of specified identities from VM/VMSS.
+Aad-pod-identity has a new flag `immutable-user-msis` which can be used to prevent deletion of specified identities from VM/VMSS.
 The list is comma separated. Example: 00000000-0000-0000-0000-000000000000,11111111-1111-1111-1111-111111111111

pkg/mic/mic.go

@@ -490,7 +490,7 @@ func (c *Client) getListOfIdsToDelete(deleteList map[string]aadpodid.AzureAssign
 
 		id := delID.Spec.AzureIdentityRef
 		isUserAssignedMSI := c.checkIfUserAssignedMSI(id)
-		isImmutableIdentity := c.checkIfIdentityImmutable(id.Spec.ResourceID)
+		isImmutableIdentity := c.checkIfIdentityImmutable(id.Spec.ClientID)
 
 		// this case includes Assigned state and empty state to ensure backward compatability
 		if delID.Status.Status == aadpodid.AssignedIDAssigned || delID.Status.Status == "" {

pkg/mic/mic_test.go

@@ -1440,8 +1440,8 @@ func TestMicAddDelVMSSwithImmutableIdentities(t *testing.T) {
 	evtRecorder.lastEvent = new(LastEvent)
 	evtRecorder.eventChannel = make(chan bool, 100)
 	var immutableUserMSIs = map[string]bool{
-		"zero-test":                true,
-		"test-user-msi-resourceid": true,
+		"zero-test":              true,
+		"test-user-msi-clientid": true,
 	}
 
 	micClient := NewMICTestClient(eventCh, cloudClient, crdClient, podClient, nodeClient, &evtRecorder, false, 4, immutableUserMSIs)

test/e2e/aadpodidentity_test.go

@@ -1288,15 +1288,13 @@ func validateAzureAssignedIdentity(azureAssignedIdentity aadpodid.AzureAssignedI
 	Expect(azureAssignedIdentity.Spec.AzureIdentityRef.ObjectMeta.Name).To(Equal(identityName))
 	Expect(azureAssignedIdentity.Spec.AzureIdentityRef.ObjectMeta.Namespace).To(Equal("default"))
 
-	if strings.HasPrefix(identityName, keyvaultIdentity) {
-		cmdOutput, err := validateUserAssignedIdentityOnPod(podName, identityClientID)
-		Expect(errors.Wrap(err, string(cmdOutput))).NotTo(HaveOccurred())
-	} else if strings.HasPrefix(identityName, clusterIdentity) {
+	if strings.HasPrefix(identityName, clusterIdentity) {
 		cmdOutput, err := validateClusterWideUserAssignedIdentity(podName, identityClientID)
 		Expect(errors.Wrap(err, string(cmdOutput))).NotTo(HaveOccurred())
 	} else {
-		err := errors.Errorf("Invalid identity name: %s", identityName)
-		Expect(err).NotTo(HaveOccurred())
+		// validates user assigned identity - this includes keyvault identities and immutable identities
+		cmdOutput, err := validateUserAssignedIdentityOnPod(podName, identityClientID)
+		Expect(errors.Wrap(err, string(cmdOutput))).NotTo(HaveOccurred())
 	}
 
 	fmt.Printf("# %s validated!\n", identityName)

test/e2e/template/deployment-rbac-old.yaml

@@ -191,7 +191,6 @@ spec:
         args:
           {{if .MICArg}}- mic {{ end }}
           - "--cloudconfig=/etc/kubernetes/azure.json"
-          {{if .ImmutableUserMSIs}}- "--immutableUserMSIs={{.ImmutableUserMSIs}}" {{end}}
           - "--logtostderr"
         volumeMounts:
           - name: k8s-azure-file

test/e2e/template/deployment-rbac.yaml

@@ -222,7 +222,7 @@ spec:
         args:
           {{if .MICArg}}- mic {{ end }}
           - "--cloudconfig=/etc/kubernetes/azure.json"
-          {{if .ImmutableUserMSIs}}- "--immutableUserMSIs={{.ImmutableUserMSIs}}" {{end}}
+          {{if .ImmutableUserMSIs}}- "--immutable-user-msis={{.ImmutableUserMSIs}}" {{end}}
           - "--logtostderr"
           {{if .EnableScaleFeatures}}- "--enableScaleFeatures=true" {{end}}
         resources: