Skip to content

Commit

Permalink
refactor: move config parsing & validation to seperate package (#546)
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.

feat(config): stricter validation for settings
`HCLOUD_LOAD_BALANCERS_ENABLED`, `HCLOUD_METRICS_ENABLED` &
`HCLOUD_NETWORK_ROUTES_ENABLED`
  • Loading branch information
@apricote
apricote committed Oct 26, 2023
1 parent a659408 commit 335a2c9
Show file tree
Hide file tree
Showing 10 changed files with 566 additions and 328 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test_e2e.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ jobs:
skaffold build --tag="e2e-${GITHUB_RUN_ID}-${GITHUB_RUN_NUMBER}"
tag=$(skaffold build --tag="e2e-${GITHUB_RUN_ID}-${GITHUB_RUN_NUMBER}" --quiet --output="{{ (index .Builds 0).Tag }}")
skaffold deploy --images=hetznercloud/hcloud-cloud-controller-manager=$tag
go test ./... -tags e2e -v -timeout 60m
go test ./tests/e2e -tags e2e -v -timeout 60m
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export KEEP_SERVER_ON_FAILURE=yes # Keep the test server after a test failure.
2. Run the tests

```bash
go test ./... -tags e2e -v -timeout 60m
go test ./tests/e2e -tags e2e -v -timeout 60m
```

The tests will now run and cleanup themselves afterward. Sometimes it might happen that you need to clean up the
Expand Down
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
cfg 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)
cfg, 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 = cfg.Validate()
if err != nil {
return nil, err
}

opts := []hcloud.ClientOption{
hcloud.WithToken(token),
hcloud.WithToken(cfg.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 cfg.Metrics.Enabled {
go metrics.Serve(cfg.Metrics.Address)
opts = append(opts, hcloud.WithInstrumentation(metrics.GetRegistry()))
}

if os.Getenv(hcloudDebugENVVar) == "true" {
if cfg.HCloudClient.Debug {
opts = append(opts, hcloud.WithDebugWriter(os.Stderr))
}
if endpoint := os.Getenv(hcloudEndpointENVVar); endpoint != "" {
opts = append(opts, hcloud.WithEndpoint(endpoint))
if cfg.HCloudClient.Endpoint != "" {
opts = append(opts, hcloud.WithEndpoint(cfg.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 cfg.Network.NameOrID != "" {
n, _, err := client.Network.Get(context.Background(), cfg.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, cfg.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 !cfg.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, cfg.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: cfg.LoadBalancer.Location,
NetworkZone: cfg.LoadBalancer.NetworkZone,
UsePrivateIP: cfg.LoadBalancer.UsePrivateIP,
},
}

loadBalancers := newLoadBalancers(lbOps, lbDisablePrivateIngress, lbDisableIPv6)
if os.Getenv(hcloudLoadBalancersEnabledENVVar) == "false" {
loadBalancers = nil
var loadBalancers *loadBalancers
if cfg.LoadBalancer.Enabled {
loadBalancers = newLoadBalancers(lbOps, cfg.LoadBalancer.DisablePrivateIngress, cfg.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, cfg.Instance.AddressFamily, networkID),
loadBalancer: loadBalancers,
cfg: cfg,
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.cfg.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)
}
Loading

0 comments on commit 335a2c9

Please sign in to comment.