diff --git a/internal/webhook/proxmoxcluster_webhook.go b/internal/webhook/proxmoxcluster_webhook.go index 924db909..eccdc76d 100644 --- a/internal/webhook/proxmoxcluster_webhook.go +++ b/internal/webhook/proxmoxcluster_webhook.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "net/netip" + "regexp" "strings" infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" @@ -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 } @@ -83,7 +84,7 @@ 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 } @@ -91,36 +92,54 @@ func (*ProxmoxCluster) ValidateUpdate(_ context.Context, _ runtime.Object, newOb 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"), }) } @@ -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 +} diff --git a/internal/webhook/proxmoxcluster_webhook_test.go b/internal/webhook/proxmoxcluster_webhook_test.go index 4bb7f982..ef4e6c3a 100644 --- a/internal/webhook/proxmoxcluster_webhook_test.go +++ b/internal/webhook/proxmoxcluster_webhook_test.go @@ -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() {