Skip to content

Commit

Permalink
feat(routes): emit warning if cluster CIDR is misconfigured
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasmetzner committed Nov 18, 2024
1 parent 782d685 commit 823eb48
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 38 deletions.
16 changes: 9 additions & 7 deletions hcloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package hcloud
import (
"context"
"fmt"
"io"
"os"
"strings"

Expand Down Expand Up @@ -52,9 +51,10 @@ type cloud struct {
cfg config.HCCMConfiguration
recorder record.EventRecorder
networkID int64
cidr string
}

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

Expand Down Expand Up @@ -132,6 +132,7 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
robotClient: robotClient,
cfg: cfg,
networkID: networkID,
cidr: cidr,
}, nil
}

Expand Down Expand Up @@ -193,7 +194,12 @@ func (c *cloud) Routes() (cloudprovider.Routes, bool) {
return nil, false
}

r, err := newRoutes(c.client, c.networkID)
r, err := newRoutes(
c.client,
c.networkID,
c.cidr,
c.recorder,
)
if err != nil {
klog.ErrorS(err, "create routes provider", "networkID", c.networkID)
return nil, false
Expand Down Expand Up @@ -222,7 +228,3 @@ func serverIsAttachedToNetwork(metadataClient *metadata.Client, networkID int64)
}
return strings.Contains(serverPrivateNetworks, fmt.Sprintf("network_id: %d\n", networkID)), nil
}

func init() {
cloudprovider.RegisterCloudProvider(providerName, newCloud)
}
13 changes: 6 additions & 7 deletions hcloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package hcloud

import (
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -92,8 +91,8 @@ func TestNewCloud(t *testing.T) {
},
)
})
var config bytes.Buffer
_, err := newCloud(&config)

_, err := NewCloud(DefaultClusterCIDR)
assert.NoError(t, err)
}

Expand All @@ -105,7 +104,7 @@ func TestNewCloudConnectionNotPossible(t *testing.T) {
)
defer resetEnv()

_, err := newCloud(&bytes.Buffer{})
_, err := NewCloud(DefaultClusterCIDR)
assert.EqualError(t, err,
`hcloud/newCloud: Get "http://127.0.0.1:4711/v1/servers?": dial tcp 127.0.0.1:4711: connect: connection refused`)
}
Expand Down Expand Up @@ -133,7 +132,7 @@ func TestNewCloudInvalidToken(t *testing.T) {
)
})

_, err := newCloud(&bytes.Buffer{})
_, err := NewCloud(DefaultClusterCIDR)
assert.EqualError(t, err, "hcloud/newCloud: unable to authenticate (unauthorized)")
}

Expand Down Expand Up @@ -196,7 +195,7 @@ func TestCloud(t *testing.T) {
)
})

cloud, err := newCloud(&bytes.Buffer{})
cloud, err := NewCloud(DefaultClusterCIDR)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -253,7 +252,7 @@ func TestCloud(t *testing.T) {
)
defer resetEnv()

