Repository: Azure/aad-pod-identity
Pull request #205 (2019-05-02 03:31:42)

Update docs and err msg in tutorial script
- Update broken docs url
- Few typos and update to tutorial docs


Patch:
diff --git a/charts/aad-pod-identity/templates/infra.yaml b/charts/aad-pod-identity/templates/infra.yaml
index 9b0f488a..91d8f64c 100644
--- a/charts/aad-pod-identity/templates/infra.yaml
+++ b/charts/aad-pod-identity/templates/infra.yaml
@@ -18,6 +18,9 @@ rules:
 - apiGroups: [""]
   resources: ["pods"]
   verbs: ["get", "list"]
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["get"]
 - apiGroups: ["aadpodidentity.k8s.io"]
   resources: ["azureidentitybindings", "azureidentities"]
   verbs: ["get", "list"]
diff --git a/deploy/demo/aadpodidentity-sp.yaml b/deploy/demo/aadpodidentity-sp.yaml
new file mode 100644
index 00000000..c38d3500
--- /dev/null
+++ b/deploy/demo/aadpodidentity-sp.yaml
@@ -0,0 +1,17 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: demo-aad1-sp
+type: Opaque
+data:
+  ClientSecret: CLIENT_SECRET
+---
+apiVersion: "aadpodidentity.k8s.io/v1"
+kind: AzureIdentity
+metadata:
+  name: demo-aad1
+spec:
+  type: 1
+  TenantID: TENANT_ID
+  ClientID: CLIENT_ID
+  ClientPassword: {"Name":"demo-aad1-sp","Namespace":"default"}
diff --git a/deploy/infra/deployment-rbac.yaml b/deploy/infra/deployment-rbac.yaml
index 0addd3e0..aa1e6a40 100644
--- a/deploy/infra/deployment-rbac.yaml
+++ b/deploy/infra/deployment-rbac.yaml
@@ -52,6 +52,9 @@ rules:
 - apiGroups: [""]
   resources: ["pods"]
   verbs: ["get", "list"]
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["get"]
 - apiGroups: ["aadpodidentity.k8s.io"]
   resources: ["azureidentitybindings", "azureidentities"]
   verbs: ["get", "list"]
diff --git a/docs/design/concept.md b/docs/design/concept.md
index d0a323f4..42799b1f 100644
--- a/docs/design/concept.md
+++ b/docs/design/concept.md
@@ -64,7 +64,7 @@ At later phases, the binding will use:
 
 ### Acquiring Tokens
 
