From 2a48df77fa7772575097ee05d19c4c46c9210682 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sat, 5 Aug 2017 17:10:46 -0400 Subject: [PATCH] Rework legacy validation to use field error helpers --- pkg/apis/kops/validation/legacy.go | 137 ++++++++++--------- pkg/client/simple/vfsclientset/cluster.go | 9 +- upup/pkg/fi/cloudup/populate_cluster_spec.go | 8 +- upup/pkg/fi/cloudup/validation_test.go | 6 +- 4 files changed, 77 insertions(+), 83 deletions(-) diff --git a/pkg/apis/kops/validation/legacy.go b/pkg/apis/kops/validation/legacy.go index c8d7157b98d83..174ba27d93c8e 100644 --- a/pkg/apis/kops/validation/legacy.go +++ b/pkg/apis/kops/validation/legacy.go @@ -30,23 +30,21 @@ import ( // legacy contains validation functions that don't match the apimachinery style -func ValidateCluster(c *kops.Cluster, strict bool) error { - specField := field.NewPath("Spec") - +func ValidateCluster(c *kops.Cluster, strict bool) *field.Error { var err error - specPath := field.NewPath("Cluster").Child("Spec") + fieldSpec := field.NewPath("Spec") // kubernetesRelease is the version with only major & minor fields var kubernetesRelease semver.Version // KubernetesVersion if c.Spec.KubernetesVersion == "" { - return field.Required(specField.Child("KubernetesVersion"), "") + return field.Required(fieldSpec.Child("KubernetesVersion"), "") } else { sv, err := util.ParseKubernetesVersion(c.Spec.KubernetesVersion) if err != nil { - return field.Invalid(specField.Child("KubernetesVersion"), c.Spec.KubernetesVersion, "unable to determine kubernetes version") + return field.Invalid(fieldSpec.Child("KubernetesVersion"), c.Spec.KubernetesVersion, "unable to determine kubernetes version") } kubernetesRelease = semver.Version{Major: sv.Major, Minor: sv.Minor} @@ -60,45 +58,45 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { // Must be a dns name errs := validation.IsDNS1123Subdomain(c.ObjectMeta.Name) if len(errs) != 0 { - return fmt.Errorf("Cluster Name must be a valid DNS name (e.g. --name=mycluster.myzone.com) errors: %s", strings.Join(errs, ", ")) + return field.Invalid(field.NewPath("Name"), c.ObjectMeta.Name, fmt.Sprintf("Cluster Name must be a valid DNS name (e.g. --name=mycluster.myzone.com) errors: %s", strings.Join(errs, ", "))) } if !strings.Contains(c.ObjectMeta.Name, ".") { // Tolerate if this is a cluster we are importing for upgrade if c.ObjectMeta.Annotations[kops.AnnotationNameManagement] != kops.AnnotationValueManagementImported { - return fmt.Errorf("Cluster Name must be a fully-qualified DNS name (e.g. --name=mycluster.myzone.com)") + return field.Invalid(field.NewPath("Name"), c.ObjectMeta.Name, "Cluster Name must be a fully-qualified DNS name (e.g. --name=mycluster.myzone.com)") } } } if len(c.Spec.Subnets) == 0 { // TODO: Auto choose zones from region? - return fmt.Errorf("must configure at least one Subnet (use --zones)") + return field.Required(fieldSpec.Child("Subnets"), "must configure at least one Subnet (use --zones)") } if strict && c.Spec.Kubelet == nil { - return fmt.Errorf("Kubelet not configured") + return field.Required(fieldSpec.Child("Kubelet"), "Kubelet not configured") } if strict && c.Spec.MasterKubelet == nil { - return fmt.Errorf("MasterKubelet not configured") + return field.Required(fieldSpec.Child("MasterKubelet"), "MasterKubelet not configured") } if strict && c.Spec.KubeControllerManager == nil { - return fmt.Errorf("KubeControllerManager not configured") + return field.Required(fieldSpec.Child("KubeControllerManager"), "KubeControllerManager not configured") } if strict && c.Spec.KubeDNS == nil { - return fmt.Errorf("KubeDNS not configured") + return field.Required(fieldSpec.Child("KubeDNS"), "KubeDNS not configured") } if strict && c.Spec.Kubelet == nil { - return fmt.Errorf("Kubelet not configured") + return field.Required(fieldSpec.Child("Kubelet"), "Kubelet not configured") } if strict && c.Spec.KubeAPIServer == nil { - return fmt.Errorf("KubeAPIServer not configured") + return field.Required(fieldSpec.Child("KubeAPIServer"), "KubeAPIServer not configured") } if strict && c.Spec.KubeProxy == nil { - return fmt.Errorf("KubeProxy not configured") + return field.Required(fieldSpec.Child("KubeProxy"), "KubeProxy not configured") } if strict && c.Spec.Docker == nil { - return fmt.Errorf("Docker not configured") + return field.Required(fieldSpec.Child("Docker"), "Docker not configured") } // Check NetworkCIDR @@ -106,11 +104,11 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { { networkCIDRString := c.Spec.NetworkCIDR if networkCIDRString == "" { - return field.Required(specField.Child("NetworkCIDR"), "Cluster did not have NetworkCIDR set") + return field.Required(fieldSpec.Child("NetworkCIDR"), "Cluster did not have NetworkCIDR set") } _, networkCIDR, err = net.ParseCIDR(networkCIDRString) if err != nil { - return field.Invalid(specField.Child("NetworkCIDR"), networkCIDRString, fmt.Sprintf("Cluster had an invalid NetworkCIDR")) + return field.Invalid(fieldSpec.Child("NetworkCIDR"), networkCIDRString, fmt.Sprintf("Cluster had an invalid NetworkCIDR")) } } @@ -119,25 +117,25 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { { nonMasqueradeCIDRString := c.Spec.NonMasqueradeCIDR if nonMasqueradeCIDRString == "" { - return fmt.Errorf("Cluster did not have NonMasqueradeCIDR set") + return field.Required(fieldSpec.Child("NonMasqueradeCIDR"), "Cluster did not have NonMasqueradeCIDR set") } _, nonMasqueradeCIDR, err = net.ParseCIDR(nonMasqueradeCIDRString) if err != nil { - return fmt.Errorf("Cluster had an invalid NonMasqueradeCIDR: %q", nonMasqueradeCIDRString) + return field.Invalid(fieldSpec.Child("NonMasqueradeCIDR"), nonMasqueradeCIDRString, "Cluster had an invalid NonMasqueradeCIDR") } if subnetsOverlap(nonMasqueradeCIDR, networkCIDR) { - return fmt.Errorf("NonMasqueradeCIDR %q cannot overlap with NetworkCIDR %q", nonMasqueradeCIDRString, c.Spec.NetworkCIDR) + return field.Invalid(fieldSpec.Child("NonMasqueradeCIDR"), nonMasqueradeCIDRString, fmt.Sprintf("NonMasqueradeCIDR %q cannot overlap with NetworkCIDR %q", nonMasqueradeCIDRString, c.Spec.NetworkCIDR)) } if c.Spec.Kubelet != nil && c.Spec.Kubelet.NonMasqueradeCIDR != nonMasqueradeCIDRString { if strict || c.Spec.Kubelet.NonMasqueradeCIDR != "" { - return fmt.Errorf("Kubelet NonMasqueradeCIDR did not match cluster NonMasqueradeCIDR") + return field.Invalid(fieldSpec.Child("NonMasqueradeCIDR"), nonMasqueradeCIDRString, "Kubelet NonMasqueradeCIDR did not match cluster NonMasqueradeCIDR") } } if c.Spec.MasterKubelet != nil && c.Spec.MasterKubelet.NonMasqueradeCIDR != nonMasqueradeCIDRString { if strict || c.Spec.MasterKubelet.NonMasqueradeCIDR != "" { - return fmt.Errorf("MasterKubelet NonMasqueradeCIDR did not match cluster NonMasqueradeCIDR") + return field.Invalid(fieldSpec.Child("NonMasqueradeCIDR"), nonMasqueradeCIDRString, "MasterKubelet NonMasqueradeCIDR did not match cluster NonMasqueradeCIDR") } } } @@ -148,21 +146,21 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { serviceClusterIPRangeString := c.Spec.ServiceClusterIPRange if serviceClusterIPRangeString == "" { if strict { - return fmt.Errorf("Cluster did not have ServiceClusterIPRange set") + return field.Required(fieldSpec.Child("ServiceClusterIPRange"), "Cluster did not have ServiceClusterIPRange set") } } else { _, serviceClusterIPRange, err = net.ParseCIDR(serviceClusterIPRangeString) if err != nil { - return fmt.Errorf("Cluster had an invalid ServiceClusterIPRange: %q", serviceClusterIPRangeString) + return field.Invalid(fieldSpec.Child("ServiceClusterIPRange"), serviceClusterIPRangeString, "Cluster had an invalid ServiceClusterIPRange") } if !isSubnet(nonMasqueradeCIDR, serviceClusterIPRange) { - return fmt.Errorf("ServiceClusterIPRange %q must be a subnet of NonMasqueradeCIDR %q", serviceClusterIPRangeString, c.Spec.NonMasqueradeCIDR) + return field.Invalid(fieldSpec.Child("ServiceClusterIPRange"), serviceClusterIPRangeString, fmt.Sprintf("ServiceClusterIPRange %q must be a subnet of NonMasqueradeCIDR %q", serviceClusterIPRangeString, c.Spec.NonMasqueradeCIDR)) } if c.Spec.KubeAPIServer != nil && c.Spec.KubeAPIServer.ServiceClusterIPRange != serviceClusterIPRangeString { if strict || c.Spec.KubeAPIServer.ServiceClusterIPRange != "" { - return fmt.Errorf("KubeAPIServer ServiceClusterIPRange did not match cluster ServiceClusterIPRange") + return field.Invalid(fieldSpec.Child("ServiceClusterIPRange"), serviceClusterIPRangeString, "KubeAPIServer ServiceClusterIPRange did not match cluster ServiceClusterIPRange") } } } @@ -175,11 +173,11 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { if clusterCIDRString != "" { _, clusterCIDR, err = net.ParseCIDR(clusterCIDRString) if err != nil { - return fmt.Errorf("Cluster had an invalid KubeControllerManager.ClusterCIDR: %q", clusterCIDRString) + return field.Invalid(fieldSpec.Child("KubeControllerManager", "ClusterCIDR"), clusterCIDRString, "Cluster had an invalid KubeControllerManager.ClusterCIDR") } if !isSubnet(nonMasqueradeCIDR, clusterCIDR) { - return fmt.Errorf("KubeControllerManager.ClusterCIDR %q must be a subnet of NonMasqueradeCIDR %q", clusterCIDRString, c.Spec.NonMasqueradeCIDR) + return field.Invalid(fieldSpec.Child("KubeControllerManager", "ClusterCIDR"), clusterCIDRString, fmt.Sprintf("KubeControllerManager.ClusterCIDR %q must be a subnet of NonMasqueradeCIDR %q", clusterCIDRString, c.Spec.NonMasqueradeCIDR)) } } } @@ -188,23 +186,23 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { if c.Spec.KubeDNS != nil { serverIPString := c.Spec.KubeDNS.ServerIP if serverIPString == "" { - return fmt.Errorf("Cluster did not have KubeDNS.ServerIP set") + return field.Required(fieldSpec.Child("KubeDNS", "ServerIP"), "Cluster did not have KubeDNS.ServerIP set") } dnsServiceIP := net.ParseIP(serverIPString) if dnsServiceIP == nil { - return fmt.Errorf("Cluster had an invalid KubeDNS.ServerIP: %q", serverIPString) + return field.Invalid(fieldSpec.Child("KubeDNS", "ServerIP"), serverIPString, "Cluster had an invalid KubeDNS.ServerIP") } if !serviceClusterIPRange.Contains(dnsServiceIP) { - return fmt.Errorf("ServiceClusterIPRange %q must contain the DNS Server IP %q", c.Spec.ServiceClusterIPRange, serverIPString) + return field.Invalid(fieldSpec.Child("KubeDNS", "ServerIP"), serverIPString, fmt.Sprintf("ServiceClusterIPRange %q must contain the DNS Server IP %q", c.Spec.ServiceClusterIPRange, serverIPString)) } if c.Spec.Kubelet != nil && c.Spec.Kubelet.ClusterDNS != c.Spec.KubeDNS.ServerIP { - return fmt.Errorf("Kubelet ClusterDNS did not match cluster KubeDNS.ServerIP") + return field.Invalid(fieldSpec.Child("KubeDNS", "ServerIP"), serverIPString, "Kubelet ClusterDNS did not match cluster KubeDNS.ServerIP") } if c.Spec.MasterKubelet != nil && c.Spec.MasterKubelet.ClusterDNS != c.Spec.KubeDNS.ServerIP { - return fmt.Errorf("MasterKubelet ClusterDNS did not match cluster KubeDNS.ServerIP") + return field.Invalid(fieldSpec.Child("KubeDNS", "ServerIP"), serverIPString, "MasterKubelet ClusterDNS did not match cluster KubeDNS.ServerIP") } } @@ -213,45 +211,46 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { cloudProvider := c.Spec.CloudProvider if cloudProvider == "" { - return field.Required(specPath.Child("CloudProvider"), "") + return field.Required(fieldSpec.Child("CloudProvider"), "") } if c.Spec.Kubelet != nil && (strict || c.Spec.Kubelet.CloudProvider != "") { if cloudProvider != c.Spec.Kubelet.CloudProvider { - return field.Invalid(specPath.Child("Kubelet", "CloudProvider"), c.Spec.Kubelet.CloudProvider, "Did not match cluster CloudProvider") + return field.Invalid(fieldSpec.Child("Kubelet", "CloudProvider"), c.Spec.Kubelet.CloudProvider, "Did not match cluster CloudProvider") } } if c.Spec.MasterKubelet != nil && (strict || c.Spec.MasterKubelet.CloudProvider != "") { if cloudProvider != c.Spec.MasterKubelet.CloudProvider { - return field.Invalid(specPath.Child("MasterKubelet", "CloudProvider"), c.Spec.MasterKubelet.CloudProvider, "Did not match cluster CloudProvider") + return field.Invalid(fieldSpec.Child("MasterKubelet", "CloudProvider"), c.Spec.MasterKubelet.CloudProvider, "Did not match cluster CloudProvider") } } if c.Spec.KubeAPIServer != nil && (strict || c.Spec.KubeAPIServer.CloudProvider != "") { if cloudProvider != c.Spec.KubeAPIServer.CloudProvider { - return field.Invalid(specPath.Child("KubeAPIServer", "CloudProvider"), c.Spec.KubeAPIServer.CloudProvider, "Did not match cluster CloudProvider") + return field.Invalid(fieldSpec.Child("KubeAPIServer", "CloudProvider"), c.Spec.KubeAPIServer.CloudProvider, "Did not match cluster CloudProvider") } } if c.Spec.KubeControllerManager != nil && (strict || c.Spec.KubeControllerManager.CloudProvider != "") { if cloudProvider != c.Spec.KubeControllerManager.CloudProvider { - return field.Invalid(specPath.Child("KubeControllerManager", "CloudProvider"), c.Spec.KubeControllerManager.CloudProvider, "Did not match cluster CloudProvider") + return field.Invalid(fieldSpec.Child("KubeControllerManager", "CloudProvider"), c.Spec.KubeControllerManager.CloudProvider, "Did not match cluster CloudProvider") } } } // Check that the subnet CIDRs are all consistent { - for _, s := range c.Spec.Subnets { + for i, s := range c.Spec.Subnets { + fieldSubnet := fieldSpec.Child("Subnets").Index(i) if s.CIDR == "" { if strict { - return fmt.Errorf("Subnet %q did not have a CIDR set", s.Name) + return field.Required(fieldSubnet.Child("CIDR"), "Subnet did not have a CIDR set") } } else { _, subnetCIDR, err := net.ParseCIDR(s.CIDR) if err != nil { - return fmt.Errorf("Subnet %q had an invalid CIDR: %q", s.Name, s.CIDR) + return field.Invalid(fieldSubnet.Child("CIDR"), s.CIDR, "Subnet had an invalid CIDR") } if !isSubnet(networkCIDR, subnetCIDR) { - return fmt.Errorf("Subnet %q had a CIDR %q that was not a subnet of the NetworkCIDR %q", s.Name, s.CIDR, c.Spec.NetworkCIDR) + return field.Invalid(fieldSubnet.Child("CIDR"), s.CIDR, fmt.Sprintf("Subnet %q had a CIDR %q that was not a subnet of the NetworkCIDR %q", s.Name, s.CIDR, c.Spec.NetworkCIDR)) } } } @@ -263,13 +262,13 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { case kops.UpdatePolicyExternal: // Valid default: - return fmt.Errorf("unrecognized value for UpdatePolicy: %v", *c.Spec.UpdatePolicy) + return field.Invalid(fieldSpec.Child("UpdatePolicy"), *c.Spec.UpdatePolicy, "unrecognized value for UpdatePolicy") } } // KubeProxy if c.Spec.KubeProxy != nil { - kubeProxyPath := specPath.Child("KubeProxy") + kubeProxyPath := fieldSpec.Child("KubeProxy") master := c.Spec.KubeProxy.Master // We no longer require the master to be set; nodeup can infer it automatically @@ -284,7 +283,7 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { // Kubelet if c.Spec.Kubelet != nil { - kubeletPath := specPath.Child("Kubelet") + kubeletPath := fieldSpec.Child("Kubelet") if kubernetesRelease.GTE(semver.MustParse("1.6.0")) { // Flag removed in 1.6 @@ -307,7 +306,7 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { // MasterKubelet if c.Spec.MasterKubelet != nil { - masterKubeletPath := specPath.Child("MasterKubelet") + masterKubeletPath := fieldSpec.Child("MasterKubelet") if kubernetesRelease.GTE(semver.MustParse("1.6.0")) { // Flag removed in 1.6 @@ -332,36 +331,37 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { if c.Spec.Topology != nil { if c.Spec.Topology.Masters != "" && c.Spec.Topology.Nodes != "" { if c.Spec.Topology.Masters != kops.TopologyPublic && c.Spec.Topology.Masters != kops.TopologyPrivate { - return fmt.Errorf("Invalid Masters value for Topology") + return field.Invalid(fieldSpec.Child("Topology", "Masters"), c.Spec.Topology.Masters, "Invalid Masters value for Topology") } else if c.Spec.Topology.Nodes != kops.TopologyPublic && c.Spec.Topology.Nodes != kops.TopologyPrivate { - return fmt.Errorf("Invalid Nodes value for Topology") + return field.Invalid(fieldSpec.Child("Topology", "Nodes"), c.Spec.Topology.Nodes, "Invalid Nodes value for Topology") } } else { - return fmt.Errorf("Topology requires non-nil values for Masters and Nodes") + return field.Required(fieldSpec.Child("Masters"), "Topology requires non-nil values for Masters and Nodes") } if c.Spec.Topology.Bastion != nil { bastion := c.Spec.Topology.Bastion if c.Spec.Topology.Masters == kops.TopologyPublic || c.Spec.Topology.Nodes == kops.TopologyPublic { - return fmt.Errorf("Bastion supports only Private Masters and Nodes") + return field.Invalid(fieldSpec.Child("Topology", "Masters"), c.Spec.Topology.Masters, "Bastion supports only Private Masters and Nodes") } if bastion.IdleTimeoutSeconds != nil && *bastion.IdleTimeoutSeconds <= 0 { - return fmt.Errorf("Bastion IdleTimeoutSeconds should be greater than zero") + return field.Invalid(fieldSpec.Child("Topology", "Bastion", "IdleTimeoutSeconds"), *bastion.IdleTimeoutSeconds, "Bastion IdleTimeoutSeconds should be greater than zero") } if bastion.IdleTimeoutSeconds != nil && *bastion.IdleTimeoutSeconds > 3600 { - return fmt.Errorf("Bastion IdleTimeoutSeconds cannot be greater than one hour") + return field.Invalid(fieldSpec.Child("Topology", "Bastion", "IdleTimeoutSeconds"), *bastion.IdleTimeoutSeconds, "Bastion IdleTimeoutSeconds cannot be greater than one hour") } } } // Egress specification support { - for _, s := range c.Spec.Subnets { + for i, s := range c.Spec.Subnets { + fieldSubnet := fieldSpec.Child("Subnets").Index(i) if s.Egress != "" && !strings.HasPrefix(s.Egress, "nat-") { - return fmt.Errorf("egress must be of type NAT Gateway") + return field.Invalid(fieldSubnet.Child("Egress"), s.Egress, "egress must be of type NAT Gateway") } if s.Egress != "" && !(s.Type == "Private") { - return fmt.Errorf("egress can only be specified for Private subnets") + return field.Invalid(fieldSubnet.Child("Egress"), s.Egress, "egress can only be specified for Private subnets") } } } @@ -369,25 +369,27 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { // Etcd { if len(c.Spec.EtcdClusters) == 0 { - return field.Required(specField.Child("EtcdClusters"), "") + return field.Required(fieldSpec.Child("EtcdClusters"), "") } - for _, etcd := range c.Spec.EtcdClusters { + for i, etcd := range c.Spec.EtcdClusters { + fieldEtcdCluster := fieldSpec.Child("EtcdClusters").Index(i) if etcd.Name == "" { - return fmt.Errorf("EtcdCluster did not have name") + return field.Required(fieldEtcdCluster.Child("Name"), "EtcdCluster did not have name") } if len(etcd.Members) == 0 { - return fmt.Errorf("No members defined in etcd cluster %q", etcd.Name) + return field.Required(fieldEtcdCluster.Child("Members"), "No members defined in etcd cluster") } if (len(etcd.Members) % 2) == 0 { // Not technically a requirement, but doesn't really make sense to allow - return fmt.Errorf("There should be an odd number of master-zones, for etcd's quorum. Hint: Use --zones and --master-zones to declare node zones and master zones separately.") + return field.Invalid(fieldEtcdCluster.Child("Members"), len(etcd.Members), "There should be an odd number of master-zones, for etcd's quorum. Hint: Use --zones and --master-zones to declare node zones and master zones separately.") } - for _, m := range etcd.Members { + for i, m := range etcd.Members { + fieldEtcdMember := fieldEtcdCluster.Child("Members").Index(i) if m.Name == "" { - return fmt.Errorf("EtcdMember did not have Name in cluster %q", etcd.Name) + return field.Required(fieldEtcdMember.Child("Name"), "EtcdMember did not have Name") } if fi.StringValue(m.InstanceGroup) == "" { - return fmt.Errorf("EtcdMember did not have InstanceGroup in cluster %q", etcd.Name) + return field.Required(fieldEtcdMember.Child("InstanceGroup"), "EtcdMember did not have InstanceGroup") } } } @@ -395,7 +397,7 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { if kubernetesRelease.GTE(semver.MustParse("1.4.0")) { if c.Spec.Networking != nil && c.Spec.Networking.Classic != nil { - return field.Invalid(specField.Child("Networking"), "classic", "classic networking is not supported with kubernetes versions 1.4 and later") + return field.Invalid(fieldSpec.Child("Networking"), "classic", "classic networking is not supported with kubernetes versions 1.4 and later") } } @@ -408,8 +410,7 @@ func ValidateCluster(c *kops.Cluster, strict bool) error { } func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) error { - err := ValidateCluster(c, strict) - if err != nil { + if err := ValidateCluster(c, strict); err != nil { return err } diff --git a/pkg/client/simple/vfsclientset/cluster.go b/pkg/client/simple/vfsclientset/cluster.go index 671976187976a..417f31a58b24f 100644 --- a/pkg/client/simple/vfsclientset/cluster.go +++ b/pkg/client/simple/vfsclientset/cluster.go @@ -90,8 +90,7 @@ func (c *ClusterVFS) List(options metav1.ListOptions) (*api.ClusterList, error) } func (r *ClusterVFS) Create(c *api.Cluster) (*api.Cluster, error) { - err := validation.ValidateCluster(c, false) - if err != nil { + if err := validation.ValidateCluster(c, false); err != nil { return nil, err } @@ -104,8 +103,7 @@ func (r *ClusterVFS) Create(c *api.Cluster) (*api.Cluster, error) { return nil, fmt.Errorf("clusterName is required") } - err = r.writeConfig(r.basePath.Join(clusterName, registry.PathCluster), c, vfs.WriteOptionCreate) - if err != nil { + if err := r.writeConfig(r.basePath.Join(clusterName, registry.PathCluster), c, vfs.WriteOptionCreate); err != nil { if os.IsExist(err) { return nil, err } @@ -126,8 +124,7 @@ func (r *ClusterVFS) Update(c *api.Cluster) (*api.Cluster, error) { return nil, fmt.Errorf("clusterName is required") } - err = r.writeConfig(r.basePath.Join(clusterName, registry.PathCluster), c, vfs.WriteOptionOnlyIfExists) - if err != nil { + if err := r.writeConfig(r.basePath.Join(clusterName, registry.PathCluster), c, vfs.WriteOptionOnlyIfExists); err != nil { if os.IsNotExist(err) { return nil, err } diff --git a/upup/pkg/fi/cloudup/populate_cluster_spec.go b/upup/pkg/fi/cloudup/populate_cluster_spec.go index 3716680e426b6..db7f00c64e737 100644 --- a/upup/pkg/fi/cloudup/populate_cluster_spec.go +++ b/upup/pkg/fi/cloudup/populate_cluster_spec.go @@ -95,8 +95,7 @@ func PopulateClusterSpec(cluster *api.Cluster, assetBuilder *assets.AssetBuilder // @kris-nova // func (c *populateClusterSpec) run() error { - err := validation.ValidateCluster(c.InputCluster, false) - if err != nil { + if err := validation.ValidateCluster(c.InputCluster, false); err != nil { return err } @@ -105,7 +104,7 @@ func (c *populateClusterSpec) run() error { utils.JsonMergeStruct(cluster, c.InputCluster) - err = c.assignSubnets(cluster) + err := c.assignSubnets(cluster) if err != nil { return err } @@ -325,8 +324,7 @@ func (c *populateClusterSpec) run() error { fullCluster.Spec = *completed tf.cluster = fullCluster - err = validation.ValidateCluster(fullCluster, true) - if err != nil { + if err := validation.ValidateCluster(fullCluster, true); err != nil { return fmt.Errorf("Completed cluster failed validation: %v", err) } diff --git a/upup/pkg/fi/cloudup/validation_test.go b/upup/pkg/fi/cloudup/validation_test.go index b092380f14520..10fca84016b80 100644 --- a/upup/pkg/fi/cloudup/validation_test.go +++ b/upup/pkg/fi/cloudup/validation_test.go @@ -108,13 +108,11 @@ func buildDefaultCluster(t *testing.T) *api.Cluster { func TestValidateFull_Default_Validates(t *testing.T) { c := buildDefaultCluster(t) - err := validation.ValidateCluster(c, false) - if err != nil { + if err := validation.ValidateCluster(c, false); err != nil { glog.Infof("Cluster: %v", c) t.Fatalf("Validate gave unexpected error (strict=false): %v", err) } - err = validation.ValidateCluster(c, true) - if err != nil { + if err := validation.ValidateCluster(c, true); err != nil { t.Fatalf("Validate gave unexpected error (strict=true): %v", err) } }