c, err := newCloud(&bytes.Buffer{})
c, err := NewCloud(DefaultClusterCIDR)
if err != nil {
t.Errorf("%s", err)
}
Expand Down
44 changes: 37 additions & 7 deletions hcloud/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import (
"time"

"golang.org/x/time/rate"
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"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog/v2"

Expand All @@ -17,6 +20,8 @@ import (
"github.com/hetznercloud/hcloud-go/v2/hcloud"
)

const DefaultClusterCIDR = "10.244.0.0/16"

var (
serversCacheMissRefreshRate = rate.Every(30 * time.Second)
)
Expand All @@ -25,9 +30,11 @@ type routes struct {
client *hcloud.Client
network *hcloud.Network
serverCache *hcops.AllServersCache
clusterCIDR *net.IPNet
recorder record.EventRecorder
}

func newRoutes(client *hcloud.Client, networkID int64) (*routes, error) {
func newRoutes(client *hcloud.Client, networkID int64, clusterCIDR string, recorder record.EventRecorder) (*routes, error) {
const op = "hcloud/newRoutes"
metrics.OperationCalled.WithLabelValues(op).Inc()

Expand All @@ -39,6 +46,11 @@ func newRoutes(client *hcloud.Client, networkID int64) (*routes, error) {
return nil, fmt.Errorf("network not found: %d", networkID)
}

_, cidr, err := net.ParseCIDR(clusterCIDR)
if err != nil {
return nil, err
}

return &routes{
client: client,
network: networkObj,
Expand All @@ -49,6 +61,8 @@ func newRoutes(client *hcloud.Client, networkID int64) (*routes, error) {
Network: networkObj,
CacheMissRefreshLimiter: rate.NewLimiter(serversCacheMissRefreshRate, 1),
},
clusterCIDR: cidr,
recorder: recorder,
}, nil
}

Expand Down Expand Up @@ -119,15 +133,31 @@ func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint s
return fmt.Errorf("%s: %w", op, err)
}

hNetSize, _ := r.network.IPRange.Mask.Size()
clusterNetSize, _ := r.clusterCIDR.Mask.Size()
destNetSize, _ := cidr.Mask.Size()

if !(r.network.IPRange.Contains(cidr.IP) && destNetSize >= hNetSize) {
return fmt.Errorf(
"route CIDR %s is not contained within CIDR %s of hcloud network %d",
if !(r.clusterCIDR.Contains(cidr.IP) && destNetSize >= clusterNetSize) {
node := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: string(route.TargetNode),
Namespace: "",
},
}
// Event is only visible via `kubectl get events` and not `kubectl describe node`,
// as we do not have the UID here and `kubectl describe node` filters by UID.
// Because of this behavior we are also dispatching a log message.
r.recorder.Eventf(
node,
corev1.EventTypeWarning,
"ClusterCIDRMisconfigured",
"route CIDR %s is not contained within cluster CIDR %s",
route.DestinationCIDR,
r.clusterCIDR.String(),
)
klog.Warningf(
"route CIDR %s is not contained within cluster CIDR %s",
route.DestinationCIDR,
r.network.IPRange.String(),
r.network.ID,
r.clusterCIDR.String(),
)
}

Expand Down
15 changes: 3 additions & 12 deletions hcloud/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestRoutes_CreateRoute(t *testing.T) {
},
})
})
routes, err := newRoutes(env.Client, 1)
routes, err := newRoutes(env.Client, 1, DefaultClusterCIDR, env.Recorder)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand All @@ -83,15 +83,6 @@ func TestRoutes_CreateRoute(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

err = routes.CreateRoute(context.TODO(), "my-cluster", "routeFail", &cloudprovider.Route{
Name: "route",
TargetNode: "node15",
DestinationCIDR: "172.16.0.0/16",
})
if err == nil {
t.Fatalf("Expected an error because DestinationCIDR is not within the cluster CIDR, but received nil instead")
}
}

func TestRoutes_ListRoutes(t *testing.T) {
Expand Down Expand Up @@ -128,7 +119,7 @@ func TestRoutes_ListRoutes(t *testing.T) {
},
})
})
routes, err := newRoutes(env.Client, 1)
routes, err := newRoutes(env.Client, 1, DefaultClusterCIDR, env.Recorder)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -196,7 +187,7 @@ func TestRoutes_DeleteRoute(t *testing.T) {
},
})
})
routes, err := newRoutes(env.Client, 1)
routes, err := newRoutes(env.Client, 1, DefaultClusterCIDR, env.Recorder)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down
7 changes: 2 additions & 5 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
_ "k8s.io/component-base/metrics/prometheus/version"
"k8s.io/klog/v2"

_ "github.com/hetznercloud/hcloud-cloud-controller-manager/hcloud"
hcloud "github.com/hetznercloud/hcloud-cloud-controller-manager/hcloud"
)

func main() {
Expand All @@ -59,10 +59,7 @@ func main() {
}

func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface {
cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider

// initialize cloud provider with the cloud provider name and config file provided
cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile)
cloud, err := hcloud.NewCloud(config.ComponentConfig.KubeCloudShared.ClusterCIDR)
if err != nil {
klog.Fatalf("Cloud provider could not be initialized: %v", err)
}
Expand Down

0 comments on commit 823eb48

Please sign in to comment.