Skip to content

Commit

Permalink
Merge pull request #2013 from vincepri/conversion-improvements
Browse files Browse the repository at this point in the history
🏃Improve conversion webhook: add tests, fix preserve logic, remove inline nolint
  • Loading branch information
k8s-ci-robot authored Jan 7, 2020
2 parents f26760d + d2eb510 commit e4c46dd
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 35 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ run:
skip-files:
- "zz_generated.*\\.go$"
- crd_manifests.go
- ".*conversion.*\\.go$"
skip-dirs:
- third_party
41 changes: 23 additions & 18 deletions api/v1alpha2/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*v1alpha3.Cluster)

if err := Convert_v1alpha2_Cluster_To_v1alpha3_Cluster(src, dst, nil); err != nil {
return err
}
Expand All @@ -43,10 +42,8 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
return nil
}

// nolint
func (dst *Cluster) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.Cluster)

if err := Convert_v1alpha3_Cluster_To_v1alpha2_Cluster(src, dst, nil); err != nil {
return err
}
Expand All @@ -70,7 +67,6 @@ func (src *ClusterList) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_ClusterList_To_v1alpha3_ClusterList(src, dst, nil)
}

// nolint
func (dst *ClusterList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.ClusterList)

Expand All @@ -79,30 +75,44 @@ func (dst *ClusterList) ConvertFrom(srcRaw conversion.Hub) error {

func (src *Machine) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*v1alpha3.Machine)
if err := Convert_v1alpha2_Machine_To_v1alpha3_Machine(src, dst, nil); err != nil {
return err
}

// Manually convert ClusterName from label, if any.
if name, ok := src.Labels[MachineClusterLabelName]; ok {
dst.Spec.ClusterName = name
}

// Manually restore data.
restored := &v1alpha3.Machine{}
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil {
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
return err
} else if ok {
if restored.Spec.Bootstrap.DataSecretName != nil {
dst.Spec.Bootstrap.DataSecretName = restored.Spec.Bootstrap.DataSecretName
}
}

return Convert_v1alpha2_Machine_To_v1alpha3_Machine(src, dst, nil)
if restored.Spec.Bootstrap.DataSecretName != nil {
dst.Spec.Bootstrap.DataSecretName = restored.Spec.Bootstrap.DataSecretName
}

if restored.Spec.ClusterName != "" {
dst.Spec.ClusterName = restored.Spec.ClusterName
}

return nil
}

// nolint
func (dst *Machine) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.Machine)
if err := Convert_v1alpha3_Machine_To_v1alpha2_Machine(src, dst, nil); err != nil {
return err
}

// Preserve Hub data on down-conversion.
if err := utilconversion.MarshalData(src, dst); err != nil {
return err
}

return Convert_v1alpha3_Machine_To_v1alpha2_Machine(src, dst, nil)
return nil
}

func (src *MachineList) ConvertTo(dstRaw conversion.Hub) error {
Expand All @@ -111,7 +121,6 @@ func (src *MachineList) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_MachineList_To_v1alpha3_MachineList(src, dst, nil)
}

// nolint
func (dst *MachineList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.MachineList)

Expand All @@ -124,7 +133,6 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_MachineSet_To_v1alpha3_MachineSet(src, dst, nil)
}

// nolint
func (dst *MachineSet) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.MachineSet)

Expand All @@ -137,7 +145,6 @@ func (src *MachineSetList) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_MachineSetList_To_v1alpha3_MachineSetList(src, dst, nil)
}

// nolint
func (dst *MachineSetList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.MachineSetList)

Expand All @@ -150,7 +157,6 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_MachineDeployment_To_v1alpha3_MachineDeployment(src, dst, nil)
}

// nolint
func (dst *MachineDeployment) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.MachineDeployment)

Expand All @@ -163,7 +169,6 @@ func (src *MachineDeploymentList) ConvertTo(dstRaw conversion.Hub) error {
return Convert_v1alpha2_MachineDeploymentList_To_v1alpha3_MachineDeploymentList(src, dst, nil)
}

// nolint
func (dst *MachineDeploymentList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha3.MachineDeploymentList)

Expand Down Expand Up @@ -284,7 +289,7 @@ func Convert_v1alpha3_MachineSetSpec_To_v1alpha2_MachineSetSpec(in *v1alpha3.Mac
}

func Convert_v1alpha3_MachineSpec_To_v1alpha2_MachineSpec(in *v1alpha3.MachineSpec, out *MachineSpec, s apiconversion.Scope) error {
return errors.New("cannot recover removed MachineSpec Cluster Name")
return autoConvert_v1alpha3_MachineSpec_To_v1alpha2_MachineSpec(in, out, s)
}

