Skip to content

test: add e2e test for provided credentials #730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 12, 2022
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
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ require (
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.1.1
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/keyvault/armkeyvault v1.0.0
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.4.1
github.com/satori/go.uuid v1.2.0
k8s.io/apiserver v0.24.3
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,8 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww=
github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
github.com/seccomp/libseccomp-golang v0.9.1/go.mod h1:GbW5+tmTXfcxTToHLXlScSlAvWlF4P2Ca7zGrPiEpWo=
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/pre_provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Pre-Provisioned", func() {
MountPathGenerate: "/mnt/test-",
},
NodeStageSecretRef: "azure-secret",
Attrib: make(map[string]string),
},
},
},
Expand Down Expand Up @@ -279,6 +280,7 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Pre-Provisioned", func() {
NameGenerate: "test-volume-",
MountPathGenerate: "/mnt/test-",
},
Attrib: make(map[string]string),
},
},
},
Expand Down Expand Up @@ -315,6 +317,7 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Pre-Provisioned", func() {
NameGenerate: "test-volume-",
MountPathGenerate: "/mnt/test-",
},
Attrib: make(map[string]string),
},
},
},
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/testsuites/pre_provisioned_keyvault_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ func (t *PreProvisionedKeyVaultTest) Run(client clientset.Interface, namespace *
accountKeySecret, err := keyVaultClient.CreateSecret(context.TODO(), accountName+"-key", accountKey)
framework.ExpectNoError(err)

pod.Volumes[n].ContainerName = containerName
pod.Volumes[n].StorageAccountname = accountName
pod.Volumes[n].KeyVaultURL = *vault.Properties.VaultURI
pod.Volumes[n].KeyVaultSecretName = *accountKeySecret.Name
pod.Volumes[n].Attrib["containerName"] = containerName
pod.Volumes[n].Attrib["storageAccountName"] = accountName
pod.Volumes[n].Attrib["keyVaultURL"] = *vault.Properties.VaultURI
pod.Volumes[n].Attrib["keyVaultSecretName"] = *accountKeySecret.Name

ginkgo.By("test storage account key...")
tpod, cleanup := pod.SetupWithPreProvisionedVolumes(client, namespace, t.CSIDriver)
Expand Down
110 changes: 89 additions & 21 deletions test/e2e/testsuites/pre_provisioned_provided_credentials_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"sigs.k8s.io/blob-csi-driver/pkg/blob"
"sigs.k8s.io/blob-csi-driver/test/e2e/driver"
"sigs.k8s.io/blob-csi-driver/test/utils/azure"

v1 "k8s.io/api/core/v1"
clientset "k8s.io/client-go/kubernetes"
Expand All @@ -39,34 +40,101 @@ type PreProvisionedProvidedCredentiasTest struct {
}

func (t *PreProvisionedProvidedCredentiasTest) Run(client clientset.Interface, namespace *v1.Namespace) {
kvClient, err := azure.NewKeyVaultClient()
framework.ExpectNoError(err)

authClient, err := azure.NewAuthorizationClient()
framework.ExpectNoError(err)

for _, pod := range t.Pods {
for n, volume := range pod.Volumes {
accountName, accountKey, accountSasToken, containerName, err := t.Driver.GetStorageAccountAndContainer(context.Background(), volume.VolumeID, nil, nil)
accountName, accountKey, _, _, err := t.Driver.GetStorageAccountAndContainer(context.Background(), volume.VolumeID, nil, nil)
framework.ExpectNoError(err, fmt.Sprintf("Error GetStorageAccountAndContainer from volumeID(%s): %v", volume.VolumeID, err))
var secretData map[string]string

var run = func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run() only provisions different secret, I think we need to add a new e2e test case for sas token stored in k8s secret scanerio

Copy link
Member Author

@cvvz cvvz Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run() will generate different secret, pvc, pv and pod and also create and clean them up in k8s each time. Also per the name of this test case: pre_provisioned_provided_credentials_tester,I think this test case is used to test the scenario of provided credentials, which includes access key, sas token, service principal and managed identity. I think it would be more reasonable and convenient to put them all in this test case.

tsecret := NewTestSecret(client, namespace, volume.NodeStageSecretRef, secretData)
tsecret.Create()
defer tsecret.Cleanup()

tpod, cleanup := pod.SetupWithPreProvisionedVolumes(client, namespace, t.CSIDriver)
// defer must be called here for resources not get removed before using them
for i := range cleanup {
defer cleanup[i]()
}

ginkgo.By("deploying the pod")
tpod.Create()
defer tpod.Cleanup()
ginkgo.By("checking that the pods command exits with no error")
tpod.WaitForSuccess()
}

ginkgo.By("creating the secret")
secreteData := map[string]string{"azurestorageaccountname": accountName}
if accountKey != "" {
secreteData["azurestorageaccountkey"] = accountKey
} else {
secreteData["azurestorageaccountsastoken"] = accountSasToken
// test for storage account key
ginkgo.By("Run for storage account key")
secretData = map[string]string{
"azurestorageaccountname": accountName,
"azurestorageaccountkey": accountKey,
}
run()

// test for storage account SAS token
ginkgo.By("Run for storage account SAS token")
sasToken := GenerateSASToken(accountName, accountKey)
secretData = map[string]string{
"azurestorageaccountname": accountName,
"azurestorageaccountsastoken": sasToken,
}
run()

// test for service principal
ginkgo.By("Run for service principal")
pod.Volumes[n].Attrib = map[string]string{
"azurestorageauthtype": "SPN",
"azurestoragespnclientid": kvClient.Cred.AADClientID,
"azurestoragespntenantid": kvClient.Cred.TenantID,
}
secretData = map[string]string{
"azurestorageaccountname": accountName,
"azurestoragespnclientsecret": kvClient.Cred.AADClientSecret,
}
tsecret := NewTestSecret(client, namespace, volume.NodeStageSecretRef, secreteData)
tsecret.Create()
defer tsecret.Cleanup()

pod.Volumes[n].ContainerName = containerName
tpod, cleanup := pod.SetupWithPreProvisionedVolumes(client, namespace, t.CSIDriver)
// defer must be called here for resources not get removed before using them
for i := range cleanup {
defer cleanup[i]()

objectID, err := kvClient.GetServicePrincipalObjectID(context.TODO(), kvClient.Cred.AADClientID)
framework.ExpectNoError(err, fmt.Sprintf("Error GetServicePrincipalObjectID from clientID(%s): %v", kvClient.Cred.AADClientID, err))

resourceID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Storage/storageAccounts/%s", kvClient.Cred.SubscriptionID, kvClient.Cred.ResourceGroup, accountName)

ginkgo.By(fmt.Sprintf("assign Storage Blob Data Contributor role to the service principal, objectID:%s", objectID))
roleDef, err := authClient.GetRoleDefinition(context.TODO(), resourceID, "Storage Blob Data Contributor")
framework.ExpectNoError(err, fmt.Sprintf("Error GetRoleDefinition from resourceID(%s): %v", resourceID, err))

roleDefID := *roleDef.ID
_, err = authClient.AssignRole(context.TODO(), resourceID, objectID, roleDefID)
framework.ExpectNoError(err, fmt.Sprintf("Error AssignRole (roleDefID(%s)) to objectID(%s) to access resource (resourceID(%s)), error: %v", roleDefID, objectID, resourceID, err))

run()

// test for managed identity
// e2e-vmss test job uses msi blobfuse-csi-driver-e2e-test-id, other jobs use service principal
objectID, err = kvClient.GetMSIObjectID(context.TODO(), "blobfuse-csi-driver-e2e-test-id")
if err != nil {
return
}

ginkgo.By("Run for managed identity")
pod.Volumes[n].Attrib = map[string]string{
"azurestorageauthtype": "MSI",
"azurestorageidentityobjectid": objectID,
}

secretData = map[string]string{
"azurestorageaccountname": accountName,
}
ginkgo.By(fmt.Sprintf("assign Storage Blob Data Contributor role to the managed identity, objectID:%s", objectID))
_, err = authClient.AssignRole(context.TODO(), resourceID, objectID, roleDefID)
framework.ExpectNoError(err, fmt.Sprintf("Error AssignRole (roleDefID(%s)) to objectID(%s) to access resource (resourceID(%s)), error: %v", roleDefID, objectID, resourceID, err))

ginkgo.By("deploying the pod")
tpod.Create()
defer tpod.Cleanup()
ginkgo.By("checking that the pods command exits with no error")
tpod.WaitForSuccess()
run()
}
}
}
12 changes: 6 additions & 6 deletions test/e2e/testsuites/pre_provisioned_sastoken_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,16 @@ func (t *PreProvisionedSASTokenTest) Run(client clientset.Interface, namespace *
}()

ginkgo.By("generating SAS token...")
sasToken := generateSASToken(accountName, accountKey)
sasToken := GenerateSASToken(accountName, accountKey)

ginkgo.By("creating secret for SAS token...")
accountSASSecret, err := keyVaultClient.CreateSecret(context.TODO(), accountName+"-sas", sasToken)
framework.ExpectNoError(err)

pod.Volumes[n].ContainerName = containerName
pod.Volumes[n].StorageAccountname = accountName
pod.Volumes[n].KeyVaultURL = *vault.Properties.VaultURI
pod.Volumes[n].KeyVaultSecretName = *accountSASSecret.Name
pod.Volumes[n].Attrib["containerName"] = containerName
pod.Volumes[n].Attrib["storageAccountName"] = accountName
pod.Volumes[n].Attrib["keyVaultURL"] = *vault.Properties.VaultURI
pod.Volumes[n].Attrib["keyVaultSecretName"] = *accountSASSecret.Name

tpod, cleanup := pod.SetupWithPreProvisionedVolumes(client, namespace, t.CSIDriver)
// defer must be called here for resources not get removed before using them
Expand All @@ -88,7 +88,7 @@ func (t *PreProvisionedSASTokenTest) Run(client clientset.Interface, namespace *
}
}

func generateSASToken(accountName, accountKey string) string {
func GenerateSASToken(accountName, accountKey string) string {
credential, err := azblob.NewSharedKeyCredential(accountName, accountKey)
framework.ExpectNoError(err)
serviceClient, err := azblob.NewServiceClientWithSharedKey(fmt.Sprintf("https://%s.blob.core.windows.net/", accountName), credential, nil)
Expand Down
19 changes: 2 additions & 17 deletions test/e2e/testsuites/specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,8 @@ type VolumeDetails struct {
VolumeID string
// Optional, used with PVCs created from snapshots
DataSource *DataSource
ContainerName string
NodeStageSecretRef string
StorageAccountname string
KeyVaultURL string
KeyVaultSecretName string
Attrib map[string]string
}

type VolumeMode int
Expand Down Expand Up @@ -203,19 +200,7 @@ func (volume *VolumeDetails) SetupDynamicPersistentVolumeClaim(client clientset.
func (volume *VolumeDetails) SetupPreProvisionedPersistentVolumeClaim(client clientset.Interface, namespace *v1.Namespace, csiDriver driver.PreProvisionedVolumeTestDriver) (*TestPersistentVolumeClaim, []func()) {
cleanupFuncs := make([]func(), 0)
ginkgo.By("setting up the PV")
attrib := make(map[string]string)
if volume.ContainerName != "" {
attrib["containerName"] = volume.ContainerName
}
if volume.StorageAccountname != "" {
attrib["storageAccountName"] = volume.StorageAccountname
}
if volume.KeyVaultURL != "" {
attrib["keyVaultURL"] = volume.KeyVaultURL
}
if volume.KeyVaultSecretName != "" {
attrib["keyVaultSecretName"] = volume.KeyVaultSecretName
}
attrib := volume.Attrib
nodeStageSecretRef := volume.NodeStageSecretRef
pv := csiDriver.GetPersistentVolume(volume.VolumeID, volume.FSType, volume.ClaimSize, volume.ReclaimPolicy, namespace.Name, attrib, nodeStageSecretRef)
tpv := NewTestPreProvisionedPersistentVolume(client, pv)
Expand Down
3 changes: 1 addition & 2 deletions test/manifest/linux-vmss.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@
}
},
"servicePrincipalProfile": {
"clientID": "{clientID}",
"secret": "{secret}"
"clientID": "msi"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep the sp test

Copy link
Member Author

@cvvz cvvz Aug 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modification is to solve the problem that blobfuse mount failed when authenticate by MSI. The root cause is MSI generated by aks-engine is not assigned to VMSS.
BTW, considering sp is already tested by the most of jobs using the arm template linux.json, which set useManagedIdentity: false and

    "servicePrincipalProfile": {
        "clientID": "{clientID}", 
        "secret": "{secret}"
    }

And only job pull-blob-csi-driver-e2e-vmss uses template linux-vmss.json which set useManagedIdentity: true. Maybe we should have set

    "servicePrincipalProfile": {
        "clientID": "msi", 
    }

to test msi scenario?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the logs, pull-blob-csi-driver-e2e is using sp

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_blob-csi-driver/730/pull-blob-csi-driver-e2e/1557924587340042240/build-log.txt

[pod/csi-blob-controller-84cf49dc6d-zs47k/blob] I0812 03:13:38.637448       1 azure.go:104] read cloud config from file: /etc/kubernetes/azure.json successfully
[pod/csi-blob-controller-84cf49dc6d-zs47k/blob] I0812 03:13:38.638028       1 azure_auth.go:245] Using AzurePublicCloud environment
[pod/csi-blob-controller-84cf49dc6d-zs47k/blob] I0812 03:13:38.638061       1 azure_auth.go:130] azure: using client_id+client_secret to retrieve access token

pull-blob-csi-driver-e2e-vmss is using msi:
https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_blob-csi-driver/730/pull-blob-csi-driver-e2e-vmss/1557924587390373888/build-log.txt

[pod/csi-blob-node-8trh8/blob] I0812 03:15:51.934107       1 azure_auth.go:245] Using AzurePublicCloud environment
[pod/csi-blob-node-8trh8/blob] I0812 03:15:51.934127       1 azure_auth.go:96] azure: using managed identity extension to retrieve access token
[pod/csi-blob-node-8trh8/blob] I0812 03:15:51.934131       1 azure_auth.go:102] azure: using User Assigned MSI ID to retrieve access token
[pod/csi-blob-node-8trh8/blob] I0812 03:15:51.934162       1 azure_auth.go:113] azure: User Assigned MSI ID is client ID. Resource ID parsing error: %+vparsing failed for 82e9bd91-c283-4b2d-8708-6d11b07d630d. Invalid resource Id format

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you are right.

}
}
}
Loading