From 9c4cb04849a388c8311b43cf67d5332e7e05577e Mon Sep 17 00:00:00 2001 From: Smriti Dahal Date: Fri, 14 Apr 2023 17:16:16 -0700 Subject: [PATCH 1/4] added unit test for validateNetworkConfig --- pkg/network/aci_network_test.go | 72 +++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/pkg/network/aci_network_test.go b/pkg/network/aci_network_test.go index c5626245..1fd36f83 100644 --- a/pkg/network/aci_network_test.go +++ b/pkg/network/aci_network_test.go @@ -2,12 +2,15 @@ package network import ( "context" + "errors" "fmt" + "os" "testing" aznetworkv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2" "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/virtual-kubelet/azure-aci/pkg/auth" testsutil "github.com/virtual-kubelet/azure-aci/pkg/tests" v1 "k8s.io/api/core/v1" ) @@ -254,3 +257,72 @@ func TestShouldCreateSubnet(t *testing.T) { } } + +func TestValidateNetworkConfig(t *testing.T) { + azConfig := auth.Config{} + err := azConfig.SetAuthConfig(context.TODO()) + if err != nil { + t.Fatal(err) + } + + pn := &ProviderNetwork{} + + cases := []struct { + description string + setEnvVar func() + expectedError error + }{ + { + description: "ACI vnet name env variable is not set", + setEnvVar: func() {}, + expectedError: errors.New("vnet name can not be empty please set ACI_VNET_NAME"), + }, + { + description: "ACI vnet resource group env variable is not set", + setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") + }, + expectedError: errors.New("vnet resourceGroup can not be empty please set ACI_VNET_RESOURCE_GROUP"), + }, + { + description: "ACI subnet CIDR env variable is set but subnet name is missing", + setEnvVar: func() { + os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") + os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") + }, + expectedError: errors.New("subnet CIDR defined but no subnet name, subnet name is required to set a subnet CIDR"), + }, + { + description: "ACI subnet CIDR env variable is set but it is malformed", + setEnvVar: func() { + os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") + }, + expectedError: errors.New("error parsing provided subnet CIDR: invalid CIDR address: 10.00.0/16"), + }, + { + description: "all environmental variables are set as expected", + setEnvVar: func() { + os.Setenv("ACI_SUBNET_CIDR", "127.0.0.1/24") + }, + expectedError: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + tc.setEnvVar() + err := pn.validateNetworkConfig(context.Background(), &azConfig) + + if tc.expectedError != nil { + assert.Equal(t, err.Error(), tc.expectedError.Error(), "Error messages should match") + } else { + assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) + assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) + assert.Equal(t, pn.SubnetName, os.Getenv("ACI_SUBNET_NAME")) + assert.Equal(t, pn.SubnetCIDR, os.Getenv("ACI_SUBNET_CIDR")) + } + }) + } +} From 3438aaf53dde7c8ce16dcf62c5766bdb908c08bb Mon Sep 17 00:00:00 2001 From: Smriti Dahal Date: Fri, 14 Apr 2023 17:34:43 -0700 Subject: [PATCH 2/4] added additional env vairables to all test cases to remove confusion --- pkg/network/aci_network_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/network/aci_network_test.go b/pkg/network/aci_network_test.go index 1fd36f83..bd112008 100644 --- a/pkg/network/aci_network_test.go +++ b/pkg/network/aci_network_test.go @@ -288,6 +288,8 @@ func TestValidateNetworkConfig(t *testing.T) { { description: "ACI subnet CIDR env variable is set but subnet name is missing", setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") }, @@ -296,6 +298,10 @@ func TestValidateNetworkConfig(t *testing.T) { { description: "ACI subnet CIDR env variable is set but it is malformed", setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") + os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") + os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") }, expectedError: errors.New("error parsing provided subnet CIDR: invalid CIDR address: 10.00.0/16"), @@ -303,6 +309,11 @@ func TestValidateNetworkConfig(t *testing.T) { { description: "all environmental variables are set as expected", setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") + os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") + os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") + os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") os.Setenv("ACI_SUBNET_CIDR", "127.0.0.1/24") }, expectedError: nil, From 5b70090bed411511e3a1457f569ae9bb68abbd16 Mon Sep 17 00:00:00 2001 From: Smriti Dahal Date: Fri, 14 Apr 2023 17:43:16 -0700 Subject: [PATCH 3/4] moving providerNetwork inside the forloop --- pkg/network/aci_network_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/network/aci_network_test.go b/pkg/network/aci_network_test.go index bd112008..3362e8c2 100644 --- a/pkg/network/aci_network_test.go +++ b/pkg/network/aci_network_test.go @@ -265,8 +265,6 @@ func TestValidateNetworkConfig(t *testing.T) { t.Fatal(err) } - pn := &ProviderNetwork{} - cases := []struct { description string setEnvVar func() @@ -323,6 +321,8 @@ func TestValidateNetworkConfig(t *testing.T) { for _, tc := range cases { t.Run(tc.description, func(t *testing.T) { tc.setEnvVar() + + pn := &ProviderNetwork{} err := pn.validateNetworkConfig(context.Background(), &azConfig) if tc.expectedError != nil { From fbd66b4daa8172493fcce98ac3d2e7ce39d32e1f Mon Sep 17 00:00:00 2001 From: Smriti Dahal Date: Mon, 17 Apr 2023 12:58:35 -0700 Subject: [PATCH 4/4] updated test cases --- pkg/network/aci_network_test.go | 116 +++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 16 deletions(-) diff --git a/pkg/network/aci_network_test.go b/pkg/network/aci_network_test.go index 3362e8c2..0ee977ca 100644 --- a/pkg/network/aci_network_test.go +++ b/pkg/network/aci_network_test.go @@ -260,41 +260,115 @@ func TestShouldCreateSubnet(t *testing.T) { func TestValidateNetworkConfig(t *testing.T) { azConfig := auth.Config{} + err := azConfig.SetAuthConfig(context.TODO()) if err != nil { t.Fatal(err) } cases := []struct { - description string - setEnvVar func() - expectedError error + description string + providerNetwork *ProviderNetwork + setEnvVar func() + expectedAssertions func(pn *ProviderNetwork) bool + expectedError error }{ { - description: "ACI vnet name env variable is not set", - setEnvVar: func() {}, + description: "Neither provider Vnet name nor env variable is set", + providerNetwork: &ProviderNetwork{}, + setEnvVar: func() {}, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, azConfig.AuthConfig.SubscriptionID, "ACI Vnet subscription ID env variable was not set so it should default to what is present in authconfig") && + assert.Equal(t, pn.VnetName, "") + }, expectedError: errors.New("vnet name can not be empty please set ACI_VNET_NAME"), }, { - description: "ACI vnet resource group env variable is not set", + description: "Provider Vnet name, RG, and subnetName is set but env variable is not set", + providerNetwork: &ProviderNetwork{ + VnetName: "fakeVnet2", + VnetResourceGroup: "fakeRG2", + SubnetName: "fakeSubnet2", + }, + setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, "fakeVnet2") && assert.Equal(t, pn.VnetResourceGroup, "fakeRG2") && + assert.Equal(t, pn.SubnetName, "fakeSubnet2") + }, + expectedError: nil, + }, + { + description: "Provider Vnet name is not set but env variable is set and neither is set for vnet resource group", + providerNetwork: &ProviderNetwork{}, setEnvVar: func() { os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") os.Setenv("ACI_VNET_NAME", "fakeVnet") }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) + }, expectedError: errors.New("vnet resourceGroup can not be empty please set ACI_VNET_RESOURCE_GROUP"), }, { - description: "ACI subnet CIDR env variable is set but subnet name is missing", + description: "Provider Vnet RG is not set, but the env variable is set", + providerNetwork: &ProviderNetwork{}, + setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") + os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") + }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) && + assert.Equal(t, pn.VnetResourceGroup, os.Getenv(("ACI_VNET_RESOURCE_GROUP"))) + }, + expectedError: nil, + }, + { + description: "Neither provider, nor env variable is set for subnet name but subnet cidr env variable is set", + providerNetwork: &ProviderNetwork{}, setEnvVar: func() { os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") os.Setenv("ACI_VNET_NAME", "fakeVnet") os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) && + assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) && + assert.Equal(t, pn.SubnetName, "") + }, expectedError: errors.New("subnet CIDR defined but no subnet name, subnet name is required to set a subnet CIDR"), }, { - description: "ACI subnet CIDR env variable is set but it is malformed", + description: "Both provider and env variable is set for Vnet name, Vnet RG, and subnet name", + providerNetwork: &ProviderNetwork{ + VnetName: "fakeVnet2", + VnetResourceGroup: "fakeRG2", + SubnetName: "fakeSubnet2", + }, + setEnvVar: func() { + os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") + os.Setenv("ACI_VNET_NAME", "fakeVnet") + os.Setenv("ACI_VNET_RESOURCE_GROUP", "fakeRG") + os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") + }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) && + assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) && + assert.Equal(t, pn.SubnetName, os.Getenv("ACI_SUBNET_NAME")) + }, + expectedError: nil, + }, + { + description: "Provider subnet name is not set, but the env variable is set and it is malformed", + providerNetwork: &ProviderNetwork{}, setEnvVar: func() { os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") os.Setenv("ACI_VNET_NAME", "fakeVnet") @@ -302,10 +376,17 @@ func TestValidateNetworkConfig(t *testing.T) { os.Setenv("ACI_SUBNET_CIDR", "10.00.0/16") os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) && + assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) && + assert.Equal(t, pn.SubnetName, os.Getenv("ACI_SUBNET_NAME")) + }, expectedError: errors.New("error parsing provided subnet CIDR: invalid CIDR address: 10.00.0/16"), }, { - description: "all environmental variables are set as expected", + description: "Provider subnet name is not set, but the env variable is set and it is valid", + providerNetwork: &ProviderNetwork{}, setEnvVar: func() { os.Setenv("ACI_VNET_SUBSCRIPTION_ID", "111111-222-3333-4444-555555") os.Setenv("ACI_VNET_NAME", "fakeVnet") @@ -314,6 +395,13 @@ func TestValidateNetworkConfig(t *testing.T) { os.Setenv("ACI_SUBNET_NAME", "fakeSubnet") os.Setenv("ACI_SUBNET_CIDR", "127.0.0.1/24") }, + expectedAssertions: func(pn *ProviderNetwork) bool { + return assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) && + assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) && + assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) && + assert.Equal(t, pn.SubnetName, os.Getenv("ACI_SUBNET_NAME")) && + assert.Equal(t, pn.SubnetCIDR, os.Getenv("ACI_SUBNET_CIDR")) + }, expectedError: nil, }, } @@ -322,17 +410,13 @@ func TestValidateNetworkConfig(t *testing.T) { t.Run(tc.description, func(t *testing.T) { tc.setEnvVar() - pn := &ProviderNetwork{} + pn := tc.providerNetwork err := pn.validateNetworkConfig(context.Background(), &azConfig) + assert.Equal(t, tc.expectedAssertions(pn), true, "Expected assertions should pass") + if tc.expectedError != nil { assert.Equal(t, err.Error(), tc.expectedError.Error(), "Error messages should match") - } else { - assert.Equal(t, pn.VnetSubscriptionID, os.Getenv("ACI_VNET_SUBSCRIPTION_ID")) - assert.Equal(t, pn.VnetName, os.Getenv("ACI_VNET_NAME")) - assert.Equal(t, pn.VnetResourceGroup, os.Getenv("ACI_VNET_RESOURCE_GROUP")) - assert.Equal(t, pn.SubnetName, os.Getenv("ACI_SUBNET_NAME")) - assert.Equal(t, pn.SubnetCIDR, os.Getenv("ACI_SUBNET_CIDR")) } }) }