diff --git a/.github/workflows/test_e2e.yml b/.github/workflows/test_e2e.yml index 4ede0c468..8c0fac95c 100644 --- a/.github/workflows/test_e2e.yml +++ b/.github/workflows/test_e2e.yml @@ -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 diff --git a/README.md b/README.md index 989252fa0..40f79f975 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/hcloud/cloud.go b/hcloud/cloud.go index d65538200..b067f1bde 100644 --- a/hcloud/cloud.go +++ b/hcloud/cloud.go @@ -18,16 +18,15 @@ 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" @@ -35,23 +34,7 @@ import ( ) 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. @@ -61,6 +44,7 @@ type cloud struct { client *hcloud.Client instances *instances loadBalancer *loadBalancers + cfg config.HCCMConfiguration networkID int64 } @@ -68,100 +52,88 @@ 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 } @@ -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) @@ -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. @@ -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) } diff --git a/hcloud/cloud_test.go b/hcloud/cloud_test.go index 0b69fc180..e9d6c24dd 100644 --- a/hcloud/cloud_test.go +++ b/hcloud/cloud_test.go @@ -27,7 +27,7 @@ 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" ) @@ -65,7 +65,7 @@ func TestNewCloud(t *testing.T) { env := newTestEnv() defer env.Teardown() - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_ENDPOINT", env.Server.URL, "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", "HCLOUD_METRICS_ENABLED", "false", @@ -86,7 +86,7 @@ func TestNewCloud(t *testing.T) { } func TestNewCloudWrongTokenSize(t *testing.T) { - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_TOKEN", "0123456789abcdef", "HCLOUD_METRICS_ENABLED", "false", ) @@ -100,7 +100,7 @@ func TestNewCloudWrongTokenSize(t *testing.T) { } func TestNewCloudConnectionNotPossible(t *testing.T) { - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_ENDPOINT", "http://127.0.0.1:4711/v1", "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", "HCLOUD_METRICS_ENABLED", "false", @@ -116,7 +116,7 @@ func TestNewCloudInvalidToken(t *testing.T) { env := newTestEnv() defer env.Teardown() - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_ENDPOINT", env.Server.URL, "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", "HCLOUD_METRICS_ENABLED", "false", @@ -143,7 +143,7 @@ func TestCloud(t *testing.T) { env := newTestEnv() defer env.Teardown() - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_ENDPOINT", env.Server.URL, "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", "HCLOUD_METRICS_ENABLED", "false", @@ -244,7 +244,7 @@ func TestCloud(t *testing.T) { }) t.Run("RoutesWithNetworks", func(t *testing.T) { - resetEnv := Setenv(t, + resetEnv := testsupport.Setenv(t, "HCLOUD_NETWORK", "1", "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK", "true", "HCLOUD_METRICS_ENABLED", "false", @@ -273,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) - }) - } -} diff --git a/hcloud/instances.go b/hcloud/instances.go index bee697aec..877cbfd1e 100644 --- a/hcloud/instances.go +++ b/hcloud/instances.go @@ -23,26 +23,19 @@ import ( corev1 "k8s.io/api/core/v1" cloudprovider "k8s.io/cloud-provider" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) -type addressFamily int - -const ( - AddressFamilyDualStack addressFamily = iota - AddressFamilyIPv6 - AddressFamilyIPv4 -) - type instances struct { client *hcloud.Client - addressFamily addressFamily + addressFamily config.AddressFamily networkID int64 } -func newInstances(client *hcloud.Client, addressFamily addressFamily, networkID int64) *instances { +func newInstances(client *hcloud.Client, addressFamily config.AddressFamily, networkID int64) *instances { return &instances{client, addressFamily, networkID} } @@ -119,14 +112,14 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (*c }, nil } -func nodeAddresses(addressFamily addressFamily, networkID int64, server *hcloud.Server) []corev1.NodeAddress { +func nodeAddresses(addressFamily config.AddressFamily, networkID int64, server *hcloud.Server) []corev1.NodeAddress { var addresses []corev1.NodeAddress addresses = append( addresses, corev1.NodeAddress{Type: corev1.NodeHostName, Address: server.Name}, ) - if addressFamily == AddressFamilyIPv4 || addressFamily == AddressFamilyDualStack { + if addressFamily == config.AddressFamilyIPv4 || addressFamily == config.AddressFamilyDualStack { if !server.PublicNet.IPv4.IsUnspecified() { addresses = append( addresses, @@ -135,7 +128,7 @@ func nodeAddresses(addressFamily addressFamily, networkID int64, server *hcloud. } } - if addressFamily == AddressFamilyIPv6 || addressFamily == AddressFamilyDualStack { + if addressFamily == config.AddressFamilyIPv6 || addressFamily == config.AddressFamilyDualStack { if !server.PublicNet.IPv6.IsUnspecified() { // For a given IPv6 network of 2001:db8:1234::/64, the instance address is 2001:db8:1234::1 hostAddress := server.PublicNet.IPv6.IP diff --git a/hcloud/instances_test.go b/hcloud/instances_test.go index 5276f99ea..fa3c717fc 100644 --- a/hcloud/instances_test.go +++ b/hcloud/instances_test.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cloudprovider "k8s.io/cloud-provider" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-go/v2/hcloud" "github.com/hetznercloud/hcloud-go/v2/hcloud/schema" ) @@ -58,7 +59,7 @@ func TestInstances_InstanceExists(t *testing.T) { json.NewEncoder(w).Encode(schema.ServerListResponse{Servers: servers}) }) - instances := newInstances(env.Client, AddressFamilyIPv4, 0) + instances := newInstances(env.Client, config.AddressFamilyIPv4, 0) tests := []struct { name string @@ -131,7 +132,7 @@ func TestInstances_InstanceShutdown(t *testing.T) { }) }) - instances := newInstances(env.Client, AddressFamilyIPv4, 0) + instances := newInstances(env.Client, config.AddressFamilyIPv4, 0) tests := []struct { name string @@ -188,7 +189,7 @@ func TestInstances_InstanceMetadata(t *testing.T) { }) }) - instances := newInstances(env.Client, AddressFamilyIPv4, 0) + instances := newInstances(env.Client, config.AddressFamilyIPv4, 0) metadata, err := instances.InstanceMetadata(context.TODO(), &corev1.Node{ Spec: corev1.NodeSpec{ProviderID: "hcloud://1"}, @@ -216,14 +217,14 @@ func TestInstances_InstanceMetadata(t *testing.T) { func TestNodeAddresses(t *testing.T) { tests := []struct { name string - addressFamily addressFamily + addressFamily config.AddressFamily server *hcloud.Server privateNetwork int64 expected []corev1.NodeAddress }{ { name: "hostname", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, server: &hcloud.Server{ Name: "foobar", }, @@ -233,7 +234,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "public ipv4", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, server: &hcloud.Server{ Name: "foobar", PublicNet: hcloud.ServerPublicNet{ @@ -252,7 +253,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "no public ipv4", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, server: &hcloud.Server{ Name: "foobar", PublicNet: hcloud.ServerPublicNet{ @@ -267,7 +268,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "public ipv6", - addressFamily: AddressFamilyIPv6, + addressFamily: config.AddressFamilyIPv6, server: &hcloud.Server{ Name: "foobar", PublicNet: hcloud.ServerPublicNet{ @@ -286,7 +287,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "no public ipv6", - addressFamily: AddressFamilyIPv6, + addressFamily: config.AddressFamilyIPv6, server: &hcloud.Server{ Name: "foobar", PublicNet: hcloud.ServerPublicNet{ @@ -301,7 +302,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "public dual stack", - addressFamily: AddressFamilyDualStack, + addressFamily: config.AddressFamilyDualStack, server: &hcloud.Server{ Name: "foobar", PublicNet: hcloud.ServerPublicNet{ @@ -322,7 +323,7 @@ func TestNodeAddresses(t *testing.T) { { name: "unknown private network", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, privateNetwork: 1, server: &hcloud.Server{ Name: "foobar", @@ -333,7 +334,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "server attached to private network", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, privateNetwork: 1, server: &hcloud.Server{ Name: "foobar", @@ -354,7 +355,7 @@ func TestNodeAddresses(t *testing.T) { }, { name: "server not attached to private network", - addressFamily: AddressFamilyIPv4, + addressFamily: config.AddressFamilyIPv4, privateNetwork: 1, server: &hcloud.Server{ Name: "foobar", diff --git a/hcloud/testing.go b/hcloud/testing.go index 77bea1a16..c33f7f02c 100644 --- a/hcloud/testing.go +++ b/hcloud/testing.go @@ -2,7 +2,6 @@ package hcloud import ( "context" - "os" "testing" corev1 "k8s.io/api/core/v1" @@ -15,57 +14,6 @@ import ( "github.com/hetznercloud/hcloud-go/v2/hcloud" ) -// Setenv prepares the environment for testing the -// hcloud-cloud-controller-manager. -func Setenv(t *testing.T, args ...string) func() { - if len(args)%2 != 0 { - t.Fatal("Sentenv: uneven number of args") - } - - newVars := make([]string, 0, len(args)/2) - oldEnv := make(map[string]string, len(newVars)) - - for i := 0; i < len(args); i += 2 { - k, v := args[i], args[i+1] - newVars = append(newVars, k) - - if old, ok := os.LookupEnv(k); ok { - oldEnv[k] = old - } - if err := os.Setenv(k, v); err != nil { - t.Fatalf("Setenv failed: %v", err) - } - } - - return func() { - for _, k := range newVars { - v, ok := oldEnv[k] - if !ok { - if err := os.Unsetenv(k); err != nil { - t.Errorf("Unsetenv failed: %v", err) - } - continue - } - if err := os.Setenv(k, v); err != nil { - t.Errorf("Setenv failed: %v", err) - } - } - } -} - -// SkipEnv skips t if any of the passed vars is not set as an environment -// variable. -// -// SkipEnv uses os.LookupEnv. The empty string is therefore a valid value. -func SkipEnv(t *testing.T, vars ...string) { - for _, v := range vars { - if _, ok := os.LookupEnv(v); !ok { - t.Skipf("Environment variable not set: %s", v) - return - } - } -} - type LoadBalancerTestCase struct { Name string diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 000000000..b71816a3b --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,188 @@ +package config + +import ( + "errors" + "fmt" + "os" + "strconv" +) + +const ( + hcloudToken = "HCLOUD_TOKEN" + hcloudEndpoint = "HCLOUD_ENDPOINT" + hcloudNetwork = "HCLOUD_NETWORK" + hcloudDebug = "HCLOUD_DEBUG" + + hcloudInstancesAddressFamily = "HCLOUD_INSTANCES_ADDRESS_FAMILY" + + // Disable the "master/server is attached to the network" check against the metadata service. + hcloudNetworkDisableAttachedCheck = "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK" + hcloudNetworkRoutesEnabled = "HCLOUD_NETWORK_ROUTES_ENABLED" + + hcloudLoadBalancersEnabled = "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" + + hcloudMetricsEnabled = "HCLOUD_METRICS_ENABLED" + hcloudMetricsAddress = ":8233" +) + +type HCloudClientConfiguration struct { + Token string + Endpoint string + Debug bool +} + +type MetricsConfiguration struct { + Enabled bool + Address string +} + +type AddressFamily string + +const ( + AddressFamilyDualStack AddressFamily = "dualstack" + AddressFamilyIPv6 AddressFamily = "ipv6" + AddressFamilyIPv4 AddressFamily = "ipv4" +) + +type InstanceConfiguration struct { + AddressFamily AddressFamily +} + +type LoadBalancerConfiguration struct { + Enabled bool + Location string + NetworkZone string + DisablePrivateIngress bool + UsePrivateIP bool + DisableIPv6 bool +} + +type NetworkConfiguration struct { + NameOrID string + DisableAttachedCheck bool +} + +type RouteConfiguration struct { + Enabled bool +} + +type HCCMConfiguration struct { + HCloudClient HCloudClientConfiguration + Metrics MetricsConfiguration + Instance InstanceConfiguration + LoadBalancer LoadBalancerConfiguration + Network NetworkConfiguration + Route RouteConfiguration +} + +// Read evaluates all environment variables and returns a [HCCMConfiguration]. It only validates as far as +// it needs to parse the values. For business logic validation, check out [HCCMConfiguration.Validate]. +func Read() (HCCMConfiguration, error) { + var err error + // Collect all errors and return them as one. + // This helps users because they will see all the errors at once + // instead of having to fix them one by one. + var errs []error + var cfg HCCMConfiguration + + cfg.HCloudClient.Token = os.Getenv(hcloudToken) + cfg.HCloudClient.Endpoint = os.Getenv(hcloudEndpoint) + cfg.HCloudClient.Debug, err = getEnvBool(hcloudDebug, false) + if err != nil { + errs = append(errs, err) + } + + cfg.Metrics.Enabled, err = getEnvBool(hcloudMetricsEnabled, true) + if err != nil { + errs = append(errs, err) + } + cfg.Metrics.Address = hcloudMetricsAddress + + // Validation happens in [HCCMConfiguration.Validate] + cfg.Instance.AddressFamily = AddressFamily(os.Getenv(hcloudInstancesAddressFamily)) + if cfg.Instance.AddressFamily == "" { + cfg.Instance.AddressFamily = AddressFamilyIPv4 + } + + cfg.LoadBalancer.Enabled, err = getEnvBool(hcloudLoadBalancersEnabled, true) + if err != nil { + errs = append(errs, err) + } + cfg.LoadBalancer.Location = os.Getenv(hcloudLoadBalancersLocation) + cfg.LoadBalancer.NetworkZone = os.Getenv(hcloudLoadBalancersNetworkZone) + cfg.LoadBalancer.DisablePrivateIngress, err = getEnvBool(hcloudLoadBalancersDisablePrivateIngress, false) + if err != nil { + errs = append(errs, err) + } + cfg.LoadBalancer.UsePrivateIP, err = getEnvBool(hcloudLoadBalancersUsePrivateIP, false) + if err != nil { + errs = append(errs, err) + } + cfg.LoadBalancer.DisableIPv6, err = getEnvBool(hcloudLoadBalancersDisableIPv6, false) + if err != nil { + errs = append(errs, err) + } + + cfg.Network.NameOrID = os.Getenv(hcloudNetwork) + cfg.Network.DisableAttachedCheck, err = getEnvBool(hcloudNetworkDisableAttachedCheck, false) + if err != nil { + errs = append(errs, err) + } + + cfg.Route.Enabled, err = getEnvBool(hcloudNetworkRoutesEnabled, true) + if err != nil { + errs = append(errs, err) + } + + if len(errs) > 0 { + return HCCMConfiguration{}, errors.Join(errs...) + } + return cfg, nil +} + +func (c HCCMConfiguration) Validate() (err error) { + // Collect all errors and return them as one. + // This helps users because they will see all the errors at once + // instead of having to fix them one by one. + var errs []error + + if c.HCloudClient.Token == "" { + errs = append(errs, fmt.Errorf("environment variable %q is required", hcloudToken)) + } else if len(c.HCloudClient.Token) != 64 { + errs = append(errs, fmt.Errorf("entered token is invalid (must be exactly 64 characters long)")) + } + + if c.Instance.AddressFamily != AddressFamilyDualStack && c.Instance.AddressFamily != AddressFamilyIPv4 && c.Instance.AddressFamily != AddressFamilyIPv6 { + errs = append(errs, fmt.Errorf("invalid value for %q, expect one of: %s,%s,%s", hcloudInstancesAddressFamily, AddressFamilyIPv4, AddressFamilyIPv6, AddressFamilyDualStack)) + } + + if c.LoadBalancer.Location != "" && c.LoadBalancer.NetworkZone != "" { + errs = append(errs, fmt.Errorf("invalid value for %q/%q, only one of them can be set", hcloudLoadBalancersLocation, hcloudLoadBalancersNetworkZone)) + } + + if len(errs) > 0 { + return errors.Join(errs...) + } + return nil +} + +// getEnvBool returns the boolean parsed from the environment variable with the given key and a potential error +// parsing the var. Returns the default value if the env var is unset. +func getEnvBool(key string, defaultValue bool) (bool, error) { + v, ok := os.LookupEnv(key) + if !ok { + return defaultValue, nil + } + + b, err := strconv.ParseBool(v) + if err != nil { + return false, fmt.Errorf("failed to parse %s: %v", key, err) + } + + return b, nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 000000000..7f398d41f --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,270 @@ +package config + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" +) + +func TestRead(t *testing.T) { + tests := []struct { + name string + env []string + want HCCMConfiguration + wantErr error + }{ + { + name: "minimal", + env: []string{}, + want: HCCMConfiguration{ + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + // Matches the default env from deploy/ccm-networks.yaml + name: "default deployment", + env: []string{ + "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", + "HCLOUD_NETWORK", "foobar", + }, + want: HCCMConfiguration{ + HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"}, + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + Network: NetworkConfiguration{ + NameOrID: "foobar", + }, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + name: "client", + env: []string{ + "HCLOUD_TOKEN", "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", + "HCLOUD_ENDPOINT", "https://api.example.com", + "HCLOUD_DEBUG", "true", + }, + want: HCCMConfiguration{ + HCloudClient: HCloudClientConfiguration{ + Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq", + Endpoint: "https://api.example.com", + Debug: true, + }, + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + name: "instance", + env: []string{ + "HCLOUD_INSTANCES_ADDRESS_FAMILY", "ipv6", + }, + want: HCCMConfiguration{ + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv6}, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + name: "network", + env: []string{ + "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK", "true", + "HCLOUD_NETWORK", "foobar", + }, + want: HCCMConfiguration{ + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Network: NetworkConfiguration{ + NameOrID: "foobar", + DisableAttachedCheck: true, + }, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + name: "route", + env: []string{ + "HCLOUD_NETWORK_ROUTES_ENABLED", "false", + }, + want: HCCMConfiguration{ + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: false}, + }, + wantErr: nil, + }, + { + name: "load balancer", + env: []string{ + "HCLOUD_LOAD_BALANCERS_ENABLED", "false", + "HCLOUD_LOAD_BALANCERS_LOCATION", "nbg1", + "HCLOUD_LOAD_BALANCERS_NETWORK_ZONE", "eu-central", + "HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS", "true", + "HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP", "true", + "HCLOUD_LOAD_BALANCERS_DISABLE_IPV6", "true", + }, + want: HCCMConfiguration{ + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{ + Enabled: false, + Location: "nbg1", + NetworkZone: "eu-central", + DisablePrivateIngress: true, + UsePrivateIP: true, + DisableIPv6: true, + }, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + { + name: "error parsing bool values", + env: []string{ + "HCLOUD_DEBUG", "foo", + "HCLOUD_METRICS_ENABLED", "bar", + "HCLOUD_LOAD_BALANCERS_ENABLED", "nej", + "HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS", "nyet", + "HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP", "nein", + "HCLOUD_LOAD_BALANCERS_DISABLE_IPV6", "ja", + "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK", "oui", + "HCLOUD_NETWORK_ROUTES_ENABLED", "si", + }, + wantErr: errors.New(`failed to parse HCLOUD_DEBUG: strconv.ParseBool: parsing "foo": invalid syntax +failed to parse HCLOUD_METRICS_ENABLED: strconv.ParseBool: parsing "bar": invalid syntax +failed to parse HCLOUD_LOAD_BALANCERS_ENABLED: strconv.ParseBool: parsing "nej": invalid syntax +failed to parse HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS: strconv.ParseBool: parsing "nyet": invalid syntax +failed to parse HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP: strconv.ParseBool: parsing "nein": invalid syntax +failed to parse HCLOUD_LOAD_BALANCERS_DISABLE_IPV6: strconv.ParseBool: parsing "ja": invalid syntax +failed to parse HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK: strconv.ParseBool: parsing "oui": invalid syntax +failed to parse HCLOUD_NETWORK_ROUTES_ENABLED: strconv.ParseBool: parsing "si": invalid syntax`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resetEnv := testsupport.Setenv(t, tt.env...) + defer resetEnv() + + got, err := Read() + if tt.wantErr == nil { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, tt.wantErr.Error()) + } + assert.Equal(t, tt.want, got) + }) + } +} + +func TestHCCMConfiguration_Validate(t *testing.T) { + type fields struct { + HCloudClient HCloudClientConfiguration + Metrics MetricsConfiguration + Instance InstanceConfiguration + LoadBalancer LoadBalancerConfiguration + Network NetworkConfiguration + Route RouteConfiguration + } + tests := []struct { + name string + fields fields + wantErr error + }{ + { + name: "minimal", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + }, + wantErr: nil, + }, + { + // Matches the default env from deploy/ccm-networks.yaml + name: "default deployment", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"}, + Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + Network: NetworkConfiguration{ + NameOrID: "foobar", + }, + LoadBalancer: LoadBalancerConfiguration{Enabled: true}, + Route: RouteConfiguration{Enabled: true}, + }, + wantErr: nil, + }, + + { + name: "token missing", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: ""}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + }, + wantErr: errors.New("environment variable \"HCLOUD_TOKEN\" is required"), + }, + { + name: "token invalid length", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: "abc"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + }, + wantErr: errors.New("entered token is invalid (must be exactly 64 characters long)"), + }, + { + name: "address family invalid", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamily("foobar")}, + }, + wantErr: errors.New("invalid value for \"HCLOUD_INSTANCES_ADDRESS_FAMILY\", expect one of: ipv4,ipv6,dualstack"), + }, + { + name: "LB location and network zone set", + fields: fields{ + HCloudClient: HCloudClientConfiguration{Token: "jr5g7ZHpPptyhJzZyHw2Pqu4g9gTqDvEceYpngPf79jN_NOT_VALID_dzhepnahq"}, + Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4}, + LoadBalancer: LoadBalancerConfiguration{ + Location: "nbg1", + NetworkZone: "eu-central", + }, + }, + wantErr: errors.New("invalid value for \"HCLOUD_LOAD_BALANCERS_LOCATION\"/\"HCLOUD_LOAD_BALANCERS_NETWORK_ZONE\", only one of them can be set"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := HCCMConfiguration{ + HCloudClient: tt.fields.HCloudClient, + Metrics: tt.fields.Metrics, + Instance: tt.fields.Instance, + LoadBalancer: tt.fields.LoadBalancer, + Network: tt.fields.Network, + Route: tt.fields.Route, + } + err := c.Validate() + if tt.wantErr == nil { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, tt.wantErr.Error()) + } + }) + } +} diff --git a/internal/testsupport/env.go b/internal/testsupport/env.go new file mode 100644 index 000000000..ce1512247 --- /dev/null +++ b/internal/testsupport/env.go @@ -0,0 +1,44 @@ +package testsupport + +import ( + "os" + "testing" +) + +// Setenv prepares the environment for testing the +// hcloud-cloud-controller-manager. +func Setenv(t *testing.T, args ...string) func() { + if len(args)%2 != 0 { + t.Fatal("Sentenv: uneven number of args") + } + + newVars := make([]string, 0, len(args)/2) + oldEnv := make(map[string]string, len(newVars)) + + for i := 0; i < len(args); i += 2 { + k, v := args[i], args[i+1] + newVars = append(newVars, k) + + if old, ok := os.LookupEnv(k); ok { + oldEnv[k] = old + } + if err := os.Setenv(k, v); err != nil { + t.Fatalf("Setenv failed: %v", err) + } + } + + return func() { + for _, k := range newVars { + v, ok := oldEnv[k] + if !ok { + if err := os.Unsetenv(k); err != nil { + t.Errorf("Unsetenv failed: %v", err) + } + continue + } + if err := os.Setenv(k, v); err != nil { + t.Errorf("Setenv failed: %v", err) + } + } + } +}