Repository: kubermatic/machine-controller
Pull request #1650 (2023-05-30 14:39:30)

[release/v1.56] Anexia Provider: Fix empty network config handling
**What this PR does / why we need it**:
Backport for #1646

**What type of PR is this?**
<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
/kind chore
-->
/kind bug

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change? Then add your Release Note here**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Anexia Provider: Fix nil pointer exception when providing no network configuration
```

**Documentation**:
<!--
Please do one of the following options:
- Add a link to the existing documentation
- Add a link to the kubermatic/docs pull request
- If no documentation change is applicable then add:
  - TBD (documentation will be added later)
  - NONE (no documentation needed for this PR)
-->
```documentation
NONE
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1650#" title="Author self-approved">eiabea</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign mfranczy for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubermatic/machine-controller/blob/release/v1.56/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["mfranczy"]} -->
kubermatic-bot: Hi @eiabea. Thanks for your PR.

I'm waiting for a [kubermatic](https://github.com/orgs/kubermatic/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/kubermatic/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

kubermatic-bot: LGTM label has been added.  <details>Git tree hash: e9ff91002527c5c4cfa1849f4d0fac298413bcee</details>

Patch:
diff --git a/pkg/cloudprovider/provider/anexia/provider.go b/pkg/cloudprovider/provider/anexia/provider.go
index 967d9578..5a4c3c01 100644
--- a/pkg/cloudprovider/provider/anexia/provider.go
+++ b/pkg/cloudprovider/provider/anexia/provider.go
@@ -167,16 +167,19 @@ func provisionVM(ctx context.Context, client anxclient.Client) error {
 
 		vm.Script = base64.StdEncoding.EncodeToString([]byte(reconcileContext.UserData))
 
-		for index, dnsServer := range reconcileContext.ProviderConfig.Network.DNS.Servers {
-			switch index {
-			case 0:
-				vm.DNS1 = dnsServer
-			case 1:
-				vm.DNS2 = dnsServer
-			case 2:
-				vm.DNS3 = dnsServer
-			case 3:
-				vm.DNS4 = dnsServer
+		providerCfg := reconcileContext.ProviderConfig
+		if providerCfg.Network != nil {
+			for index, dnsServer := range providerCfg.Network.DNS.Servers {
+				switch index {
+				case 0:
+					vm.DNS1 = dnsServer
+				case 1:
+					vm.DNS2 = dnsServer
+				case 2:
+					vm.DNS3 = dnsServer
+				case 3:
+					vm.DNS4 = dnsServer
+				}
 			}
 		}
 


Test patch:
diff --git a/pkg/cloudprovider/provider/anexia/helper_test.go b/pkg/cloudprovider/provider/anexia/helper_test.go
index 38c3a37e..c6256fa2 100644
--- a/pkg/cloudprovider/provider/anexia/helper_test.go
+++ b/pkg/cloudprovider/provider/anexia/helper_test.go
@@ -20,15 +20,27 @@ import (
 	"encoding/json"
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"github.com/gophercloud/gophercloud/testhelper"
 
 	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
+	clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
 	anxtypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/anexia/types"
+	cloudprovidertypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/types"
 	"github.com/kubermatic/machine-controller/pkg/providerconfig/types"
+	providerconfigtypes "github.com/kubermatic/machine-controller/pkg/providerconfig/types"
 
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+type jsonObject = map[string]interface{}
+
+type ProvisionVMTestCase struct {
+	ReconcileContext reconcileContext
+	AssertJSONBody   func(jsonBody jsonObject)
+}
+
 type ConfigTestCase struct {
 	Config anxtypes.RawConfig
 	Error  error
@@ -67,3 +79,78 @@ func newConfigVarString(str string) types.ConfigVarString {
 		Value: str,
 	}
 }
+
+// this generates a full config and allows hooking into it to e.g. remove a value.
+func hookableConfig(hook func(*anxtypes.RawConfig)) anxtypes.RawConfig {
+	config := anxtypes.RawConfig{
+		CPUs: 1,
+
+		Memory: 2,
+
+		Disks: []anxtypes.RawDisk{
+			{Size: 5, PerformanceType: newConfigVarString("ENT6")},
+		},
+
+		Token:      newConfigVarString("test-token"),
+		VlanID:     newConfigVarString("test-vlan"),
+		LocationID: newConfigVarString("test-location"),
+		TemplateID: newConfigVarString("test-template-id"),
+	}
+
+	if hook != nil {
+		hook(&config)
+	}
+
+	return config
+}
+
+// this generates a full reconcileContext with some default values and allows hooking into it to e.g. remove/overwrite a value.
+func hookableReconcileContext(locationID string, templateID string, hook func(*reconcileContext)) reconcileContext {
+	context := reconcileContext{
+		Machine: &v1alpha1.Machine{
+			ObjectMeta: metav1.ObjectMeta{Name: "TestMachine"},
+		},
+		Status:   &anxtypes.ProviderStatus{},
+		UserData: "",
+		Config: resolvedConfig{
+			VlanID:     "VLAN-ID",
+			LocationID: locationID,
+			TemplateID: templateID,
+			Disks: []resolvedDisk{
+				{
+					RawDisk: anxtypes.RawDisk{
+						Size: 5,
+					},
+				},
+			},
+			RawConfig: anxtypes.RawConfig{
+				CPUs:   5,
+				Memory: 5,
+			},
+		},
+		ProviderData: &cloudprovidertypes.ProviderData{
+			Update: func(m *clusterv1alpha1.Machine, mods ...cloudprovidertypes.MachineModifier) error {
+				return nil
+			},
+		},
+		ProviderConfig: &providerconfigtypes.Config{
+			Network: &providerconfigtypes.NetworkConfig{
+				DNS: providerconfigtypes.DNSConfig{
+					Servers: []string{
+						"1.1.1.1",
+						"",
+						"192.168.0.1",
+						"192.168.0.2",
+						"192.168.0.3",
+					},
+				},
+			},
+		},
+	}
+
+	if hook != nil {
+		hook(&context)
+	}
+
+	return context
+}
diff --git a/pkg/cloudprovider/provider/anexia/provider_test.go b/pkg/cloudprovider/provider/anexia/provider_test.go
index 5ba8e656..6e885326 100644
--- a/pkg/cloudprovider/provider/anexia/provider_test.go
+++ b/pkg/cloudprovider/provider/anexia/provider_test.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -40,7 +41,6 @@ import (
 	"go.anx.io/go-anxcloud/pkg/vsphere/provisioning/vm"
 
 	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
-	clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
 	cloudprovidererrors "github.com/kubermatic/machine-controller/pkg/cloudprovider/errors"
 	anxtypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/provider/anexia/types"
 	cloudprovidertypes "github.com/kubermatic/machine-controller/pkg/cloudprovider/types"
@@ -64,6 +64,7 @@ func TestAnexiaProvider(t *testing.T) {
 	a.FakeExisting(&vspherev1.Template{Identifier: "TEMPLATE-ID-OLD-BUILD", Name: testTemplateName, Build: "b01"})
 	a.FakeExisting(&vspherev1.Template{Identifier: "TEMPLATE-ID", Name: testTemplateName, Build: "b02"})
 	a.FakeExisting(&vspherev1.Template{Identifier: "WRONG-TEMPLATE-NAME", Name: "Wrong Template Name", Build: "b02"})
+	a.FakeExisting(&vspherev1.Template{Identifier: "TEMPLATE-ID-NO-NETWORK-CONFIG", Name: "no-network-config", Build: "b03"})
 
 	t.Cleanup(func() {
 		testhelper.TeardownHTTP()
@@ -72,6 +73,61 @@ func TestAnexiaProvider(t *testing.T) {
 
 	t.Run("Test provision VM", func(t *testing.T) {
 		t.Parallel()
+
+		testCases := []ProvisionVMTestCase{
+			{
+				// Provision a generic VM with some custom dns entries
+				ReconcileContext: hookableReconcileContext("LOCATION-ID", "TEMPLATE-ID", func(rc *reconcileContext) {
+					rc.ProviderConfig = &providerconfigtypes.Config{
+						Network: &providerconfigtypes.NetworkConfig{
+							DNS: providerconfigtypes.DNSConfig{
+								Servers: []string{
+									"1.1.1.1",
+									"",
+									"192.168.0.1",
+									"192.168.0.2",
+									"192.168.0.3",
+								},
+							},
+						},
+					}
+				}),
+				AssertJSONBody: func(jsonBody jsonObject) {
+					testhelper.AssertEquals(t, jsonBody["cpu_performance_type"], "performance")
+					testhelper.AssertEquals(t, jsonBody["hostname"], "TestMachine")
+					testhelper.AssertEquals(t, jsonBody["memory_mb"], json.Number("5"))
+
+					testhelper.AssertEquals(t, jsonBody["dns1"], "1.1.1.1")
+					_, exists := jsonBody["dns2"]
+					testhelper.AssertEquals(t, exists, false)
+					testhelper.AssertEquals(t, jsonBody["dns3"], "192.168.0.1")
+					testhelper.AssertEquals(t, jsonBody["dns4"], "192.168.0.2")
+
+					networkArray := jsonBody["network"].([]interface{})
+					networkObject := networkArray[0].(jsonObject)
+					testhelper.AssertEquals(t, networkObject["vlan"], "VLAN-ID")
+					testhelper.AssertEquals(t, networkObject["nic_type"], "vmxnet3")
+					testhelper.AssertEquals(t, networkObject["ips"].([]interface{})[0], "8.8.8.8")
+				},
+			},
+			{
+				// Provision a VM without any ProviderConfig
+				ReconcileContext: hookableReconcileContext("LOCATION-ID", "TEMPLATE-ID-NO-NETWORK-CONFIG", func(rc *reconcileContext) {
+					rc.ProviderConfig = &providerconfigtypes.Config{}
+				}),
+				AssertJSONBody: func(jsonBody jsonObject) {
+					_, exists := jsonBody["dns1"]
+					testhelper.AssertEquals(t, exists, false)
+					_, exists = jsonBody["dns2"]
+					testhelper.AssertEquals(t, exists, false)
+					_, exists = jsonBody["dns3"]
+					testhelper.AssertEquals(t, exists, false)
+					_, exists = jsonBody["dns4"]
+					testhelper.AssertEquals(t, exists, false)
+				},
+			},
+		}
+
 		testhelper.Mux.HandleFunc("/api/ipam/v1/address/reserve/ip/count.json", func(writer http.ResponseWriter, request *http.Request) {
 			err := json.NewEncoder(writer).Encode(address.ReserveRandomSummary{
 				Data: []address.ReservedIP{
@@ -84,108 +140,46 @@ func TestAnexiaProvider(t *testing.T) {
 			testhelper.AssertNoErr(t, err)
 		})
 
-		testhelper.Mux.HandleFunc("/api/vsphere/v1/provisioning/vm.json/LOCATION-ID/templates/TEMPLATE-ID", func(writer http.ResponseWriter, request *http.Request) {
-			testhelper.TestMethod(t, request, http.MethodPost)
-			type jsonObject = map[string]interface{}
-			expectedJSON := map[string]interface{}{
-				"cpu_performance_type": "performance",
-				"hostname":             "TestMachine",
-				"memory_mb":            json.Number("5"),
-				"network": []jsonObject{
-					{
-						"vlan":     "VLAN-ID",
-						"nic_type": "vmxnet3",
-						"ips":      []interface{}{"8.8.8.8"},
-					},
-				},
-			}
-			var jsonBody jsonObject
-			decoder := json.NewDecoder(request.Body)
-			decoder.UseNumber()
-			testhelper.AssertNoErr(t, decoder.Decode(&jsonBody))
-			testhelper.AssertEquals(t, expectedJSON["cpu_performance_type"], jsonBody["cpu_performance_type"])
-			testhelper.AssertEquals(t, expectedJSON["hostname"], jsonBody["hostname"])
-			testhelper.AssertEquals(t, expectedJSON["memory_mb"], jsonBody["memory_mb"])
-
-			testhelper.AssertEquals(t, jsonBody["dns1"], "1.1.1.1")
-			testhelper.AssertEquals(t, jsonBody["dns2"], nil)
-			testhelper.AssertEquals(t, jsonBody["dns3"], "192.168.0.1")
-			testhelper.AssertEquals(t, jsonBody["dns4"], "192.168.0.2")
-			testhelper.AssertEquals(t, expectedJSON["count"], jsonBody["count"])
-
-			expectedNetwork := expectedJSON["network"].([]jsonObject)[0]
-			bodyNetwork := jsonBody["network"].([]interface{})[0].(jsonObject)
-			testhelper.AssertEquals(t, expectedNetwork["vlan"], bodyNetwork["vlan"])
-			testhelper.AssertEquals(t, expectedNetwork["nic_type"], bodyNetwork["nic_type"])
-			testhelper.AssertEquals(t, expectedNetwork["ips"].([]interface{})[0], bodyNetwork["ips"].([]interface{})[0])
-
-			err := json.NewEncoder(writer).Encode(vm.ProvisioningResponse{
-				Progress:   100,
-				Errors:     nil,
-				Identifier: "TEST-IDENTIFIER",
-				Queued:     false,
+		for _, testCase := range testCases {
+			templateID := testCase.ReconcileContext.Config.TemplateID
+			locationID := testCase.ReconcileContext.Config.LocationID
+
+			testhelper.Mux.HandleFunc(fmt.Sprintf("/api/vsphere/v1/provisioning/vm.json/%s/templates/%s", locationID, templateID), func(writer http.ResponseWriter, request *http.Request) {
+				testhelper.TestMethod(t, request, http.MethodPost)
+				var jsonBody jsonObject
+				decoder := json.NewDecoder(request.Body)
+				decoder.UseNumber()
+				testhelper.AssertNoErr(t, decoder.Decode(&jsonBody))
+
+				testCase.AssertJSONBody(jsonBody)
+
+				err := json.NewEncoder(writer).Encode(vm.ProvisioningResponse{
+					Progress:   100,
+					Errors:     nil,
+					Identifier: templateID,
+					Queued:     false,
+				})
+				testhelper.AssertNoErr(t, err)
 			})
-			testhelper.AssertNoErr(t, err)
-		})
 
-		testhelper.Mux.HandleFunc("/api/vsphere/v1/provisioning/progress.json/TEST-IDENTIFIER", func(writer http.ResponseWriter, request *http.Request) {
-			testhelper.TestMethod(t, request, http.MethodGet)
+			testhelper.Mux.HandleFunc(fmt.Sprintf("/api/vsphere/v1/provisioning/progress.json/%s", templateID), func(writer http.ResponseWriter, request *http.Request) {
+				testhelper.TestMethod(t, request, http.MethodGet)
 
-			err := json.NewEncoder(writer).Encode(progress.Progress{
-				TaskIdentifier: "TEST-IDENTIFIER",
-				Queued:         false,
-				Progress:       100,
-				VMIdentifier:   "VM-IDENTIFIER",
-				Errors:         nil,
+				err := json.NewEncoder(writer).Encode(progress.Progress{
+					TaskIdentifier: templateID,
+					Queued:         false,
+					Progress:       100,
+					VMIdentifier:   "VM-IDENTIFIER",
+					Errors:         nil,
+				})
+				testhelper.AssertNoErr(t, err)
 			})
-			testhelper.AssertNoErr(t, err)
-		})
 
-		providerStatus := anxtypes.ProviderStatus{}
-		ctx := createReconcileContext(context.Background(), reconcileContext{
-			Machine: &v1alpha1.Machine{
-				ObjectMeta: metav1.ObjectMeta{Name: "TestMachine"},
-			},
-			Status:   &providerStatus,
-			UserData: "",
-			Config: resolvedConfig{
-				VlanID:     "VLAN-ID",
-				LocationID: "LOCATION-ID",
-				TemplateID: "TEMPLATE-ID",
-				Disks: []resolvedDisk{
-					{
-						RawDisk: anxtypes.RawDisk{
-							Size: 5,
-						},
-					},
-				},
-				RawConfig: anxtypes.RawConfig{
-					CPUs:   5,
-					Memory: 5,
-				},
-			},
-			ProviderData: &cloudprovidertypes.ProviderData{
-				Update: func(m *clusterv1alpha1.Machine, mods ...cloudprovidertypes.MachineModifier) error {
-					return nil
-				},
-			},
-			ProviderConfig: &providerconfigtypes.Config{
-				Network: &providerconfigtypes.NetworkConfig{
-					DNS: providerconfigtypes.DNSConfig{
-						Servers: []string{
-							"1.1.1.1",
-							"",
-							"192.168.0.1",
-							"192.168.0.2",
-							"192.168.0.3",
-						},
-					},
-				},
-			},
-		})
+			ctx := createReconcileContext(context.Background(), testCase.ReconcileContext)
 
-		err := provisionVM(ctx, client)
-		testhelper.AssertNoErr(t, err)
+			err := provisionVM(ctx, client)
+			testhelper.AssertNoErr(t, err)
+		}
 	})
 
 	t.Run("Test resolve template", func(t *testing.T) {
@@ -296,30 +290,6 @@ func TestAnexiaProvider(t *testing.T) {
 	})
 }
 
-// this generates a full config and allows hooking into it to e.g. remove a value.
-func hookableConfig(hook func(*anxtypes.RawConfig)) anxtypes.RawConfig {
-	config := anxtypes.RawConfig{
-		CPUs: 1,
-
-		Memory: 2,
-
-		Disks: []anxtypes.RawDisk{
-			{Size: 5, PerformanceType: newConfigVarString("ENT6")},
-		},
-
-		Token:      newConfigVarString("test-token"),
-		VlanID:     newConfigVarString("test-vlan"),
-		LocationID: newConfigVarString("test-location"),
-		TemplateID: newConfigVarString("test-template-id"),
-	}
-
-	if hook != nil {
-		hook(&config)
-	}
-
-	return config
-}
-
 func TestValidate(t *testing.T) {
 	t.Parallel()