Skip to content

Commit

Permalink
refactor: move config parsing & validation to seperate package
Browse files Browse the repository at this point in the history 
This significantly reduces the scope of the hcloud.newCloud() method,
and allows us to cleanly introduce new validations for the Robot
support.
  • Loading branch information
@apricote
apricote committed Oct 25, 2023
1 parent a9e3536 commit 575901b
Show file tree
Hide file tree
Showing 6 changed files with 496 additions and 260 deletions.
164 changes: 35 additions & 129 deletions hcloud/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,23 @@ package hcloud

import (
"context"
"errors"
"fmt"
"io"
"os"
"strconv"
"strings"

cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog/v2"

"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/metrics"
"github.com/hetznercloud/hcloud-go/v2/hcloud"
"github.com/hetznercloud/hcloud-go/v2/hcloud/metadata"
)

const (
hcloudTokenENVVar = "HCLOUD_TOKEN"
hcloudEndpointENVVar = "HCLOUD_ENDPOINT"
hcloudNetworkENVVar = "HCLOUD_NETWORK"
hcloudDebugENVVar = "HCLOUD_DEBUG"
// Disable the "master/server is attached to the network" check against the metadata service.
hcloudNetworkDisableAttachedCheckENVVar = "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK"
hcloudNetworkRoutesEnabledENVVar = "HCLOUD_NETWORK_ROUTES_ENABLED"
hcloudInstancesAddressFamily = "HCLOUD_INSTANCES_ADDRESS_FAMILY"
hcloudLoadBalancersEnabledENVVar = "HCLOUD_LOAD_BALANCERS_ENABLED"
hcloudLoadBalancersLocation = "HCLOUD_LOAD_BALANCERS_LOCATION"
hcloudLoadBalancersNetworkZone = "HCLOUD_LOAD_BALANCERS_NETWORK_ZONE"
hcloudLoadBalancersDisablePrivateIngress = "HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS"
hcloudLoadBalancersUsePrivateIP = "HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP"
hcloudLoadBalancersDisableIPv6 = "HCLOUD_LOAD_BALANCERS_DISABLE_IPV6"
hcloudMetricsEnabledENVVar = "HCLOUD_METRICS_ENABLED"
hcloudMetricsAddress = ":8233"
providerName = "hcloud"
providerName = "hcloud"
)

// providerVersion is set by the build process using -ldflags -X.
Expand All @@ -61,107 +44,96 @@ type cloud struct {
client *hcloud.Client
instances *instances
loadBalancer *loadBalancers
config config.HCCMConfiguration
networkID int64
}

func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
const op = "hcloud/newCloud"
metrics.OperationCalled.WithLabelValues(op).Inc()

