diff --git a/hcloud/cloud.go b/hcloud/cloud.go index 35f595b24..da71bf754 100644 --- a/hcloud/cloud.go +++ b/hcloud/cloud.go @@ -213,7 +213,7 @@ func (c *cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) { Recorder: c.recorder, } - return newLoadBalancers(lbOps, &c.cfg.LoadBalancer), true + return newLoadBalancers(lbOps, &c.cfg.LoadBalancer, c.recorder), true } func (c *cloud) Clusters() (cloudprovider.Clusters, bool) { diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go index 18d5c9e27..b7ba486fc 100644 --- a/hcloud/load_balancers.go +++ b/hcloud/load_balancers.go @@ -4,17 +4,19 @@ import ( "context" "errors" "fmt" + "slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - cloudprovider "k8s.io/cloud-provider" + "k8s.io/client-go/tools/record" "k8s.io/klog/v2" - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-go/v2/hcloud" + "github.com/hetznercloud/hcloud-go/v2/hcloud/exp/kit/sliceutil" ) // LoadBalancerOps defines the Load Balancer related operations required by @@ -23,52 +25,45 @@ type LoadBalancerOps interface { GetByName(ctx context.Context, name string) (*hcloud.LoadBalancer, error) GetByID(ctx context.Context, id int64) (*hcloud.LoadBalancer, error) GetByK8SServiceUID(ctx context.Context, svc *corev1.Service) (*hcloud.LoadBalancer, error) - Create(ctx context.Context, lbName string, service *corev1.Service) (*hcloud.LoadBalancer, error) + Create(ctx context.Context, service *corev1.Service, spec lbspec.Spec) (*hcloud.LoadBalancer, error) Delete(ctx context.Context, lb *hcloud.LoadBalancer) error - ReconcileHCLB(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) - ReconcileHCLBTargets(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, nodes []*corev1.Node) (bool, error) - ReconcileHCLBServices(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) + ReconcileHCLB(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec) (bool, error) + ReconcileHCLBTargets( + ctx context.Context, + lb *hcloud.LoadBalancer, + svc *corev1.Service, + spec lbspec.Spec, + nodes []*corev1.Node, + ) (bool, error) + ReconcileHCLBServices(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec) (bool, error) } type loadBalancers struct { - lbOps LoadBalancerOps - cfg *config.LoadBalancerConfiguration + lbOps LoadBalancerOps + cfg *config.LoadBalancerConfiguration + recorder record.EventRecorder } -func newLoadBalancers(lbOps LoadBalancerOps, lbCfg *config.LoadBalancerConfiguration) *loadBalancers { +func newLoadBalancers(lbOps LoadBalancerOps, lbCfg *config.LoadBalancerConfiguration, recorder record.EventRecorder) *loadBalancers { return &loadBalancers{ - lbOps: lbOps, - cfg: lbCfg, + lbOps: lbOps, + cfg: lbCfg, + recorder: recorder, } } -func matchNodeSelector(svc *corev1.Service, nodes []*corev1.Node) ([]*corev1.Node, error) { - var selectedNodes []*corev1.Node - - selector := labels.Everything() - if v, err := annotation.LBNodeSelector.FromService(svc); err == nil { - parsed, err := labels.Parse(v) - if err != nil { - return nil, fmt.Errorf("unable to parse the node-selector annotation: %w", err) - } - selector = parsed - } - - for _, n := range nodes { - if selector.Matches(labels.Set(n.GetLabels())) { - selectedNodes = append(selectedNodes, n) - } - } - - return selectedNodes, nil -} - func (l *loadBalancers) GetLoadBalancer( ctx context.Context, _ string, service *corev1.Service, ) (status *corev1.LoadBalancerStatus, exists bool, err error) { const op = "hcloud/loadBalancers.GetLoadBalancer" metrics.OperationCalled.WithLabelValues(op).Inc() + // The lookup comes before resolving the annotations on purpose. The service + // controller calls us before deleting a Load Balancer and only removes its + // finalizer once we returned without an error, so reporting that a Load + // Balancer does not exist must not depend on the annotations being valid. + // Otherwise a Service that never got a Load Balancer because one of its + // annotations is invalid could not be deleted either. lb, err := l.lbOps.GetByK8SServiceUID(ctx, service) if err != nil { if errors.Is(err, hcops.ErrNotFound) { @@ -77,56 +72,41 @@ func (l *loadBalancers) GetLoadBalancer( return nil, false, fmt.Errorf("%s: %w", op, err) } - if v, err := annotation.LBHostname.FromService(service); err == nil { - return &corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{{Hostname: v}}, - }, true, nil - } - - ingress, err := l.buildLoadBalancerStatusIngress(lb, service) + spec, err := lbspec.Resolve(l.recorder, service, *l.cfg) if err != nil { return nil, false, fmt.Errorf("%s: %w", op, err) } - return &corev1.LoadBalancerStatus{Ingress: ingress}, true, nil + return &corev1.LoadBalancerStatus{Ingress: l.buildLoadBalancerStatusIngress(lb, spec)}, true, nil } func (l *loadBalancers) GetLoadBalancerName(_ context.Context, _ string, service *corev1.Service) string { - if v, err := annotation.LBName.FromService(service); err == nil { - return v - } - return cloudprovider.DefaultLoadBalancerName(service) + return lbspec.Name(service) } func (l *loadBalancers) EnsureLoadBalancer( - ctx context.Context, clusterName string, svc *corev1.Service, nodes []*corev1.Node, + ctx context.Context, _ string, svc *corev1.Service, nodes []*corev1.Node, ) (*corev1.LoadBalancerStatus, error) { const op = "hcloud/loadBalancers.EnsureLoadBalancer" metrics.OperationCalled.WithLabelValues(op).Inc() var ( - reload bool - lb *hcloud.LoadBalancer - err error - selectedNodes []*corev1.Node + reload bool + lb *hcloud.LoadBalancer + err error ) - selectedNodes, err = matchNodeSelector(svc, nodes) + spec, err := lbspec.Resolve(l.recorder, svc, *l.cfg) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } - nodeNames := make([]string, len(selectedNodes)) - for i, n := range selectedNodes { - nodeNames[i] = n.Name - } + selectedNodes := filterNodes(spec.NodeSelector, nodes) + nodeNames := sliceutil.Transform(selectedNodes, func(n *corev1.Node) string { + return n.GetName() + }) klog.InfoS("ensure Load Balancer", "op", op, "service", svc.Name, "nodes", nodeNames) - lb, err = l.lbOps.GetByK8SServiceUID(ctx, svc) - if err != nil && !errors.Is(err, hcops.ErrNotFound) { - return nil, fmt.Errorf("%s: %w", op, err) - } - // Try the load balancer's name if we were not able to find it using the // service UID. This is required for two reasons: // @@ -135,23 +115,26 @@ func (l *loadBalancers) EnsureLoadBalancer( // // 2. Import of load balancers which were created by other means but // should be re-used by the cloud controller manager. - lbName := l.GetLoadBalancerName(ctx, clusterName, svc) + lb, err = l.lbOps.GetByK8SServiceUID(ctx, svc) + if err != nil && !errors.Is(err, hcops.ErrNotFound) { + return nil, fmt.Errorf("%s: %w", op, err) + } + + // Not found by UID label; try name if errors.Is(err, hcops.ErrNotFound) { - lb, err = l.lbOps.GetByName(ctx, lbName) - if err != nil && !errors.Is(err, hcops.ErrNotFound) { - return nil, fmt.Errorf("%s: %w", op, err) - } + lb, err = l.lbOps.GetByName(ctx, spec.Name) } - // If we were still not able to find the load balancer we create it. + // Load Balancer does not exist yet if errors.Is(err, hcops.ErrNotFound) { - lb, err = l.lbOps.Create(ctx, lbName, svc) - if err != nil { - return nil, fmt.Errorf("%s: %w", op, err) - } + lb, err = l.lbOps.Create(ctx, svc, spec) + } + + if err != nil { + return nil, fmt.Errorf("%s: %w", op, err) } - lbChanged, err := l.lbOps.ReconcileHCLB(ctx, lb, svc) + lbChanged, err := l.lbOps.ReconcileHCLB(ctx, lb, svc, spec) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } @@ -172,13 +155,13 @@ func (l *loadBalancers) EnsureLoadBalancer( reload = false } - servicesChanged, err := l.lbOps.ReconcileHCLBServices(ctx, lb, svc) + servicesChanged, err := l.lbOps.ReconcileHCLBServices(ctx, lb, svc, spec) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } reload = reload || servicesChanged - targetsChanged, err := l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, selectedNodes) + targetsChanged, err := l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, spec, selectedNodes) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } @@ -192,49 +175,34 @@ func (l *loadBalancers) EnsureLoadBalancer( } } - // Either set the Hostname or the IPs (below). - // See: https://github.com/kubernetes/kubernetes/issues/66607 - if v, err := annotation.LBHostname.FromService(svc); err == nil { - return &corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{{Hostname: v}}, - }, nil - } - - ingress, err := l.buildLoadBalancerStatusIngress(lb, svc) - if err != nil { - return nil, fmt.Errorf("%s: %w", op, err) - } - - return &corev1.LoadBalancerStatus{Ingress: ingress}, nil + return &corev1.LoadBalancerStatus{Ingress: l.buildLoadBalancerStatusIngress(lb, spec)}, nil } -func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, svc *corev1.Service) ([]corev1.LoadBalancerIngress, error) { +// buildLoadBalancerStatusIngress reports the addresses the Service is reachable +// on. A configured hostname replaces the IPs entirely. +// See: https://github.com/kubernetes/kubernetes/issues/66607 +func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, spec lbspec.Spec) []corev1.LoadBalancerIngress { const op = "hcloud/loadBalancers.getLoadBalancerStatusIngress" metrics.OperationCalled.WithLabelValues(op).Inc() - var ingress []corev1.LoadBalancerIngress - ipMode := corev1.LoadBalancerIPModeVIP - - proxyProtocolEnabled, err := l.getProxyProtocolEnabled(svc) - if err != nil { - return nil, fmt.Errorf("%s: %w", op, err) + if spec.Hostname != "" { + return []corev1.LoadBalancerIngress{{Hostname: spec.Hostname}} } - if proxyProtocolEnabled { + ipMode := corev1.LoadBalancerIPModeVIP + if spec.Service.ProxyProtocol != nil && *spec.Service.ProxyProtocol { ipMode = corev1.LoadBalancerIPModeProxy } + var ingress []corev1.LoadBalancerIngress + if lb.PublicNet.Enabled { ingress = append(ingress, corev1.LoadBalancerIngress{ IP: lb.PublicNet.IPv4.IP.String(), IPMode: &ipMode, }) - ipv6Enabled, err := l.getIPv6Enabled(svc) - if err != nil { - return nil, fmt.Errorf("%s: %w", op, err) - } - if ipv6Enabled { + if spec.IPv6 { ingress = append(ingress, corev1.LoadBalancerIngress{ IP: lb.PublicNet.IPv6.IP.String(), IPMode: &ipMode, @@ -242,12 +210,7 @@ func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, } } - privateIngressEnabled, err := l.getPrivateIngressEnabled(svc) - if err != nil { - return nil, fmt.Errorf("%s: %w", op, err) - } - - if privateIngressEnabled { + if spec.PrivateIngress { for _, privateNet := range lb.PrivateNet { ingress = append(ingress, corev1.LoadBalancerIngress{ IP: privateNet.IP.String(), @@ -256,62 +219,30 @@ func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, } } - return ingress, nil -} - -func (l *loadBalancers) getPrivateIngressEnabled(svc *corev1.Service) (bool, error) { - disable, err := annotation.LBDisablePrivateIngress.FromService(svc) - if err == nil { - return !disable, nil - } - if errors.Is(err, annotation.ErrNotSet) { - return l.cfg.PrivateIngressEnabled, nil - } - return true, err -} - -func (l *loadBalancers) getProxyProtocolEnabled(svc *corev1.Service) (bool, error) { - enable, err := annotation.LBSvcProxyProtocol.FromService(svc) - if err == nil { - return enable, nil - } - if errors.Is(err, annotation.ErrNotSet) { - if l.cfg.ProxyProtocolEnabled == nil { - return false, nil - } - return *l.cfg.ProxyProtocolEnabled, nil - } - return false, err -} - -func (l *loadBalancers) getIPv6Enabled(svc *corev1.Service) (bool, error) { - disable, err := annotation.LBIPv6Disabled.FromService(svc) - if err == nil { - return !disable, nil - } - if errors.Is(err, annotation.ErrNotSet) { - return l.cfg.IPv6Enabled, nil - } - return true, err + return ingress } func (l *loadBalancers) UpdateLoadBalancer( - ctx context.Context, clusterName string, svc *corev1.Service, nodes []*corev1.Node, + ctx context.Context, + _ string, + svc *corev1.Service, + nodes []*corev1.Node, ) error { const op = "hcloud/loadBalancers.UpdateLoadBalancer" metrics.OperationCalled.WithLabelValues(op).Inc() var ( - lb *hcloud.LoadBalancer - err error - selectedNodes []*corev1.Node + lb *hcloud.LoadBalancer + err error ) - selectedNodes, err = matchNodeSelector(svc, nodes) + spec, err := lbspec.Resolve(l.recorder, svc, *l.cfg) if err != nil { return fmt.Errorf("%s: %w", op, err) } + selectedNodes := filterNodes(spec.NodeSelector, nodes) + nodeNames := make([]string, len(selectedNodes)) for i, n := range selectedNodes { nodeNames[i] = n.Name @@ -320,25 +251,23 @@ func (l *loadBalancers) UpdateLoadBalancer( lb, err = l.lbOps.GetByK8SServiceUID(ctx, svc) if errors.Is(err, hcops.ErrNotFound) { - lbName := l.GetLoadBalancerName(ctx, clusterName, svc) - - lb, err = l.lbOps.GetByName(ctx, lbName) - if errors.Is(err, hcops.ErrNotFound) { - return nil - } - // further error types handled below + lb, err = l.lbOps.GetByName(ctx, spec.Name) } - if err != nil { + switch { + case errors.Is(err, hcops.ErrNotFound): + // Nothing to do, the Load Balancer does not exist. + return nil + case err != nil: return fmt.Errorf("%s: %w", op, err) } - if _, err = l.lbOps.ReconcileHCLB(ctx, lb, svc); err != nil { + if _, err = l.lbOps.ReconcileHCLB(ctx, lb, svc, spec); err != nil { return fmt.Errorf("%s: %w", op, err) } - if _, err = l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, selectedNodes); err != nil { + if _, err = l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, spec, selectedNodes); err != nil { return fmt.Errorf("%s: %w", op, err) } - if _, err = l.lbOps.ReconcileHCLBServices(ctx, lb, svc); err != nil { + if _, err = l.lbOps.ReconcileHCLBServices(ctx, lb, svc, spec); err != nil { return fmt.Errorf("%s: %w", op, err) } return nil @@ -372,3 +301,13 @@ func (l *loadBalancers) EnsureLoadBalancerDeleted(ctx context.Context, _ string, return nil } + +func filterNodes(selector labels.Selector, nodes []*corev1.Node) []*corev1.Node { + if selector.Empty() { + return nodes + } + + return slices.DeleteFunc(slices.Clone(nodes), func(n *corev1.Node) bool { + return !selector.Matches(labels.Set(n.GetLabels())) + }) +} diff --git a/hcloud/load_balancers_test.go b/hcloud/load_balancers_test.go index 99a37834c..15552f81c 100644 --- a/hcloud/load_balancers_test.go +++ b/hcloud/load_balancers_test.go @@ -4,14 +4,18 @@ import ( "errors" "net" "reflect" + "slices" "testing" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -57,6 +61,24 @@ func TestLoadBalancers_GetLoadBalancer(t *testing.T) { assert.Equal(t, tt.LB.PublicNet.IPv4.IP.String(), status.Ingress[0].IP) }, }, + { + Name: "report a missing load balancer despite an invalid annotation", + ServiceUID: "1", + ServiceAnnotations: map[string]string{ + string(annotation.LBSvcHealthCheckInterval): "10", + }, + Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { + tt.LBOps. + On("GetByK8SServiceUID", tt.Ctx, tt.Service). + Return(nil, hcops.ErrNotFound) + }, + Perform: func(t *testing.T, tt *LoadBalancerTestCase) { + status, exists, err := tt.LoadBalancers.GetLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service) + assert.NoError(t, err) + assert.False(t, exists) + assert.Nil(t, status) + }, + }, { Name: "get load balancer without host name", ServiceUID: "1", @@ -231,16 +253,16 @@ func TestLoadBalancers_EnsureLoadBalancer_CreateLoadBalancer(t *testing.T) { On("GetByName", tt.Ctx, lbName). Return(nil, hcops.ErrNotFound) tt.LBOps. - On("Create", tt.Ctx, tt.LB.Name, tt.Service). + On("Create", tt.Ctx, tt.Service, tt.Spec). Return(tt.LB, nil) tt.LBOps. - On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service). + On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec). Return(false, nil) tt.LBOps. - On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes). + On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes). Return(false, nil) tt.LBOps. - On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service). + On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec). Return(false, nil) } @@ -484,16 +506,16 @@ func TestLoadBalancers_EnsureLoadBalancer_CreateLoadBalancer(t *testing.T) { On("GetByName", tt.Ctx, "priv-net-only"). Return(nil, hcops.ErrNotFound) tt.LBOps. - On("Create", tt.Ctx, tt.LB.Name, tt.Service). + On("Create", tt.Ctx, tt.Service, tt.Spec). Return(tt.LB, nil) tt.LBOps. - On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes). + On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes). Return(false, nil) tt.LBOps. - On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service). + On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec). Return(false, nil) tt.LBOps. - On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service). + On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec). Return(false, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { @@ -571,9 +593,9 @@ func TestLoadBalancer_EnsureLoadBalancer_UpdateLoadBalancer(t *testing.T) { }, Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(false, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { _, err := tt.LoadBalancers.EnsureLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service, tt.Nodes) @@ -593,9 +615,9 @@ func TestLoadBalancer_EnsureLoadBalancer_UpdateLoadBalancer(t *testing.T) { }, Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(true, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(false, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(true, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) tt.LBOps.On("GetByID", tt.Ctx, tt.LB.ID).Times(1).Return(tt.LB, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { @@ -616,9 +638,9 @@ func TestLoadBalancer_EnsureLoadBalancer_UpdateLoadBalancer(t *testing.T) { }, Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(true, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(false, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(true, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) tt.LBOps.On("GetByID", tt.Ctx, tt.LB.ID).Times(1).Return(tt.LB, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { @@ -639,9 +661,9 @@ func TestLoadBalancer_EnsureLoadBalancer_UpdateLoadBalancer(t *testing.T) { }, Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(true, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(true, nil) tt.LBOps.On("GetByID", tt.Ctx, tt.LB.ID).Times(1).Return(tt.LB, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { @@ -664,9 +686,9 @@ func TestLoadBalancer_EnsureLoadBalancer_UpdateLoadBalancer(t *testing.T) { Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(nil, hcops.ErrNotFound) tt.LBOps.On("GetByName", tt.Ctx, "pre-existing-lb").Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(true, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(true, nil) tt.LBOps.On("GetByID", tt.Ctx, tt.LB.ID).Times(1).Return(tt.LB, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { @@ -709,9 +731,9 @@ func TestLoadBalancer_UpdateLoadBalancer(t *testing.T) { }, Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(false, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { err := tt.LoadBalancers.UpdateLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service, tt.Nodes) @@ -732,9 +754,9 @@ func TestLoadBalancer_UpdateLoadBalancer(t *testing.T) { Mock: func(_ *testing.T, tt *LoadBalancerTestCase) { tt.LBOps.On("GetByK8SServiceUID", tt.Ctx, tt.Service).Return(nil, hcops.ErrNotFound) tt.LBOps.On("GetByName", tt.Ctx, "previously-created-lb").Return(tt.LB, nil) - tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service).Return(false, nil) - tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Nodes).Return(false, nil) - tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service).Return(false, nil) + tt.LBOps.On("ReconcileHCLB", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) + tt.LBOps.On("ReconcileHCLBTargets", tt.Ctx, tt.LB, tt.Service, tt.Spec, tt.Nodes).Return(false, nil) + tt.LBOps.On("ReconcileHCLBServices", tt.Ctx, tt.LB, tt.Service, tt.Spec).Return(false, nil) }, Perform: func(t *testing.T, tt *LoadBalancerTestCase) { err := tt.LoadBalancers.UpdateLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service, tt.Nodes) @@ -931,6 +953,21 @@ func TestLoadBalancer_matchNodeSelector(t *testing.T) { newNodeSelectorNode("node1", map[string]string{"environment": "production"}), }, }, + { + name: "single node selector to select none", + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + string(annotation.LBNodeSelector): "environment=production", + }, + }, + }, + k8sNodes: []*corev1.Node{ + newNodeSelectorNode("node1", map[string]string{"environment": "staging"}), + newNodeSelectorNode("node2", map[string]string{"environment": "staging"}), + }, + expected: []*corev1.Node{}, + }, { name: "multiple node selector to select all", service: &corev1.Service{ @@ -952,14 +989,24 @@ func TestLoadBalancer_matchNodeSelector(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - nodes, err := matchNodeSelector(c.service, c.k8sNodes) + spec, err := lbspec.Resolve(record.NewFakeRecorder(10), c.service, config.LoadBalancerConfiguration{}) if err != nil { t.Fatal(err) } + // The service controller keeps a reference to the Nodes it hands + // us, so filtering must leave them untouched. + unfiltered := slices.Clone(c.k8sNodes) + + nodes := filterNodes(spec.NodeSelector, c.k8sNodes) + if !reflect.DeepEqual(nodes, c.expected) { t.Errorf("expected: %+v got %+v", c.expected, nodes) } + + if !reflect.DeepEqual(c.k8sNodes, unfiltered) { + t.Errorf("filterNodes modified the Nodes passed to it: %+v", c.k8sNodes) + } }) } } diff --git a/hcloud/testing.go b/hcloud/testing.go index 0e572111a..31bce3f22 100644 --- a/hcloud/testing.go +++ b/hcloud/testing.go @@ -11,6 +11,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -40,6 +41,7 @@ type LoadBalancerTestCase struct { LBClient *mocks.LoadBalancerClient LoadBalancers *loadBalancers Service *corev1.Service + Spec lbspec.Spec } func (tt *LoadBalancerTestCase) run(t *testing.T) { @@ -73,16 +75,20 @@ func (tt *LoadBalancerTestCase) run(t *testing.T) { tt.Ctx = context.Background() } - if tt.Mock != nil { - tt.Mock(t, tt) - } - lbConfig := &config.LoadBalancerConfiguration{ PrivateIngressEnabled: *tt.UsePrivateIngressDefault, IPv6Enabled: *tt.UseIPv6Default, } - tt.LoadBalancers = newLoadBalancers(tt.LBOps, lbConfig) + // The error is ignored on purpose: a test case may set an invalid + // annotation to check how the cloud provider reports it. + tt.Spec, _ = lbspec.Resolve(&MockEventRecorder{}, tt.Service, *lbConfig) + + if tt.Mock != nil { + tt.Mock(t, tt) + } + + tt.LoadBalancers = newLoadBalancers(tt.LBOps, lbConfig, &MockEventRecorder{}) tt.Perform(t, tt) tt.LBOps.AssertExpectations(t) diff --git a/internal/annotation/annotation.go b/internal/annotation/annotation.go index 3b6b6cd3e..8c7615d15 100644 --- a/internal/annotation/annotation.go +++ b/internal/annotation/annotation.go @@ -16,7 +16,7 @@ package annotation import ( "errors" "fmt" - "net" + "net/netip" "strconv" "strings" "time" @@ -68,7 +68,7 @@ func (a Strings) FromService(svc *corev1.Service) ([]string, error) { }) } -func (a IP) FromService(svc *corev1.Service) (net.IP, error) { +func (a IP) FromService(svc *corev1.Service) (netip.Addr, error) { return parse(string(a), svc, parseIP) } @@ -88,7 +88,7 @@ func (a Certificates) FromService(svc *corev1.Service) ([]*hcloud.Certificate, e func value(name string, svc *corev1.Service) (string, error) { v, ok := svc.Annotations[name] if !ok { - return "", fmt.Errorf("%s: %w", name, ErrNotSet) + return "", fmt.Errorf("%q: %w", name, ErrNotSet) } return v, nil } @@ -103,16 +103,16 @@ func parse[T any](name string, svc *corev1.Service, convert func(string) (T, err converted, err := convert(v) if err != nil { - return zero, fmt.Errorf("%s: %w", name, err) + return zero, fmt.Errorf("%q: %w", name, err) } return converted, nil } -func parseIP(v string) (net.IP, error) { - ip := net.ParseIP(v) - if ip == nil { - return nil, fmt.Errorf("invalid ip address: %s", v) +func parseIP(v string) (netip.Addr, error) { + ip, err := netip.ParseAddr(v) + if err != nil { + return netip.Addr{}, fmt.Errorf("invalid ip address: %s", v) } return ip, nil } @@ -125,7 +125,7 @@ func parseAlgorithmType(v string) (hcloud.LoadBalancerAlgorithmType, error) { hcloud.LoadBalancerAlgorithmTypeRoundRobin: return algorithm, nil default: - return "", fmt.Errorf("invalid: %s", v) + return "", fmt.Errorf("invalid algorithm type: %s", v) } } @@ -138,7 +138,7 @@ func parseServiceProtocol(v string) (hcloud.LoadBalancerServiceProtocol, error) hcloud.LoadBalancerServiceProtocolHTTPS: return protocol, nil default: - return "", fmt.Errorf("invalid: %s", v) + return "", fmt.Errorf("invalid protocol: %s", v) } } diff --git a/internal/annotation/annotation_test.go b/internal/annotation/annotation_test.go index c4e12e6e0..ab9872cc3 100644 --- a/internal/annotation/annotation_test.go +++ b/internal/annotation/annotation_test.go @@ -2,7 +2,8 @@ package annotation_test import ( "errors" - "net" + "fmt" + "net/netip" "strconv" "testing" "time" @@ -18,6 +19,8 @@ import ( // per type. const annName = "some/annotation" +var qAnnName = fmt.Sprintf("%q", annName) + const ( annString annotation.String = annName annBool annotation.Bool = annName @@ -48,7 +51,7 @@ func TestString(t *testing.T) { name: "value not set", notSet: true, expected: "", - err: errors.New(annName + ": not set"), + err: errors.New(qAnnName + ": not set"), }, } @@ -108,7 +111,7 @@ func TestInt(t *testing.T) { name: "value not set", notSet: true, expected: 0, - err: errors.New(annName + ": not set"), + err: errors.New(qAnnName + ": not set"), }, { name: "value invalid", @@ -138,7 +141,7 @@ func TestDuration(t *testing.T) { { name: "value invalid", value: "invalid", - err: errors.New(annName + `: time: invalid duration "invalid"`), + err: errors.New(qAnnName + `: time: invalid duration "invalid"`), }, } @@ -171,17 +174,17 @@ func TestIP(t *testing.T) { { name: "value set to valid IPv4", value: "1.2.3.4", - expected: net.ParseIP("1.2.3.4"), + expected: netip.MustParseAddr("1.2.3.4"), }, { name: "value set to valid IPv6", value: "3c2e:2ef9:a7e9:1a5b:30ba:4912:e3fe:91b2", - expected: net.ParseIP("3c2e:2ef9:a7e9:1a5b:30ba:4912:e3fe:91b2"), + expected: netip.MustParseAddr("3c2e:2ef9:a7e9:1a5b:30ba:4912:e3fe:91b2"), }, { name: "value invalid", value: "invalid", - err: errors.New(annName + ": invalid ip address: invalid"), + err: errors.New(qAnnName + ": invalid ip address: invalid"), }, { name: "value not set", @@ -214,8 +217,8 @@ func TestProtocol(t *testing.T) { }, { name: "value invalid", - value: "invalid", - err: errors.New(annName + ": invalid: invalid"), + value: "hppt", + err: errors.New(qAnnName + ": invalid protocol: hppt"), }, } @@ -243,8 +246,8 @@ func TestAlgorithmType(t *testing.T) { }, { name: "value invalid", - value: "invalid", - err: errors.New(annName + ": invalid: invalid"), + value: "round_ronald", + err: errors.New(qAnnName + ": invalid algorithm type: round_ronald"), }, } diff --git a/internal/annotation/load_balancer.go b/internal/annotation/load_balancer.go index 9fbe08cc0..03dba0467 100644 --- a/internal/annotation/load_balancer.go +++ b/internal/annotation/load_balancer.go @@ -56,7 +56,7 @@ const ( // LBPrivateIPv4 specifies the IPv4 address to assign to the load balancer in the // private network that it's attached to. - LBPrivateIPv4 String = "load-balancer.hetzner.cloud/private-ipv4" + LBPrivateIPv4 IP = "load-balancer.hetzner.cloud/private-ipv4" // PrivateSubnetIPRange specifies an existing subnet to which the load balancer will be attached. // The value must be in the CIDR notation. The subnet must belong to the network defined diff --git a/internal/config/config.go b/internal/config/config.go index 30891e03f..23a840870 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,7 +3,7 @@ package config import ( "errors" "fmt" - "net" + "net/netip" "os" "strconv" "strings" @@ -321,7 +321,7 @@ func (c HCCMConfiguration) Validate() (err error) { } if c.LoadBalancer.PrivateSubnetIPRange != "" { - if _, _, err := net.ParseCIDR(c.LoadBalancer.PrivateSubnetIPRange); err != nil { + if _, err := netip.ParsePrefix(c.LoadBalancer.PrivateSubnetIPRange); err != nil { errs = append(errs, fmt.Errorf("invalid value for %q: must be a valid CIDR: %w", hcloudLoadBalancersPrivateSubnetIPRange, err)) } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f133b6734..575fe5659 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -476,7 +476,7 @@ func TestHCCMConfiguration_Validate(t *testing.T) { PrivateSubnetIPRange: "10.0.0.0/33", }, }, - wantErr: errors.New("invalid value for \"HCLOUD_LOAD_BALANCERS_PRIVATE_SUBNET_IP_RANGE\": must be a valid CIDR: invalid CIDR address: 10.0.0.0/33"), + wantErr: errors.New("invalid value for \"HCLOUD_LOAD_BALANCERS_PRIVATE_SUBNET_IP_RANGE\": must be a valid CIDR: netip.ParsePrefix(\"10.0.0.0/33\"): prefix length out of range"), }, { name: "algorithm type invalid", diff --git a/internal/hcops/certificates_test.go b/internal/hcops/certificates_test.go index b5747a27a..3687eca36 100644 --- a/internal/hcops/certificates_test.go +++ b/internal/hcops/certificates_test.go @@ -130,6 +130,14 @@ func TestCertificateOps_GetCertificateByLabel(t *testing.T) { runCertificateOpsTestCases(t, tests) } +// managedCertificateOpts is the certificate the cases below create. +var managedCertificateOpts = hcloud.CertificateCreateOpts{ + Name: "test-cert", + Type: hcloud.CertificateTypeManaged, + DomainNames: []string{"example.com", "*.example.com"}, + Labels: map[string]string{"key": "value"}, +} + func TestCertificateOps_CreateManagedCertificate(t *testing.T) { tests := []certificateOpsTestCase{ { @@ -140,12 +148,7 @@ func TestCertificateOps_CreateManagedCertificate(t *testing.T) { Return(nil, nil, errors.New("test error")) }, Perform: func(t *testing.T, tt *certificateOpsTestCase) { - err := tt.CertOps.CreateManagedCertificate(tt.Ctx, hcloud.CertificateCreateOpts{ - Name: "test-cert", - Type: hcloud.CertificateTypeManaged, - DomainNames: []string{"example.com", "*.example.com"}, - Labels: map[string]string{"key": "value"}, - }) + err := tt.CertOps.CreateManagedCertificate(tt.Ctx, managedCertificateOpts) assert.Error(t, err) assert.True(t, strings.HasSuffix(err.Error(), "test error")) }, @@ -159,12 +162,7 @@ func TestCertificateOps_CreateManagedCertificate(t *testing.T) { Return(nil, nil, err) }, Perform: func(t *testing.T, tt *certificateOpsTestCase) { - err := tt.CertOps.CreateManagedCertificate(tt.Ctx, hcloud.CertificateCreateOpts{ - Name: "test-cert", - Type: hcloud.CertificateTypeManaged, - DomainNames: []string{"example.com", "*.example.com"}, - Labels: map[string]string{"key": "value"}, - }) + err := tt.CertOps.CreateManagedCertificate(tt.Ctx, managedCertificateOpts) assert.ErrorIs(t, err, hcops.ErrAlreadyExists) }, }, @@ -173,22 +171,12 @@ func TestCertificateOps_CreateManagedCertificate(t *testing.T) { Mock: func(_ *testing.T, tt *certificateOpsTestCase) { res := hcloud.CertificateCreateResult{Certificate: &hcloud.Certificate{ID: 1}, Action: &hcloud.Action{ID: 2}} tt.CertClient. - On("CreateCertificate", tt.Ctx, hcloud.CertificateCreateOpts{ - Name: "test-cert", - Type: hcloud.CertificateTypeManaged, - DomainNames: []string{"example.com", "*.example.com"}, - Labels: map[string]string{"key": "value"}, - }). + On("CreateCertificate", tt.Ctx, managedCertificateOpts). Return(res, nil, nil) tt.ActionClient.On("WaitFor", tt.Ctx, &hcloud.Action{ID: 2}).Return(nil) }, Perform: func(t *testing.T, tt *certificateOpsTestCase) { - err := tt.CertOps.CreateManagedCertificate(tt.Ctx, hcloud.CertificateCreateOpts{ - Name: "test-cert", - Type: hcloud.CertificateTypeManaged, - DomainNames: []string{"example.com", "*.example.com"}, - Labels: map[string]string{"key": "value"}, - }) + err := tt.CertOps.CreateManagedCertificate(tt.Ctx, managedCertificateOpts) assert.NoError(t, err) }, }, diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index 27ed43515..80db9d2eb 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -4,9 +4,8 @@ import ( "context" "errors" "fmt" - "maps" "net" - "sync" + "net/netip" "time" hrobot "github.com/syself/hrobot-go" @@ -17,6 +16,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" @@ -24,14 +24,7 @@ import ( "github.com/hetznercloud/hcloud-go/v2/hcloud/exp/deprecationutil" ) -const ( - // LabelServiceUID is a label added to the Hetzner Cloud backend to uniquely - // identify a load balancer managed by Hetzner Cloud Cloud Controller Manager. - LabelServiceUID = "hcloud-ccm/service-uid" - - defaultLoadBalancerType = "lb11" - loadBalancerSubsystem = "load_balancer" -) +const loadBalancerSubsystem = "load_balancer" // LoadBalancerOps implements all operations regarding Hetzner Cloud Load Balancers. type LoadBalancerOps struct { @@ -59,7 +52,7 @@ func (l *LoadBalancerOps) GetByK8SServiceUID(ctx context.Context, svc *corev1.Se opts := hcloud.LoadBalancerListOpts{ ListOpts: hcloud.ListOpts{ - LabelSelector: fmt.Sprintf("%s=%s", LabelServiceUID, svc.ObjectMeta.UID), + LabelSelector: fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, svc.ObjectMeta.UID), }, } lbs, err := l.LBClient.AllWithOpts(ctx, opts) @@ -118,128 +111,25 @@ func (l *LoadBalancerOps) GetByID(ctx context.Context, id int64) (*hcloud.LoadBa return lb, nil } -func (l *LoadBalancerOps) getType(ctx context.Context, svc *corev1.Service) (*hcloud.LoadBalancerType, bool, error) { - ctx = cache.SetSubsystem(ctx, loadBalancerSubsystem) - var lbTypeName string - var unset bool - - if l.Cfg.LoadBalancer.Type != "" { - lbTypeName = l.Cfg.LoadBalancer.Type - } - - if v, err := annotation.LBType.FromService(svc); err == nil { - lbTypeName = v - } - - if lbTypeName == "" { - lbTypeName = defaultLoadBalancerType - unset = true - utils.WarnEventLogf( - l.Recorder, - svc, - "LoadBalancerTypeUnconfigured", - "Load Balancer Type unconfigured: this will be required in the future, set it with the annotation %q or cluster-wide with the environment variable %q", - annotation.LBType, - config.HcloudLoadBalancersType, - ) - } - - lbType, err := l.LBTypeCache.ByName(ctx, lbTypeName) - if err != nil { - return nil, unset, err - } - - if lbType == nil { - return nil, unset, fmt.Errorf("load balancer type not found: %s", lbTypeName) - } - - msg, unavailable := deprecationutil.LoadBalancerTypeMessage(lbType) - if unavailable { - return nil, false, errors.New(msg) - } - if msg != "" { - utils.WarnEventLogf( - l.Recorder, - svc, - "LoadBalancerTypeDeprecated", - "%s", msg, - ) - } - - return lbType, unset, nil -} - // Create creates a new Load Balancer using the Hetzner Cloud API. // -// It adds annotations identifying the HC Load Balancer to svc. +// svc is only used to report events about the created Load Balancer. func (l *LoadBalancerOps) Create( - ctx context.Context, lbName string, svc *corev1.Service, + ctx context.Context, svc *corev1.Service, spec lbspec.Spec, ) (*hcloud.LoadBalancer, error) { const op = "hcops/LoadBalancerOps.Create" metrics.OperationCalled.WithLabelValues(op).Inc() - opts := hcloud.LoadBalancerCreateOpts{ - Name: lbName, - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, - Labels: map[string]string{ - LabelServiceUID: string(svc.ObjectMeta.UID), - }, + if spec.Location == "" && spec.NetworkZone == "" { + return nil, fmt.Errorf("%s: neither %s nor %s set", op, annotation.LBLocation, annotation.LBNetworkZone) } - lbType, _, err := l.getType(ctx, svc) + lbType, err := l.verifyType(ctx, svc, spec) if err != nil { return nil, fmt.Errorf("error getting load balancer type: %w", err) } - opts.LoadBalancerType = lbType - - if l.Cfg.LoadBalancer.Location != "" { - opts.Location = &hcloud.Location{Name: l.Cfg.LoadBalancer.Location} - } - if v, err := annotation.LBLocation.FromService(svc); err == nil { - if v == "" { - // Allow resetting the location in case someone wants to specify a network zone in an annotation - // and a location as default. - opts.Location = nil - } else { - opts.Location = &hcloud.Location{Name: v} - } - } - opts.NetworkZone = hcloud.NetworkZone(l.Cfg.LoadBalancer.NetworkZone) - if v, err := annotation.LBNetworkZone.FromService(svc); err == nil { - opts.NetworkZone = hcloud.NetworkZone(v) - } - if opts.Location == nil && opts.NetworkZone == "" { - return nil, fmt.Errorf("%s: neither %s nor %s set", op, annotation.LBLocation, annotation.LBNetworkZone) - } - if opts.Location != nil && opts.NetworkZone != "" { - opts.NetworkZone = "" - } - - algType, err := annotation.LBAlgorithmType.FromService(svc) - switch { - case err == nil: - opts.Algorithm = &hcloud.LoadBalancerAlgorithm{Type: algType} - case errors.Is(err, annotation.ErrNotSet): - if l.Cfg.LoadBalancer.AlgorithmType != "" { - opts.Algorithm = &hcloud.LoadBalancerAlgorithm{Type: l.Cfg.LoadBalancer.AlgorithmType} - } - default: - return nil, fmt.Errorf("%s: %w", op, err) - } - disablePubIface, err := annotation.LBDisablePublicNetwork.FromService(svc) - switch { - case err == nil: - opts.PublicInterface = new(!disablePubIface) - case errors.Is(err, annotation.ErrNotSet): - if l.Cfg.LoadBalancer.DisablePublicNetwork != nil { - opts.PublicInterface = new(!*l.Cfg.LoadBalancer.DisablePublicNetwork) - } - default: - return nil, fmt.Errorf("%s: %w", op, err) - } - - result, _, err := l.LBClient.Create(ctx, opts) + result, _, err := l.LBClient.Create(ctx, spec.CreateOpts(lbType)) if err != nil { return nil, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) } @@ -251,6 +141,7 @@ func (l *LoadBalancerOps) Create( if err != nil { return nil, fmt.Errorf("%s: get Load Balancer: %d: %w", op, result.LoadBalancer.ID, err) } + return lb, nil } @@ -269,57 +160,60 @@ func (l *LoadBalancerOps) Delete(ctx context.Context, lb *hcloud.LoadBalancer) e return nil } -// ReconcileHCLB configures the Hetzner Cloud Load Balancer to match what is -// defined for the K8S Load Balancer svc. -func (l *LoadBalancerOps) ReconcileHCLB(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +// ReconcileHCLB configures the Hetzner Cloud Load Balancer to match spec. +// +// svc is only used to report events about the Load Balancer. +func (l *LoadBalancerOps) ReconcileHCLB( + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, +) (bool, error) { const op = "hcops/LoadBalancerOps.ReconcileHCLB" metrics.OperationCalled.WithLabelValues(op).Inc() var changed bool - labelSet, err := l.changeHCLBInfo(ctx, lb, svc) + labelSet, err := l.changeHCLBInfo(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || labelSet - ipv4RDNSChanged, err := l.changeIPv4RDNS(ctx, lb, svc) + ipv4RDNSChanged, err := l.changeIPv4RDNS(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || ipv4RDNSChanged - ipv6RDNSChanged, err := l.changeIPv6RDNS(ctx, lb, svc) + ipv6RDNSChanged, err := l.changeIPv6RDNS(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || ipv6RDNSChanged - algorithmChanged, err := l.changeAlgorithm(ctx, lb, svc) + algorithmChanged, err := l.changeAlgorithm(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || algorithmChanged - typeChanged, err := l.changeType(ctx, lb, svc) + typeChanged, err := l.changeType(ctx, lb, svc, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || typeChanged - networkDetached, err := l.detachFromNetwork(ctx, lb, svc) + networkDetached, err := l.detachFromNetwork(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || networkDetached - networkAttached, err := l.attachToNetwork(ctx, lb, svc) + networkAttached, err := l.attachToNetwork(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } changed = changed || networkAttached - pubIfaceToggled, err := l.togglePublicInterface(ctx, lb, svc) + pubIfaceToggled, err := l.togglePublicInterface(ctx, lb, spec) if err != nil { return changed, fmt.Errorf("%s: %w", op, err) } @@ -334,31 +228,15 @@ func (l *LoadBalancerOps) ReconcileHCLB(ctx context.Context, lb *hcloud.LoadBala // This is implemented in one method as both changes need to be made using // hcloud.LoadBalancerUpdateOpts. Using one method reduces the number of API // requests should more than one change be necessary. -func (l *LoadBalancerOps) changeHCLBInfo(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) changeHCLBInfo( + ctx context.Context, + lb *hcloud.LoadBalancer, + spec lbspec.Spec, +) (bool, error) { const op = "hcops/LoadBalancerOps.changeHCLBInfo" metrics.OperationCalled.WithLabelValues(op).Inc() - var ( - update bool - opts hcloud.LoadBalancerUpdateOpts - ) - - if lb.Labels[LabelServiceUID] != string(svc.ObjectMeta.UID) { - // Make a defensive copy of labels. This way we do not modify lb unless - // updating is really successful. The service UID is set after copying, - // so that it replaces a stale value instead of being overwritten by it. - labels := make(map[string]string, len(lb.Labels)+1) - maps.Copy(labels, lb.Labels) - labels[LabelServiceUID] = string(svc.ObjectMeta.UID) - opts.Labels = labels - update = true - } - - if lbName, err := annotation.LBName.FromService(svc); err == nil && lbName != lb.Name { - opts.Name = lbName - update = true - } - + opts, update := spec.UpdateOpts(lb) if !update { return false, nil } @@ -373,24 +251,20 @@ func (l *LoadBalancerOps) changeHCLBInfo(ctx context.Context, lb *hcloud.LoadBal return true, nil } -func (l *LoadBalancerOps) changeIPv4RDNS(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) changeIPv4RDNS(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.changeIPv4RDNS" metrics.OperationCalled.WithLabelValues(op).Inc() - rdns, err := annotation.LBPublicIPv4RDNS.FromService(svc) // If the annotation is not set, no changes are needed - if errors.Is(err, annotation.ErrNotSet) { + if spec.IPv4RDNS == nil { return false, nil } - if err != nil { - return false, fmt.Errorf("%s: %w", op, err) - } // If the annotation and the actual value match, no changes are needed - if rdns == lb.PublicNet.IPv4.DNSPtr { + if *spec.IPv4RDNS == lb.PublicNet.IPv4.DNSPtr { return false, nil } - action, _, err := l.LBClient.ChangeDNSPtr(ctx, lb, lb.PublicNet.IPv4.IP.String(), &rdns) + action, _, err := l.LBClient.ChangeDNSPtr(ctx, lb, lb.PublicNet.IPv4.IP.String(), spec.IPv4RDNS) if err != nil { return false, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) } @@ -401,24 +275,20 @@ func (l *LoadBalancerOps) changeIPv4RDNS(ctx context.Context, lb *hcloud.LoadBal return true, nil } -func (l *LoadBalancerOps) changeIPv6RDNS(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) changeIPv6RDNS(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.changeIPv6RDNS" metrics.OperationCalled.WithLabelValues(op).Inc() - rdns, err := annotation.LBPublicIPv6RDNS.FromService(svc) // If the annotation is not set, no changes are needed - if errors.Is(err, annotation.ErrNotSet) { + if spec.IPv6RDNS == nil { return false, nil } - if err != nil { - return false, fmt.Errorf("%s: %w", op, err) - } // If the annotation and the actual value match, no changes are needed - if rdns == lb.PublicNet.IPv6.DNSPtr { + if *spec.IPv6RDNS == lb.PublicNet.IPv6.DNSPtr { return false, nil } - action, _, err := l.LBClient.ChangeDNSPtr(ctx, lb, lb.PublicNet.IPv6.IP.String(), &rdns) + action, _, err := l.LBClient.ChangeDNSPtr(ctx, lb, lb.PublicNet.IPv6.IP.String(), spec.IPv6RDNS) if err != nil { return false, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) } @@ -429,26 +299,16 @@ func (l *LoadBalancerOps) changeIPv6RDNS(ctx context.Context, lb *hcloud.LoadBal return true, nil } -func (l *LoadBalancerOps) changeAlgorithm(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) changeAlgorithm(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.changeAlgorithm" metrics.OperationCalled.WithLabelValues(op).Inc() - at, err := annotation.LBAlgorithmType.FromService(svc) - if err != nil { - if errors.Is(err, annotation.ErrNotSet) { - if l.Cfg.LoadBalancer.AlgorithmType == "" { - return false, nil - } - at = l.Cfg.LoadBalancer.AlgorithmType - } else { - return false, fmt.Errorf("%s: %w", op, err) - } - } - if at == lb.Algorithm.Type { + // An unconfigured algorithm is left alone. + if spec.Algorithm == "" || spec.Algorithm == lb.Algorithm.Type { return false, nil } - opts := hcloud.LoadBalancerChangeAlgorithmOpts{Type: at} + opts := spec.ChangeAlgorithmOpts() action, _, err := l.LBClient.ChangeAlgorithm(ctx, lb, opts) if err != nil { return false, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) @@ -460,20 +320,25 @@ func (l *LoadBalancerOps) changeAlgorithm(ctx context.Context, lb *hcloud.LoadBa return true, nil } -func (l *LoadBalancerOps) changeType(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) changeType( + ctx context.Context, + lb *hcloud.LoadBalancer, + svc *corev1.Service, + spec lbspec.Spec, +) (bool, error) { const op = "hcops/LoadBalancerOps.changeType" metrics.OperationCalled.WithLabelValues(op).Inc() opts := hcloud.LoadBalancerChangeTypeOpts{} - lbType, unset, err := l.getType(ctx, svc) + lbType, err := l.verifyType(ctx, svc, spec) if err != nil { return false, fmt.Errorf("error getting load balancer type: %w", err) } // If the user removes the annotation, we do not downgrade the Load Balancer // back to its default value. This could be changed in a next major release. - if unset { + if spec.TypeUnset { return false, nil } @@ -494,19 +359,17 @@ func (l *LoadBalancerOps) changeType(ctx context.Context, lb *hcloud.LoadBalance return true, nil } -func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.detachFromNetwork" metrics.OperationCalled.WithLabelValues(op).Inc() var changed bool - privateIPv4, err := annotation.LBPrivateIPv4.FromService(svc) - privateIPv4configured := err == nil for _, lbpn := range lb.PrivateNet { // Don't detach the Load Balancer from the network it is supposed to // be attached to and the current private IP of the load balancer matches // the one configured by the user, if one is configured. - if l.NetworkID == lbpn.Network.ID && (!privateIPv4configured || privateIPv4 == lbpn.IP.String()) { + if l.NetworkID == lbpn.Network.ID && (!spec.PrivateIPv4.IsValid() || sameIP(spec.PrivateIPv4, lbpn.IP)) { continue } klog.InfoS("detach from network", "op", op, "loadBalancerID", lb.ID, "networkID", lbpn.Network.ID, "privateIPv4", lbpn.IP.String()) @@ -524,42 +387,18 @@ func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.Load return changed, nil } -func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.attachToNetwork" metrics.OperationCalled.WithLabelValues(op).Inc() - privateIPv4String, err := annotation.LBPrivateIPv4.FromService(svc) - privateIPv4configured := err == nil - subnetString, err := annotation.PrivateSubnetIPRange.FromService(svc) - subnetConfigured := err == nil - if !subnetConfigured && l.Cfg.LoadBalancer.PrivateSubnetIPRange != "" { - subnetString = l.Cfg.LoadBalancer.PrivateSubnetIPRange - subnetConfigured = true - } // Don't attach the Load Balancer if network is not set, or the load // balancer is already attached. - if l.NetworkID == 0 || lbAttached(lb, l.NetworkID, privateIPv4String) { + if l.NetworkID == 0 || lbAttached(lb, l.NetworkID, spec.PrivateIPv4) { return false, nil } - var privateIPv4 net.IP - if privateIPv4configured { - privateIPv4 = net.ParseIP(privateIPv4String) - if privateIPv4 == nil { - return false, fmt.Errorf("%s: %w", op, fmt.Errorf("could not parse private IPv4 '%s'", privateIPv4)) - } - } - - var subnet *net.IPNet - if subnetConfigured { - _, subnet, err = net.ParseCIDR(subnetString) - if err != nil { - return false, fmt.Errorf("%s: could not parse private subnet IP range '%s'", op, subnetString) - } - } - - if privateIPv4 != nil { - klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID, "privateIP", privateIPv4) + if spec.PrivateIPv4.IsValid() { + klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID, "privateIP", spec.PrivateIPv4) } else { klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID) } @@ -576,13 +415,7 @@ func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBa if retryDelay == 0 { retryDelay = time.Second } - opts := hcloud.LoadBalancerAttachToNetworkOpts{Network: nw} - if privateIPv4 != nil { - opts.IP = privateIPv4 - } - if subnet != nil { - opts.IPRange = subnet - } + opts := spec.AttachToNetworkOpts(nw) a, _, err := l.LBClient.AttachToNetwork(ctx, lb, opts) if hcloud.IsError(err, hcloud.ErrorCodeConflict, hcloud.ErrorCodeLocked) { klog.InfoS("retry due to conflict or lock", @@ -602,35 +435,22 @@ func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBa return true, nil } -func (l *LoadBalancerOps) togglePublicInterface(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) { +func (l *LoadBalancerOps) togglePublicInterface(ctx context.Context, lb *hcloud.LoadBalancer, spec lbspec.Spec) (bool, error) { const op = "hcops/LoadBalancerOps.togglePublicInterface" metrics.OperationCalled.WithLabelValues(op).Inc() var a *hcloud.Action + var err error - disable, err := annotation.LBDisablePublicNetwork.FromService(svc) - var desiredDisable *bool - switch { - case err == nil: - desiredDisable = new(disable) - case errors.Is(err, annotation.ErrNotSet): - desiredDisable = l.Cfg.LoadBalancer.DisablePublicNetwork - default: - return false, fmt.Errorf("%s: %w", op, err) - } - - if desiredDisable == nil { + // An unconfigured public interface is left alone. + if spec.PublicInterface == nil || *spec.PublicInterface == lb.PublicNet.Enabled { return false, nil } - if *desiredDisable == !lb.PublicNet.Enabled { - return false, nil - } - - if *desiredDisable { - a, _, err = l.LBClient.DisablePublicInterface(ctx, lb) - } else { + if *spec.PublicInterface { a, _, err = l.LBClient.EnablePublicInterface(ctx, lb) + } else { + a, _, err = l.LBClient.DisablePublicInterface(ctx, lb) } if err != nil { return false, fmt.Errorf("%s: %w", op, err) @@ -645,7 +465,7 @@ func (l *LoadBalancerOps) togglePublicInterface(ctx context.Context, lb *hcloud. // ReconcileHCLBTargets adds or removes target nodes from the Hetzner Cloud // Load Balancer when nodes are added or removed to the K8S cluster. func (l *LoadBalancerOps) ReconcileHCLBTargets( - ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, nodes []*corev1.Node, + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, nodes []*corev1.Node, ) (bool, error) { const op = "hcops/LoadBalancerOps.ReconcileHCLBTargets" metrics.OperationCalled.WithLabelValues(op).Inc() @@ -674,10 +494,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( changed bool ) - privateIPEnabled, err := l.getPrivateIPEnabled(svc) - if err != nil { - return changed, fmt.Errorf("%s: %w", op, err) - } + privateIPEnabled := spec.UsePrivateIP if privateIPEnabled && l.NetworkID == 0 { return changed, fmt.Errorf("%s: use private ip: missing network id", op) } @@ -882,10 +699,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( } klog.InfoS("add target", "op", op, "service", svc.ObjectMeta.Name, "targetName", node.Name) - opts := hcloud.LoadBalancerAddServerTargetOpts{ - Server: &hcloud.Server{ID: id}, - UsePrivateIP: &privateIPEnabled, - } + opts := spec.AddServerTargetOpts(id) a, _, err := l.LBClient.AddServerTarget(ctx, lb, opts) if err != nil { if hcloud.IsError(err, hcloud.ErrorCodeResourceLimitExceeded) { @@ -956,28 +770,24 @@ func (l *LoadBalancerOps) emitMaxTargetsReachedError(node *corev1.Node, svc *cor klog.InfoS("cannot add server target because max number of targets have been reached", "op", op, "service", svc.ObjectMeta.Name, "targetName", node.Name) } -func (l *LoadBalancerOps) getPrivateIPEnabled(svc *corev1.Service) (bool, error) { - usePrivateIP, err := annotation.LBUsePrivateIP.FromService(svc) - if err != nil { - if errors.Is(err, annotation.ErrNotSet) { - return l.Cfg.LoadBalancer.PrivateIPEnabled, nil - } - return false, err - } - return usePrivateIP, nil -} - // ReconcileHCLBServices synchronizes services exposed by the Hetzner Cloud // Load Balancer with the kubernetes cluster. func (l *LoadBalancerOps) ReconcileHCLBServices( - ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, ) (bool, error) { const op = "hcops/LoadBalancerOps.ReconcileHCLBServices" metrics.OperationCalled.WithLabelValues(op).Inc() var changed bool - if err := l.reconcileManagedCertificate(ctx, svc); err != nil { + if err := l.reconcileOwnedCertificate(ctx, spec); err != nil { + return false, fmt.Errorf("%s: %w", op, err) + } + + // Resolved once for the whole Service, as every port uses the same + // certificates. + certificates, err := l.resolveCertificates(ctx, svc, spec) + if err != nil { return false, fmt.Errorf("%s: %w", op, err) } @@ -990,13 +800,7 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( // balancer. Remove the ports from the set of HC Load Balancer listen // ports. for _, port := range svc.Spec.Ports { - var ( - addOpts hcloud.LoadBalancerAddServiceOpts - updOpts hcloud.LoadBalancerUpdateServiceOpts - action *hcloud.Action - - err error - ) + var action *hcloud.Action if port.Protocol != "" && port.Protocol != corev1.ProtocolTCP { utils.WarnEventLogf( @@ -1014,31 +818,19 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( portExists := hclbListenPorts[portNo] delete(hclbListenPorts, portNo) - b := &hclbServiceOptsBuilder{ - Port: port, - Service: svc, - CertOps: l.CertOps, - cfg: l.Cfg.LoadBalancer, - } if portExists { klog.InfoS("update service", "op", op, "port", portNo, "loadBalancerID", lb.ID) - updOpts, err = b.buildUpdateServiceOpts() - if err != nil { - return changed, fmt.Errorf("%s: %w", op, err) - } - action, _, err = l.LBClient.UpdateService(ctx, lb, b.listenPort, updOpts) + opts := spec.Service.UpdateServiceOpts(port, certificates) + action, _, err = l.LBClient.UpdateService(ctx, lb, portNo, opts) if err != nil { return changed, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) } } else { klog.InfoS("add service", "op", op, "port", portNo, "loadBalancerID", lb.ID) - addOpts, err = b.buildAddServiceOpts() - if err != nil { - return changed, fmt.Errorf("%s: %w", op, err) - } - action, _, err = l.LBClient.AddService(ctx, lb, addOpts) + opts := spec.Service.AddServiceOpts(port, certificates) + action, _, err = l.LBClient.AddService(ctx, lb, opts) if err != nil { return changed, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) } @@ -1067,38 +859,18 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( return changed, nil } -func (l *LoadBalancerOps) reconcileManagedCertificate(ctx context.Context, svc *corev1.Service) error { - const op = "hcops/LoadBalancerOps.reconcileManagedCertificate" +func (l *LoadBalancerOps) reconcileOwnedCertificate( + ctx context.Context, + spec lbspec.Spec, +) error { + const op = "hcops/LoadBalancerOps.reconcileOwnedCertificate" metrics.OperationCalled.WithLabelValues(op).Inc() - // Compared as a raw string: only the exact value selects managed - // certificates, and an unset annotation reads as the empty string. - if typ, _ := annotation.LBSvcHTTPCertificateType.FromService(svc); typ != string(hcloud.CertificateTypeManaged) { + if spec.OwnedCertificate == nil { return nil } - name, _ := annotation.LBSvcHTTPManagedCertificateName.FromService(svc) - if name == "" { - name = fmt.Sprintf("ccm-managed-certificate-%s", svc.ObjectMeta.UID) - } - domains, err := annotation.LBSvcHTTPManagedCertificateDomains.FromService(svc) - if errors.Is(err, annotation.ErrNotSet) { - return fmt.Errorf("%s: no domains for managed certificate", op) - } - labels := map[string]string{ - LabelServiceUID: string(svc.ObjectMeta.UID), - } - // It's ok to ignore the error here. We are only interested if the - // annotation is set and parseable as a truthy boolean. Anything else tells - // us we do not want to use ACME staging. - if ok, _ := annotation.LBSvcHTTPManagedCertificateUseACMEStaging.FromService(svc); ok { - labels["HC-Use-Staging-CA"] = "true" - } - err = l.CertOps.CreateManagedCertificate(ctx, hcloud.CertificateCreateOpts{ - Name: name, - Type: hcloud.CertificateTypeManaged, - DomainNames: domains, - Labels: labels, - }) + + err := l.CertOps.CreateManagedCertificate(ctx, spec.OwnedCertificate.CreateOpts()) if errors.Is(err, ErrAlreadyExists) { return nil } @@ -1108,460 +880,110 @@ func (l *LoadBalancerOps) reconcileManagedCertificate(ctx context.Context, svc * return nil } -type hclbServiceOptsBuilder struct { - Port corev1.ServicePort - Service *corev1.Service - CertOps *CertificateOps - cfg config.LoadBalancerConfiguration - - listenPort int - destinationPort int - proxyProtocol *bool - protocol hcloud.LoadBalancerServiceProtocol - httpOpts struct { - CookieName *string - CookieLifetime *time.Duration - Certificates []*hcloud.Certificate - RedirectHTTP *bool - StickySessions *bool - TimeoutIdle *time.Duration - } - addHTTP bool - healthCheckOpts struct { - Protocol hcloud.LoadBalancerServiceProtocol - Port *int - Interval *time.Duration - Timeout *time.Duration - Retries *int - httpOpts struct { - Domain *string - Path *string - Response *string - StatusCodes []string - TLS *bool - } - } - addHealthCheck bool - - once sync.Once - err error -} - -func (b *hclbServiceOptsBuilder) extract() { - const op = "hcops/hclbServiceOptsBuilder.extract" +// resolveCertificates turns the certificate references of the Service into +// references the API accepts, which means looking up certificates referenced by +// name. A certificate we own ourselves is looked up by the label it was created +// with. +// +// It returns nil when the Service has no certificates configured. +func (l *LoadBalancerOps) resolveCertificates( + ctx context.Context, + svc *corev1.Service, + spec lbspec.Spec, +) ([]*hcloud.Certificate, error) { + const op = "hcops/LoadBalancerOps.resolveCertificates" metrics.OperationCalled.WithLabelValues(op).Inc() - b.listenPort = int(b.Port.Port) - b.destinationPort = int(b.Port.NodePort) - - b.do(func() error { - pp, err := annotation.LBSvcProxyProtocol.FromService(b.Service) - if err == nil { - b.proxyProtocol = new(pp) - return nil - } - if errors.Is(err, annotation.ErrNotSet) { - b.proxyProtocol = b.cfg.ProxyProtocolEnabled - return nil - } - return fmt.Errorf("%s: %w", op, err) - }) - - b.protocol = hcloud.LoadBalancerServiceProtocolTCP - b.do(func() error { - p, err := annotation.LBSvcProtocol.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } + if spec.OwnedCertificate != nil { + cert, err := l.CertOps.GetCertificateByLabel(ctx, fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, svc.ObjectMeta.UID)) if err != nil { - return fmt.Errorf("%s: %w", op, err) + return nil, fmt.Errorf("%s: %w", op, err) } - b.protocol = p - return nil - }) - - if v, err := annotation.LBSvcHTTPCookieName.FromService(b.Service); err == nil { - b.httpOpts.CookieName = &v - b.addHTTP = true + return []*hcloud.Certificate{{ID: cert.ID}}, nil } - b.do(func() error { - lt, err := annotation.LBSvcHTTPCookieLifetime.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.httpOpts.CookieLifetime = < - b.addHTTP = true - return nil - }) - - b.do(func() error { - timeout, err := annotation.LBSvcHTTPTimeoutIdle.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.httpOpts.TimeoutIdle = &timeout - b.addHTTP = true - return nil - }) - - b.do(func() error { - certtyp, _ := annotation.LBSvcHTTPCertificateType.FromService(b.Service) - if certtyp == string(hcloud.CertificateTypeManaged) { - // Continue with managed certificates below - return nil - } - - certs, err := annotation.LBSvcHTTPCertificates.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - certs, err = b.resolveCertsByNameOrID(ctx, certs) - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.httpOpts.Certificates = certs - b.addHTTP = true - return nil - }) - - b.do(func() error { - certtyp, _ := annotation.LBSvcHTTPCertificateType.FromService(b.Service) - if certtyp != string(hcloud.CertificateTypeManaged) { - // Not a a managed certificate. - return nil - } - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - svcUID := b.Service.ObjectMeta.UID - cert, err := b.CertOps.GetCertificateByLabel(ctx, fmt.Sprintf("%s=%s", LabelServiceUID, svcUID)) - if err != nil { - return err - } - b.httpOpts.Certificates = []*hcloud.Certificate{{ID: cert.ID}} - b.addHTTP = true - return nil - }) - - b.do(func() error { - redirectHTTP, err := annotation.LBSvcRedirectHTTP.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.httpOpts.RedirectHTTP = &redirectHTTP - b.addHTTP = true - return nil - }) - - b.do(func() error { - stickySessions, err := annotation.LBSvcHTTPStickySessions.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.httpOpts.StickySessions = &stickySessions - b.addHTTP = true - return nil - }) - - b.extractHealthCheck() -} - -func (b *hclbServiceOptsBuilder) resolveCertsByNameOrID(ctx context.Context, cs []*hcloud.Certificate) ([]*hcloud.Certificate, error) { - const op = "hcops/hclbServiceOptsBuilder.resolveCertsByNameOrID" - metrics.OperationCalled.WithLabelValues(op).Inc() + if spec.Service.HTTP == nil || len(spec.Service.HTTP.Certificates) == 0 { + return nil, nil + } - resolved := make([]*hcloud.Certificate, len(cs)) - for i, c := range cs { + resolved := make([]*hcloud.Certificate, len(spec.Service.HTTP.Certificates)) + for i, c := range spec.Service.HTTP.Certificates { if c.ID != 0 { resolved[i] = c continue } - c, err := b.CertOps.GetCertificateByNameOrID(ctx, c.Name) + cert, err := l.CertOps.GetCertificateByNameOrID(ctx, c.Name) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } - resolved[i] = &hcloud.Certificate{ID: c.ID} + resolved[i] = &hcloud.Certificate{ID: cert.ID} } return resolved, nil } -func (b *hclbServiceOptsBuilder) extractHealthCheck() { - const op = "hcops/hclbServiceOptsBuilder.extractHealthCheck" - metrics.OperationCalled.WithLabelValues(op).Inc() - - b.do(func() error { - p, err := annotation.LBSvcHealthCheckProtocol.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - // Set the service protocol but do not set the addHealthCheck flag. - // This way the health check is configured using the service - // protocol only if at least one health check annotation is - // present. - b.healthCheckOpts.Protocol = b.protocol - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.healthCheckOpts.Protocol = p - b.addHealthCheck = true - return nil - }) - - b.do(func() error { - hcPort, err := annotation.LBSvcHealthCheckPort.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.healthCheckOpts.Port = new(hcPort) - b.addHealthCheck = true - return nil - }) - - b.do(func() error { - hcInterval, err := annotation.LBSvcHealthCheckInterval.FromService(b.Service) - if err == nil { - b.healthCheckOpts.Interval = &hcInterval - b.addHealthCheck = true - return nil - } - if errors.Is(err, annotation.ErrNotSet) { - if b.cfg.HealthCheckInterval != 0 { - b.healthCheckOpts.Interval = &b.cfg.HealthCheckInterval - b.addHealthCheck = true - } - return nil - } - return fmt.Errorf("%s: %w", op, err) - }) - - b.do(func() error { - t, err := annotation.LBSvcHealthCheckTimeout.FromService(b.Service) - if err == nil { - b.healthCheckOpts.Timeout = &t - b.addHealthCheck = true - return nil - } - if errors.Is(err, annotation.ErrNotSet) { - if b.cfg.HealthCheckTimeout != 0 { - b.healthCheckOpts.Timeout = &b.cfg.HealthCheckTimeout - b.addHealthCheck = true - } - return nil - } - return fmt.Errorf("%s: %w", op, err) - }) - - b.do(func() error { - v, err := annotation.LBSvcHealthCheckRetries.FromService(b.Service) - if err == nil { - b.healthCheckOpts.Retries = &v - b.addHealthCheck = true - return nil - } - if errors.Is(err, annotation.ErrNotSet) { - if b.cfg.HealthCheckRetries != 0 { - b.healthCheckOpts.Retries = &b.cfg.HealthCheckRetries - b.addHealthCheck = true - } - return nil - } - return fmt.Errorf("%s: %w", op, err) - }) - - if b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolTCP { - return - } - - if v, err := annotation.LBSvcHealthCheckHTTPDomain.FromService(b.Service); err == nil { - b.healthCheckOpts.httpOpts.Domain = &v - } - - if v, err := annotation.LBSvcHealthCheckHTTPPath.FromService(b.Service); err == nil { - b.healthCheckOpts.httpOpts.Path = &v - } - - b.do(func() error { - tls, err := annotation.LBSvcHealthCheckHTTPValidateCertificate.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.healthCheckOpts.httpOpts.TLS = &tls - return nil - }) - - b.do(func() error { - scs, err := annotation.LBSvcHealthCheckHTTPStatusCodes.FromService(b.Service) - if errors.Is(err, annotation.ErrNotSet) { - return nil - } - if err != nil { - return fmt.Errorf("%s: %w", op, err) - } - b.healthCheckOpts.httpOpts.StatusCodes = scs - return nil - }) -} - -func (b *hclbServiceOptsBuilder) initialize() error { - b.once.Do(b.extract) - return b.err -} - -func (b *hclbServiceOptsBuilder) do(f func() error) { - if b.err != nil { - return - } - b.err = f() -} - -func (b *hclbServiceOptsBuilder) buildAddServiceOpts() (hcloud.LoadBalancerAddServiceOpts, error) { - const op = "hcops/hclbServiceOptsBuilder.buildAddServiceOpts" - metrics.OperationCalled.WithLabelValues(op).Inc() +// verifyType looks up the Load Balancer type requested by spec and warns about a +// type that is unconfigured, deprecated or unavailable. +func (l *LoadBalancerOps) verifyType( + ctx context.Context, + svc *corev1.Service, + spec lbspec.Spec, +) (*hcloud.LoadBalancerType, error) { + ctx = cache.SetSubsystem(ctx, loadBalancerSubsystem) - if err := b.initialize(); err != nil { - return hcloud.LoadBalancerAddServiceOpts{}, fmt.Errorf("%s: %w", op, err) + if spec.TypeUnset { + utils.WarnEventLogf( + l.Recorder, + svc, + "LoadBalancerTypeUnconfigured", + "Load Balancer Type unconfigured: this will be required in the future, set it with the annotation %q or cluster-wide with the environment variable %q", + annotation.LBType, + config.HcloudLoadBalancersType, + ) } - opts := hcloud.LoadBalancerAddServiceOpts{ - ListenPort: new(b.listenPort), - DestinationPort: new(b.destinationPort), - Protocol: b.protocol, - Proxyprotocol: b.proxyProtocol, - } - if b.addHTTP { - opts.HTTP = &hcloud.LoadBalancerAddServiceOptsHTTP{ - CookieName: b.httpOpts.CookieName, - CookieLifetime: b.httpOpts.CookieLifetime, - Certificates: b.httpOpts.Certificates, - RedirectHTTP: b.httpOpts.RedirectHTTP, - StickySessions: b.httpOpts.StickySessions, - TimeoutIdle: b.httpOpts.TimeoutIdle, - } - } - if b.addHealthCheck { - port := b.healthCheckOpts.Port - if port == nil { - port = new(b.destinationPort) - } - opts.HealthCheck = &hcloud.LoadBalancerAddServiceOptsHealthCheck{ - Protocol: b.healthCheckOpts.Protocol, - Interval: b.healthCheckOpts.Interval, - Port: port, - Retries: b.healthCheckOpts.Retries, - Timeout: b.healthCheckOpts.Timeout, - } - if b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTP || - b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTPS { - opts.HealthCheck.HTTP = &hcloud.LoadBalancerAddServiceOptsHealthCheckHTTP{ - Domain: b.healthCheckOpts.httpOpts.Domain, - Path: b.healthCheckOpts.httpOpts.Path, - Response: b.healthCheckOpts.httpOpts.Response, - StatusCodes: b.healthCheckOpts.httpOpts.StatusCodes, - TLS: b.healthCheckOpts.httpOpts.TLS, - } - } - } else { - opts.HealthCheck = &hcloud.LoadBalancerAddServiceOptsHealthCheck{ - Protocol: hcloud.LoadBalancerServiceProtocolTCP, - Port: new(b.destinationPort), - } + lbType, err := l.LBTypeCache.ByName(ctx, spec.Type) + if err != nil { + return nil, err } - return opts, nil -} - -func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUpdateServiceOpts, error) { - const op = "hcops/hclbServiceOptsBuilder.buildUpdateServiceOpts" - metrics.OperationCalled.WithLabelValues(op).Inc() - - if err := b.initialize(); err != nil { - return hcloud.LoadBalancerUpdateServiceOpts{}, fmt.Errorf("%s: %w", op, err) + if lbType == nil { + return nil, fmt.Errorf("load balancer type not found: %s", spec.Type) } - opts := hcloud.LoadBalancerUpdateServiceOpts{ - DestinationPort: new(b.destinationPort), - Protocol: b.protocol, - Proxyprotocol: b.proxyProtocol, - } - if b.addHTTP { - opts.HTTP = &hcloud.LoadBalancerUpdateServiceOptsHTTP{ - CookieName: b.httpOpts.CookieName, - CookieLifetime: b.httpOpts.CookieLifetime, - RedirectHTTP: b.httpOpts.RedirectHTTP, - Certificates: b.httpOpts.Certificates, - StickySessions: b.httpOpts.StickySessions, - TimeoutIdle: b.httpOpts.TimeoutIdle, - } + msg, unavailable := deprecationutil.LoadBalancerTypeMessage(lbType) + if unavailable { + return nil, errors.New(msg) } - if b.addHealthCheck { - port := b.healthCheckOpts.Port - if port == nil { - port = new(b.destinationPort) - } - opts.HealthCheck = &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{ - Protocol: b.healthCheckOpts.Protocol, - Interval: b.healthCheckOpts.Interval, - Port: port, - Retries: b.healthCheckOpts.Retries, - Timeout: b.healthCheckOpts.Timeout, - } - if b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTP || - b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTPS { - opts.HealthCheck.HTTP = &hcloud.LoadBalancerUpdateServiceOptsHealthCheckHTTP{ - Domain: b.healthCheckOpts.httpOpts.Domain, - Path: b.healthCheckOpts.httpOpts.Path, - Response: b.healthCheckOpts.httpOpts.Response, - StatusCodes: b.healthCheckOpts.httpOpts.StatusCodes, - TLS: b.healthCheckOpts.httpOpts.TLS, - } - } - } else { - opts.HealthCheck = &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{ - Protocol: hcloud.LoadBalancerServiceProtocolTCP, - Port: new(b.destinationPort), - } + if msg != "" { + utils.WarnEventLogf( + l.Recorder, + svc, + "LoadBalancerTypeDeprecated", + "%s", msg, + ) } - return opts, nil + return lbType, nil } -func lbAttached(lb *hcloud.LoadBalancer, nwID int64, privateIPv4 string) bool { +func lbAttached(lb *hcloud.LoadBalancer, nwID int64, privateIPv4 netip.Addr) bool { for _, nw := range lb.PrivateNet { - if nw.Network.ID == nwID && (privateIPv4 == "" || privateIPv4 == nw.IP.String()) { + if nw.Network.ID == nwID && (!privateIPv4.IsValid() || sameIP(privateIPv4, nw.IP)) { return true } } return false } +// sameIP reports whether addr and ip are the same address. Both sides are +// unmapped first, so that an address written as ::ffff:10.0.0.1 equals the +// same address written as 10.0.0.1, as net.IP.Equal did. +func sameIP(addr netip.Addr, ip net.IP) bool { + other, ok := netip.AddrFromSlice(ip) + return ok && addr.Unmap() == other.Unmap() +} + func getNodeInternalIP(node *corev1.Node) string { for _, addr := range node.Status.Addresses { if addr.Type == corev1.NodeInternalIP { diff --git a/internal/hcops/load_balancer_internal_test.go b/internal/hcops/load_balancer_internal_test.go index f8a25593b..272757bed 100644 --- a/internal/hcops/load_balancer_internal_test.go +++ b/internal/hcops/load_balancer_internal_test.go @@ -1,6 +1,7 @@ package hcops import ( + "context" "fmt" "maps" "testing" @@ -8,17 +9,23 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) -func TestHCLBServiceOptsBuilder(t *testing.T) { +// TestBuildServiceOpts covers the path from the annotations of a Service to the +// options sent to the API: [lbspec.Resolve], the certificate lookups and the +// two opts builders. +func TestBuildServiceOpts(t *testing.T) { type testCase struct { name string servicePort corev1.ServicePort @@ -241,7 +248,7 @@ func TestHCLBServiceOptsBuilder(t *testing.T) { tt.certClient. On("AllWithOpts", mock.Anything, hcloud.CertificateListOpts{ ListOpts: hcloud.ListOpts{ - LabelSelector: fmt.Sprintf("%s=%s", LabelServiceUID, "some-service-uid"), + LabelSelector: fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, "some-service-uid"), }, }). Return([]*hcloud.Certificate{{ID: 1}}, nil, nil) @@ -441,24 +448,27 @@ func TestHCLBServiceOptsBuilder(t *testing.T) { tt.mock(t, &tt) } - builder := &hclbServiceOptsBuilder{ - Port: tt.servicePort, - Service: &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(tt.serviceUID), - Annotations: map[string]string{}, - }, + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(tt.serviceUID), + Annotations: map[string]string{}, }, + } + maps.Copy(svc.Annotations, tt.serviceAnnotations) + + spec, err := lbspec.Resolve(record.NewFakeRecorder(10), svc, tt.cfg) + require.NoError(t, err) + + lbOps := &LoadBalancerOps{ CertOps: &CertificateOps{ActionClient: tt.actionClient, CertClient: tt.certClient}, - cfg: tt.cfg, } - maps.Copy(builder.Service.Annotations, tt.serviceAnnotations) - addOpts, err := builder.buildAddServiceOpts() - assert.NoError(t, err) + certificates, err := lbOps.resolveCertificates(context.Background(), svc, spec) + require.NoError(t, err) + + addOpts := spec.Service.AddServiceOpts(tt.servicePort, certificates) assert.Equal(t, tt.expectedAddOpts, addOpts) - updateOpts, err := builder.buildUpdateServiceOpts() - assert.NoError(t, err) + updateOpts := spec.Service.UpdateServiceOpts(tt.servicePort, certificates) assert.Equal(t, tt.expectedUpdateOpts, updateOpts) }) } diff --git a/internal/hcops/load_balancer_test.go b/internal/hcops/load_balancer_test.go index ac38c68a8..36a22da13 100644 --- a/internal/hcops/load_balancer_test.go +++ b/internal/hcops/load_balancer_test.go @@ -19,6 +19,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -205,7 +206,7 @@ func TestGetByK8SServiceUID(t *testing.T) { opts := hcloud.LoadBalancerListOpts{ ListOpts: hcloud.ListOpts{ - LabelSelector: fmt.Sprintf("%s=%s", hcops.LabelServiceUID, tt.uid), + LabelSelector: fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, tt.uid), }, } fx.LBClient. @@ -259,6 +260,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, }, serviceAnnotations: map[string]string{ + string(annotation.LBName): "some-lb", string(annotation.LBLocation): "fsn1", }, createOpts: hcloud.LoadBalancerCreateOpts{ @@ -268,7 +270,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { Name: "fsn1", }, Labels: map[string]string{ - hcops.LabelServiceUID: "some-lb-uid", + lbspec.LabelServiceUID: "some-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 1}, @@ -281,6 +283,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, }, serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBNetworkZone): "eu-central", }, createOpts: hcloud.LoadBalancerCreateOpts{ @@ -288,7 +291,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 2}, @@ -300,6 +303,9 @@ func TestLoadBalancerOps_Create(t *testing.T) { Location: "fsn1", }, }, + serviceAnnotations: map[string]string{ + string(annotation.LBName): "some-lb", + }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "some-lb", LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, @@ -307,7 +313,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { Name: "fsn1", }, Labels: map[string]string{ - hcops.LabelServiceUID: "some-lb-uid", + lbspec.LabelServiceUID: "some-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 3}, @@ -319,12 +325,15 @@ func TestLoadBalancerOps_Create(t *testing.T) { NetworkZone: "eu-central", }, }, + serviceAnnotations: map[string]string{ + string(annotation.LBName): "some-lb", + }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "some-lb", LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ - hcops.LabelServiceUID: "some-lb-uid", + lbspec.LabelServiceUID: "some-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 4}, @@ -337,6 +346,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, }, serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBLocation): "", string(annotation.LBNetworkZone): "eu-central", }, @@ -345,7 +355,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 2}, @@ -358,6 +368,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, }, serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBLocation): "fsn1", string(annotation.LBNetworkZone): "", }, @@ -368,11 +379,28 @@ func TestLoadBalancerOps_Create(t *testing.T) { Name: "fsn1", }, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 2}, }, + { + name: "create derives the name from the service uid", + serviceAnnotations: map[string]string{ + string(annotation.LBLocation): "fsn1", + }, + createOpts: hcloud.LoadBalancerCreateOpts{ + Name: "asomelbuid", + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, + Location: &hcloud.Location{ + Name: "fsn1", + }, + Labels: map[string]string{ + lbspec.LabelServiceUID: "some-lb-uid", + }, + }, + lb: &hcloud.LoadBalancer{ID: 1}, + }, { name: "fails if location and network zone missing", serviceAnnotations: map[string]string{}, @@ -382,6 +410,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { { name: "gives preference to location name", serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBLocation): "nbg1", string(annotation.LBNetworkZone): "eu-central", }, @@ -390,7 +419,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 2}, @@ -398,6 +427,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { { name: "set Load Balancer type name", serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBType): "lb21", string(annotation.LBLocation): "nbg1", }, @@ -406,7 +436,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 3}, @@ -414,6 +444,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { { name: "set Load Balancer algorithm type", serviceAnnotations: map[string]string{ + string(annotation.LBName): "another-lb", string(annotation.LBLocation): "nbg1", string(annotation.LBAlgorithmType): "least_connections", }, @@ -423,7 +454,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { Location: &hcloud.Location{Name: "nbg1"}, Algorithm: &hcloud.LoadBalancerAlgorithm{Type: hcloud.LoadBalancerAlgorithmTypeLeastConnections}, Labels: map[string]string{ - hcops.LabelServiceUID: "another-lb-uid", + lbspec.LabelServiceUID: "another-lb-uid", }, }, lb: &hcloud.LoadBalancer{ID: 4}, @@ -436,12 +467,15 @@ func TestLoadBalancerOps_Create(t *testing.T) { Type: "lb21", }, }, + serviceAnnotations: map[string]string{ + string(annotation.LBName): "lb-default-type", + }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "lb-default-type", LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ - hcops.LabelServiceUID: "lb-default-type-uid", + lbspec.LabelServiceUID: "lb-default-type-uid", }, }, lb: &hcloud.LoadBalancer{ID: 7}, @@ -454,28 +488,24 @@ func TestLoadBalancerOps_Create(t *testing.T) { DisablePublicNetwork: new(true), }, }, + serviceAnnotations: map[string]string{ + string(annotation.LBName): "lb-disable-public", + }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "lb-disable-public", LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, PublicInterface: new(false), Labels: map[string]string{ - hcops.LabelServiceUID: "lb-disable-public-uid", + lbspec.LabelServiceUID: "lb-disable-public-uid", }, }, lb: &hcloud.LoadBalancer{ID: 8}, }, - { - name: "fail on invalid Load Balancer algorithm type", - serviceAnnotations: map[string]string{ - string(annotation.LBLocation): "nbg1", - string(annotation.LBAlgorithmType): "invalidType", - }, - err: fmt.Errorf("hcops/LoadBalancerOps.Create: load-balancer.hetzner.cloud/algorithm-type: invalid: invalidType"), - }, { name: "disable public interface", serviceAnnotations: map[string]string{ + string(annotation.LBName): "lb-with-priv", string(annotation.LBLocation): "nbg1", string(annotation.LBDisablePublicNetwork): "true", }, @@ -485,7 +515,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { Location: &hcloud.Location{Name: "nbg1"}, PublicInterface: new(false), Labels: map[string]string{ - hcops.LabelServiceUID: "lb-with-priv-uid", + lbspec.LabelServiceUID: "lb-with-priv-uid", }, }, mock: func(_ *testing.T, tt *testCase, fx *hcops.LoadBalancerOpsFixture) { @@ -517,13 +547,13 @@ func TestLoadBalancerOps_Create(t *testing.T) { service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(tt.createOpts.Labels[hcops.LabelServiceUID]), + UID: types.UID(tt.createOpts.Labels[lbspec.LabelServiceUID]), Annotations: map[string]string{}, }, } maps.Copy(service.Annotations, tt.serviceAnnotations) - lb, err := fx.LBOps.Create(fx.Ctx, tt.createOpts.Name, service) + lb, err := fx.LBOps.Create(fx.Ctx, service, fx.ResolveSpec(service)) if tt.err != nil { assert.EqualError(t, err, tt.err.Error()) } else { @@ -587,6 +617,7 @@ type LBReconcilementTestCase struct { // set during test execution service *corev1.Service + spec lbspec.Spec fx *hcops.LoadBalancerOpsFixture } @@ -606,6 +637,7 @@ func (tt *LBReconcilementTestCase) run(t *testing.T) { } } maps.Copy(tt.service.Annotations, tt.serviceAnnotations) + tt.spec = tt.fx.ResolveSpec(tt.service) if tt.mock != nil { tt.mock(t, tt) } @@ -639,32 +671,11 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, }, - { - name: "update to invalid algorithm", - serviceAnnotations: map[string]string{ - string(annotation.LBAlgorithmType): "invalidType", - }, - initialLB: &hcloud.LoadBalancer{ - ID: 2, - Algorithm: hcloud.LoadBalancerAlgorithm{ - Type: hcloud.LoadBalancerAlgorithmTypeRoundRobin, - }, - PublicNet: hcloud.LoadBalancerPublicNet{ - Enabled: true, - }, - }, - perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) - assert.EqualError(t, err, - "hcops/LoadBalancerOps.ReconcileHCLB: hcops/LoadBalancerOps.changeAlgorithm: load-balancer.hetzner.cloud/algorithm-type: invalid: invalidType") - assert.False(t, changed) - }, - }, { name: "don't update unchanged algorithm", serviceAnnotations: map[string]string{ @@ -680,7 +691,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -711,7 +722,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -744,7 +755,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -764,7 +775,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -784,7 +795,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -811,7 +822,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -831,7 +842,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -858,7 +869,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -885,7 +896,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -917,7 +928,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { attachOpts := hcloud.LoadBalancerAttachToNetworkOpts{ Network: nw, - IP: net.ParseIP("10.10.10.2"), + IP: net.ParseIP("10.10.10.2").To4(), } detachAction := &hcloud.Action{ID: rand.Int63()} @@ -930,7 +941,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, attachAction).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -952,7 +963,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.LBOps.NetworkID = tt.initialLB.PrivateNet[0].Network.ID }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -977,7 +988,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -999,13 +1010,13 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.LBOps.NetworkID = nw.ID - opts := hcloud.LoadBalancerAttachToNetworkOpts{Network: nw, IP: net.ParseIP("10.10.10.2")} + opts := hcloud.LoadBalancerAttachToNetworkOpts{Network: nw, IP: net.ParseIP("10.10.10.2").To4()} action := &hcloud.Action{ID: rand.Int63()} tt.fx.LBClient.On("AttachToNetwork", tt.fx.Ctx, tt.initialLB, opts).Return(action, nil, nil) tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1039,7 +1050,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1073,7 +1084,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1095,7 +1106,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.LBOps.NetworkID = tt.initialLB.PrivateNet[0].Network.ID }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -1119,7 +1130,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1145,7 +1156,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1162,7 +1173,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -1186,7 +1197,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1203,7 +1214,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) }, @@ -1223,13 +1234,13 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { mock: func(_ *testing.T, tt *LBReconcilementTestCase) { updated := *tt.initialLB updated.Labels = map[string]string{ - hcops.LabelServiceUID: tt.serviceUID, - "some-label": "some-value", + lbspec.LabelServiceUID: tt.serviceUID, + "some-label": "some-value", } opts := hcloud.LoadBalancerUpdateOpts{ Labels: map[string]string{ - hcops.LabelServiceUID: tt.serviceUID, - "some-label": "some-value", + lbspec.LabelServiceUID: tt.serviceUID, + "some-label": "some-value", }, } tt.fx.LBClient. @@ -1237,10 +1248,10 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { Return(&updated, nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) - assert.Equal(t, tt.serviceUID, tt.initialLB.Labels[hcops.LabelServiceUID]) + assert.Equal(t, tt.serviceUID, tt.initialLB.Labels[lbspec.LabelServiceUID]) assert.Equal(t, "some-value", tt.initialLB.Labels["some-label"]) }, }, @@ -1250,8 +1261,8 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { initialLB: &hcloud.LoadBalancer{ ID: 12, Labels: map[string]string{ - hcops.LabelServiceUID: "stale-uid", - "some-label": "some-value", + lbspec.LabelServiceUID: "stale-uid", + "some-label": "some-value", }, PublicNet: hcloud.LoadBalancerPublicNet{ Enabled: true, @@ -1260,13 +1271,13 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { mock: func(_ *testing.T, tt *LBReconcilementTestCase) { updated := *tt.initialLB updated.Labels = map[string]string{ - hcops.LabelServiceUID: tt.serviceUID, - "some-label": "some-value", + lbspec.LabelServiceUID: tt.serviceUID, + "some-label": "some-value", } opts := hcloud.LoadBalancerUpdateOpts{ Labels: map[string]string{ - hcops.LabelServiceUID: tt.serviceUID, - "some-label": "some-value", + lbspec.LabelServiceUID: tt.serviceUID, + "some-label": "some-value", }, } tt.fx.LBClient. @@ -1274,15 +1285,15 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { Return(&updated, nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) - assert.Equal(t, tt.serviceUID, tt.initialLB.Labels[hcops.LabelServiceUID]) + assert.Equal(t, tt.serviceUID, tt.initialLB.Labels[lbspec.LabelServiceUID]) assert.Equal(t, "some-value", tt.initialLB.Labels["some-label"]) // The stale label must actually be replaced, otherwise every // reconcile issues the same update again. - changed, err = tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err = tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) tt.fx.LBClient.AssertNumberOfCalls(t, "Update", 1) @@ -1298,7 +1309,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { ID: 11, Name: "old-name", Labels: map[string]string{ - hcops.LabelServiceUID: "11", + lbspec.LabelServiceUID: "11", }, PublicNet: hcloud.LoadBalancerPublicNet{ Enabled: true, @@ -1313,7 +1324,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { Return(&updated, nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) assert.Equal(t, "new-name", tt.initialLB.Name) @@ -1376,7 +1387,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.MockListRobotServers(tt.robotServers, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1441,7 +1452,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.MockListRobotServers(tt.robotServers, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1470,7 +1481,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { // will fail if an action would be taken instead. }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.False(t, changed) }, @@ -1498,7 +1509,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { // Nothing to mock because no action will be taken besides emitting an event }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.False(t, changed) }, @@ -1536,7 +1547,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.MockListRobotServers(nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1577,7 +1588,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.MockListRobotServers(nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1624,7 +1635,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.MockListRobotServers(nil, nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1683,7 +1694,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) }, @@ -1716,7 +1727,7 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { tt.fx.LBOps.NetworkID = 4711 }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes) + changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.False(t, changed) }, @@ -1743,7 +1754,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { }, }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - _, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + _, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) }, }, @@ -1785,7 +1796,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1822,7 +1833,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { }) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.EqualError(t, err, "hcops/LoadBalancerOps.ReconcileHCLBServices: invalid input in field 'http' (invalid_input): "+ "invalid fields: http.timeout_idle (must be between 5 and 3600)") @@ -1862,7 +1873,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1903,7 +1914,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -1925,7 +1936,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { Name: "ccm-managed-certificate-some service uid", Type: hcloud.CertificateTypeManaged, DomainNames: []string{"example.com", "*.example.com"}, - Labels: map[string]string{hcops.LabelServiceUID: tt.serviceUID}, + Labels: map[string]string{lbspec.LabelServiceUID: tt.serviceUID}, }). Return(hcloud.CertificateCreateResult{Certificate: cert}, nil, nil) @@ -1935,7 +1946,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { mock.Anything, hcloud.CertificateListOpts{ ListOpts: hcloud.ListOpts{ - LabelSelector: fmt.Sprintf("%s=%s", hcops.LabelServiceUID, tt.serviceUID), + LabelSelector: fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, tt.serviceUID), }, }). Return([]*hcloud.Certificate{cert}, nil, nil) @@ -1956,7 +1967,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -2008,7 +2019,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, @@ -2050,7 +2061,7 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) { tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { - changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service) + changed, err := tt.fx.LBOps.ReconcileHCLBServices(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.True(t, changed) }, diff --git a/internal/hcops/mocks.go b/internal/hcops/mocks.go index 77deff677..6dc5e1e00 100644 --- a/internal/hcops/mocks.go +++ b/internal/hcops/mocks.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/mock" corev1 "k8s.io/api/core/v1" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -25,9 +26,9 @@ func (m *MockLoadBalancerOps) GetByID(ctx context.Context, id int64) (*hcloud.Lo } func (m *MockLoadBalancerOps) Create( - ctx context.Context, lbName string, service *corev1.Service, + ctx context.Context, service *corev1.Service, spec lbspec.Spec, ) (*hcloud.LoadBalancer, error) { - args := m.Called(ctx, lbName, service) + args := m.Called(ctx, service, spec) return mocks.GetLoadBalancerPtr(args, 0), args.Error(1) } @@ -37,23 +38,23 @@ func (m *MockLoadBalancerOps) Delete(ctx context.Context, lb *hcloud.LoadBalance } func (m *MockLoadBalancerOps) ReconcileHCLB( - ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, ) (bool, error) { - args := m.Called(ctx, lb, svc) + args := m.Called(ctx, lb, svc, spec) return args.Bool(0), args.Error(1) } func (m *MockLoadBalancerOps) ReconcileHCLBTargets( - ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, nodes []*corev1.Node, + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, nodes []*corev1.Node, ) (bool, error) { - args := m.Called(ctx, lb, svc, nodes) + args := m.Called(ctx, lb, svc, spec, nodes) return args.Bool(0), args.Error(1) } func (m *MockLoadBalancerOps) ReconcileHCLBServices( - ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, + ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service, spec lbspec.Spec, ) (bool, error) { - args := m.Called(ctx, lb, svc) + args := m.Called(ctx, lb, svc, spec) return args.Bool(0), args.Error(1) } diff --git a/internal/hcops/same_ip_test.go b/internal/hcops/same_ip_test.go new file mode 100644 index 000000000..5f719604d --- /dev/null +++ b/internal/hcops/same_ip_test.go @@ -0,0 +1,61 @@ +package hcops + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSameIP(t *testing.T) { + tests := []struct { + name string + addr netip.Addr + ip net.IP + expected bool + }{ + { + name: "the zero value equals nothing", + addr: netip.Addr{}, + ip: net.ParseIP("10.0.0.1"), + expected: false, + }, + { + name: "the zero value does not equal a missing IP either", + addr: netip.Addr{}, + ip: nil, + expected: false, + }, + { + name: "same address", + addr: netip.MustParseAddr("10.0.0.1"), + ip: net.ParseIP("10.0.0.1"), + expected: true, + }, + { + name: "same address written as IPv4-in-IPv6", + addr: netip.MustParseAddr("::ffff:10.0.0.1"), + ip: net.ParseIP("10.0.0.1"), + expected: true, + }, + { + name: "different address", + addr: netip.MustParseAddr("10.0.0.1"), + ip: net.ParseIP("10.0.0.2"), + expected: false, + }, + { + name: "a missing IP equals no address", + addr: netip.MustParseAddr("10.0.0.1"), + ip: nil, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.expected, sameIP(test.addr, test.ip)) + }) + } +} diff --git a/internal/hcops/testing.go b/internal/hcops/testing.go index b95d9cefb..363d1e4e8 100644 --- a/internal/hcops/testing.go +++ b/internal/hcops/testing.go @@ -10,9 +10,11 @@ import ( "time" hrobotmodels "github.com/syself/hrobot-go/models" + corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -87,6 +89,19 @@ func newLBTypeCacheFixture(t *testing.T) *cache.Cache[hcloud.LoadBalancerType] { return cache.NewLoadBalancerTypeCache(client, cache.ModeAll, time.Minute) } +// ResolveSpec resolves the desired Load Balancer state for svc, the way the +// cloud provider does before it calls into [LoadBalancerOps]. Invalid +// annotations fail the test. +func (fx *LoadBalancerOpsFixture) ResolveSpec(svc *corev1.Service) lbspec.Spec { + fx.T.Helper() + + spec, err := lbspec.Resolve(fx.LBOps.Recorder, svc, fx.LBOps.Cfg.LoadBalancer) + if err != nil { + fx.T.Fatalf("resolve Load Balancer spec: %v", err) + } + return spec +} + func (fx *LoadBalancerOpsFixture) MockGetByID(lb *hcloud.LoadBalancer, err error) { fx.LBClient.On("GetByID", fx.Ctx, lb.ID).Return(lb, nil, err) } diff --git a/internal/lbspec/opts.go b/internal/lbspec/opts.go new file mode 100644 index 000000000..f13836298 --- /dev/null +++ b/internal/lbspec/opts.go @@ -0,0 +1,225 @@ +package lbspec + +import ( + "maps" + "net" + "net/netip" + + corev1 "k8s.io/api/core/v1" + + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +// labelUseStagingCA tells the backend to issue managed certificates through the +// Let's Encrypt staging environment. +const labelUseStagingCA = "HC-Use-Staging-CA" + +func (s Spec) CreateOpts(lbType *hcloud.LoadBalancerType) hcloud.LoadBalancerCreateOpts { + opts := hcloud.LoadBalancerCreateOpts{ + Name: s.Name, + LoadBalancerType: lbType, + Labels: s.Labels, + NetworkZone: s.NetworkZone, + PublicInterface: s.PublicInterface, + } + + if s.Location != "" { + opts.Location = &hcloud.Location{Name: s.Location} + } + if s.Algorithm != "" { + opts.Algorithm = &hcloud.LoadBalancerAlgorithm{Type: s.Algorithm} + } + + return opts +} + +func (s Spec) UpdateOpts(lb *hcloud.LoadBalancer) (hcloud.LoadBalancerUpdateOpts, bool) { + var ( + opts hcloud.LoadBalancerUpdateOpts + update bool + ) + + if !hasLabels(lb.Labels, s.Labels) { + labels := make(map[string]string, len(lb.Labels)+len(s.Labels)) + maps.Copy(labels, lb.Labels) + maps.Copy(labels, s.Labels) + + opts.Labels = labels + update = true + } + + if !s.NameUnset && s.Name != lb.Name { + opts.Name = s.Name + update = true + } + + return opts, update +} + +func (s Spec) ChangeAlgorithmOpts() hcloud.LoadBalancerChangeAlgorithmOpts { + return hcloud.LoadBalancerChangeAlgorithmOpts{Type: s.Algorithm} +} + +func (s Spec) AttachToNetworkOpts(network *hcloud.Network) hcloud.LoadBalancerAttachToNetworkOpts { + return hcloud.LoadBalancerAttachToNetworkOpts{ + Network: network, + IP: toIP(s.PrivateIPv4), + IPRange: toIPNet(s.PrivateSubnetIPRange), + } +} + +func (s Spec) AddServerTargetOpts(serverID int64) hcloud.LoadBalancerAddServerTargetOpts { + return hcloud.LoadBalancerAddServerTargetOpts{ + Server: &hcloud.Server{ID: serverID}, + UsePrivateIP: new(s.UsePrivateIP), + } +} + +func (c OwnedCertificate) CreateOpts() hcloud.CertificateCreateOpts { + labels := c.Labels + if c.UseACMEStaging { + // A copy, so that requesting staging does not leave the label behind + // in the spec. + labels = make(map[string]string, len(c.Labels)+1) + maps.Copy(labels, c.Labels) + labels[labelUseStagingCA] = "true" + } + + return hcloud.CertificateCreateOpts{ + Name: c.Name, + Type: hcloud.CertificateTypeManaged, + DomainNames: c.Domains, + Labels: labels, + } +} + +func toIP(addr netip.Addr) net.IP { + if !addr.IsValid() { + return nil + } + return addr.AsSlice() +} + +func toIPNet(prefix netip.Prefix) *net.IPNet { + if !prefix.IsValid() { + return nil + } + return &net.IPNet{ + IP: prefix.Addr().AsSlice(), + Mask: net.CIDRMask(prefix.Bits(), prefix.Addr().BitLen()), + } +} + +// hasLabels reports whether actual carries every label in required. +func hasLabels(actual, required map[string]string) bool { + for k, v := range required { + if actual[k] != v { + return false + } + } + return true +} + +func (s ServiceSpec) AddServiceOpts( + port corev1.ServicePort, certificates []*hcloud.Certificate, +) hcloud.LoadBalancerAddServiceOpts { + destinationPort := int(port.NodePort) + + opts := hcloud.LoadBalancerAddServiceOpts{ + ListenPort: new(int(port.Port)), + DestinationPort: new(destinationPort), + Protocol: s.Protocol, + Proxyprotocol: s.ProxyProtocol, + HealthCheck: s.addHealthCheckOpts(destinationPort), + } + + if s.HTTP != nil { + opts.HTTP = &hcloud.LoadBalancerAddServiceOptsHTTP{ + CookieName: s.HTTP.CookieName, + CookieLifetime: s.HTTP.CookieLifetime, + Certificates: certificates, + RedirectHTTP: s.HTTP.RedirectHTTP, + StickySessions: s.HTTP.StickySessions, + TimeoutIdle: s.HTTP.TimeoutIdle, + } + } + + return opts +} + +func (s ServiceSpec) addHealthCheckOpts(destinationPort int) *hcloud.LoadBalancerAddServiceOptsHealthCheck { + if s.HealthCheck == nil { + return &hcloud.LoadBalancerAddServiceOptsHealthCheck{ + Protocol: hcloud.LoadBalancerServiceProtocolTCP, + Port: new(destinationPort), + } + } + + port := s.HealthCheck.Port + if port == nil { + port = new(destinationPort) + } + + opts := &hcloud.LoadBalancerAddServiceOptsHealthCheck{ + Protocol: s.HealthCheck.Protocol, + Interval: s.HealthCheck.Interval, + Port: port, + Retries: s.HealthCheck.Retries, + Timeout: s.HealthCheck.Timeout, + } + + if s.HealthCheck.Protocol == hcloud.LoadBalancerServiceProtocolHTTP || + s.HealthCheck.Protocol == hcloud.LoadBalancerServiceProtocolHTTPS { + opts.HTTP = &hcloud.LoadBalancerAddServiceOptsHealthCheckHTTP{ + Domain: s.HealthCheck.HTTP.Domain, + Path: s.HealthCheck.HTTP.Path, + StatusCodes: s.HealthCheck.HTTP.StatusCodes, + TLS: s.HealthCheck.HTTP.TLS, + } + } + + return opts +} + +func (s ServiceSpec) UpdateServiceOpts( + port corev1.ServicePort, certificates []*hcloud.Certificate, +) hcloud.LoadBalancerUpdateServiceOpts { + add := s.AddServiceOpts(port, certificates) + + opts := hcloud.LoadBalancerUpdateServiceOpts{ + DestinationPort: add.DestinationPort, + Protocol: add.Protocol, + Proxyprotocol: add.Proxyprotocol, + } + + if add.HTTP != nil { + opts.HTTP = &hcloud.LoadBalancerUpdateServiceOptsHTTP{ + CookieName: add.HTTP.CookieName, + CookieLifetime: add.HTTP.CookieLifetime, + Certificates: add.HTTP.Certificates, + RedirectHTTP: add.HTTP.RedirectHTTP, + StickySessions: add.HTTP.StickySessions, + TimeoutIdle: add.HTTP.TimeoutIdle, + } + } + + if add.HealthCheck != nil { + opts.HealthCheck = &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{ + Protocol: add.HealthCheck.Protocol, + Interval: add.HealthCheck.Interval, + Port: add.HealthCheck.Port, + Retries: add.HealthCheck.Retries, + Timeout: add.HealthCheck.Timeout, + } + if add.HealthCheck.HTTP != nil { + opts.HealthCheck.HTTP = &hcloud.LoadBalancerUpdateServiceOptsHealthCheckHTTP{ + Domain: add.HealthCheck.HTTP.Domain, + Path: add.HealthCheck.HTTP.Path, + StatusCodes: add.HealthCheck.HTTP.StatusCodes, + TLS: add.HealthCheck.HTTP.TLS, + } + } + } + + return opts +} diff --git a/internal/lbspec/opts_test.go b/internal/lbspec/opts_test.go new file mode 100644 index 000000000..b9069599d --- /dev/null +++ b/internal/lbspec/opts_test.go @@ -0,0 +1,219 @@ +package lbspec_test + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +func TestSpecCreateOpts(t *testing.T) { + lbType := &hcloud.LoadBalancerType{ID: 2, Name: "lb21"} + labels := map[string]string{lbspec.LabelServiceUID: "some-uid"} + + t.Run("in a location", func(t *testing.T) { + spec := lbspec.Spec{Name: "some-lb", Labels: labels, Location: "fsn1"} + + opts := spec.CreateOpts(lbType) + + assert.Equal(t, hcloud.LoadBalancerCreateOpts{ + Name: "some-lb", + LoadBalancerType: lbType, + Labels: labels, + Location: &hcloud.Location{Name: "fsn1"}, + }, opts) + }) + + t.Run("in a network zone", func(t *testing.T) { + spec := lbspec.Spec{Name: "some-lb", Labels: labels, NetworkZone: hcloud.NetworkZoneEUCentral} + + opts := spec.CreateOpts(lbType) + + assert.Nil(t, opts.Location) + assert.Equal(t, hcloud.NetworkZoneEUCentral, opts.NetworkZone) + }) + + t.Run("with an algorithm and a disabled public interface", func(t *testing.T) { + spec := lbspec.Spec{ + Name: "some-lb", + Labels: labels, + Location: "fsn1", + Algorithm: hcloud.LoadBalancerAlgorithmTypeLeastConnections, + PublicInterface: new(false), + } + + opts := spec.CreateOpts(lbType) + + assert.Equal(t, &hcloud.LoadBalancerAlgorithm{ + Type: hcloud.LoadBalancerAlgorithmTypeLeastConnections, + }, opts.Algorithm) + assert.Equal(t, new(false), opts.PublicInterface) + }) + + t.Run("unconfigured settings are left to the API", func(t *testing.T) { + spec := lbspec.Spec{Name: "some-lb", Labels: labels, Location: "fsn1"} + + opts := spec.CreateOpts(lbType) + + assert.Nil(t, opts.Algorithm, "an unconfigured algorithm is not sent") + assert.Nil(t, opts.PublicInterface, "an unconfigured public interface is not sent") + }) +} + +func TestSpecUpdateOpts(t *testing.T) { + required := map[string]string{lbspec.LabelServiceUID: "new-uid"} + + t.Run("nothing to update", func(t *testing.T) { + lb := &hcloud.LoadBalancer{ + Name: "some-lb", + Labels: map[string]string{lbspec.LabelServiceUID: "new-uid"}, + } + + _, update := lbspec.Spec{Name: "some-lb", Labels: required}.UpdateOpts(lb) + + assert.False(t, update) + }) + + t.Run("an unset name leaves the name alone", func(t *testing.T) { + lb := &hcloud.LoadBalancer{ + Name: "imported-lb", + Labels: map[string]string{lbspec.LabelServiceUID: "new-uid"}, + } + + opts, update := lbspec.Spec{Name: "derived-name", NameUnset: true, Labels: required}.UpdateOpts(lb) + + assert.False(t, update) + assert.Empty(t, opts.Name) + }) + + t.Run("renames the Load Balancer", func(t *testing.T) { + lb := &hcloud.LoadBalancer{ + Name: "old-name", + Labels: map[string]string{lbspec.LabelServiceUID: "new-uid"}, + } + + opts, update := lbspec.Spec{Name: "new-name", Labels: required}.UpdateOpts(lb) + + require.True(t, update) + assert.Equal(t, "new-name", opts.Name) + assert.Nil(t, opts.Labels, "the labels are already correct") + }) + + t.Run("adds the required labels to a Load Balancer without any", func(t *testing.T) { + lb := &hcloud.LoadBalancer{Name: "some-lb"} + + opts, update := lbspec.Spec{Name: "some-lb", Labels: required}.UpdateOpts(lb) + + require.True(t, update) + assert.Equal(t, map[string]string{lbspec.LabelServiceUID: "new-uid"}, opts.Labels) + }) + + t.Run("replaces a stale label and keeps the others", func(t *testing.T) { + // A Service that was deleted and recreated keeps its Load Balancer, which + // then carries the UID of the previous Service. The required label has to + // win, or every reconcile writes the stale value back. + lb := &hcloud.LoadBalancer{ + Name: "some-lb", + Labels: map[string]string{ + lbspec.LabelServiceUID: "old-uid", + "team": "keep-me", + }, + } + + opts, update := lbspec.Spec{Name: "some-lb", Labels: required}.UpdateOpts(lb) + + require.True(t, update) + assert.Equal(t, map[string]string{ + lbspec.LabelServiceUID: "new-uid", + "team": "keep-me", + }, opts.Labels) + assert.Equal(t, "old-uid", lb.Labels[lbspec.LabelServiceUID], + "the Load Balancer is only updated once the API call succeeds") + }) +} + +func TestSpecChangeAlgorithmOpts(t *testing.T) { + spec := lbspec.Spec{Algorithm: hcloud.LoadBalancerAlgorithmTypeLeastConnections} + + assert.Equal(t, hcloud.LoadBalancerChangeAlgorithmOpts{ + Type: hcloud.LoadBalancerAlgorithmTypeLeastConnections, + }, spec.ChangeAlgorithmOpts()) +} + +func TestSpecAttachToNetworkOpts(t *testing.T) { + network := &hcloud.Network{ID: 4711} + + t.Run("without an address or subnet", func(t *testing.T) { + opts := lbspec.Spec{}.AttachToNetworkOpts(network) + + assert.Equal(t, network, opts.Network) + assert.Nil(t, opts.IP, "the API picks an address") + assert.Nil(t, opts.IPRange) + }) + + t.Run("with an address and a subnet", func(t *testing.T) { + spec := lbspec.Spec{ + PrivateIPv4: netip.MustParseAddr("10.0.1.5"), + PrivateSubnetIPRange: netip.MustParsePrefix("10.0.1.0/24"), + } + + opts := spec.AttachToNetworkOpts(network) + + assert.True(t, net.ParseIP("10.0.1.5").Equal(opts.IP)) + require.NotNil(t, opts.IPRange) + assert.Equal(t, "10.0.1.0/24", opts.IPRange.String()) + }) +} + +func TestSpecAddServerTargetOpts(t *testing.T) { + for _, usePrivateIP := range []bool{true, false} { + spec := lbspec.Spec{UsePrivateIP: usePrivateIP} + + opts := spec.AddServerTargetOpts(42) + + assert.Equal(t, &hcloud.Server{ID: 42}, opts.Server) + assert.Equal(t, new(usePrivateIP), opts.UsePrivateIP) + } +} + +func TestOwnedCertificateCreateOpts(t *testing.T) { + t.Run("certificate for the configured domains", func(t *testing.T) { + cert := lbspec.OwnedCertificate{ + Name: "some-cert", + Labels: map[string]string{lbspec.LabelServiceUID: "some-uid"}, + Domains: []string{"example.com", "*.example.com"}, + } + + opts := cert.CreateOpts() + + assert.Equal(t, hcloud.CertificateCreateOpts{ + Name: "some-cert", + Type: hcloud.CertificateTypeManaged, + DomainNames: []string{"example.com", "*.example.com"}, + Labels: map[string]string{lbspec.LabelServiceUID: "some-uid"}, + }, opts) + }) + + t.Run("ACME staging is requested through a label", func(t *testing.T) { + cert := lbspec.OwnedCertificate{ + Name: "some-cert", + Labels: map[string]string{lbspec.LabelServiceUID: "some-uid"}, + Domains: []string{"example.com"}, + UseACMEStaging: true, + } + + opts := cert.CreateOpts() + + assert.Equal(t, map[string]string{ + lbspec.LabelServiceUID: "some-uid", + "HC-Use-Staging-CA": "true", + }, opts.Labels) + assert.Equal(t, map[string]string{lbspec.LabelServiceUID: "some-uid"}, cert.Labels, + "the labels of the spec are not modified") + }) +} diff --git a/internal/lbspec/solver.go b/internal/lbspec/solver.go new file mode 100644 index 000000000..97230d707 --- /dev/null +++ b/internal/lbspec/solver.go @@ -0,0 +1,303 @@ +package lbspec + +import ( + "errors" + "fmt" + "net/netip" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/tools/record" + cloudprovider "k8s.io/cloud-provider" + "k8s.io/klog/v2" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +// Name returns the effective Load Balancer name for svc: the requested name if +// the annotation is set, otherwise the name derived from the Service UID. +func Name(svc *corev1.Service) string { + if v, err := annotation.LBName.FromService(svc); err == nil && v != "" { + return v + } + return cloudprovider.DefaultLoadBalancerName(svc) +} + +// Resolve builds the desired state from the Service annotations and the +// cluster-wide Load Balancer configuration. Annotations take precedence over +// the configuration, which takes precedence over the built-in defaults. +// +// All invalid annotations are reported together in the returned error. The +// returned Spec is still fully populated, with the offending settings left at +// their fallback, so callers must not use it when err is non-nil. +func Resolve(recorder record.EventRecorder, svc *corev1.Service, cfg config.LoadBalancerConfiguration) (Spec, error) { + var errs []error + var spec Spec + + spec.Name = resolve(&errs, svc, annotation.LBName, "") + if spec.Name == "" { + spec.Name = cloudprovider.DefaultLoadBalancerName(svc) + spec.NameUnset = true + } + + spec.Hostname = resolve(&errs, svc, annotation.LBHostname, "") + spec.NodeSelector = resolveNodeSelector(&errs, svc) + spec.Labels = map[string]string{ + LabelServiceUID: string(svc.ObjectMeta.UID), + } + + spec.Type = resolve(&errs, svc, annotation.LBType, cfg.Type) + if spec.Type == "" { + spec.Type = DefaultType + spec.TypeUnset = true + } + + spec.Location = resolve(&errs, svc, annotation.LBLocation, cfg.Location) + spec.NetworkZone = hcloud.NetworkZone(resolve(&errs, svc, annotation.LBNetworkZone, cfg.NetworkZone)) + if spec.Location != "" { + spec.NetworkZone = "" + } + + spec.Algorithm = resolve(&errs, svc, annotation.LBAlgorithmType, cfg.AlgorithmType) + spec.PublicInterface = negate(resolvePtr(&errs, svc, annotation.LBDisablePublicNetwork, cfg.DisablePublicNetwork)) + spec.IPv4RDNS = resolvePtr(&errs, svc, annotation.LBPublicIPv4RDNS, nil) + spec.IPv6RDNS = resolvePtr(&errs, svc, annotation.LBPublicIPv6RDNS, nil) + spec.PrivateIPv4 = resolve(&errs, svc, annotation.LBPrivateIPv4, netip.Addr{}) + spec.PrivateSubnetIPRange = resolvePrivateSubnetIPRange(&errs, svc, cfg) + spec.UsePrivateIP = resolve(&errs, svc, annotation.LBUsePrivateIP, cfg.PrivateIPEnabled) + spec.PrivateIngress = !resolve(&errs, svc, annotation.LBDisablePrivateIngress, !cfg.PrivateIngressEnabled) + spec.IPv6 = !resolve(&errs, svc, annotation.LBIPv6Disabled, !cfg.IPv6Enabled) + spec.OwnedCertificate = resolveOwnedCertificate(&errs, svc) + spec.Service = resolveService(&errs, svc, cfg, spec.OwnedCertificate != nil) + + if len(errs) == 0 { + return spec, nil + } + + for _, err := range errs { + klog.ErrorS( + err, + "invalid Load Balancer annotation", + "service", + klog.KObj(svc), + ) + recorder.Eventf( + svc, + corev1.EventTypeWarning, + "InvalidLoadBalancerAnnotation", + "Invalid Load Balancer annotation %v", + err, + ) + } + + return spec, fmt.Errorf("%d Load Balancer annotation(s) are invalid", len(errs)) +} + +func resolveNodeSelector(errs *[]error, svc *corev1.Service) labels.Selector { + v := resolvePtr(errs, svc, annotation.LBNodeSelector, nil) + if v == nil { + return labels.Everything() + } + + selector, err := labels.Parse(*v) + if err != nil { + *errs = append(*errs, fmt.Errorf("%s: unable to parse the node-selector annotation: %w", + annotation.LBNodeSelector, err)) + return labels.Everything() + } + + return selector +} + +func resolvePrivateSubnetIPRange(errs *[]error, svc *corev1.Service, cfg config.LoadBalancerConfiguration) netip.Prefix { + value := resolvePtr(errs, svc, annotation.PrivateSubnetIPRange, nil) + if value == nil { + if cfg.PrivateSubnetIPRange == "" { + return netip.Prefix{} + } + value = &cfg.PrivateSubnetIPRange + } + + subnet, err := netip.ParsePrefix(*value) + if err != nil { + *errs = append(*errs, fmt.Errorf("invalid private subnet IP range %q: %w", *value, err)) + return netip.Prefix{} + } + + // The annotation names a subnet, so host bits carry no meaning. + return subnet.Masked() +} + +func resolveOwnedCertificate(errs *[]error, svc *corev1.Service) *OwnedCertificate { + // Compared as a raw string: only the exact value asks us to provide the + // certificate. + if typ, err := annotation.LBSvcHTTPCertificateType.FromService(svc); err != nil || typ != string(hcloud.CertificateTypeManaged) { + return nil + } + + cert := OwnedCertificate{ + Name: fmt.Sprintf("ccm-managed-certificate-%s", svc.ObjectMeta.UID), + Labels: map[string]string{LabelServiceUID: string(svc.ObjectMeta.UID)}, + } + if v, err := annotation.LBSvcHTTPManagedCertificateName.FromService(svc); err == nil && v != "" { + cert.Name = v + } + + domains, err := annotation.LBSvcHTTPManagedCertificateDomains.FromService(svc) + if err != nil { + *errs = append(*errs, fmt.Errorf("%s: no domains for managed certificate", + annotation.LBSvcHTTPManagedCertificateDomains)) + return nil + } + cert.Domains = domains + + // The error is ignored on purpose: we are only interested in whether the + // annotation is set and parses as a truthy boolean. Anything else tells us + // not to use ACME staging. + cert.UseACMEStaging, _ = annotation.LBSvcHTTPManagedCertificateUseACMEStaging.FromService(svc) + + return &cert +} + +func resolveService( + errs *[]error, svc *corev1.Service, cfg config.LoadBalancerConfiguration, hasOwnedCertificate bool, +) ServiceSpec { + var spec ServiceSpec + + spec.Protocol = resolve(errs, svc, annotation.LBSvcProtocol, hcloud.LoadBalancerServiceProtocolTCP) + spec.ProxyProtocol = resolvePtr(errs, svc, annotation.LBSvcProxyProtocol, cfg.ProxyProtocolEnabled) + + var http HTTPSpec + var httpConfigured bool + + if v := resolvePtr(errs, svc, annotation.LBSvcHTTPCookieName, nil); v != nil { + http.CookieName = v + httpConfigured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHTTPCookieLifetime, nil); v != nil { + http.CookieLifetime = v + httpConfigured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHTTPTimeoutIdle, nil); v != nil { + http.TimeoutIdle = v + httpConfigured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcRedirectHTTP, nil); v != nil { + http.RedirectHTTP = v + httpConfigured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHTTPStickySessions, nil); v != nil { + http.StickySessions = v + httpConfigured = true + } + + if hasOwnedCertificate { + // Our own certificate replaces the certificate list, so that + // annotation is not read at all. The caller fills in the certificate + // it looked up by label. + httpConfigured = true + } else { + certs := resolve(errs, svc, annotation.LBSvcHTTPCertificates, nil) + if len(certs) > 0 { + http.Certificates = certs + httpConfigured = true + } + } + + if httpConfigured { + spec.HTTP = &http + } + spec.HealthCheck = resolveHealthCheck(errs, svc, cfg, spec.Protocol) + + return spec +} + +func resolveHealthCheck( + errs *[]error, + svc *corev1.Service, + cfg config.LoadBalancerConfiguration, + serviceProtocol hcloud.LoadBalancerServiceProtocol, +) *HealthCheckSpec { + // Without an explicit health check protocol the service protocol is used, + // but that alone does not configure a health check. + check := HealthCheckSpec{Protocol: serviceProtocol} + var configured bool + + if v := resolvePtr(errs, svc, annotation.LBSvcHealthCheckProtocol, nil); v != nil { + check.Protocol = *v + configured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHealthCheckPort, nil); v != nil { + check.Port = v + configured = true + } + // The cluster-wide defaults only apply when set to a non-zero value. + if v := resolvePtr(errs, svc, annotation.LBSvcHealthCheckInterval, nonZero(cfg.HealthCheckInterval)); v != nil { + check.Interval = v + configured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHealthCheckTimeout, nonZero(cfg.HealthCheckTimeout)); v != nil { + check.Timeout = v + configured = true + } + if v := resolvePtr(errs, svc, annotation.LBSvcHealthCheckRetries, nonZero(cfg.HealthCheckRetries)); v != nil { + check.Retries = v + configured = true + } + + // A TCP health check has no HTTP options. + if check.Protocol != hcloud.LoadBalancerServiceProtocolTCP { + check.HTTP.Domain = resolvePtr(errs, svc, annotation.LBSvcHealthCheckHTTPDomain, nil) + check.HTTP.Path = resolvePtr(errs, svc, annotation.LBSvcHealthCheckHTTPPath, nil) + check.HTTP.TLS = resolvePtr(errs, svc, annotation.LBSvcHealthCheckHTTPValidateCertificate, nil) + check.HTTP.StatusCodes = resolve(errs, svc, annotation.LBSvcHealthCheckHTTPStatusCodes, nil) + } + + if !configured { + return nil + } + return &check +} + +func resolve[T any](errs *[]error, svc *corev1.Service, a annotation.Annotation[T], fallback T) T { + v, err := a.FromService(svc) + switch { + case err == nil: + return v + case errors.Is(err, annotation.ErrNotSet): + return fallback + default: + *errs = append(*errs, err) + return fallback + } +} + +func resolvePtr[T any](errs *[]error, svc *corev1.Service, a annotation.Annotation[T], fallback *T) *T { + v, err := a.FromService(svc) + switch { + case err == nil: + return &v + case errors.Is(err, annotation.ErrNotSet): + return fallback + default: + *errs = append(*errs, err) + return fallback + } +} + +func nonZero[T comparable](v T) *T { + var zero T + if v == zero { + return nil + } + return &v +} + +func negate(v *bool) *bool { + if v == nil { + return nil + } + return new(!*v) +} diff --git a/internal/lbspec/solver_test.go b/internal/lbspec/solver_test.go new file mode 100644 index 000000000..ad56ec7e4 --- /dev/null +++ b/internal/lbspec/solver_test.go @@ -0,0 +1,435 @@ +package lbspec_test + +import ( + "maps" + "net/netip" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +func service(uid string, annotations map[string]string) *corev1.Service { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(uid), + Annotations: make(map[string]string, len(annotations)), + }, + } + maps.Copy(svc.Annotations, annotations) + return svc +} + +func TestResolve(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + cfg config.LoadBalancerConfiguration + check func(t *testing.T, spec lbspec.Spec) + }{ + { + name: "defaults without annotations or configuration", + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "asomeuid", spec.Name, "derived from the service uid") + assert.True(t, spec.NameUnset) + assert.Empty(t, spec.Hostname) + assert.True(t, spec.NodeSelector.Empty(), "selects every Node") + assert.Equal(t, map[string]string{lbspec.LabelServiceUID: "some-uid"}, spec.Labels) + assert.Equal(t, lbspec.DefaultType, spec.Type) + assert.True(t, spec.TypeUnset) + assert.Empty(t, spec.Location) + assert.Empty(t, spec.NetworkZone) + assert.Empty(t, spec.Algorithm, "an unconfigured algorithm is left alone") + assert.Nil(t, spec.PublicInterface, "an unconfigured public interface is left alone") + assert.Nil(t, spec.IPv4RDNS) + assert.Nil(t, spec.IPv6RDNS) + assert.False(t, spec.PrivateIPv4.IsValid()) + assert.False(t, spec.PrivateSubnetIPRange.IsValid()) + assert.False(t, spec.UsePrivateIP) + assert.Nil(t, spec.OwnedCertificate) + assert.Equal(t, hcloud.LoadBalancerServiceProtocolTCP, spec.Service.Protocol) + assert.Nil(t, spec.Service.ProxyProtocol) + assert.Nil(t, spec.Service.HTTP, "no HTTP block without HTTP options") + assert.Nil(t, spec.Service.HealthCheck, "no health check without health check options") + }, + }, + { + name: "name and hostname are separate settings", + annotations: map[string]string{ + string(annotation.LBName): "my-lb", + string(annotation.LBHostname): "lb.example.com", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "my-lb", spec.Name) + assert.False(t, spec.NameUnset) + assert.Equal(t, "lb.example.com", spec.Hostname) + }, + }, + { + name: "annotations take precedence over the configuration", + cfg: config.LoadBalancerConfiguration{ + Type: "lb21", + Location: "hel1", + AlgorithmType: hcloud.LoadBalancerAlgorithmTypeRoundRobin, + PrivateIPEnabled: true, + }, + annotations: map[string]string{ + string(annotation.LBType): "lb31", + string(annotation.LBLocation): "fsn1", + string(annotation.LBAlgorithmType): "least_connections", + string(annotation.LBUsePrivateIP): "false", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "lb31", spec.Type) + assert.False(t, spec.TypeUnset) + assert.Equal(t, "fsn1", spec.Location) + assert.Equal(t, hcloud.LoadBalancerAlgorithmTypeLeastConnections, spec.Algorithm) + assert.False(t, spec.UsePrivateIP) + }, + }, + { + name: "configuration applies without annotations", + cfg: config.LoadBalancerConfiguration{ + Type: "lb21", + NetworkZone: "eu-central", + AlgorithmType: hcloud.LoadBalancerAlgorithmTypeLeastConnections, + DisablePublicNetwork: new(true), + PrivateSubnetIPRange: "10.0.0.0/24", + PrivateIPEnabled: true, + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "lb21", spec.Type) + assert.False(t, spec.TypeUnset) + assert.Equal(t, hcloud.NetworkZone("eu-central"), spec.NetworkZone) + assert.Equal(t, hcloud.LoadBalancerAlgorithmTypeLeastConnections, spec.Algorithm) + assert.Equal(t, new(false), spec.PublicInterface, "DISABLE_PUBLIC_NETWORK=true means disabled") + assert.Equal(t, "10.0.0.0/24", spec.PrivateSubnetIPRange.String()) + assert.True(t, spec.UsePrivateIP) + }, + }, + { + name: "an empty annotation resets a configured location", + cfg: config.LoadBalancerConfiguration{ + Location: "hel1", + }, + annotations: map[string]string{ + string(annotation.LBLocation): "", + string(annotation.LBNetworkZone): "eu-central", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Empty(t, spec.Location) + assert.Equal(t, hcloud.NetworkZone("eu-central"), spec.NetworkZone) + }, + }, + { + name: "a location wins over a network zone", + annotations: map[string]string{ + string(annotation.LBLocation): "nbg1", + string(annotation.LBNetworkZone): "eu-central", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "nbg1", spec.Location) + assert.Empty(t, spec.NetworkZone, "the API accepts only one of them") + }, + }, + { + name: "disable annotations are resolved as enabled settings", + cfg: config.LoadBalancerConfiguration{ + PrivateIngressEnabled: true, + IPv6Enabled: true, + }, + annotations: map[string]string{ + string(annotation.LBDisablePublicNetwork): "true", + string(annotation.LBDisablePrivateIngress): "true", + string(annotation.LBIPv6Disabled): "true", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, new(false), spec.PublicInterface) + assert.False(t, spec.PrivateIngress) + assert.False(t, spec.IPv6) + }, + }, + { + name: "reverse DNS records tell an empty value from an unset one", + annotations: map[string]string{ + string(annotation.LBPublicIPv4RDNS): "", + string(annotation.LBPublicIPv6RDNS): "lb.example.com", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, new(""), spec.IPv4RDNS, "an empty record is a value, not an omission") + assert.Equal(t, new("lb.example.com"), spec.IPv6RDNS) + }, + }, + { + name: "private network addressing is parsed", + annotations: map[string]string{ + string(annotation.LBPrivateIPv4): "10.0.1.5", + string(annotation.PrivateSubnetIPRange): "10.0.1.0/24", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, netip.MustParseAddr("10.0.1.5"), spec.PrivateIPv4) + assert.Equal(t, "10.0.1.0/24", spec.PrivateSubnetIPRange.String()) + }, + }, + { + name: "node selector is parsed", + annotations: map[string]string{ + string(annotation.LBNodeSelector): "environment=production", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, "environment=production", spec.NodeSelector.String()) + }, + }, + { + name: "HTTP options configure the HTTP block", + annotations: map[string]string{ + string(annotation.LBSvcProtocol): "http", + string(annotation.LBSvcHTTPCookieName): "my-cookie", + string(annotation.LBSvcHTTPCookieLifetime): "1h", + string(annotation.LBSvcHTTPTimeoutIdle): "30s", + string(annotation.LBSvcRedirectHTTP): "true", + string(annotation.LBSvcHTTPStickySessions): "true", + string(annotation.LBSvcHTTPCertificates): "1,some-cert", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Equal(t, hcloud.LoadBalancerServiceProtocolHTTP, spec.Service.Protocol) + require.NotNil(t, spec.Service.HTTP) + assert.Equal(t, new("my-cookie"), spec.Service.HTTP.CookieName) + assert.Equal(t, new(time.Hour), spec.Service.HTTP.CookieLifetime) + assert.Equal(t, new(30*time.Second), spec.Service.HTTP.TimeoutIdle) + assert.Equal(t, new(true), spec.Service.HTTP.RedirectHTTP) + assert.Equal(t, new(true), spec.Service.HTTP.StickySessions) + assert.Equal(t, []*hcloud.Certificate{{ID: 1}, {Name: "some-cert"}}, + spec.Service.HTTP.Certificates, "references are kept as written") + }, + }, + { + name: "a managed certificate defaults its name to the service uid", + annotations: map[string]string{ + string(annotation.LBSvcHTTPCertificateType): "managed", + string(annotation.LBSvcHTTPManagedCertificateDomains): "example.com,*.example.com", + string(annotation.LBSvcHTTPCertificates): "ignored", + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.OwnedCertificate) + assert.Equal(t, "ccm-managed-certificate-some-uid", spec.OwnedCertificate.Name) + assert.Equal(t, []string{"example.com", "*.example.com"}, spec.OwnedCertificate.Domains) + assert.False(t, spec.OwnedCertificate.UseACMEStaging) + + require.NotNil(t, spec.Service.HTTP, "the certificate is attached to an HTTP service") + assert.Empty(t, spec.Service.HTTP.Certificates, + "the managed certificate is looked up by label, and the list annotation is ignored") + }, + }, + { + name: "a managed certificate can be named and use ACME staging", + annotations: map[string]string{ + string(annotation.LBSvcHTTPCertificateType): "managed", + string(annotation.LBSvcHTTPManagedCertificateName): "my-cert", + string(annotation.LBSvcHTTPManagedCertificateDomains): "example.com", + string(annotation.LBSvcHTTPManagedCertificateUseACMEStaging): "true", + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.OwnedCertificate) + assert.Equal(t, "my-cert", spec.OwnedCertificate.Name) + assert.True(t, spec.OwnedCertificate.UseACMEStaging) + }, + }, + { + name: "an uploaded certificate type is not a managed certificate", + annotations: map[string]string{ + string(annotation.LBSvcHTTPCertificateType): "uploaded", + string(annotation.LBSvcHTTPCertificates): "1", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Nil(t, spec.OwnedCertificate) + require.NotNil(t, spec.Service.HTTP) + assert.Equal(t, []*hcloud.Certificate{{ID: 1}}, spec.Service.HTTP.Certificates) + }, + }, + { + name: "health check options configure a health check", + annotations: map[string]string{ + string(annotation.LBSvcHealthCheckProtocol): "http", + string(annotation.LBSvcHealthCheckPort): "8080", + string(annotation.LBSvcHealthCheckInterval): "1h", + string(annotation.LBSvcHealthCheckTimeout): "30s", + string(annotation.LBSvcHealthCheckRetries): "5", + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.Service.HealthCheck) + assert.Equal(t, hcloud.LoadBalancerServiceProtocolHTTP, spec.Service.HealthCheck.Protocol) + assert.Equal(t, new(8080), spec.Service.HealthCheck.Port) + assert.Equal(t, new(time.Hour), spec.Service.HealthCheck.Interval) + assert.Equal(t, new(30*time.Second), spec.Service.HealthCheck.Timeout) + assert.Equal(t, new(5), spec.Service.HealthCheck.Retries) + }, + }, + { + name: "cluster-wide health check defaults configure a health check", + cfg: config.LoadBalancerConfiguration{ + HealthCheckInterval: 30 * time.Second, + HealthCheckTimeout: 5 * time.Second, + HealthCheckRetries: 5, + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.Service.HealthCheck) + assert.Equal(t, hcloud.LoadBalancerServiceProtocolTCP, spec.Service.HealthCheck.Protocol) + assert.Equal(t, new(30*time.Second), spec.Service.HealthCheck.Interval) + assert.Equal(t, new(5*time.Second), spec.Service.HealthCheck.Timeout) + assert.Equal(t, new(5), spec.Service.HealthCheck.Retries) + }, + }, + { + name: "an unset health check protocol follows the service protocol", + annotations: map[string]string{ + string(annotation.LBSvcProtocol): "https", + string(annotation.LBSvcHealthCheckPort): "8080", + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.Service.HealthCheck) + assert.Equal(t, hcloud.LoadBalancerServiceProtocolHTTPS, spec.Service.HealthCheck.Protocol) + }, + }, + { + name: "health check HTTP options alone do not configure a health check", + annotations: map[string]string{ + string(annotation.LBSvcHealthCheckHTTPPath): "/healthz", + }, + check: func(t *testing.T, spec lbspec.Spec) { + assert.Nil(t, spec.Service.HealthCheck, + "a TCP health check has no HTTP options, so the path alone is not enough") + }, + }, + { + name: "health check HTTP options are read for HTTP health checks", + annotations: map[string]string{ + string(annotation.LBSvcHealthCheckProtocol): "http", + string(annotation.LBSvcHealthCheckHTTPDomain): "example.com", + string(annotation.LBSvcHealthCheckHTTPPath): "/healthz", + string(annotation.LBSvcHealthCheckHTTPValidateCertificate): "true", + string(annotation.LBSvcHealthCheckHTTPStatusCodes): "200,202", + }, + check: func(t *testing.T, spec lbspec.Spec) { + require.NotNil(t, spec.Service.HealthCheck) + assert.Equal(t, new("example.com"), spec.Service.HealthCheck.HTTP.Domain) + assert.Equal(t, new("/healthz"), spec.Service.HealthCheck.HTTP.Path) + assert.Equal(t, new(true), spec.Service.HealthCheck.HTTP.TLS) + assert.Equal(t, []string{"200", "202"}, spec.Service.HealthCheck.HTTP.StatusCodes) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + spec, err := lbspec.Resolve(record.NewFakeRecorder(10), service("some-uid", tt.annotations), tt.cfg) + require.NoError(t, err) + tt.check(t, spec) + }) + } +} + +func TestResolveReportsEveryInvalidAnnotation(t *testing.T) { + svc := service("some-uid", map[string]string{ + string(annotation.LBAlgorithmType): "sideways", + string(annotation.LBUsePrivateIP): "maybe", + string(annotation.LBSvcHealthCheckRetries): "many", + string(annotation.LBPrivateIPv4): "not-an-ip", + string(annotation.PrivateSubnetIPRange): "not-a-cidr", + }) + + logs := testsupport.CaptureKlog(t) + recorder := record.NewFakeRecorder(10) + + _, err := lbspec.Resolve(recorder, svc, config.LoadBalancerConfiguration{}) + + // The error only counts the invalid annotations, the details are reported + // through the logs and the Kubernetes events. + require.EqualError(t, err, "5 Load Balancer annotation(s) are invalid") + + // One reconcile tells the user about all of their typos, not just the first. + for _, want := range []string{"sideways", "maybe", "many", "not-an-ip", "not-a-cidr"} { + assert.Contains(t, logs.String(), want) + } + + // Every invalid annotation gets its own event, so that a single event stays + // readable. + assert.Len(t, recorder.Events, 5) +} + +func TestResolveErrors(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + wantErr string + }{ + { + name: "invalid algorithm", + annotations: map[string]string{string(annotation.LBAlgorithmType): "sideways"}, + wantErr: "invalid algorithm type: sideways", + }, + { + name: "invalid service protocol", + annotations: map[string]string{string(annotation.LBSvcProtocol): "smtp"}, + wantErr: "invalid protocol: smtp", + }, + { + name: "invalid node selector", + annotations: map[string]string{string(annotation.LBNodeSelector): "environment=production=staging"}, + wantErr: "unable to parse the node-selector annotation", + }, + { + name: "invalid duration", + annotations: map[string]string{string(annotation.LBSvcHTTPTimeoutIdle): "30 fortnights"}, + wantErr: "30 fortnights", + }, + { + name: "managed certificate without domains", + annotations: map[string]string{ + string(annotation.LBSvcHTTPCertificateType): "managed", + }, + wantErr: "no domains for managed certificate", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logs := testsupport.CaptureKlog(t) + recorder := record.NewFakeRecorder(10) + + _, err := lbspec.Resolve(recorder, service("some-uid", tt.annotations), config.LoadBalancerConfiguration{}) + + require.EqualError(t, err, "1 Load Balancer annotation(s) are invalid") + + // The reason is only reported through the logs and the Kubernetes + // event. + assert.Contains(t, logs.String(), tt.wantErr) + require.Len(t, recorder.Events, 1) + event := <-recorder.Events + assert.Contains(t, event, "InvalidLoadBalancerAnnotation") + assert.Contains(t, event, tt.wantErr) + }) + } +} + +func TestName(t *testing.T) { + t.Run("derived from the service uid", func(t *testing.T) { + assert.Equal(t, "a0000000000000000000000000000000", lbspec.Name(service("0000000-0000-0000-0000-000000000000", nil))) + }) + + t.Run("from the annotation", func(t *testing.T) { + svc := service("some-uid", map[string]string{string(annotation.LBName): "my-lb"}) + assert.Equal(t, "my-lb", lbspec.Name(svc)) + }) +} diff --git a/internal/lbspec/spec.go b/internal/lbspec/spec.go new file mode 100644 index 000000000..af28f1f05 --- /dev/null +++ b/internal/lbspec/spec.go @@ -0,0 +1,160 @@ +// Package lbspec resolves the desired state of a Hetzner Cloud Load Balancer +// from the annotations of a Kubernetes Service and the cluster-wide +// configuration. +// +// Resolution is pure: it makes no API calls, emits no events and reads no +// clock. Anything that needs the Hetzner Cloud API - looking up a Load Balancer +// type by name, turning certificate names into IDs - is left to the caller, +// which receives the parsed intent and nothing else. +// +// Every invalid annotation is reported, not just the first one, so a user with +// two typos does not need two reconciles to learn about both. +package lbspec + +import ( + "net/netip" + "time" + + "k8s.io/apimachinery/pkg/labels" + + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +const DefaultType = "lb11" + +// LabelServiceUID is a label added to the Hetzner Cloud backend to uniquely +// identify a load balancer managed by Hetzner Cloud Cloud Controller Manager. +const LabelServiceUID = "hcloud-ccm/service-uid" + +// Spec is the desired state of the Hetzner Cloud Load Balancer for a Service. +// +// Pointer fields distinguish "not configured anywhere" (nil) from a configured +// value, so we never ask the API to change something the user never set. +type Spec struct { + // Name is the effective Load Balancer name, never empty. + Name string + // NameUnset reports that Name is derived from the Service UID because no + // name was requested. An unrequested name is not applied to an existing + // Load Balancer, so that one created by other means keeps its own. + NameUnset bool + // Labels are the labels the Load Balancer must carry. + Labels map[string]string + // Hostname is published as the ingress address instead of the IPs when set. + Hostname string + // NodeSelector restricts which Nodes become targets. Never nil. + NodeSelector labels.Selector + + // Type is the Load Balancer type name, never empty. + Type string + // TypeUnset reports that Type holds [DefaultType] because nothing + // configured it. An unconfigured type is not applied to an existing Load + // Balancer, to avoid downgrading it. + TypeUnset bool + + Location string + NetworkZone hcloud.NetworkZone + + // Algorithm is empty when unconfigured, in which case it is left alone. + Algorithm hcloud.LoadBalancerAlgorithmType + // PublicInterface reports whether the public interface should be enabled. + // Nil when unconfigured, in which case it is left alone. + PublicInterface *bool + // IPv4RDNS and IPv6RDNS are nil when unconfigured. The empty string is a + // valid value and resets the record. + IPv4RDNS *string + IPv6RDNS *string + + // PrivateIPv4 is the address the Load Balancer should have in the private + // network. The zero value when unconfigured. + PrivateIPv4 netip.Addr + // PrivateSubnetIPRange is the existing subnet to attach to. The zero value + // when unconfigured. + PrivateSubnetIPRange netip.Prefix + // UsePrivateIP makes server targets use their private IP. + UsePrivateIP bool + // PrivateIngress publishes the private IPs as ingress addresses. + PrivateIngress bool + // IPv6 publishes the public IPv6 address as an ingress address. + IPv6 bool + + // Service applies to every port of the Kubernetes Service. Every HTTP and + // health check annotation is read from the Service, so only the listen and + // destination ports differ per port. + Service ServiceSpec + + // OwnedCertificate is set when the Service asks the cloud controller + // manager to provide the certificate itself. The certificate is created + // and looked up by the caller. + OwnedCertificate *OwnedCertificate +} + +// ServiceSpec is the desired state of the services exposed by the Load +// Balancer. +type ServiceSpec struct { + Protocol hcloud.LoadBalancerServiceProtocol + ProxyProtocol *bool + + // HTTP is nil unless at least one HTTP option is configured, in which case + // no HTTP block is sent to the API at all. + HTTP *HTTPSpec + // HealthCheck is nil unless at least one health check option is + // configured, in which case a TCP check against the destination port is + // used. + HealthCheck *HealthCheckSpec +} + +// HTTPSpec holds the HTTP options of a Load Balancer service. +type HTTPSpec struct { + CookieName *string + CookieLifetime *time.Duration + TimeoutIdle *time.Duration + RedirectHTTP *bool + StickySessions *bool + + // Certificates reference certificates either by ID or by name, exactly as + // the annotation spelled them. Resolving names to IDs needs the API and is + // left to the caller. Certificates that already exist are referenced here + // whatever their type, managed ones included. Empty for an + // [OwnedCertificate], which the caller looks up by label instead. + Certificates []*hcloud.Certificate +} + +// HealthCheckSpec holds the health check options of a Load Balancer service. +type HealthCheckSpec struct { + // Protocol defaults to the service protocol when no health check protocol + // is configured. + Protocol hcloud.LoadBalancerServiceProtocol + // Port is nil when unconfigured, in which case the destination port is + // checked. + Port *int + Interval *time.Duration + Timeout *time.Duration + Retries *int + + // HTTP is only applied for the HTTP and HTTPS protocols, so it is a value: + // its options can all be unset while the block itself is still sent. + HTTP HealthCheckHTTPSpec +} + +// HealthCheckHTTPSpec holds the HTTP options of a health check. +type HealthCheckHTTPSpec struct { + Domain *string + Path *string + TLS *bool + StatusCodes []string +} + +// OwnedCertificate is a certificate the cloud controller manager creates on +// behalf of the Service and owns for its lifetime. +// +// It is of the Hetzner Cloud "managed" certificate type, so the backend renews +// it, but not every managed certificate is an OwnedCertificate: one created +// outside the cluster is referenced by ID or name through +// [HTTPSpec.Certificates] like an uploaded one. +type OwnedCertificate struct { + Name string + Labels map[string]string + Domains []string + // UseACMEStaging is for Hetzner internal testing only. + UseACMEStaging bool +} diff --git a/internal/testsupport/klog.go b/internal/testsupport/klog.go new file mode 100644 index 000000000..5b0e5354e --- /dev/null +++ b/internal/testsupport/klog.go @@ -0,0 +1,33 @@ +package testsupport + +import ( + "bytes" + "testing" + + "k8s.io/klog/v2" +) + +// KlogCapture holds the log output collected by [CaptureKlog]. +type KlogCapture struct { + buf *bytes.Buffer +} + +// String returns everything klog has logged so far. +func (c *KlogCapture) String() string { + klog.Flush() + return c.buf.String() +} + +// CaptureKlog redirects the klog output into a buffer for the duration of the test. +func CaptureKlog(t *testing.T) *KlogCapture { + t.Helper() + + state := klog.CaptureState() + t.Cleanup(state.Restore) + + var buf bytes.Buffer + klog.LogToStderr(false) + klog.SetOutput(&buf) + + return &KlogCapture{buf: &buf} +} diff --git a/internal/utils/eventlog_test.go b/internal/utils/eventlog_test.go index 262bdf9d7..099450f42 100644 --- a/internal/utils/eventlog_test.go +++ b/internal/utils/eventlog_test.go @@ -1,28 +1,15 @@ package utils import ( - "bytes" "testing" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - "k8s.io/klog/v2" -) - -func captureKlog(t *testing.T) *bytes.Buffer { - t.Helper() - - state := klog.CaptureState() - t.Cleanup(state.Restore) - var buf bytes.Buffer - klog.LogToStderr(false) - klog.SetOutput(&buf) - - return &buf -} + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" +) func TestWarnEventLogf(t *testing.T) { tests := []struct { @@ -56,14 +43,13 @@ func TestWarnEventLogf(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - logs := captureKlog(t) + logs := testsupport.CaptureKlog(t) recorder := record.NewFakeRecorder(1) WarnEventLogf(recorder, &corev1.Node{}, tt.reason, tt.msg, tt.args...) assert.Equal(t, "Warning "+tt.reason+" "+tt.expected, <-recorder.Events) - klog.Flush() // klog prefix `W` for warning assert.Regexp(t, `^W\d`, logs.String()) assert.Contains(t, logs.String(), tt.expected) @@ -72,7 +58,7 @@ func TestWarnEventLogf(t *testing.T) { } func TestWarnEventLogfEventObject(t *testing.T) { - captureKlog(t) + testsupport.CaptureKlog(t) recorder := record.NewFakeRecorder(1) recorder.IncludeObject = true diff --git a/tests/e2e/cloud_test.go b/tests/e2e/cloud_test.go index 0d12c92e1..b56f99dc4 100644 --- a/tests/e2e/cloud_test.go +++ b/tests/e2e/cloud_test.go @@ -18,7 +18,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/legacydatacenter" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -140,7 +140,7 @@ func TestServiceLoadBalancersHTTPSWithManagedCertificate(t *testing.T) { certs, err := testCluster.hcloud.Certificate.AllWithOpts(t.Context(), hcloud.CertificateListOpts{ ListOpts: hcloud.ListOpts{ - LabelSelector: fmt.Sprintf("%s=%s", hcops.LabelServiceUID, lbSvc.ObjectMeta.UID), + LabelSelector: fmt.Sprintf("%s=%s", lbspec.LabelServiceUID, lbSvc.ObjectMeta.UID), }, }) assert.NoError(t, err) diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index f4f1e7c54..fca6dd8a0 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -25,7 +25,7 @@ import ( "k8s.io/client-go/tools/clientcmd" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" "github.com/hetznercloud/hcloud-go/v2/hcloud" @@ -140,7 +140,7 @@ func (tc *TestCluster) Stop() error { ctx := context.Background() uids := tc.loadBalancers.All() - selector := fmt.Sprintf("%s in (%s)", hcops.LabelServiceUID, strings.Join(uids, ",")) + selector := fmt.Sprintf("%s in (%s)", lbspec.LabelServiceUID, strings.Join(uids, ",")) lbs, err := tc.hcloud.LoadBalancer.AllWithOpts(ctx, hcloud.LoadBalancerListOpts{ ListOpts: hcloud.ListOpts{ LabelSelector: selector,