Merge pull request #317 from ederst/os-restrict-fixed-ip-range

OpenStack: Filter fixed IP addresses with CIDR notation

etcd-manager/cmd/etcd-manager/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
 
 go_library(
     name = "etcd-manager_lib",
@@ -40,3 +40,9 @@ go_binary(
     embed = [":etcd-manager_lib"],
     visibility = ["//visibility:public"],
 )
+
+go_test(
+    name = "etcd-manager_test",
+    srcs = ["main_test.go"],
+    embed = [":etcd-manager_lib"],
+)

etcd-manager/cmd/etcd-manager/main.go

@@ -101,6 +101,8 @@ func main() {
 	var volumeTags stringSliceFlag
 	flag.Var(&volumeTags, "volume-tag", "tag which volume is required to have")
 
+	flag.StringVar(&o.NetworkCIDR, "network-cidr", o.NetworkCIDR, "filter for a specific IP address by network CIDR (OpenStack only)")
+
 	flag.Parse()
 
 	o.VolumeTags = volumeTags
@@ -162,6 +164,9 @@ type EtcdManagerOptions struct {
 
 	// EtcdManagerMetricsPort allows exposing statistics from etcd-manager
 	EtcdManagerMetricsPort int
+
+	// NetworkCIDR allows filtering for a specific IP address by network CIDR (OpenStack only)
+	NetworkCIDR string
 }
 
 // InitDefaults populates the default flag values
@@ -202,6 +207,19 @@ func RunEtcdManager(o *EtcdManagerOptions) error {
 		return fmt.Errorf("backup-store is required")
 	}
 
+	var networkCIDR *net.IPNet
+	if o.NetworkCIDR != "" {
+		if o.VolumeProviderID != "openstack" {
+			return fmt.Errorf("network-cidr is only supported with provider 'openstack'")
+		}
+
+		var err error
+		_, networkCIDR, err = net.ParseCIDR(o.NetworkCIDR)
+		if err != nil {
+			return fmt.Errorf("parsing network-cidr: %w", err)
+		}
+	}
+
 	backupInterval, err := time.ParseDuration(o.BackupInterval)
 	if err != nil {
 		return fmt.Errorf("invalid backup-interval duration %q", o.BackupInterval)
@@ -244,7 +262,7 @@ func RunEtcdManager(o *EtcdManagerOptions) error {
 			discoveryProvider = gceVolumeProvider
 
 		case "openstack":
-			osVolumeProvider, err := openstack.NewOpenstackVolumes(o.ClusterName, o.VolumeTags, o.NameTag)
+			osVolumeProvider, err := openstack.NewOpenstackVolumes(o.ClusterName, o.VolumeTags, o.NameTag, networkCIDR)
 			if err != nil {
 				fmt.Fprintf(os.Stderr, "%v\n", err)
 				os.Exit(1)

etcd-manager/cmd/etcd-manager/main_test.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"fmt"
+	"net"
+	"reflect"
+	"testing"
+)
+
+func getTestData(networkCIDR string, volumeProviderID string) *EtcdManagerOptions {
+	var o EtcdManagerOptions
+	o.InitDefaults()
+
+	o.ClusterName = "test"
+	o.BackupStorePath = "s3://test"
+
+	o.NetworkCIDR = networkCIDR
+	o.VolumeProviderID = volumeProviderID
+
+	return &o
+}
+
+func assertTestResults(t *testing.T, err error, expected interface{}, actual interface{}) {
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	if !reflect.DeepEqual(expected, actual) {
+		t.Fatalf("expected '%+v', but got '%+v'", expected, actual)
+	}
+}
+
+func TestRunEtcdManagerReturnsNetworkCIDROnlySupportedWithOpenstack(t *testing.T) {
+	expectedErr := fmt.Errorf("network-cidr is only supported with provider 'openstack'")
+	o := getTestData("192.168.0.0/16", "aws")
+
+	actualErr := RunEtcdManager(o)
+
+	assertTestResults(t, nil, expectedErr, actualErr)
+}
+
+func TestRunEtcdManagerReturnsErrorOnInvalidCIDR(t *testing.T) {
+	invalidCIDR := "192.168.0.0/123"
+	expectedErr := fmt.Errorf("parsing network-cidr: %w", &net.ParseError{Type: "CIDR address", Text: invalidCIDR})
+	o := getTestData(invalidCIDR, "openstack")
+
+	actualErr := RunEtcdManager(o)
+
+	assertTestResults(t, nil, expectedErr, actualErr)
+}

etcd-manager/pkg/volumes/openstack/BUILD.bazel

@@ -31,7 +31,10 @@ go_library(
 
 go_test(
     name = "openstack_test",
-    srcs = ["volumes_test.go"],
+    srcs = [
+        "util_test.go",
+        "volumes_test.go",
+    ],
     data = glob(["testdata/**"]),
     embed = [":openstack"],
     deps = [

etcd-manager/pkg/volumes/openstack/discovery.go

@@ -52,7 +52,7 @@ func (os *OpenstackVolumes) Poll() (map[string]discovery.Node, error) {
 		node := discovery.Node{
 			ID: volume.EtcdName,
 		}
-		address, err := GetServerFixedIP(server.Addresses, server.Name)
+		address, err := GetServerFixedIP(server.Addresses, server.Name, os.networkCIDR)
 		if err != nil {
 			klog.Warningf("Could not find servers fixed ip %s: %v", server.Name, err)
 			continue

etcd-manager/pkg/volumes/openstack/util.go

@@ -18,6 +18,7 @@ package openstack
 
 import (
 	"fmt"
+	"net"
 )
 
 const (
@@ -26,14 +27,17 @@ const (
 	openstackAddress        = "addr"
 )
 
-func GetServerFixedIP(addrs map[string]interface{}, name string) (poolAddress string, err error) {
+func GetServerFixedIP(addrs map[string]interface{}, name string, networkCIDR *net.IPNet) (poolAddress string, err error) {
 	for _, address := range addrs {
 		if addresses, ok := address.([]interface{}); ok {
 			for _, addr := range addresses {
 				addrMap := addr.(map[string]interface{})
 				if addrType, ok := addrMap[openstackExternalIPType]; ok && addrType == openstackAddressFixed {
 					if fixedIP, ok := addrMap[openstackAddress]; ok {
 						if fixedIPStr, ok := fixedIP.(string); ok {
+							if networkCIDR != nil && !networkCIDR.Contains(net.ParseIP(fixedIPStr)) {
+								continue
+							}
 							return fixedIPStr, nil
 						}
 					}

etcd-manager/pkg/volumes/openstack/util_test.go

@@ -0,0 +1,131 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package openstack
+
+import (
+	"fmt"
+	"net"
+	"testing"
+)
+
+type TestData struct {
+	clusterName string
+	ips         []string
+	addrs       map[string]interface{}
+}
+
+func getTestData() *TestData {
+	ips := []string{
+		"10.135.1.1",
+		"2001:db8::1:0",
+		"192.168.1.1",
+		"192.168.2.1",
+	}
+
+	addrs := map[string]interface{}{
+		"test_network_1": []interface{}{
+			map[string]interface{}{
+				"OS-EXT-IPS-MAC:mac_addr": "00:00:00:00:00:01",
+				"OS-EXT-IPS:type":         "fixed",
+				"addr":                    ips[0],
+				"version":                 "4",
+			},
+			map[string]interface{}{
+				"OS-EXT-IPS-MAC:mac_addr": "00:00:00:00:00:02",
+				"OS-EXT-IPS:type":         "fixed",
+				"addr":                    ips[1],
+				"version":                 "6",
+			},
+		},
+		"test_network_2": []interface{}{
+			map[string]interface{}{
+				"OS-EXT-IPS-MAC:mac_addr": "00:00:00:00:00:03",
+				"OS-EXT-IPS:type":         "fixed",
+				"addr":                    ips[2],
+				"version":                 "4",
+			},
+			map[string]interface{}{
+				"OS-EXT-IPS-MAC:mac_addr": "00:00:00:00:00:04",
+				"OS-EXT-IPS:type":         "fixed",
+				"addr":                    ips[3],
+				"version":                 "4",
+			},
+		},
+	}
+
+	return &TestData{
+		clusterName: "test",
+		ips:         ips,
+		addrs:       addrs,
+	}
+}
+
+func TestReturnErrorOnEmptyAddresses(t *testing.T) {
+	td := getTestData()
+	td.addrs = map[string]interface{}{}
+
+	expectedErr := fmt.Errorf("failed to find Fixed IP address for server %s", td.clusterName)
+
+	_, actualErr := GetServerFixedIP(td.addrs, td.clusterName, nil)
+
+	assertTestResults(t, nil, fmt.Sprintf("%+v", expectedErr), fmt.Sprintf("%+v", actualErr))
+}
+
+func TestReturnFirstFixedIP(t *testing.T) {
+	td := getTestData()
+	expectedIP := td.ips[0]
+
+	actualIP, err := GetServerFixedIP(td.addrs, td.clusterName, nil)
+
+	assertTestResults(t, err, expectedIP, actualIP)
+}
+
+func TestReturnErrorOnNonMatchingCIDR(t *testing.T) {
+	td := getTestData()
+
+	_, cidr, _ := net.ParseCIDR("172.16.0.0/16")
+
+	expectedErr := fmt.Errorf("failed to find Fixed IP address for server %s", td.clusterName)
+
+	_, actualErr := GetServerFixedIP(td.addrs, td.clusterName, cidr)
+
+	assertTestResults(t, nil, expectedErr, actualErr)
+}
+
+func TestReturnFirstIPv4MatchingCIDR(t *testing.T) {
+	td := getTestData()
+
+	_, cidr, _ := net.ParseCIDR("192.168.2.0/24")
+
+	expectedIP := td.ips[3]
+
+	actualIP, err := GetServerFixedIP(td.addrs, td.clusterName, cidr)
+
+	assertTestResults(t, err, expectedIP, actualIP)
+}
+
+func TestReturnFirstIPv6MatchingCIDR(t *testing.T) {
+	td := getTestData()
+
+	_, cidr, _ := net.ParseCIDR("2001:db8::/64")
+
+	expectedIP := td.ips[1]
+
+	actualIP, err := GetServerFixedIP(td.addrs, td.clusterName, cidr)
+
+	assertTestResults(t, err, expectedIP, actualIP)
+}

etcd-manager/pkg/volumes/openstack/volumes.go

@@ -73,6 +73,7 @@ type OpenstackVolumes struct {
 
 	matchTagKeys []string
 	matchTags    map[string]string
+	networkCIDR  *net.IPNet
 
 	computeClient *gophercloud.ServiceClient
 	volumeClient  *gophercloud.ServiceClient
@@ -96,7 +97,7 @@ type MetadataService struct {
 var _ volumes.Volumes = &OpenstackVolumes{}
 
 // NewOpenstackVolumes builds a OpenstackVolume
-func NewOpenstackVolumes(clusterName string, volumeTags []string, nameTag string) (*OpenstackVolumes, error) {
+func NewOpenstackVolumes(clusterName string, volumeTags []string, nameTag string, networkCIDR *net.IPNet) (*OpenstackVolumes, error) {
 
 	metadata, err := getLocalMetadata()
 	if err != nil {
@@ -108,6 +109,7 @@ func NewOpenstackVolumes(clusterName string, volumeTags []string, nameTag string
 		meta:        metadata,
 		matchTags:   make(map[string]string),
 		nameTag:     nameTag,
+		networkCIDR: networkCIDR,
 	}
 
 	for _, volumeTag := range volumeTags {
@@ -368,7 +370,7 @@ func (stack *OpenstackVolumes) discoverTags() error {
 		if mc.ObserveRequest(err) != nil {
 			return fmt.Errorf("failed to retrieve server information from cloud: %v", err)
 		}
-		ip, err := GetServerFixedIP(extendedServer.Addresses, extendedServer.Name)
+		ip, err := GetServerFixedIP(extendedServer.Addresses, extendedServer.Name, stack.networkCIDR)
 		if err != nil {
 			return fmt.Errorf("error querying InternalIP from name: %v", err)
 		}