token := os.Getenv(hcloudTokenENVVar)
if token == "" {
return nil, fmt.Errorf("environment variable %q is required", hcloudTokenENVVar)
configuration, err := config.Read()
if err != nil {
return nil, err
}
if len(token) != 64 {
return nil, fmt.Errorf("entered token is invalid (must be exactly 64 characters long)")
err = configuration.Validate()
if err != nil {
return nil, err
}

opts := []hcloud.ClientOption{
hcloud.WithToken(token),
hcloud.WithToken(configuration.HCloudClient.Token),
hcloud.WithApplication("hcloud-cloud-controller", providerVersion),
}

// start metrics server if enabled (enabled by default)
if os.Getenv(hcloudMetricsEnabledENVVar) != "false" {
go metrics.Serve(hcloudMetricsAddress)

if configuration.Metrics.Enabled {
go metrics.Serve(configuration.Metrics.Address)
opts = append(opts, hcloud.WithInstrumentation(metrics.GetRegistry()))
}

if os.Getenv(hcloudDebugENVVar) == "true" {
if configuration.HCloudClient.Debug {
opts = append(opts, hcloud.WithDebugWriter(os.Stderr))
}
if endpoint := os.Getenv(hcloudEndpointENVVar); endpoint != "" {
opts = append(opts, hcloud.WithEndpoint(endpoint))
if configuration.HCloudClient.Endpoint != "" {
opts = append(opts, hcloud.WithEndpoint(configuration.HCloudClient.Endpoint))
}
client := hcloud.NewClient(opts...)
metadataClient := metadata.NewClient()

var networkID int64
if v, ok := os.LookupEnv(hcloudNetworkENVVar); ok {
n, _, err := client.Network.Get(context.Background(), v)
if configuration.Network.NameOrID != "" {
n, _, err := client.Network.Get(context.Background(), configuration.Network.NameOrID)
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
}
if n == nil {
return nil, fmt.Errorf("%s: Network %s not found", op, v)
return nil, fmt.Errorf("%s: Network %s not found", op, configuration.Network.NameOrID)
}
networkID = n.ID

networkDisableAttachedCheck, err := getEnvBool(hcloudNetworkDisableAttachedCheckENVVar)
if err != nil {
return nil, fmt.Errorf("%s: checking if server is in Network not possible: %w", op, err)
}
if !networkDisableAttachedCheck {
e, err := serverIsAttachedToNetwork(metadataClient, networkID)
if !configuration.Network.DisableAttachedCheck {
attached, err := serverIsAttachedToNetwork(metadataClient, networkID)
if err != nil {
return nil, fmt.Errorf("%s: checking if server is in Network not possible: %w", op, err)
}
if !e {
return nil, fmt.Errorf("%s: This node is not attached to Network %s", op, v)
if !attached {
return nil, fmt.Errorf("%s: This node is not attached to Network %s", op, configuration.Network.NameOrID)
}
}
}
if networkID == 0 {
klog.Infof("%s: %s empty", op, hcloudNetworkENVVar)
}

// Validate that the provided token works, and we have network connectivity to the Hetzner Cloud API
_, _, err := client.Server.List(context.Background(), hcloud.ServerListOpts{})
_, _, err = client.Server.List(context.Background(), hcloud.ServerListOpts{})
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
}

lbOpsDefaults, lbDisablePrivateIngress, lbDisableIPv6, err := loadBalancerDefaultsFromEnv()
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
}

klog.Infof("Hetzner Cloud k8s cloud controller %s started\n", providerVersion)

lbOps := &hcops.LoadBalancerOps{
LBClient: &client.LoadBalancer,
CertOps: &hcops.CertificateOps{CertClient: &client.Certificate},
ActionClient: &client.Action,
NetworkClient: &client.Network,
NetworkID: networkID,
Defaults: lbOpsDefaults,
Defaults: hcops.LoadBalancerDefaults{
Location: configuration.LoadBalancer.Location,
NetworkZone: configuration.LoadBalancer.NetworkZone,
UsePrivateIP: configuration.LoadBalancer.UsePrivateIP,
},
}

loadBalancers := newLoadBalancers(lbOps, lbDisablePrivateIngress, lbDisableIPv6)
if os.Getenv(hcloudLoadBalancersEnabledENVVar) == "false" {
loadBalancers = nil
var loadBalancers *loadBalancers
if configuration.LoadBalancer.Enabled {
loadBalancers = newLoadBalancers(lbOps, configuration.LoadBalancer.DisablePrivateIngress, configuration.LoadBalancer.DisableIPv6)
}

instancesAddressFamily, err := addressFamilyFromEnv()
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
}
klog.Infof("Hetzner Cloud k8s cloud controller %s started\n", providerVersion)

return &cloud{
client: client,
instances: newInstances(client, instancesAddressFamily, networkID),
instances: newInstances(client, configuration.Instance.AddressFamily, networkID),
loadBalancer: loadBalancers,
config: configuration,
networkID: networkID,
}, nil
}
Expand Down Expand Up @@ -195,7 +167,7 @@ func (c *cloud) Clusters() (cloudprovider.Clusters, bool) {
}

func (c *cloud) Routes() (cloudprovider.Routes, bool) {
if c.networkID > 0 && os.Getenv(hcloudNetworkRoutesEnabledENVVar) != "false" {
if c.networkID > 0 && c.config.Route.Enabled {
r, err := newRoutes(c.client, c.networkID)
if err != nil {
klog.ErrorS(err, "create routes provider", "networkID", c.networkID)
Expand All @@ -214,35 +186,6 @@ func (c *cloud) HasClusterID() bool {
return false
}

func loadBalancerDefaultsFromEnv() (hcops.LoadBalancerDefaults, bool, bool, error) {
defaults := hcops.LoadBalancerDefaults{
Location: os.Getenv(hcloudLoadBalancersLocation),
NetworkZone: os.Getenv(hcloudLoadBalancersNetworkZone),
}

if defaults.Location != "" && defaults.NetworkZone != "" {
return defaults, false, false, errors.New(
"HCLOUD_LOAD_BALANCERS_LOCATION/HCLOUD_LOAD_BALANCERS_NETWORK_ZONE: Only one of these can be set")
}

disablePrivateIngress, err := getEnvBool(hcloudLoadBalancersDisablePrivateIngress)
if err != nil {
return defaults, false, false, err
}

disableIPv6, err := getEnvBool(hcloudLoadBalancersDisableIPv6)
if err != nil {
return defaults, false, false, err
}

defaults.UsePrivateIP, err = getEnvBool(hcloudLoadBalancersUsePrivateIP)
if err != nil {
return defaults, false, false, err
}

return defaults, disablePrivateIngress, disableIPv6, nil
}

// serverIsAttachedToNetwork checks if the server where the master is running on is attached to the configured private network
// We use this measurement to protect users against some parts of misconfiguration, like configuring a master in a not attached
// network.
Expand All @@ -257,43 +200,6 @@ func serverIsAttachedToNetwork(metadataClient *metadata.Client, networkID int64)
return strings.Contains(serverPrivateNetworks, fmt.Sprintf("network_id: %d\n", networkID)), nil
}

// addressFamilyFromEnv returns the address family for the instance address from the environment
// variable. Returns AddressFamilyIPv4 if unset.
func addressFamilyFromEnv() (addressFamily, error) {
family, ok := os.LookupEnv(hcloudInstancesAddressFamily)
if !ok {
return AddressFamilyIPv4, nil
}

switch strings.ToLower(family) {
case "ipv6":
return AddressFamilyIPv6, nil
case "ipv4":
return AddressFamilyIPv4, nil
case "dualstack":
return AddressFamilyDualStack, nil
default:
return -1, fmt.Errorf(
"%v: Invalid value, expected one of: ipv4,ipv6,dualstack", hcloudInstancesAddressFamily)
}
}

// getEnvBool returns the boolean parsed from the environment variable with the given key and a potential error
// parsing the var. Returns false if the env var is unset.
func getEnvBool(key string) (bool, error) {
v, ok := os.LookupEnv(key)
if !ok {
return false, nil
}

b, err := strconv.ParseBool(v)
if err != nil {
return false, fmt.Errorf("%s: %v", key, err)
}

return b, nil
}

func init() {
cloudprovider.RegisterCloudProvider(providerName, newCloud)
}
113 changes: 0 additions & 113 deletions hcloud/cloud_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/stretchr/testify/assert"

"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops"
"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport"
"github.com/hetznercloud/hcloud-go/v2/hcloud"
"github.com/hetznercloud/hcloud-go/v2/hcloud/schema"
Expand Down Expand Up @@ -274,115 +273,3 @@ func TestCloud(t *testing.T) {
}
})
}

func TestLoadBalancerDefaultsFromEnv(t *testing.T) {
cases := []struct {
name string
env map[string]string
expDefaults hcops.LoadBalancerDefaults
expDisablePrivateIngress bool
expDisableIPv6 bool
expErr string
}{
{
name: "None set",
env: map[string]string{},
expDefaults: hcops.LoadBalancerDefaults{
// strings should be empty (zero value)
// bools should be false (zero value)
},
},
{
name: "All set (except network zone)",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_LOCATION": "hel1",
"HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS": "true",
"HCLOUD_LOAD_BALANCERS_DISABLE_IPV6": "true",
"HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP": "true",
},
expDefaults: hcops.LoadBalancerDefaults{
Location: "hel1",
UsePrivateIP: true,
},
expDisablePrivateIngress: true,
expDisableIPv6: true,
},
{
name: "Network zone set",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_NETWORK_ZONE": "eu-central",
},
expDefaults: hcops.LoadBalancerDefaults{
NetworkZone: "eu-central",
},
},
{
name: "Both location and network zone set (error)",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_LOCATION": "hel1",
"HCLOUD_LOAD_BALANCERS_NETWORK_ZONE": "eu-central",
},
expErr: "HCLOUD_LOAD_BALANCERS_LOCATION/HCLOUD_LOAD_BALANCERS_NETWORK_ZONE: Only one of these can be set",
},
{
name: "Invalid DISABLE_PRIVATE_INGRESS",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS": "invalid",
},
expErr: `HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS: strconv.ParseBool: parsing "invalid": invalid syntax`,
},
{
name: "Invalid DISABLE_IPV6",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_DISABLE_IPV6": "invalid",
},
expErr: `HCLOUD_LOAD_BALANCERS_DISABLE_IPV6: strconv.ParseBool: parsing "invalid": invalid syntax`,
},
{
name: "Invalid USE_PRIVATE_IP",
env: map[string]string{
"HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP": "invalid",
},
expErr: `HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP: strconv.ParseBool: parsing "invalid": invalid syntax`,
},
}

for _, c := range cases {
c := c // prevent scopelint from complaining
t.Run(c.name, func(t *testing.T) {
previousEnvVars := map[string]string{}
unsetEnvVars := []string{}

for k, v := range c.env {
// Store previous value, so we can later restore it and not affect other tests in this package.
if v, ok := os.LookupEnv(k); ok {
previousEnvVars[k] = v
} else if !ok {
unsetEnvVars = append(unsetEnvVars, k)
}
os.Setenv(k, v)
}

// Make sure this is always executed, even on panic
defer func() {
for k, v := range previousEnvVars {
os.Setenv(k, v)
}
for _, k := range unsetEnvVars {
os.Unsetenv(k)
}
}()

defaults, disablePrivateIngress, disableIPv6, err := loadBalancerDefaultsFromEnv()

if c.expErr != "" {
assert.EqualError(t, err, c.expErr)
return
}
assert.NoError(t, err)
assert.Equal(t, c.expDefaults, defaults)
assert.Equal(t, c.expDisablePrivateIngress, disablePrivateIngress)
assert.Equal(t, c.expDisableIPv6, disableIPv6)
})
}
}
Loading

0 comments on commit 575901b

Please sign in to comment.