func Convert_v1alpha3_Bootstrap_To_v1alpha2_Bootstrap(in *v1alpha3.Bootstrap, out *Bootstrap, s apiconversion.Scope) error {
Expand Down
89 changes: 89 additions & 0 deletions api/v1alpha2/conversion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
Copyright 2019 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha2

import (
"testing"

. "github.com/onsi/gomega"
"k8s.io/utils/pointer"
"sigs.k8s.io/cluster-api/api/v1alpha3"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

func TestConvertCluster(t *testing.T) {
g := NewWithT(t)

t.Run("to hub", func(t *testing.T) {
t.Run("should convert the first value in Status.APIEndpoints to Spec.ControlPlaneEndpoint", func(t *testing.T) {
src := &Cluster{
Status: ClusterStatus{
APIEndpoints: []APIEndpoint{
{
Host: "example.com",
Port: 6443,
},
},
},
}
dst := &v1alpha3.Cluster{}

g.Expect(src.ConvertTo(dst)).To(Succeed())
g.Expect(dst.Spec.ControlPlaneEndpoint.Host).To(Equal("example.com"))
g.Expect(dst.Spec.ControlPlaneEndpoint.Port).To(BeEquivalentTo(6443))
})
})

t.Run("from hub", func(t *testing.T) {
t.Run("should convert Spec.ControlPlaneEndpoint to Status.APIEndpoints[0]", func(t *testing.T) {
src := &v1alpha3.Cluster{
Spec: v1alpha3.ClusterSpec{
ControlPlaneEndpoint: v1alpha3.APIEndpoint{
Host: "example.com",
Port: 6443,
},
},
}
dst := &Cluster{}

g.Expect(dst.ConvertFrom(src)).To(Succeed())
g.Expect(dst.Status.APIEndpoints[0].Host).To(Equal("example.com"))
g.Expect(dst.Status.APIEndpoints[0].Port).To(BeEquivalentTo(6443))
})
})
}

func TestConvertMachine(t *testing.T) {
g := NewWithT(t)

t.Run("from hub", func(t *testing.T) {
t.Run("preserves Spec.Bootstrap.DataSecretName", func(t *testing.T) {
src := &v1alpha3.Machine{
Spec: v1alpha3.MachineSpec{
Bootstrap: v1alpha3.Bootstrap{
DataSecretName: pointer.StringPtr("secret-data"),
},
},
}
dst := &Machine{}

g.Expect(dst.ConvertFrom(src)).To(Succeed())
g.Expect(dst.GetAnnotations()[utilconversion.DataAnnotation]).To(ContainSubstring("secret-data"))
})
})

}
40 changes: 23 additions & 17 deletions bootstrap/kubeadm/api/v1alpha2/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,72 +24,78 @@ import (
)

// ConvertTo converts this KubeadmConfig to the Hub version (v1alpha3).
func (src *KubeadmConfig) ConvertTo(dstRaw conversion.Hub) error { // nolint
func (src *KubeadmConfig) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfig)
if err := Convert_v1alpha2_KubeadmConfig_To_v1alpha3_KubeadmConfig(src, dst, nil); err != nil {
return err
}

// Manually restore data.
restored := &kubeadmbootstrapv1alpha3.KubeadmConfig{}
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil {
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
return err
} else if ok {
if restored.Status.DataSecretName != nil {
dst.Status.DataSecretName = restored.Status.DataSecretName
}
}

return Convert_v1alpha2_KubeadmConfig_To_v1alpha3_KubeadmConfig(src, dst, nil)
if restored.Status.DataSecretName != nil {
dst.Status.DataSecretName = restored.Status.DataSecretName
}

return nil
}

// ConvertFrom converts from the KubeadmConfig Hub version (v1alpha3) to this version.
func (dst *KubeadmConfig) ConvertFrom(srcRaw conversion.Hub) error { // nolint
func (dst *KubeadmConfig) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfig)
if err := Convert_v1alpha3_KubeadmConfig_To_v1alpha2_KubeadmConfig(src, dst, nil); err != nil {
return nil
}

// Preserve Hub data on down-conversion.
if err := utilconversion.MarshalData(src, dst); err != nil {
return err
}

return Convert_v1alpha3_KubeadmConfig_To_v1alpha2_KubeadmConfig(src, dst, nil)
return nil
}

