✨ enable BYO IP for control plane

Signed-off-by: Alexander Eldeib <alexeldeib@gmail.com>

api/v1alpha2/azurecluster_conversion.go

@@ -59,6 +59,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
 	}
 
 	dst.Status.FailureDomains = restored.Status.FailureDomains
+	dst.Spec.NetworkSpec.PublicIP = restored.Spec.NetworkSpec.PublicIP
 
 	for _, restoredSubnet := range restored.Spec.NetworkSpec.Subnets {
 		if restoredSubnet != nil {

api/v1alpha3/azurecluster_validation.go

@@ -20,8 +20,6 @@ import (
 	"fmt"
 	"regexp"
 
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
@@ -34,16 +32,8 @@ const (
 )
 
 // validateCluster validates a cluster
-func (c *AzureCluster) validateCluster() error {
-	var allErrs field.ErrorList
-	allErrs = append(allErrs, c.validateClusterSpec()...)
-	if len(allErrs) == 0 {
-		return nil
-	}
-
-	return apierrors.NewInvalid(
-		schema.GroupKind{Group: "infrastructure.cluster.x-k8s.io", Kind: "AzureCluster"},
-		c.Name, allErrs)
+func (c *AzureCluster) validateCluster() field.ErrorList {
+	return c.validateClusterSpec()
 }
 
 // validateClusterSpec validates a ClusterSpec
@@ -156,6 +146,21 @@ func validateIngressRule(ingressRule *IngressRule, fldPath *field.Path) *field.E
 		return field.Invalid(fldPath, ingressRule.Priority,
 			fmt.Sprintf("ingress priorities should be between 100 and 4096"))
 	}
+	return nil
+}
 
+func validateControlPlaneIP(old, new *PublicIPSpec, fldPath *field.Path) *field.Error {
+	if old == nil && new != nil {
+		return field.Invalid(fldPath, new, fmt.Sprintf("setting control plane endpoint after cluster creation is not allowed"))
+	}
+	if old != nil && new == nil {
+		return field.Invalid(fldPath, new, fmt.Sprintf("removing control plane endpoint after cluster creation is not allowed"))
+	}
+	if old != nil && new != nil && old.Name != new.Name {
+		return field.Invalid(fldPath, new, fmt.Sprintf("changing control plane endpoint after cluster creation is not allowed"))
+	}
+	if new != nil && new.Name == "" {
+		return field.Invalid(fldPath, new, fmt.Sprintf("control plane endpoint IP name must be non-empty"))
+	}
 	return nil
 }

api/v1alpha3/azurecluster_webhook.go

@@ -17,7 +17,9 @@ limitations under the License.
 package v1alpha3
 
 import (
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -48,15 +50,51 @@ func (c *AzureCluster) Default() {
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (c *AzureCluster) ValidateCreate() error {
 	clusterlog.Info("validate create", "name", c.Name)
-
-	return c.validateCluster()
+	allErrs := c.validateCluster()
+
+	// validation is smart/lazy enough to work with the same object compared to itself
+	// it works well for update to take both objects.
+	if err := validateControlPlaneIP(
+		c.Spec.NetworkSpec.PublicIP,
+		c.Spec.NetworkSpec.PublicIP,
+		field.NewPath("spec").Child("networkSpec").Child("publicIp"),
+	); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
+	if len(allErrs) == 0 {
+		return nil
+	}
+
+	return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (c *AzureCluster) ValidateUpdate(old runtime.Object) error {
+func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
 	clusterlog.Info("validate update", "name", c.Name)
 
-	return c.validateCluster()
+	old := oldRaw.(*AzureCluster)
+
+	var allErrs field.ErrorList
+
+	// validate cluster may return a list of errors
+	if errs := c.validateCluster(); errs != nil {
+		allErrs = append(allErrs, errs...)
+	}
+
+	if err := validateControlPlaneIP(
+		old.Spec.NetworkSpec.PublicIP,
+		c.Spec.NetworkSpec.PublicIP,
+		field.NewPath("spec").Child("networkSpec").Child("publicIp"),
+	); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
+	if len(allErrs) == 0 {
+		return nil
+	}
+
+	return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type

api/v1alpha3/types.go

@@ -38,6 +38,11 @@ type Network struct {
 
 // NetworkSpec specifies what the Azure networking resources should look like.
 type NetworkSpec struct {
+	// PublicIP is the public IP to attach to the Azure load balanacer.
+	// If name is not set in the spec, one will be generated.
+	// +optional
+	PublicIP *PublicIPSpec `json:"publicIp,omitempty"`
+
 	// Vnet is the configuration for the Azure virtual network.
 	// +optional
 	Vnet VnetSpec `json:"vnet,omitempty"`
@@ -141,9 +146,15 @@ type IngressRules []*IngressRule
 // PublicIP defines an Azure public IP address.
 type PublicIP struct {
 	ID        string `json:"id,omitempty"`
-	Name      string `json:"name,omitempty"`
 	IPAddress string `json:"ipAddress,omitempty"`
 	DNSName   string `json:"dnsName,omitempty"`
+	Name      string `json:"name,omitempty"`
+}
+
+// PublicIPSpec defines the inputs to create an Azure public IP address.
+type PublicIPSpec struct {
+	// +kubebuilder:validation:MinLength=1
+	Name string `json:"name,omitempty"`
 }
 
 // LoadBalancer defines an Azure load balancer.

cloud/scope/cluster.go

@@ -19,6 +19,8 @@ package scope
 import (
 	"context"
 	"fmt"
+	"hash/fnv"
+
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
@@ -104,8 +106,18 @@ func (s *ClusterScope) Network() *infrav1.Network {
 	return &s.AzureCluster.Status.Network
 }
 
-// PublicIPSpec returns the public IP specs.
+// PublicIPSpecs returns the public IP specs.
 func (s *ClusterScope) PublicIPSpecs() []azure.PublicIPSpec {
+	// If user provides an API server IP, do not return it return it here.
+	// We expect it to be created by them and will not reconcile it.
+	if s.AzureCluster.Spec.NetworkSpec.PublicIP != nil && s.AzureCluster.Spec.NetworkSpec.PublicIP.Name != "" {
+		return []azure.PublicIPSpec{
+			{
+				Name: azure.GenerateNodeOutboundIPName(s.ClusterName()),
+			},
+		}
+	}
+
 	return []azure.PublicIPSpec{
 		{
 			Name: azure.GenerateNodeOutboundIPName(s.ClusterName()),
@@ -231,3 +243,22 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS
 	}
 	s.AzureCluster.Status.FailureDomains[id] = spec
 }
+
+// SetAPIServerIP will set the spec for a for a given key
+func (s *ClusterScope) SetAPIServerIP() error {
+	// If IP provided by name, expect to exist and don't precreate.
+	if s.AzureCluster.Spec.NetworkSpec.PublicIP != nil {
+		s.AzureCluster.Status.Network.APIServerIP.Name = s.AzureCluster.Spec.NetworkSpec.PublicIP.Name
+	} else if s.Network().APIServerIP.Name == "" {
+		// otherwise, generate
+		h := fnv.New32a()
+		if _, err := h.Write([]byte(fmt.Sprintf("%s/%s/%s", s.SubscriptionID(), s.ResourceGroup(), s.ClusterName()))); err != nil {
+			return errors.Wrapf(err, "failed to write hash sum for api server ip")
+		}
+		s.AzureCluster.Status.Network.APIServerIP.Name = azure.GeneratePublicIPName(s.ClusterName(), fmt.Sprintf("%x", h.Sum32()))
+	}
+
+	s.AzureCluster.Status.Network.APIServerIP.DNSName = s.GenerateFQDN()
+
+	return nil
+}

cloud/scope/machine.go

@@ -88,13 +88,14 @@ type MachineScope struct {
 
 // PublicIPSpec returns the public IP specs.
 func (m *MachineScope) PublicIPSpecs() []azure.PublicIPSpec {
-	var spec []azure.PublicIPSpec
-	if m.AzureMachine.Spec.AllocatePublicIP == true {
-		spec = append(spec, azure.PublicIPSpec{
-			Name: azure.GenerateNodePublicIPName(m.Name()),
-		})
+	if m.AzureMachine.Spec.AllocatePublicIP {
+		return []azure.PublicIPSpec{
+			{
+				Name: azure.GenerateNodePublicIPName(azure.GenerateNICName(m.Name())),
+			},
+		}
 	}
-	return spec
+	return nil
 }
 
 // NICSpecs returns the network interface specs.

cloud/services/publicips/publicips.go

@@ -28,51 +28,56 @@ import (
 
 // Reconcile gets/creates/updates a public ip.
 func (s *Service) Reconcile(ctx context.Context) error {
-	for _, ip := range s.Scope.PublicIPSpecs() {
-		s.Scope.V(2).Info("creating public IP", "public ip", ip.Name)
-		err := s.Client.CreateOrUpdate(
-			ctx,
-			s.Scope.ResourceGroup(),
-			ip.Name,
-			network.PublicIPAddress{
-				Sku:      &network.PublicIPAddressSku{Name: network.PublicIPAddressSkuNameStandard},
-				Name:     to.StringPtr(ip.Name),
-				Location: to.StringPtr(s.Scope.Location()),
-				PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
-					PublicIPAddressVersion:   network.IPv4,
-					PublicIPAllocationMethod: network.Static,
-					DNSSettings: &network.PublicIPAddressDNSSettings{
-						DomainNameLabel: to.StringPtr(strings.ToLower(ip.Name)),
-						Fqdn:            to.StringPtr(ip.DNSName),
-					},
+	for _, publicIPSpec := range s.Scope.PublicIPSpecs() {
+		s.Scope.V(2).Info("creating public IP", "public-ip", publicIPSpec.Name)
+
+		ipSpec := network.PublicIPAddress{
+			Sku:      &network.PublicIPAddressSku{Name: network.PublicIPAddressSkuNameStandard},
+			Name:     to.StringPtr(publicIPSpec.Name),
+			Location: to.StringPtr(s.Scope.Location()),
+			PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
+				PublicIPAddressVersion:   network.IPv4,
+				PublicIPAllocationMethod: network.Static,
+				DNSSettings: &network.PublicIPAddressDNSSettings{
+					DomainNameLabel: to.StringPtr(strings.ToLower(publicIPSpec.Name)),
+					Fqdn:            to.StringPtr(publicIPSpec.DNSName),
 				},
 			},
-		)
+		}
 
-		if err != nil {
+		if err := s.Client.CreateOrUpdate(
+			ctx,
+			s.Scope.ResourceGroup(),
+			publicIPSpec.Name,
+			ipSpec,
+		); err != nil {
 			return errors.Wrap(err, "cannot create public IP")
 		}
 
-		s.Scope.V(2).Info("successfully created public IP", "public ip", ip.Name)
+		s.Scope.V(2).Info("successfully created public IP", "public ip", publicIPSpec.Name)
 	}
 
 	return nil
 }
 
 // Delete deletes the public IP with the provided scope.
 func (s *Service) Delete(ctx context.Context) error {
-	for _, ip := range s.Scope.PublicIPSpecs() {
-		s.Scope.V(2).Info("deleting public IP", "public ip", ip.Name)
-		err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), ip.Name)
-		if err != nil && azure.ResourceNotFound(err) {
-			// already deleted
-			return nil
+	for _, publicIPSpec := range s.Scope.PublicIPSpecs() {
+		s.Scope.V(2).Info("deleting public IP", "public ip", publicIPSpec.Name)
+
+		err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), publicIPSpec.Name)
+		if err == nil {
+			s.Scope.V(2).Info("deleted public IP", "public ip", publicIPSpec.Name)
+			continue
 		}
-		if err != nil {
-			return errors.Wrapf(err, "failed to delete public IP %s in resource group %s", ip.Name, s.Scope.ResourceGroup())
+
+		if azure.ResourceNotFound(err) {
+			s.Scope.V(2).Info("tried to delete public IP which didn't exist", "public ip", publicIPSpec.Name)
+			continue
 		}
 
-		s.Scope.V(2).Info("deleted public IP", "public ip", ip.Name)
+		return errors.Wrapf(err, "failed to delete public IP %s in resource group %s", publicIPSpec.Name, s.Scope.ResourceGroup())
 	}
+
 	return nil
 }