Skip to content

Commit

Permalink
Complete auth support (#8)
Browse files Browse the repository at this point in the history
* Adding Unit teste

* resolve golinbt issues
  • Loading branch information
Michael Edegware authored Jan 21, 2022
1 parent c7b78df commit 598c1f3
Show file tree
Hide file tree
Showing 7 changed files with 510 additions and 59 deletions.
7 changes: 4 additions & 3 deletions controllers/csm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ var configVersionKey = fmt.Sprintf("%s/%s", MetadataPrefix, "CSIDriverConfigVers
// +kubebuilder:rbac:groups="storage.k8s.io",resources=storageclasses,verbs=get;list;watch;create;update;delete
// +kubebuilder:rbac:groups="storage.k8s.io",resources=volumeattachments,verbs=get;list;watch;create;update;patch
// +kubebuilder:rbac:groups="storage.k8s.io",resources=csinodes,verbs=get;list;watch;create;update
// +kubebuilder:rbac:groups="csi.storage.k8s.io",resources=csinodeinfos,verbs=get;list;watch
// +kubebuilder:rbac:groups="snapshot.storage.k8s.io",resources=volumesnapshotclasses;volumesnapshotcontents,verbs=get;list;watch;create;update;delete
// +kubebuilder:rbac:groups="snapshot.storage.k8s.io",resources=volumesnapshotcontents/status,verbs=update
// +kubebuilder:rbac:groups="snapshot.storage.k8s.io",resources=volumesnapshots;volumesnapshots/status,verbs=get;list;watch;update
Expand Down Expand Up @@ -145,7 +146,7 @@ func (r *ContainerStorageModuleReconciler) Reconcile(ctx context.Context, req ct
}

// Before doing anything else, check for config version and apply annotation if not set
isUpdated, err := checkAndApplyConfigVersionAnnotations(*csm, log, false)
isUpdated, err := checkAndApplyConfigVersionAnnotations(csm, log, false)
if err != nil {
return utils.HandleValidationError(ctx, csm, r, reqLogger, err)
} else if isUpdated {
Expand Down Expand Up @@ -525,7 +526,7 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs
if m.Enabled {
switch m.Name {
case csmv1.Authorization:
err := modules.AuthorizationPrecheck(ctx, cr, m, r, reqLogger)
err := modules.AuthorizationPrecheck(ctx, cr.GetNamespace(), string(cr.Spec.Driver.CSIDriverType), operatorConfig, m, r.GetClient(), reqLogger)
if err != nil {
return fmt.Errorf("failed authorization validation: %v", err)
}
Expand All @@ -542,7 +543,7 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs

}

func checkAndApplyConfigVersionAnnotations(instance csmv1.ContainerStorageModule, log logr.Logger, update bool) (bool, error) {
func checkAndApplyConfigVersionAnnotations(instance *csmv1.ContainerStorageModule, log logr.Logger, update bool) (bool, error) {
if instance.Spec.Driver.ConfigVersion == "" {
// fail immediately
return false, fmt.Errorf("mandatory argument: ConfigVersion missing")
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/kubernetes-csi/external-snapshotter/client/v3 v3.0.0
github.com/onsi/ginkgo v1.16.4
github.com/onsi/gomega v1.13.0
github.com/stretchr/testify v1.6.1 // indirect
k8s.io/api v0.21.2
k8s.io/apimachinery v0.21.2
k8s.io/client-go v0.21.2
Expand Down
47 changes: 11 additions & 36 deletions k8s/client_test.go
Original file line number Diff line number Diff line change
@@ -1,49 +1,24 @@
package k8s_test
package k8s

/*import (
"context"
"encoding/json"
import (
"errors"
"fmt"
"reflect"
"testing"
"time"

"github.com/dell/csm-deployment/utils"
"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
k8stesting "k8s.io/client-go/testing"

"k8s.io/apimachinery/pkg/version"
discoveryfake "k8s.io/client-go/discovery/fake"
"k8s.io/client-go/tools/clientcmd"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
ctrlClient "sigs.k8s.io/controller-runtime/pkg/client"
ctrlClientFake "sigs.k8s.io/controller-runtime/pkg/client/fake"
)

type testOverrides struct {
getClientSetWrapper func() (kubernetes.Interface, error)
ignoreError bool
}

var (
namespace = "csm"
csmDataCollector = "csm-data-collector"
secretName = "testing-secret"
configMapName = "testing-configMap"
)
func Test_IsOpenShift(t *testing.T) {
tests := map[string]func(t *testing.T) (bool, testOverrides){
"success ": func(*testing.T) (bool, testOverrides) {
Expand All @@ -66,6 +41,9 @@ func Test_IsOpenShift(t *testing.T) {
},
}
},
"bad config data ": func(*testing.T) (bool, testOverrides) {
return false, testOverrides{ignoreError: true}
},
"fail - not found ": func(*testing.T) (bool, testOverrides) {
return false, testOverrides{
getClientSetWrapper: func() (kubernetes.Interface, error) {
Expand Down Expand Up @@ -106,9 +84,6 @@ func Test_IsOpenShift(t *testing.T) {
},
}
},
"bad config data ": func(*testing.T) (bool, testOverrides) {
return false, testOverrides{}
},
"fail - to get client set": func(*testing.T) (bool, testOverrides) {
return false, testOverrides{
getClientSetWrapper: func() (kubernetes.Interface, error) {
Expand All @@ -129,7 +104,9 @@ func Test_IsOpenShift(t *testing.T) {
}

isOpenshift, err := IsOpenShift()
if !success {
if patch.ignoreError {
t.Log("cover real Openshift setup")
} else if !success {
assert.False(t, isOpenshift)
} else {
assert.NoError(t, err)
Expand All @@ -138,8 +115,8 @@ func Test_IsOpenShift(t *testing.T) {

})
}
}

func Test_GetVersion(t *testing.T) {
tests := map[string]func(t *testing.T) (bool, string, string, testOverrides){
"success ": func(*testing.T) (bool, string, string, testOverrides) {
Expand All @@ -163,7 +140,6 @@ func Test_GetVersion(t *testing.T) {
return fake.NewSimpleClientset(), errors.New(" error listing pods")
},
}
},
}
for name, tc := range tests {
Expand All @@ -176,7 +152,7 @@ func Test_GetVersion(t *testing.T) {
GetClientSetWrapper = patch.getClientSetWrapper
}

out, err := GetVersion([]byte(""))
out, err := GetVersion()
if !success {
assert.Error(t, err)
} else {
Expand All @@ -187,4 +163,3 @@ func Test_GetVersion(t *testing.T) {
})
}
}
*/
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ metadata:
namespace: <DriverDefaultReleaseNamespace>
data:
driver-config-params.yaml: |
CSI_LOG_LEVEL: debug
CSI_LOG_LEVEL: debug
50 changes: 31 additions & 19 deletions pkg/modules/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
crclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)

const (
// DefaultPluginIdentifier constant
// DefaultPluginIdentifier - spring placeholder for driver plugin
DefaultPluginIdentifier = "<DriverPluginIdentifier>"
)

Expand All @@ -29,10 +30,6 @@ var SupportedDrivers = map[string]string{

func getAuthCR(cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*csmv1.Module, *corev1.Container, error) {
var err error
configMapName := map[string]string{
"powerscale": "csi-isilon",
"isilon": "csi-isilon",
}
authModule := csmv1.Module{}
for _, m := range cr.Spec.Modules {
if m.Name == csmv1.Authorization {
Expand All @@ -55,10 +52,7 @@ func getAuthCR(cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*csmv1
return nil, nil, err
}

YamlString := string(buf)
if cr.Name != "" {
YamlString = strings.ReplaceAll(YamlString, utils.DefaultReleaseName, configMapName[string(cr.Spec.Driver.CSIDriverType)])
}
YamlString := utils.ModifyCommonCR(string(buf), cr)

if string(cr.Spec.Driver.Common.ImagePullPolicy) != "" {
YamlString = strings.ReplaceAll(YamlString, utils.DefaultImagePullPolicy, string(cr.Spec.Driver.Common.ImagePullPolicy))
Expand Down Expand Up @@ -133,7 +127,7 @@ func getAuthVolumes(cr csmv1.ContainerStorageModule, op utils.OperatorConfig, au
return vols, nil
}

// AuthInjectDaemonset auth module
// AuthInjectDaemonset - inject authorization into daemonset
func AuthInjectDaemonset(ds appsv1.DaemonSet, cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*appsv1.DaemonSet, error) {
authModule, containerPtr, err := getAuthCR(cr, op)
if err != nil {
Expand Down Expand Up @@ -161,7 +155,7 @@ func AuthInjectDaemonset(ds appsv1.DaemonSet, cr csmv1.ContainerStorageModule, o
return &ds, nil
}

// AuthInjectDeployment deploy
// AuthInjectDeployment - inject authorization into deployment
func AuthInjectDeployment(dp appsv1.Deployment, cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*appsv1.Deployment, error) {
authModule, containerPtr, err := getAuthCR(cr, op)
if err != nil {
Expand Down Expand Up @@ -190,10 +184,30 @@ func AuthInjectDeployment(dp appsv1.Deployment, cr csmv1.ContainerStorageModule,

}

// AuthorizationPrecheck precheck
func AuthorizationPrecheck(ctx context.Context, cr *csmv1.ContainerStorageModule, auth csmv1.Module, r utils.ReconcileCSM, log logr.Logger) error {
if _, ok := SupportedDrivers[string(cr.Spec.Driver.CSIDriverType)]; !ok {
return fmt.Errorf("csm authorization does not support %s driver", string(cr.Spec.Driver.CSIDriverType))
// AuthorizationPrecheck - runs precheck for CSM Authorization
func AuthorizationPrecheck(ctx context.Context, namespace, driverType string, op utils.OperatorConfig, auth csmv1.Module, ctrlClient crclient.Client, log logr.Logger) error {
if _, ok := SupportedDrivers[driverType]; !ok {
return fmt.Errorf("CSM Authorization does not support %s driver", driverType)
}

// check if provided version is supported
if auth.ConfigVersion != "" {
files, err := ioutil.ReadDir(fmt.Sprintf("%s/moduleconfig/authorization/", op.ConfigDirectory))
if err != nil {
return err
}
found := false
authVersions := ""
for _, file := range files {
authVersions += (file.Name() + ",")
if file.Name() == auth.ConfigVersion {
found = true
}
}
if !found {
return fmt.Errorf("CSM Authorization does not have %s version. The following are supported versions: %s", auth.ConfigVersion, authVersions[:len(authVersions)-1])
}

}

// Check for secrets
Expand All @@ -219,8 +233,8 @@ func AuthorizationPrecheck(ctx context.Context, cr *csmv1.ContainerStorageModule

for _, name := range secrets {
found := &corev1.Secret{}
err := r.GetClient().Get(ctx, types.NamespacedName{Name: name,
Namespace: cr.GetNamespace()}, found)
err := ctrlClient.Get(ctx, types.NamespacedName{Name: name,
Namespace: namespace}, found)
if err != nil {
if errors.IsNotFound(err) {
return fmt.Errorf("failed to find secret %s and certificate validation is requested", name)
Expand All @@ -229,7 +243,5 @@ func AuthorizationPrecheck(ctx context.Context, cr *csmv1.ContainerStorageModule
}
}

// TODO(Michael): Do Other configuration checks if needed

return nil
}
Loading

0 comments on commit 598c1f3

Please sign in to comment.