Skip to content
New issue

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

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

Already on GitHub? # to your account

chore: refactor SetAzureCredentials func #616

Merged
merged 1 commit into from
Apr 14, 2021
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
37 changes: 37 additions & 0 deletions pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (

"golang.org/x/net/context"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -727,3 +729,38 @@ func (d *Driver) useDataPlaneAPI(volumeID, accountName string) bool {
}
return useDataPlaneAPI
}

func (d *Driver) SetAzureCredentials(accountName, accountKey, secretName, secretNamespace string) (string, error) {
if d.cloud.KubeClient == nil {
klog.Warningf("could not create secret: kubeClient is nil")
return "", nil
}
if accountName == "" || accountKey == "" {
return "", fmt.Errorf("the account info is not enough, accountName(%v), accountKey(%v)", accountName, accountKey)
}
if secretNamespace == "" {
secretNamespace = defaultSecretNamespace
}
if secretName == "" {
secretName = fmt.Sprintf(secretNameTemplate, accountName)
}
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: secretNamespace,
Name: secretName,
},
Data: map[string][]byte{
defaultSecretAccountName: []byte(accountName),
defaultSecretAccountKey: []byte(accountKey),
},
Type: "Opaque",
}
_, err := d.cloud.KubeClient.CoreV1().Secrets(secretNamespace).Create(context.TODO(), secret, metav1.CreateOptions{})
if errors.IsAlreadyExists(err) {
err = nil
}
if err != nil {
return "", fmt.Errorf("couldn't create secret %v", err)
}
return secretName, err
}
2 changes: 1 addition & 1 deletion pkg/azurefile/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
}
}
storeSecretName, err := setAzureCredentials(d.cloud.KubeClient, accountName, accountKey, secretName, secretNamespace)
storeSecretName, err := d.SetAzureCredentials(accountName, accountKey, secretName, secretNamespace)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to store storage account key: %v", err)
}
Expand Down
78 changes: 78 additions & 0 deletions pkg/azurefile/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
cloudprovider "k8s.io/cloud-provider"

Expand Down Expand Up @@ -1930,3 +1931,80 @@ func TestListSnapshots(t *testing.T) {
t.Errorf("Unexpected error: %v", err)
}
}

func TestSetAzureCredentials(t *testing.T) {
d := NewFakeDriver()
d.cloud = &azure.Cloud{
Config: azure.Config{
ResourceGroup: "rg",
Location: "loc",
VnetName: "fake-vnet",
SubnetName: "fake-subnet",
},
}
fakeClient := fake.NewSimpleClientset()

tests := []struct {
desc string
kubeClient kubernetes.Interface
accountName string
accountKey string
secretName string
secretNamespace string
expectedName string
expectedErr error
}{
{
desc: "[failure] accountName is nil",
kubeClient: fakeClient,
expectedErr: fmt.Errorf("the account info is not enough, accountName(), accountKey()"),
},
{
desc: "[failure] accountKey is nil",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "",
expectedErr: fmt.Errorf("the account info is not enough, accountName(testName), accountKey()"),
},
{
desc: "[success] kubeClient is nil",
kubeClient: nil,
expectedErr: nil,
},
{
desc: "[success] normal scenario",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
expectedName: "azure-storage-account-testName-secret",
expectedErr: nil,
},
{
desc: "[success] already exist",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
expectedName: "azure-storage-account-testName-secret",
expectedErr: nil,
},
{
desc: "[success] normal scenario using secretName",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
secretName: "secretName",
secretNamespace: "secretNamespace",
expectedName: "secretName",
expectedErr: nil,
},
}

for _, test := range tests {
d.cloud.KubeClient = test.kubeClient
result, err := d.SetAzureCredentials(test.accountName, test.accountKey, test.secretName, test.secretNamespace)
if result != test.expectedName || !reflect.DeepEqual(err, test.expectedErr) {
t.Errorf("desc: %s,\n input: accountName(%v), accountKey(%v),\n setAzureCredentials result: %v, expectedName: %v err: %v, expectedErr: %v",
test.desc, test.accountName, test.accountKey, result, test.expectedName, err, test.expectedErr)
}
}
}
41 changes: 0 additions & 41 deletions pkg/azurefile/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,10 @@ limitations under the License.
package azurefile

