From 065888d6c7aee79d638971ce3976f165b28a556d Mon Sep 17 00:00:00 2001
From: Kostiantyn Kalynovskyi <kkalynovskyi@mirantis.com>
Date: Tue, 22 Sep 2020 14:40:46 -0500
Subject: [PATCH] Add functions for default api objects

This allows to avoid nil pointers when working with unmarshaled
objects, that may be missing some fields.

Next step would be to revisit why pointers are used in such obj
as Clusterctl and ImageConfiguration

Change-Id: I99a711529a53943ef1b0cb1ebb4d240f2482fac8
---
 pkg/api/v1alpha1/cluster_map_types.go        | 7 +++++++
 pkg/api/v1alpha1/clusterctl_types.go         | 9 +++++++++
 pkg/api/v1alpha1/imageconfiguration_types.go | 8 ++++++++
 pkg/api/v1alpha1/phase_types.go              | 9 +++++++++
 pkg/bootstrap/isogen/executor.go             | 2 +-
 pkg/clusterctl/client/executor.go            | 2 +-
 pkg/phase/helper.go                          | 6 ++++--
 7 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/pkg/api/v1alpha1/cluster_map_types.go b/pkg/api/v1alpha1/cluster_map_types.go
index 939f6c7c8..e003ed78b 100644
--- a/pkg/api/v1alpha1/cluster_map_types.go
+++ b/pkg/api/v1alpha1/cluster_map_types.go
@@ -36,3 +36,10 @@ type Cluster struct {
 	// expecting it to be in document bundle. Parent kubeconfig will be used to get kubeconfig
 	DynamicKubeConfig bool `json:"dynamicKubeConf,omitempty"`
 }
+
+// DefaultClusterMap can be used to safely unmarshal ClusterMap object without nil pointers
+func DefaultClusterMap() *ClusterMap {
+	return &ClusterMap{
+		Map: make(map[string]*Cluster),
+	}
+}
diff --git a/pkg/api/v1alpha1/clusterctl_types.go b/pkg/api/v1alpha1/clusterctl_types.go
index d3ff667ac..1f90df457 100644
--- a/pkg/api/v1alpha1/clusterctl_types.go
+++ b/pkg/api/v1alpha1/clusterctl_types.go
@@ -105,3 +105,12 @@ type MoveOptions struct {
 	// The namespace where the workload cluster is hosted. If unspecified, the target context's namespace is used.
 	Namespace string `json:"namespace,omitempty"`
 }
+
+// DefaultClusterctl can be used to safely unmarshal Clusterctl object without nil pointers
+func DefaultClusterctl() *Clusterctl {
+	return &Clusterctl{
+		InitOptions: &InitOptions{},
+		MoveOptions: &MoveOptions{},
+		Providers:   make([]*Provider, 0),
+	}
+}
diff --git a/pkg/api/v1alpha1/imageconfiguration_types.go b/pkg/api/v1alpha1/imageconfiguration_types.go
index 52d0d75f3..0ebb882a4 100644
--- a/pkg/api/v1alpha1/imageconfiguration_types.go
+++ b/pkg/api/v1alpha1/imageconfiguration_types.go
@@ -49,3 +49,11 @@ type ImageConfiguration struct {
 	Container *Container `json:"container,omitempty"`
 	Builder   *Builder   `json:"builder,omitempty"`
 }
+
+// DefaultImageConfiguration can be used to safely unmarshal ImageConfiguration object without nil pointers
+func DefaultImageConfiguration() *ImageConfiguration {
+	return &ImageConfiguration{
+		Container: &Container{},
+		Builder:   &Builder{},
+	}
+}
diff --git a/pkg/api/v1alpha1/phase_types.go b/pkg/api/v1alpha1/phase_types.go
index e33abb216..7ed4394cc 100644
--- a/pkg/api/v1alpha1/phase_types.go
+++ b/pkg/api/v1alpha1/phase_types.go
@@ -34,3 +34,12 @@ type PhaseConfig struct {
 	ExecutorRef        *corev1.ObjectReference `json:"executorRef"`
 	DocumentEntryPoint string                  `json:"documentEntryPoint"`
 }
+
+// DefaultPhase can be used to safely unmarshal phase object without nil pointers
+func DefaultPhase() *Phase {
+	return &Phase{
+		Config: PhaseConfig{
+			ExecutorRef: &corev1.ObjectReference{},
+		},
+	}
+}
diff --git a/pkg/bootstrap/isogen/executor.go b/pkg/bootstrap/isogen/executor.go
index b21313ff4..7c0e147af 100644
--- a/pkg/bootstrap/isogen/executor.go
+++ b/pkg/bootstrap/isogen/executor.go
@@ -42,7 +42,7 @@ type Executor struct {
 
 // RegisterExecutor adds executor to phase executor registry
 func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory) error {
-	obj := &v1alpha1.ImageConfiguration{}
+	obj := v1alpha1.DefaultImageConfiguration()
 	gvks, _, err := v1alpha1.Scheme.ObjectKinds(obj)
 	if err != nil {
 		return err
diff --git a/pkg/clusterctl/client/executor.go b/pkg/clusterctl/client/executor.go
index aa77f7e1d..e968e702b 100644
--- a/pkg/clusterctl/client/executor.go
+++ b/pkg/clusterctl/client/executor.go
@@ -55,7 +55,7 @@ func RegisterExecutor(registry map[schema.GroupVersionKind]ifc.ExecutorFactory)
 
 // NewExecutor creates instance of 'clusterctl init' phase executor
 func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
-	options := &airshipv1.Clusterctl{}
+	options := airshipv1.DefaultClusterctl()
 	if err := cfg.ExecutorDocument.ToAPIObject(options, airshipv1.Scheme); err != nil {
 		return nil, err
 	}
diff --git a/pkg/phase/helper.go b/pkg/phase/helper.go
index 00c095e05..d90ca61f5 100644
--- a/pkg/phase/helper.go
+++ b/pkg/phase/helper.go
@@ -76,6 +76,8 @@ func (helper *Helper) Phase(phaseID ifc.ID) (*v1alpha1.Phase, error) {
 	if err != nil {
 		return nil, err
 	}
+	// Overwrite phase used for selector, with a phase with default values
+	phase = v1alpha1.DefaultPhase()
 	if err = doc.ToAPIObject(phase, v1alpha1.Scheme); err != nil {
 		return nil, err
 	}
@@ -126,7 +128,7 @@ func (helper *Helper) ListPhases() ([]*v1alpha1.Phase, error) {
 
 	phases := []*v1alpha1.Phase{}
 	for _, doc := range docs {
-		p := &v1alpha1.Phase{}
+		p := v1alpha1.DefaultPhase()
 		if err = doc.ToAPIObject(p, v1alpha1.Scheme); err != nil {
 			return nil, err
 		}
@@ -142,7 +144,7 @@ func (helper *Helper) ClusterMapAPIobj() (*v1alpha1.ClusterMap, error) {
 		return nil, err
 	}
 
-	cMap := &v1alpha1.ClusterMap{}
+	cMap := v1alpha1.DefaultClusterMap()
 	selector, err := document.NewSelector().ByObject(cMap, v1alpha1.Scheme)
 	if err != nil {
 		return nil, err