// ConvertTo converts this KubeadmConfigList to the Hub version (v1alpha3).
func (src *KubeadmConfigList) ConvertTo(dstRaw conversion.Hub) error { // nolint
func (src *KubeadmConfigList) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigList)
return Convert_v1alpha2_KubeadmConfigList_To_v1alpha3_KubeadmConfigList(src, dst, nil)
}

// ConvertFrom converts from the KubeadmConfigList Hub version (v1alpha3) to this version.
func (dst *KubeadmConfigList) ConvertFrom(srcRaw conversion.Hub) error { // nolint
func (dst *KubeadmConfigList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigList)
return Convert_v1alpha3_KubeadmConfigList_To_v1alpha2_KubeadmConfigList(src, dst, nil)
}

// ConvertTo converts this KubeadmConfigTemplate to the Hub version (v1alpha3).
func (src *KubeadmConfigTemplate) ConvertTo(dstRaw conversion.Hub) error { // nolint
func (src *KubeadmConfigTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigTemplate)
return Convert_v1alpha2_KubeadmConfigTemplate_To_v1alpha3_KubeadmConfigTemplate(src, dst, nil)
}

// ConvertFrom converts from the KubeadmConfigTemplate Hub version (v1alpha3) to this version.
func (dst *KubeadmConfigTemplate) ConvertFrom(srcRaw conversion.Hub) error { // nolint
func (dst *KubeadmConfigTemplate) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigTemplate)
return Convert_v1alpha3_KubeadmConfigTemplate_To_v1alpha2_KubeadmConfigTemplate(src, dst, nil)
}

// ConvertTo converts this KubeadmConfigTemplateList to the Hub version (v1alpha3).
func (src *KubeadmConfigTemplateList) ConvertTo(dstRaw conversion.Hub) error { // nolint
func (src *KubeadmConfigTemplateList) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigTemplateList)
return Convert_v1alpha2_KubeadmConfigTemplateList_To_v1alpha3_KubeadmConfigTemplateList(src, dst, nil)
}

// ConvertFrom converts from the KubeadmConfigTemplateList Hub version (v1alpha3) to this version.
func (dst *KubeadmConfigTemplateList) ConvertFrom(srcRaw conversion.Hub) error { // nolint
func (dst *KubeadmConfigTemplateList) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*kubeadmbootstrapv1alpha3.KubeadmConfigTemplateList)
return Convert_v1alpha3_KubeadmConfigTemplateList_To_v1alpha2_KubeadmConfigTemplateList(src, dst, nil)
}

// Convert_v1alpha2_KubeadmConfigStatus_To_v1alpha3_KubeadmConfigStatus converts this KubeadmConfigStatus to the Hub version (v1alpha3).
func Convert_v1alpha2_KubeadmConfigStatus_To_v1alpha3_KubeadmConfigStatus(in *KubeadmConfigStatus, out *kubeadmbootstrapv1alpha3.KubeadmConfigStatus, s apiconversion.Scope) error { // nolint
func Convert_v1alpha2_KubeadmConfigStatus_To_v1alpha3_KubeadmConfigStatus(in *KubeadmConfigStatus, out *kubeadmbootstrapv1alpha3.KubeadmConfigStatus, s apiconversion.Scope) error {
if err := autoConvert_v1alpha2_KubeadmConfigStatus_To_v1alpha3_KubeadmConfigStatus(in, out, s); err != nil {
return err
}
Expand All @@ -102,7 +108,7 @@ func Convert_v1alpha2_KubeadmConfigStatus_To_v1alpha3_KubeadmConfigStatus(in *Ku
}

// Convert_v1alpha3_KubeadmConfigStatus_To_v1alpha2_KubeadmConfigStatus converts from the Hub version (v1alpha3) of the KubeadmConfigStatus to this version.
func Convert_v1alpha3_KubeadmConfigStatus_To_v1alpha2_KubeadmConfigStatus(in *kubeadmbootstrapv1alpha3.KubeadmConfigStatus, out *KubeadmConfigStatus, s apiconversion.Scope) error { // nolint
func Convert_v1alpha3_KubeadmConfigStatus_To_v1alpha2_KubeadmConfigStatus(in *kubeadmbootstrapv1alpha3.KubeadmConfigStatus, out *KubeadmConfigStatus, s apiconversion.Scope) error {
if err := autoConvert_v1alpha3_KubeadmConfigStatus_To_v1alpha2_KubeadmConfigStatus(in, out, s); err != nil {
return err
}
Expand Down

0 comments on commit e4c46dd

Please sign in to comment.