Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow FQDNs as controlPlaneEndpoint #157

Merged
merged 4 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 42 additions & 9 deletions internal/webhook/proxmoxcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"context"
"fmt"
"net/netip"
"regexp"
"strings"

infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1"
Expand Down Expand Up @@ -63,7 +64,7 @@ func (*ProxmoxCluster) ValidateCreate(_ context.Context, obj runtime.Object) (wa
return warnings, err
}

if err := validateIPs(cluster); err != nil {
if err := validateControlPlaneEndpoint(cluster); err != nil {
warnings = append(warnings, fmt.Sprintf("cannot create proxmox cluster %s", cluster.GetName()))
return warnings, err
}
Expand All @@ -83,44 +84,62 @@ func (*ProxmoxCluster) ValidateUpdate(_ context.Context, _ runtime.Object, newOb
return warnings, apierrors.NewBadRequest(fmt.Sprintf("expected a ProxmoxCluster but got %T", newCluster))
}

if err := validateIPs(newCluster); err != nil {
if err := validateControlPlaneEndpoint(newCluster); err != nil {
warnings = append(warnings, fmt.Sprintf("cannot update proxmox cluster %s", newCluster.GetName()))
return warnings, err
}

return warnings, nil
}

func validateIPs(cluster *infrav1.ProxmoxCluster) error {
func validateControlPlaneEndpoint(cluster *infrav1.ProxmoxCluster) error {
ep := cluster.Spec.ControlPlaneEndpoint

gk, name := cluster.GroupVersionKind().GroupKind(), cluster.GetName()

endpointHostIP := ep.Host
endpoint := ep.Host

addr, err := netip.ParseAddr(endpoint)

/*
No further validation is done on hostnames. Checking DNS records
incures a lot of complexity. To list a few of the problems:
- DNS TTL will lead to incorrect results
- IP addresses can be PTR records
- Both A and AAAA records would need checking
- A record can have multiple entries, each of which need to be checked
- A valid record can start with _, but that is not a valid hostname
- ...
Most importantly, cluster-api does not validate controlPlaneEndpoint
at all.
*/
match := isHostname(endpoint)
if match {
return nil
}

addr, err := netip.ParseAddr(endpointHostIP)
if err != nil {
return apierrors.NewInvalid(
gk,
name,
field.ErrorList{
field.Invalid(
field.NewPath("spec", "controlplaneEndpoint"), endpointHostIP, "provided endpoint address is not a valid IP"),
field.NewPath("spec", "controlplaneEndpoint"), endpoint, "provided endpoint address is not a valid IP or FQDN"),
})
}
// If the passed control-plane endppoint is an IPv6 address, wrap it in [], so it can properly pass ParseAddrPort validation
if addr.Is6() {
endpointHostIP = fmt.Sprintf("[%s]", ep.Host)
endpoint = fmt.Sprintf("[%s]", endpoint)
}

ipAddr, err := netip.ParseAddrPort(fmt.Sprintf("%s:%d", endpointHostIP, ep.Port))
ipAddr, err := netip.ParseAddrPort(fmt.Sprintf("%s:%d", endpoint, ep.Port))
if err != nil {
return apierrors.NewInvalid(
gk,
name,
field.ErrorList{
field.Invalid(
field.NewPath("spec", "controlplaneEndpoint"), fmt.Sprintf("%s:%d", endpointHostIP, ep.Port), "provided endpoint is not in a valid IP and port format"),
field.NewPath("spec", "controlplaneEndpoint"), fmt.Sprintf("%s:%d", endpoint, ep.Port), "provided endpoint is not in a valid IP and port format"),
})
}

Expand Down Expand Up @@ -213,3 +232,17 @@ func buildSetFromAddresses(addresses []string) (*netipx.IPSet, error) {
func hasNoIPPoolConfig(cluster *infrav1.ProxmoxCluster) bool {
return cluster.Spec.IPv4Config == nil && cluster.Spec.IPv6Config == nil
}

func isHostname(h string) bool {
// shortname is up to 253 bytes long
shortname := `([a-z0-9]{1,253}|[a-z0-9][a-z0-9-]{1,251}[a-z0-9])`
// hostname is optional in a domain
hostname := `([a-z0-9]{1,63}|[a-z0-9][a-z0-9-]{1,61}[a-z0-9]\.)?`
domain := `((([a-z0-9]{1,63}|[a-z0-9][a-z0-9-]{1,61}[a-z0-9])\.)+?[a-z]{2,63})`

// make hostname match case insensitive, match complete string
hostmatch := `(?i)^(` + shortname + `|` + hostname + domain + `)$`

match, _ := regexp.Match(hostmatch, []byte(h))
return match
}
36 changes: 33 additions & 3 deletions internal/webhook/proxmoxcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,40 @@ var _ = Describe("Controller Test", func() {
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("at least one ip config must be set")))
})

It("should disallow invalid endpoint IP", func() {
It("should disallow invalid endpoint FQDN", func() {
cluster := invalidProxmoxCluster("test-cluster")
cluster.Spec.ControlPlaneEndpoint.Host = "invalid"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP")))
cluster.Spec.ControlPlaneEndpoint.Host = "_this.is.a.txt.record"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP or FQDN")))
})

It("should disallow invalid endpoint short hostname", func() {
cluster := invalidProxmoxCluster("test-cluster")
cluster.Spec.ControlPlaneEndpoint.Host = "invalid-"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP or FQDN")))
})

It("should allow valid endpoint FQDN", func() {
cluster := validProxmoxCluster("succeed-test-cluster-with-fqdn")
cluster.Spec.ControlPlaneEndpoint.Host = "host.example.com"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
})

It("should allow valid upper case endpoint FQDN", func() {
cluster := validProxmoxCluster("succeed-test-cluster-with-uppercase-fqdn")
cluster.Spec.ControlPlaneEndpoint.Host = "HOST.EXAMPLE.COM"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
})

It("should allow valid endpoint IP4", func() {
cluster := validProxmoxCluster("succeed-test-cluster-with-ip4")
cluster.Spec.ControlPlaneEndpoint.Host = "127.0.0.1"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
})

It("should allow valid endpoint IP6", func() {
cluster := validProxmoxCluster("succeed-test-cluster-with-ip6")
cluster.Spec.ControlPlaneEndpoint.Host = "::1"
g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
})

It("should disallow invalid endpoint IP + port combination", func() {
Expand Down