Merge pull request #5 from moadqassem/validate-secrets

validating secrets on machine server calls

pkg/kubevirt/machine_server.go

@@ -19,9 +19,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/util"
-	"github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/validation"
-
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/driver"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
@@ -55,20 +52,14 @@ func (p *MachinePlugin) CreateMachine(ctx context.Context, req *driver.CreateMac
 	klog.V(2).Infof("Machine creation request has been recieved for %q", req.Machine.Name)
 	defer klog.V(2).Infof("Machine creation request has been processed for %q", req.Machine.Name)
 
-	providerSpec, err := util.DecodeProviderSpecAndSecret(req.MachineClass)
+	providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Create machine %q failed on DecodeProviderSpecAndSecret", req.Machine.Name)
-	}
-
-	validationErrors := validation.ValidateKubevirtSecret(providerSpec, req.Secret)
-	if validationErrors != nil {
-		err = fmt.Errorf("error while validating ProviderSpec %v", validationErrors)
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, prepareErrorf(err, "Create machine %q failed on decodeProviderSpecAndSecret", req.Machine.Name)
 	}
 
 	providerID, err := p.SPI.CreateMachine(ctx, req.Machine.Name, providerSpec, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Create machine %q failed", req.Machine.Name)
+		return nil, prepareErrorf(err, "Create machine %q failed", req.Machine.Name)
 	}
 
 	response := &driver.CreateMachineResponse{
@@ -95,14 +86,14 @@ func (p *MachinePlugin) DeleteMachine(ctx context.Context, req *driver.DeleteMac
 	klog.V(2).Infof("Machine deletion request has been recieved for %q", req.Machine.Name)
 	defer klog.V(2).Infof("Machine deletion request has been processed for %q", req.Machine.Name)
 
-	providerSpec, err := util.DecodeProviderSpecAndSecret(req.MachineClass)
+	providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Create machine %q failed on DecodeProviderSpecAndSecret", req.Machine.Name)
+		return nil, prepareErrorf(err, "Create machine %q failed on decodeProviderSpecAndSecret", req.Machine.Name)
 	}
 
 	providerID, err := p.SPI.DeleteMachine(ctx, req.Machine.Name, req.Machine.Spec.ProviderID, providerSpec, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Create machine %q failed", req.Machine.Name)
+		return nil, prepareErrorf(err, "Create machine %q failed", req.Machine.Name)
 	}
 
 	response := &driver.DeleteMachineResponse{
@@ -132,14 +123,14 @@ func (p *MachinePlugin) GetMachineStatus(ctx context.Context, req *driver.GetMac
 	klog.V(2).Infof("Get request has been recieved for %q", req.Machine.Name)
 	defer klog.V(2).Infof("Machine get request has been processed successfully for %q", req.Machine.Name)
 
-	providerSpec, err := util.DecodeProviderSpecAndSecret(req.MachineClass)
+	providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Create machine %q failed on DecodeProviderSpecAndSecret", req.Machine.Name)
+		return nil, prepareErrorf(err, "Create machine %q failed on decodeProviderSpecAndSecret", req.Machine.Name)
 	}
 
 	providerID, err := p.SPI.GetMachineStatus(ctx, req.Machine.Name, req.Machine.Spec.ProviderID, providerSpec, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "Machine status %q failed", req.Machine.Name)
+		return nil, prepareErrorf(err, "Machine status %q failed", req.Machine.Name)
 	}
 
 	response := &driver.GetMachineStatusResponse{
@@ -170,14 +161,14 @@ func (p *MachinePlugin) ListMachines(ctx context.Context, req *driver.ListMachin
 	klog.V(2).Infof("List machines request has been recieved for %q", req.MachineClass.Name)
 	defer klog.V(2).Infof("List machines request has been recieved for %q", req.MachineClass.Name)
 
-	providerSpec, err := util.DecodeProviderSpecAndSecret(req.MachineClass)
+	providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "List machines failed on DecodeProviderSpecAndSecret")
+		return nil, prepareErrorf(err, "List machines failed on decodeProviderSpecAndSecret")
 	}
 
 	machineList, err := p.SPI.ListMachines(ctx, providerSpec, req.Secret)
 	if err != nil {
-		return nil, util.PrepareErrorf(err, "List machines failed")
+		return nil, prepareErrorf(err, "List machines failed")
 	}
 
 	klog.V(2).Infof("List machines request for kubevirt cluster, found %d machines", len(machineList))

pkg/kubevirt/machine_server_util.go

@@ -0,0 +1,56 @@
+package kubevirt
+
+import (
+	"encoding/json"
+	"fmt"
+
+	api "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/apis"
+	clouderrors "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/errors"
+	"github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/validation"
+
+	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
+	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
+	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
+	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/klog"
+)
+
+// decodeProviderSpecAndSecret converts request parameters to api.ProviderSpec
+func decodeProviderSpecAndSecret(machineClass *v1alpha1.MachineClass, secret *corev1.Secret) (*api.KubeVirtProviderSpec, error) {
+	var (
+		providerSpec *api.KubeVirtProviderSpec
+	)
+
+	// Extract providerSpec
+	err := json.Unmarshal(machineClass.ProviderSpec.Raw, &providerSpec)
+	if err != nil {
+		return nil, status.Error(codes.Internal, err.Error())
+	}
+
+	validationErrors := validation.ValidateKubevirtProviderSpecAndSecret(providerSpec, secret)
+	if validationErrors != nil {
+		err = fmt.Errorf("error while validating ProviderSpec %v", validationErrors)
+		return nil, status.Error(codes.Internal, err.Error())
+	}
+
+	return providerSpec, nil
+}
+
+// prepareErrorf preapre, format and wrap an error on the machine server level.
+func prepareErrorf(err error, format string, args ...interface{}) error {
+	var (
+		code    codes.Code
+		wrapped error
+	)
+	switch err.(type) {
+	case *clouderrors.MachineNotFoundError:
+		code = codes.NotFound
+		wrapped = err
+	default:
+		code = codes.Internal
+		wrapped = errors.Wrap(err, fmt.Sprintf(format, args...))
+	}
+	klog.V(2).Infof(wrapped.Error())
+	return status.Error(code, wrapped.Error())
+}

pkg/kubevirt/util/util.go

@@ -15,54 +15,12 @@
 package util
 
 import (
-	"encoding/json"
 	"fmt"
 
-	api "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/apis"
-	clouderrors "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/errors"
-
-	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
-	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
-	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
-	"k8s.io/klog"
 )
 
-// DecodeProviderSpecAndSecret converts request parameters to api.ProviderSpec
-func DecodeProviderSpecAndSecret(machineClass *v1alpha1.MachineClass) (*api.KubeVirtProviderSpec, error) {
-	var (
-		providerSpec *api.KubeVirtProviderSpec
-	)
-
-	// Extract providerSpec
-	err := json.Unmarshal(machineClass.ProviderSpec.Raw, &providerSpec)
-	if err != nil {
-		return nil, status.Error(codes.Internal, err.Error())
-	}
-
-	return providerSpec, nil
-}
-
-// PrepareErrorf preapre, format and wrap an error on the machine server level.
-func PrepareErrorf(err error, format string, args ...interface{}) error {
-	var (
-		code    codes.Code
-		wrapped error
-	)
-	switch err.(type) {
-	case *clouderrors.MachineNotFoundError:
-		code = codes.NotFound
-		wrapped = err
-	default:
-		code = codes.Internal
-		wrapped = errors.Wrap(err, fmt.Sprintf(format, args...))
-	}
-	klog.V(2).Infof(wrapped.Error())
-	return status.Error(code, wrapped.Error())
-}
-
 // DNSPolicy receives a policy as a string and converts it to a kubevirt DNSPolicy to be used in the virtual machine.
 func DNSPolicy(policy string) (corev1.DNSPolicy, error) {
 	switch policy {

pkg/kubevirt/validation/validation.go

@@ -28,8 +28,8 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 )
 
-// ValidateKubevirtSecret validates kubevirt spec and secret to check if all fields are present and valid
-func ValidateKubevirtSecret(spec *api.KubeVirtProviderSpec, secrets *corev1.Secret) []error {
+// ValidateKubevirtProviderSpecAndSecret validates kubevirt spec and secret to check if all fields are present and valid
+func ValidateKubevirtProviderSpecAndSecret(spec *api.KubeVirtProviderSpec, secrets *corev1.Secret) []error {
 	var validationErrors []error
 
 	if spec.CPUs == "" {