import (
"context"
"fmt"
"strings"
"sync"
"time"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
)

Expand Down Expand Up @@ -78,41 +72,6 @@ func (lm *lockMap) unlockEntry(entry string) {
lm.mutexMap[entry].Unlock()
}

func setAzureCredentials(kubeClient kubernetes.Interface, accountName, accountKey, secretName, secretNamespace string) (string, error) {
if kubeClient == nil {
klog.Warningf("could not create secret: kubeClient is nil")
return "", nil
}
if accountName == "" || accountKey == "" {
return "", fmt.Errorf("the account info is not enough, accountName(%v), accountKey(%v)", accountName, accountKey)
}
if secretNamespace == "" {
secretNamespace = defaultSecretNamespace
}
if secretName == "" {
secretName = fmt.Sprintf(secretNameTemplate, accountName)
}
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: secretNamespace,
Name: secretName,
},
Data: map[string][]byte{
defaultSecretAccountName: []byte(accountName),
defaultSecretAccountKey: []byte(accountKey),
},
Type: "Opaque",
}
_, err := kubeClient.CoreV1().Secrets(secretNamespace).Create(context.TODO(), secret, metav1.CreateOptions{})
if errors.IsAlreadyExists(err) {
err = nil
}
if err != nil {
return "", fmt.Errorf("couldn't create secret %v", err)
}
return secretName, err
}