-> for reference please read [Azure VM Managed Service Identity (MSI)](https://docs.microsoft.com/en-us/azure/active-directory/managed-service-identity/how-to-use-vm-token) and [Assign a Managed Service Identity](https://docs.microsoft.com/en-us/azure/active-directory/managed-service-identity/howto-assign-access-portal)
+> for reference please read [Azure VM Managed Service Identity (MSI)](https://docs.microsoft.com/en-us/azure/active-directory/managed-service-identity/how-to-use-vm-token) and [Assign a Managed Service Identity](https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/howto-assign-access-portal)
 
 1. Kubernetes applications (pods) will default to use MSI endpoint.
 2. All traffic to MSI endpoint is routed via `iptables` to a daemon-set that will use `sourceIp` to identify pod, then find an appropriate `crd:azureIdnetityBinding`. The daemon-set mimics all the REST api offered by MSI endpoint. All tokens are presented to pods on MSI endpoint irrespective of the identity used to back this request (EMSI or service principal).
diff --git a/docs/tutorial/README.md b/docs/tutorial/README.md
index 949bec29..578b2633 100644
--- a/docs/tutorial/README.md
+++ b/docs/tutorial/README.md
@@ -64,7 +64,7 @@ K8S_NAME="Cluster-Name"
 
 With `kubectl` installed, run the following script
 
-`./scripts/1-init-aks/4-configure-sli.sh`
+`./scripts/1-init-aks/4-configure-cli.sh`
 
 Now the `kubectl` command should control your AKS cluster. Try it out, it should look similar to below:
 
@@ -109,6 +109,10 @@ Then set the environment variable
 
 `export MC_RG = "resource-group-name"`
 
+Run the following to create an azure id
+
+`./scripts/3-deploy-demo/1-create-azure-id.sh`
+
 ### 3.2. Deploy demo
 
 The `/deploy/demo/deployment.yaml` describes the pod that will be deployed. 
diff --git a/docs/tutorial/scripts/1-init-aks/3-create-aks.sh b/docs/tutorial/scripts/1-init-aks/3-create-aks.sh
index f7944df1..9a15d953 100644
--- a/docs/tutorial/scripts/1-init-aks/3-create-aks.sh
+++ b/docs/tutorial/scripts/1-init-aks/3-create-aks.sh
@@ -12,7 +12,7 @@ fi
 if [ -z "$K8S_NAME" ]
 then
       echo "Kubernetes Cluster Name Not Set. Set the env variable with the following command:"
-      echo "export RG=\"K8S_NAME\" "
+      echo "export K8S_NAME=\"K8S_NAME\" "
       return 1
 fi
 
diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go
index 9ab2fae3..d3ff7958 100644
--- a/pkg/auth/auth.go
+++ b/pkg/auth/auth.go
@@ -47,6 +47,9 @@ func GetServicePrincipalToken(tenantID, clientID, secret string) (*adal.Token, e
 		secret,
 		resourceManagerEndpoint,
 	)
+	if err != nil {
+		return nil, err
+	}
 	// Evectively acqurie the token
 	err = spt.Refresh()
 	if err != nil {
diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go
index 80715bf0..f89714a3 100644
--- a/pkg/k8s/client.go
+++ b/pkg/k8s/client.go
@@ -4,8 +4,8 @@ import (
 	"fmt"
 	"net"
 	"os"
-	"time"
 	"strings"
+	"time"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes"
@@ -25,7 +25,7 @@ const (
 var (
 	// We only want to allow pod-identity with Pending or Running phase status
 	ignorePodPhaseStatuses = []string{"Succeeded", "Failed", "Unknown", "Completed", "CrashLoopBackOff"}
-	phaseStatusFilter = getPodPhaseFilter()
+	phaseStatusFilter      = getPodPhaseFilter()
 )
 
 func getPodPhaseFilter() string {
@@ -38,12 +38,14 @@ type Client interface {
 	GetPodName(podip string) (podns, podname string, err error)
 	// ListPodIds pod matching azure identity or nil
 	ListPodIds(podns, podname string) (*[]aadpodid.AzureIdentity, error)
+	// GetSecret returns secret the secretRef represents
+	GetSecret(secretRef *v1.SecretReference) (*v1.Secret, error)
 }
 
 // KubeClient k8s client
 type KubeClient struct {
 	// Main Kubernetes client
-	ClientSet *kubernetes.Clientset
+	ClientSet kubernetes.Interface
 	// Crd client used to access our CRD resources.
 	CrdClient *crd.Client
 }
@@ -86,7 +88,7 @@ func (c *KubeClient) GetPodName(podip string) (podns, poddname string, err error
 	return "", "", fmt.Errorf("match failed, ip:%s matching pods:%v", podip, podList)
 }
 
-func (c *KubeClient) getPodListWithTries(podip string, tries int) (*v1.PodList, error) {	
+func (c *KubeClient) getPodListWithTries(podip string, tries int) (*v1.PodList, error) {
 	podList, err := c.ClientSet.CoreV1().Pods("").List(metav1.ListOptions{
 		FieldSelector: "status.podIP==" + podip + phaseStatusFilter,
 	})
@@ -123,6 +125,15 @@ func (c *KubeClient) ListPodIds(podns, podname string) (*[]aadpodid.AzureIdentit
 	return c.CrdClient.ListPodIds(podns, podname)
 }
 
+// GetSecret returns secret the secretRef represents
+func (c *KubeClient) GetSecret(secretRef *v1.SecretReference) (*v1.Secret, error) {
+	secret, err := c.ClientSet.CoreV1().Secrets(secretRef.Namespace).Get(secretRef.Name, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+	return secret, nil
+}
+
 func getkubeclient(config *rest.Config) (*kubernetes.Clientset, error) {
 	// creates the clientset
 	kubeClient, err := kubernetes.NewForConfig(config)
diff --git a/pkg/k8s/fake.go b/pkg/k8s/fake.go
index 411e9ab7..236d042d 100644
--- a/pkg/k8s/fake.go
+++ b/pkg/k8s/fake.go
@@ -2,6 +2,7 @@ package k8s
 
 import (
 	aadpodid "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
+	"k8s.io/api/core/v1"
 )
 
 // FakeClient implements Interface
@@ -25,3 +26,8 @@ func (c *FakeClient) GetPodName(podip string) (podns, podname string, err error)
 func (c *FakeClient) ListPodIds(podns string, podname string) (*[]aadpodid.AzureIdentity, error) {
 	return nil, nil
 }
+
+// GetSecret returns secret the secretRef represents
+func (c *FakeClient) GetSecret(secretRef *v1.SecretReference) (*v1.Secret, error) {
+	return nil, nil
+}
diff --git a/pkg/nmi/server/server.go b/pkg/nmi/server/server.go
index 62d2f8e2..960a5103 100644
--- a/pkg/nmi/server/server.go
+++ b/pkg/nmi/server/server.go
@@ -178,7 +178,7 @@ func (s *Server) hostHandler(logger *log.Entry, w http.ResponseWriter, r *http.R
 	}
 	podIDs = &filterPodIdentities
 	rqClientID, rqResource := parseRequestClientIDAndResource(r)
-	token, clientID, err := getTokenForMatchingID(logger, rqClientID, rqResource, podIDs)
+	token, clientID, err := getTokenForMatchingID(s.KubeClient, logger, rqClientID, rqResource, podIDs)
 	if err != nil {
 		logger.Errorf("failed to get service principal token for pod:%s/%s, %+v", podns, podname, err)
 		http.Error(w, err.Error(), http.StatusForbidden)
@@ -224,7 +224,7 @@ func (s *Server) msiHandler(logger *log.Entry, w http.ResponseWriter, r *http.Re
 		return
 	}
 	rqClientID, rqResource := parseRequestClientIDAndResource(r)
-	token, _, err := getTokenForMatchingID(logger, rqClientID, rqResource, podIDs)
+	token, _, err := getTokenForMatchingID(s.KubeClient, logger, rqClientID, rqResource, podIDs)
 	if err != nil {
 		logger.Errorf("failed to get service principal token for pod:%s/%s, %+v", podns, podname, err)
 		http.Error(w, err.Error(), http.StatusForbidden)
@@ -239,7 +239,7 @@ func (s *Server) msiHandler(logger *log.Entry, w http.ResponseWriter, r *http.Re
 	w.Write(response)
 }
 
-func getTokenForMatchingID(logger *log.Entry, rqClientID string, rqResource string, podIDs *[]aadpodid.AzureIdentity) (token *adal.Token, clientID string, err error) {
+func getTokenForMatchingID(kubeClient k8s.Client, logger *log.Entry, rqClientID string, rqResource string, podIDs *[]aadpodid.AzureIdentity) (token *adal.Token, clientID string, err error) {
 	rqHasClientID := len(rqClientID) != 0
 	for _, v := range *podIDs {
 		clientID := v.Spec.ClientID
@@ -256,8 +256,16 @@ func getTokenForMatchingID(logger *log.Entry, rqClientID string, rqResource stri
 		case aadpodid.ServicePrincipal:
 			tenantid := v.Spec.TenantID
 			logger.Infof("matched identityType:%v tenantid:%s clientid:%s", idType, tenantid, clientID)
-			secret := v.Spec.ClientPassword.String()
-			token, err := auth.GetServicePrincipalToken(tenantid, clientID, secret)
+			secret, err := kubeClient.GetSecret(&v.Spec.ClientPassword)
+			if err != nil {
+				return nil, clientID, err
+			}
+			clientSecret := ""
+			for _, v := range secret.Data {
+				clientSecret = string(v)
+				break
+			}
+			token, err := auth.GetServicePrincipalToken(tenantid, clientID, clientSecret)
 			return token, clientID, err
 		default:
 			return nil, clientID, fmt.Errorf("unsupported identity type %+v", idType)


Test patch:
diff --git a/pkg/auth/auth_test.go b/pkg/auth/auth_test.go
new file mode 100644
index 00000000..195980bb
--- /dev/null
+++ b/pkg/auth/auth_test.go
@@ -0,0 +1,12 @@
+package auth
+
+import (
+	"testing"
+)
+
+func TestGetServicePrincipalToken(t *testing.T) {
+	_, err := GetServicePrincipalToken("tid", "cid", "")
+	if err == nil {
+		t.Fatal("should be error with empty secret")
+	}
+}
diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go
new file mode 100644
index 00000000..4722b001
--- /dev/null
+++ b/pkg/k8s/client_test.go
@@ -0,0 +1,32 @@
+package k8s
+
+import (
+	"testing"
+
+	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestGetSecret(t *testing.T) {
+	secretName := "clientSecret"
+
+	fakeClient := fake.NewSimpleClientset()
+
+	secret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName}}
+	fakeClient.CoreV1().Secrets("default").Create(secret)
+
+	kubeClient := &KubeClient{ClientSet: fakeClient}
+
+	secretRef := &v1.SecretReference{
+		Name:      secretName,
+		Namespace: "default",
+	}
+	retrievedSecret, err := kubeClient.GetSecret(secretRef)
+	if err != nil {
+		t.Fatalf("Error getting secret: %v", err)
+	}
+	if retrievedSecret.ObjectMeta.Name != secretName {
+		t.Fatalf("Incorrect secret name: %v", retrievedSecret.ObjectMeta.Name)
+	}
+}
diff --git a/pkg/nmi/server/server_test.go b/pkg/nmi/server/server_test.go
new file mode 100644
index 00000000..772ff1d2
--- /dev/null
+++ b/pkg/nmi/server/server_test.go
@@ -0,0 +1,41 @@
+package server
+
+import (
+	"encoding/base64"
+	"testing"
+
+	aadpodid "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
+	"github.com/Azure/aad-pod-identity/pkg/k8s"
+	log "github.com/sirupsen/logrus"
+	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestGetTokenForMatchingIDBySP(t *testing.T) {
+	fakeClient := fake.NewSimpleClientset()
+
+	secret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "clientSecret"}, Data: make(map[string][]byte)}
+	val, _ := base64.StdEncoding.DecodeString("YWJjZA==")
+	secret.Data["key1"] = val
+	fakeClient.CoreV1().Secrets("default").Create(secret)
+
+	kubeClient := &k8s.KubeClient{ClientSet: fakeClient}
+
+	secretRef := v1.SecretReference{
+		Name:      "clientSecret",
+		Namespace: "default",
+	}
+
+	podID := aadpodid.AzureIdentity{
+		Spec: aadpodid.AzureIdentitySpec{
+			Type:           aadpodid.ServicePrincipal,
+			TenantID:       "tid",
+			ClientID:       "cid",
+			ClientPassword: secretRef,
+		},
+	}
+	podIDs := []aadpodid.AzureIdentity{podID}
+	logger := log.WithError(nil)
+	getTokenForMatchingID(kubeClient, logger, podID.Spec.ClientID, "https://management.azure.com/", &podIDs)
+}