From 302c08312574d3cb0aca19f66d3e9132edf2db25 Mon Sep 17 00:00:00 2001 From: Samarth Deyagond Date: Thu, 1 Apr 2021 16:48:52 +0530 Subject: [PATCH 1/4] add a check on provider of the machineclass in the driver methods --- pkg/azure/core.go | 28 +++++++++++-- pkg/azure/core_test.go | 93 +++++++++++++++++++++++++++++++++++++++++- pkg/azure/utils.go | 1 + 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/pkg/azure/core.go b/pkg/azure/core.go index 14f25ea4d..24a5ab680 100644 --- a/pkg/azure/core.go +++ b/pkg/azure/core.go @@ -23,6 +23,11 @@ import ( "k8s.io/klog" ) +const ( + AzureMachineClassKind = "AzureMachineClass" + ProviderAzure = "azure" +) + // NOTE // // The basic working of the controller will work with just implementing the CreateMachine() & DeleteMachine() methods. @@ -57,9 +62,6 @@ type MachinePlugin struct { Secret *corev1.Secret } -// AzureMachineClassKind for Azure Machine Class -const AzureMachineClassKind = "AzureMachineClass" - // NewAzureDriver returns an empty AzureDriver object func NewAzureDriver(spi spi.SessionProviderInterface) *MachinePlugin { return &MachinePlugin{ @@ -77,6 +79,11 @@ func (d *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) + // Check if incoming CR is a CR we support + if req.MachineClass.Provider != ProviderAzure { + return nil, fmt.Errorf("Requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, ProviderAzure) + } + d.Secret = req.Secret virtualMachine, err := d.createVMNicDisk(req) if err != nil { @@ -105,6 +112,11 @@ func (d *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) + // Check if incoming CR is a CR we support + if req.MachineClass.Provider != ProviderAzure { + return nil, fmt.Errorf("Requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, ProviderAzure) + } + providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret) if err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -166,6 +178,11 @@ func (d *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) + // Check if incoming CR is a CR we support + if req.MachineClass.Provider != ProviderAzure { + return nil, fmt.Errorf("Requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, ProviderAzure) + } + var machineStatusResponse = &driver.GetMachineStatusResponse{} listMachineRequest := &driver.ListMachinesRequest{MachineClass: req.MachineClass, Secret: req.Secret} @@ -203,6 +220,11 @@ func (d *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) + // Check if incoming CR is a CR we support + if req.MachineClass.Provider != ProviderAzure { + return nil, fmt.Errorf("Requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, ProviderAzure) + } + providerSpec, err := decodeProviderSpecAndSecret(req.MachineClass, req.Secret) d.AzureProviderSpec = providerSpec diff --git a/pkg/azure/core_test.go b/pkg/azure/core_test.go index 0025fe73c..3bcfa8d34 100644 --- a/pkg/azure/core_test.go +++ b/pkg/azure/core_test.go @@ -183,6 +183,24 @@ var _ = Describe("MachineController", func() { false, "", ), + Entry("#1 Create a simple machine with wrong provider value in Machine Class", + &mock.AzureProviderSpec, + &driver.CreateMachineRequest{ + Machine: newMachine("dummy-machine"), + MachineClass: newAzureMachineClassWithProvider(mock.AzureProviderSpec, "aws"), + Secret: newSecret(azureProviderSecret), + }, + &driver.CreateMachineResponse{ + ProviderID: "azure:///westeurope/dummy-machine", + NodeName: "dummy-machine", + }, + nil, + nil, + nil, + nil, + true, + fmt.Errorf("Requested for Provider 'aws', we only support '%s'", ProviderAzure).Error(), + ), Entry("#2 Create machine without client id in secret", &mock.AzureProviderSpec, &driver.CreateMachineRequest{ @@ -741,7 +759,24 @@ var _ = Describe("MachineController", func() { false, nil, ), - + Entry("#2 Delete a machine", + &mock.AzureProviderSpec, + &driver.DeleteMachineRequest{ + Machine: newMachine("dummy-machine"), + MachineClass: newAzureMachineClassWithProvider(mock.AzureProviderSpec, "aws"), + Secret: newSecret(azureProviderSecret), + }, + &driver.DeleteMachineResponse{ + LastKnownState: "", + }, + nil, + false, + false, + false, + nil, + true, + fmt.Errorf("Requested for Provider '%s', we only support '%s'", "aws", ProviderAzure).Error(), + ), Entry("#2 Delete a machine while a NIC is still attached", &mock.AzureProviderSpec, &driver.DeleteMachineRequest{ @@ -970,6 +1005,22 @@ var _ = Describe("MachineController", func() { false, "", ), + Entry("#1 List machines with wrong MachineClass Provider", + &mock.AzureProviderSpec, + &driver.ListMachinesRequest{ + MachineClass: newAzureMachineClassWithProvider(mock.AzureProviderSpec, "aws"), + Secret: newSecret(azureProviderSecret), + }, + &driver.ListMachinesResponse{ + MachineList: map[string]string{ + "azure:///westeurope/dummy-machine": "dummy-machine", + }, + }, + false, + nil, + true, + fmt.Errorf("Requested for Provider '%s', we only support '%s'", "aws", ProviderAzure).Error(), + ), Entry("#2 List machines with VM List error scenario", &mock.AzureProviderSpec, &driver.ListMachinesRequest{ @@ -1085,6 +1136,30 @@ var _ = Describe("MachineController", func() { false, "", ), + Entry("#1 GetMachineStatus of machine with wrong MachineClass reference", + &mock.AzureProviderSpec, + &driver.GetMachineStatusRequest{ + Machine: newMachine("dummy-machine"), + MachineClass: newAzureMachineClassWithProvider(mock.AzureProviderSpec, "aws"), + Secret: newSecret(azureProviderSecret), + }, + &driver.GetMachineStatusResponse{ + NodeName: "dummy-machine", + ProviderID: "azure:///westeurope/dummy-machine", + }, + compute.VirtualMachineListResult{ + Value: &[]compute.VirtualMachine{ + { + Name: getStringPointer("dummy-machine"), + Location: getStringPointer("westeurope"), + }, + }, + NextLink: getStringPointer(""), + }, + nil, + true, + fmt.Errorf("Requested for Provider '%s', we only support '%s'", "aws", ProviderAzure).Error(), + ), Entry("#2 GetMachineStatus of non existing machine", &mock.AzureProviderSpec, &driver.GetMachineStatusRequest{ @@ -1533,6 +1608,19 @@ func newMachine(name string) *v1alpha1.Machine { } } +func newAzureMachineClassWithProvider(azureProviderSpec apis.AzureProviderSpec, provider string) *v1alpha1.MachineClass { + byteData, _ := json.Marshal(azureProviderSpec) + return &v1alpha1.MachineClass{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + }, + ProviderSpec: runtime.RawExtension{ + Raw: byteData, + }, + Provider: provider, + } +} + func newAzureMachineClass(azureProviderSpec apis.AzureProviderSpec) *v1alpha1.MachineClass { byteData, _ := json.Marshal(azureProviderSpec) return &v1alpha1.MachineClass{ @@ -1542,6 +1630,7 @@ func newAzureMachineClass(azureProviderSpec apis.AzureProviderSpec) *v1alpha1.Ma ProviderSpec: runtime.RawExtension{ Raw: byteData, }, + Provider: ProviderAzure, } } @@ -1555,6 +1644,7 @@ func newAzureMachineClassWithError() *v1alpha1.MachineClass { ProviderSpec: runtime.RawExtension{ Raw: byteData, }, + Provider: ProviderAzure, } } @@ -1600,6 +1690,7 @@ func getMigratedMachineClass(providerSpecificMachineClass interface{}) *v1alpha1 ProviderSpec: runtime.RawExtension{ Raw: providerSpecMarshal, }, + Provider: ProviderAzure, SecretRef: providerSpecificMachineClass.(*v1alpha1.AzureMachineClass).Spec.SecretRef, } diff --git a/pkg/azure/utils.go b/pkg/azure/utils.go index 3f1390c95..ef8d762b4 100644 --- a/pkg/azure/utils.go +++ b/pkg/azure/utils.go @@ -606,6 +606,7 @@ func fillUpMachineClass(azureMachineClass *v1alpha1.AzureMachineClass, machineCl return status.Error(codes.Internal, err.Error()) } + machineClass.Provider = ProviderAzure machineClass.SecretRef = azureMachineClass.Spec.SecretRef machineClass.CredentialsSecretRef = azureMachineClass.Spec.CredentialsSecretRef machineClass.Name = azureMachineClass.Name From 141ab5ca7fd8bde425482aa9bac6aa6f219c6bb0 Mon Sep 17 00:00:00 2001 From: Samarth Deyagond Date: Thu, 1 Apr 2021 16:52:43 +0530 Subject: [PATCH 2/4] Update example files with useful fields --- kubernetes/machine-class.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/kubernetes/machine-class.yaml b/kubernetes/machine-class.yaml index 74e5fc76a..58cafd813 100644 --- a/kubernetes/machine-class.yaml +++ b/kubernetes/machine-class.yaml @@ -10,6 +10,7 @@ kind: MachineClass metadata: name: namespace: +provider: azure providerSpec: location: westeurope properties: From b8208b82541c0fb39f4aa9711cfcbf710da0bcf8 Mon Sep 17 00:00:00 2001 From: Samarth Deyagond Date: Thu, 1 Apr 2021 17:48:41 +0530 Subject: [PATCH 3/4] Commenting for an exported constant --- pkg/azure/core.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/azure/core.go b/pkg/azure/core.go index 24a5ab680..5663db0ba 100644 --- a/pkg/azure/core.go +++ b/pkg/azure/core.go @@ -24,6 +24,7 @@ import ( ) const ( + // AzureMachineClassKind is the constant presenting the AzureMachineClass AzureMachineClassKind = "AzureMachineClass" ProviderAzure = "azure" ) From 360a160d53ef6c9fc12bd2f087103776a5cc93ad Mon Sep 17 00:00:00 2001 From: Samarth Deyagond Date: Thu, 1 Apr 2021 17:58:10 +0530 Subject: [PATCH 4/4] Commenting on an exported constant --- pkg/azure/core.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/azure/core.go b/pkg/azure/core.go index 5663db0ba..c5aae664c 100644 --- a/pkg/azure/core.go +++ b/pkg/azure/core.go @@ -24,9 +24,11 @@ import ( ) const ( - // AzureMachineClassKind is the constant presenting the AzureMachineClass + // AzureMachineClassKind is the constant representing the AzureMachineClass AzureMachineClassKind = "AzureMachineClass" - ProviderAzure = "azure" + + // ProviderAzure is the constant representing the Cloud Provider Azure + ProviderAzure = "azure" ) // NOTE