func isDiskFsType(fsType string) bool {
for _, v := range supportedDiskFsTypeList {
if fsType == v {
Expand Down
72 changes: 0 additions & 72 deletions pkg/azurefile/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,8 @@ package azurefile

import (
"errors"
"fmt"
"reflect"
"testing"
"time"

"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
)

func TestSimpleLockEntry(t *testing.T) {
Expand Down Expand Up @@ -99,73 +94,6 @@ func TestUnlockEntryNotExists(t *testing.T) {
testLockMap.UnlockEntry("entry1")
}

func TestSetAzureCredentials(t *testing.T) {
fakeClient := fake.NewSimpleClientset()

tests := []struct {
desc string
kubeClient kubernetes.Interface
accountName string
accountKey string
secretName string
secretNamespace string
expectedName string
expectedErr error
}{
{
desc: "[failure] accountName is nil",
kubeClient: fakeClient,
expectedErr: fmt.Errorf("the account info is not enough, accountName(), accountKey()"),
},
{
desc: "[failure] accountKey is nil",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "",
expectedErr: fmt.Errorf("the account info is not enough, accountName(testName), accountKey()"),
},
{
desc: "[success] kubeClient is nil",
kubeClient: nil,
expectedErr: nil,
},
{
desc: "[success] normal scenario",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
expectedName: "azure-storage-account-testName-secret",
expectedErr: nil,
},
{
desc: "[success] already exist",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
expectedName: "azure-storage-account-testName-secret",
expectedErr: nil,
},
{
desc: "[success] normal scenario using secretName",
kubeClient: fakeClient,
accountName: "testName",
accountKey: "testKey",
secretName: "secretName",
secretNamespace: "secretNamespace",
expectedName: "secretName",
expectedErr: nil,
},
}

for _, test := range tests {
result, err := setAzureCredentials(test.kubeClient, test.accountName, test.accountKey, test.secretName, test.secretNamespace)
if result != test.expectedName || !reflect.DeepEqual(err, test.expectedErr) {
t.Errorf("desc: %s,\n input: kubeClient(%v), accountName(%v), accountKey(%v),\n setAzureCredentials result: %v, expectedName: %v err: %v, expectedErr: %v",
test.desc, test.kubeClient, test.accountName, test.accountKey, result, test.expectedName, err, test.expectedErr)
}
}
}

func TestIsDiskFsType(t *testing.T) {
tests := []struct {
fsType string
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/pre_provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ var _ = ginkgo.Describe("Pre-Provisioned", func() {
ginkgo.Fail(fmt.Sprintf("create volume error: %v", err))
}
volumeID = resp.Volume.VolumeId
ginkgo.By(fmt.Sprintf("Successfully provisioned BlobFuse volume: %q\n", volumeID))
ginkgo.By(fmt.Sprintf("Successfully provisioned Azure File volume: %q\n", volumeID))

volumeSize := fmt.Sprintf("%dGi", defaultDiskSize)
reclaimPolicy := v1.PersistentVolumeReclaimRetain
Expand Down
52 changes: 52 additions & 0 deletions test/e2e/testsuites/dynamically_provisioned_inline_volume.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright 2019 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package testsuites

import (
"sigs.k8s.io/azurefile-csi-driver/test/e2e/driver"

"github.com/onsi/ginkgo"
v1 "k8s.io/api/core/v1"
clientset "k8s.io/client-go/kubernetes"
)

// DynamicallyProvisionedInlineVolumeTest will provision required SecretName, ShareName
// Waiting for the PV provisioner to create an inline volume
// Testing if the Pod(s) Cmd is run with a 0 exit code
type DynamicallyProvisionedInlineVolumeTest struct {
CSIDriver driver.DynamicPVTestDriver
Pods []PodDetails
SecretName string
ShareName string
ReadOnly bool
}

func (t *DynamicallyProvisionedInlineVolumeTest) Run(client clientset.Interface, namespace *v1.Namespace) {
for _, pod := range t.Pods {
tpod, cleanup := pod.SetupWithInlineVolumes(client, namespace, t.CSIDriver, t.SecretName, t.ShareName, t.ReadOnly)
// 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()
}
}
9 changes: 9 additions & 0 deletions test/e2e/testsuites/specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,15 @@ func (pod *PodDetails) SetupWithDynamicVolumesWithSubpath(client clientset.Inter
return tpod, cleanupFuncs
}

func (pod *PodDetails) SetupWithInlineVolumes(client clientset.Interface, namespace *v1.Namespace, csiDriver driver.DynamicPVTestDriver, secretName, shareName string, readOnly bool) (*TestPod, []func()) {
tpod := NewTestPod(client, namespace, pod.Cmd, pod.IsWindows)
cleanupFuncs := make([]func(), 0)
for n, v := range pod.Volumes {
tpod.SetupInlineVolume(fmt.Sprintf("%s%d", v.VolumeMount.NameGenerate, n+1), fmt.Sprintf("%s%d", v.VolumeMount.MountPathGenerate, n+1), secretName, shareName, readOnly)
}
return tpod, cleanupFuncs
}

func (pod *PodDetails) SetupWithPreProvisionedVolumes(client clientset.Interface, namespace *v1.Namespace, csiDriver driver.PreProvisionedVolumeTestDriver) (*TestPod, []func()) {
tpod := NewTestPod(client, namespace, pod.Cmd, pod.IsWindows)
cleanupFuncs := make([]func(), 0)
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/testsuites/testsuites.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,27 @@ func (t *TestPod) SetupVolumeMountWithSubpath(pvc *v1.PersistentVolumeClaim, nam
t.pod.Spec.Volumes = append(t.pod.Spec.Volumes, volume)
}

func (t *TestPod) SetupInlineVolume(name, mountPath, secretName, shareName string, readOnly bool) {
volumeMount := v1.VolumeMount{
Name: name,
MountPath: mountPath,
ReadOnly: readOnly,
}
t.pod.Spec.Containers[0].VolumeMounts = append(t.pod.Spec.Containers[0].VolumeMounts, volumeMount)

volume := v1.Volume{
Name: name,
VolumeSource: v1.VolumeSource{
AzureFile: &v1.AzureFileVolumeSource{
SecretName: secretName,
ShareName: shareName,
ReadOnly: readOnly,
},
},
}
t.pod.Spec.Volumes = append(t.pod.Spec.Volumes, volume)
}

func (t *TestPod) SetNodeSelector(nodeSelector map[string]string) {
t.pod.Spec.NodeSelector = nodeSelector
}
Expand Down