diff --git a/api/v1beta1/hetznercluster_webhook.go b/api/v1beta1/hetznercluster_webhook.go index c888ba77f..b95496910 100644 --- a/api/v1beta1/hetznercluster_webhook.go +++ b/api/v1beta1/hetznercluster_webhook.go @@ -86,20 +86,22 @@ func (r *HetznerCluster) Default(_ context.Context, obj runtime.Object) error { return apierrors.NewBadRequest(fmt.Sprintf("expected an HetznerCluster but got a %T", obj)) } - if cluster.Spec.HCloudNetwork.Enabled { - if cluster.Spec.HCloudNetwork.ID != nil { - return nil - } + if !cluster.Spec.HCloudNetwork.Enabled { + return nil + } - if cluster.Spec.HCloudNetwork.CIDRBlock == nil { - cluster.Spec.HCloudNetwork.CIDRBlock = ptr.To(DefaultCIDRBlock) - } - if cluster.Spec.HCloudNetwork.SubnetCIDRBlock == nil { - cluster.Spec.HCloudNetwork.SubnetCIDRBlock = ptr.To(DefaultSubnetCIDRBlock) - } - if cluster.Spec.HCloudNetwork.NetworkZone == nil { - cluster.Spec.HCloudNetwork.NetworkZone = ptr.To[HCloudNetworkZone](DefaultNetworkZone) - } + if cluster.Spec.HCloudNetwork.ID != nil { + return nil + } + + if cluster.Spec.HCloudNetwork.CIDRBlock == nil { + cluster.Spec.HCloudNetwork.CIDRBlock = ptr.To(DefaultCIDRBlock) + } + if cluster.Spec.HCloudNetwork.SubnetCIDRBlock == nil { + cluster.Spec.HCloudNetwork.SubnetCIDRBlock = ptr.To(DefaultSubnetCIDRBlock) + } + if cluster.Spec.HCloudNetwork.NetworkZone == nil { + cluster.Spec.HCloudNetwork.NetworkZone = ptr.To[HCloudNetworkZone](DefaultNetworkZone) } return nil @@ -250,12 +252,24 @@ func (r *HetznerCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an HetznerCluster but got a %T", old)) } - if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.Enabled, r.Spec.HCloudNetwork.Enabled) { + if oldC.Spec.HCloudNetwork.Enabled != r.Spec.HCloudNetwork.Enabled { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "hcloudNetwork", "enabled"), r.Spec.HCloudNetwork.Enabled, "field is immutable"), ) } + if !oldC.Spec.HCloudNetwork.Enabled { + // If the network is disabled check that all other network related fields are empty. + if r.Spec.HCloudNetwork.ID != nil { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "hcloudNetwork", "id"), oldC.Spec.HCloudNetwork.ID, "field must be empty"), + ) + } + if errs := areCIDRsAndNetworkZoneEmpty(r.Spec.HCloudNetwork); errs != nil { + allErrs = append(allErrs, errs...) + } + } + if oldC.Spec.HCloudNetwork.Enabled { // Only allow updating the network ID when it was not set previously. This makes it possible to e.g. adopt the // network that was created initially by CAPH. @@ -265,28 +279,22 @@ func (r *HetznerCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, ) } - if r.Spec.HCloudNetwork.ID != nil { - if errs := areCIDRsAndNetworkZoneEmpty(r.Spec.HCloudNetwork); errs != nil { - allErrs = append(allErrs, errs...) - } - } else { - if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.CIDRBlock, r.Spec.HCloudNetwork.CIDRBlock) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "hcloudNetwork", "cidrBlock"), r.Spec.HCloudNetwork.CIDRBlock, "field is immutable"), - ) - } + if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.CIDRBlock, r.Spec.HCloudNetwork.CIDRBlock) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "hcloudNetwork", "cidrBlock"), r.Spec.HCloudNetwork.CIDRBlock, "field is immutable"), + ) + } - if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.SubnetCIDRBlock, r.Spec.HCloudNetwork.SubnetCIDRBlock) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "hcloudNetwork", "subnetCIDRBlock"), r.Spec.HCloudNetwork.SubnetCIDRBlock, "field is immutable"), - ) - } + if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.SubnetCIDRBlock, r.Spec.HCloudNetwork.SubnetCIDRBlock) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "hcloudNetwork", "subnetCIDRBlock"), r.Spec.HCloudNetwork.SubnetCIDRBlock, "field is immutable"), + ) + } - if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.NetworkZone, r.Spec.HCloudNetwork.NetworkZone) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "hcloudNetwork", "networkZone"), r.Spec.HCloudNetwork.NetworkZone, "field is immutable"), - ) - } + if !reflect.DeepEqual(oldC.Spec.HCloudNetwork.NetworkZone, r.Spec.HCloudNetwork.NetworkZone) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "hcloudNetwork", "networkZone"), r.Spec.HCloudNetwork.NetworkZone, "field is immutable"), + ) } } @@ -304,14 +312,14 @@ func (r *HetznerCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, } // Load balancer enabled/disabled is immutable - if !reflect.DeepEqual(oldC.Spec.ControlPlaneLoadBalancer.Enabled, r.Spec.ControlPlaneLoadBalancer.Enabled) { + if oldC.Spec.ControlPlaneLoadBalancer.Enabled != r.Spec.ControlPlaneLoadBalancer.Enabled { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "enabled"), r.Spec.ControlPlaneLoadBalancer.Enabled, "field is immutable"), ) } // Load balancer region and port are immutable - if !reflect.DeepEqual(oldC.Spec.ControlPlaneLoadBalancer.Port, r.Spec.ControlPlaneLoadBalancer.Port) { + if oldC.Spec.ControlPlaneLoadBalancer.Port != r.Spec.ControlPlaneLoadBalancer.Port { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "port"), r.Spec.ControlPlaneLoadBalancer.Port, "field is immutable"), ) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 3097466d6..9f76f39dd 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -232,7 +232,7 @@ type HCloudNetworkSpec struct { CIDRBlock *string `json:"cidrBlock,omitempty"` // SubnetCIDRBlock defines the cidrBlock for the subnet of the HCloud Network. - // Defaults to "10.0.0.0/24". + // The webhook defaults this to "10.0.0.0/24". // Mutually exclusive with ID. // Note: A subnet is required. // +optional @@ -240,7 +240,7 @@ type HCloudNetworkSpec struct { // NetworkZone specifies the HCloud network zone of the private network. // The zones must be one of eu-central, us-east, or us-west. - // Defaults to "eu-central". + // The webhook defaults this to "eu-central". // Mutually exclusive with ID. // +optional NetworkZone *HCloudNetworkZone `json:"networkZone,omitempty"` diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml index f8105619a..0fbc3d11a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml @@ -209,13 +209,13 @@ spec: description: |- NetworkZone specifies the HCloud network zone of the private network. The zones must be one of eu-central, us-east, or us-west. - Defaults to "eu-central". + The webhook defaults this to "eu-central". Mutually exclusive with ID. type: string subnetCidrBlock: description: |- SubnetCIDRBlock defines the cidrBlock for the subnet of the HCloud Network. - Defaults to "10.0.0.0/24". + The webhook defaults this to "10.0.0.0/24". Mutually exclusive with ID. Note: A subnet is required. type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml index 3a69a0f0b..3e63c22db 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml @@ -240,13 +240,13 @@ spec: description: |- NetworkZone specifies the HCloud network zone of the private network. The zones must be one of eu-central, us-east, or us-west. - Defaults to "eu-central". + The webhook defaults this to "eu-central". Mutually exclusive with ID. type: string subnetCidrBlock: description: |- SubnetCIDRBlock defines the cidrBlock for the subnet of the HCloud Network. - Defaults to "10.0.0.0/24". + The webhook defaults this to "10.0.0.0/24". Mutually exclusive with ID. Note: A subnet is required. type: string diff --git a/pkg/services/hcloud/network/network.go b/pkg/services/hcloud/network/network.go index 24df1ffc2..d97d30e8d 100644 --- a/pkg/services/hcloud/network/network.go +++ b/pkg/services/hcloud/network/network.go @@ -19,7 +19,6 @@ package network import ( "context" - "errors" "fmt" "net" "slices" @@ -119,7 +118,7 @@ func (s *Service) createOpts() (hcloud.NetworkCreateOpts, error) { spec := s.scope.HetznerCluster.Spec.HCloudNetwork if spec.CIDRBlock == nil || spec.SubnetCIDRBlock == nil || spec.NetworkZone == nil { - return hcloud.NetworkCreateOpts{}, errors.New("nil CIDRs or NetworkZone given") + return hcloud.NetworkCreateOpts{}, fmt.Errorf("nil CIDRs or NetworkZone given") } _, network, err := net.ParseCIDR(*spec.CIDRBlock)