Allow user defined egress-cidrs for use with cmr

modongsongrio · Jun 29, 2017 · f18be4b · f18be4b
1 parent 0668d39
commit f18be4b
Show file tree

Hide file tree

Showing 5 changed files with 183 additions and 10 deletions.
diff --git a/apiserver/firewaller/ingressaddresswatcher.go b/apiserver/firewaller/ingressaddresswatcher.go
@@ -4,12 +4,15 @@
 package firewaller
 
 import (
+	"net"
+
 	"github.com/juju/errors"
 	"github.com/juju/utils/set"
 	"gopkg.in/juju/worker.v1"
 
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/network"
+	"github.com/juju/juju/state/watcher"
 	"github.com/juju/juju/worker/catacomb"
 )
 
@@ -40,6 +43,9 @@ type IngressAddressWatcher struct {
 
 	// A map of known unit addresses, keyed on unit name.
 	known map[string]string
+
+	// A set of known egress cidrs
+	knownEgress set.Strings
 }
 
 // machineData holds the information we track at the machine level.
@@ -97,6 +103,11 @@ func (w *IngressAddressWatcher) initialise() error {
 		}
 
 	}
+	cfg, err := w.backend.ModelConfig()
+	if err != nil {
+		return err
+	}
+	w.knownEgress = set.NewStrings(cfg.EgressCidrs()...)
 	return nil
 }
 
@@ -113,6 +124,18 @@ func (w *IngressAddressWatcher) loop() error {
 		return errors.Trace(err)
 	}
 
+	// TODO(wallyworld) - we just want to watch for egress
+	// address changes but right now can only watch for
+	// any model config change.
+	mw := w.backend.WatchForModelConfigChanges()
+	if err := w.catacomb.Add(mw); err != nil {
+		return errors.Trace(err)
+	}
+	// Consume initial event.
+	if _, ok := <-mw.Changes(); !ok {
+		return watcher.EnsureErr(mw)
+	}
+
 	var (
 		sentInitial bool
 		out         chan<- []string
@@ -125,17 +148,26 @@ func (w *IngressAddressWatcher) loop() error {
 	if err != nil {
 		return errors.Trace(err)
 	}
-	changed := false
+	unitAddressesChanged := false
+	userConfiguredEgressChanged := false
 	for {
-		if !sentInitial || changed {
-			addressSet := set.NewStrings()
-			for _, addr := range w.known {
-				addressSet.Add(addr)
+		if !sentInitial || unitAddressesChanged || (userConfiguredEgressChanged && len(w.known) > 0) {
+			var addressSet set.Strings
+			// Egress cidrs, if configured, override unit machine addresses.
+			if len(w.known) > 0 {
+				addressSet = set.NewStrings(w.knownEgress.Values()...)
+				if addressSet.Size() == 0 {
+					// No user configured egress so just use the unit addresses.
+					for _, addr := range w.known {
+						addressSet.Add(addr)
+					}
+				}
 			}
-			changed = false
+			unitAddressesChanged = false
 			addresses = formatAsCIDR(addressSet.Values())
 			out = w.out
 		}
+		userConfiguredEgressChanged = false
 
 		select {
 		case <-w.catacomb.Dying():
@@ -144,22 +176,37 @@ func (w *IngressAddressWatcher) loop() error {
 		case out <- addresses:
 			sentInitial = true
 			out = nil
+		case _, ok := <-mw.Changes():
+			if !ok {
+				return w.catacomb.ErrDying()
+			}
+			cfg, err := w.backend.ModelConfig()
+			if err != nil {
+				return err
+			}
+			egress := set.NewStrings(cfg.EgressCidrs()...)
+			// Have the egress addresses changed.
+			if egress.Size() != w.knownEgress.Size() ||
+				egress.Difference(w.knownEgress).Size() != 0 || w.knownEgress.Difference(egress).Size() != 0 {
+				userConfiguredEgressChanged = true
+				w.knownEgress = egress
+			}
 		case c, ok := <-ruw.Changes():
 			if !ok {
 				return w.catacomb.ErrDying()
 			}
 			// A unit has entered or left scope.
 			// Get the new set of addresses resulting from that
 			// change, and if different to what we know, send the change.
-			changed, err = w.processUnitChanges(c)
+			unitAddressesChanged, err = w.processUnitChanges(c)
 			if err != nil {
 				return err
 			}
 		case machineId, ok := <-w.addressChanges:
 			if !ok {
 				continue
 			}
-			changed, err = w.processMachineAddresses(machineId)
+			unitAddressesChanged, err = w.processMachineAddresses(machineId)
 			if err != nil {
 				return errors.Trace(err)
 			}
@@ -171,7 +218,17 @@ func (w *IngressAddressWatcher) loop() error {
 func formatAsCIDR(addresses []string) []string {
 	result := make([]string, len(addresses))
 	for i, a := range addresses {
-		result[i] = a + "/32"
+		cidr := a
+		// If address is not already a cidr, add a /32 (ipv4) or /128 (ipv6).
+		if _, _, err := net.ParseCIDR(a); err != nil {
+			ip := net.ParseIP(a)
+			if ip.To4() != nil {
+				cidr = a + "/32"
+			} else {
+				cidr = a + "/128"
+			}
+		}
+		result[i] = cidr
 	}
 	return result
 }

diff --git a/apiserver/firewaller/ingressaddresswatcher_test.go b/apiserver/firewaller/ingressaddresswatcher_test.go
@@ -403,3 +403,44 @@ func (s *addressWatcherSuite) TestHandlesUnitGoneWhenMachineAddressChanges(c *gc
 	wc.AssertChange("6.7.8.9/32")
 	wc.AssertNoChange()
 }
+
+func (s *addressWatcherSuite) TestEgressAddressConfigured(c *gc.C) {
+	s.st.configAttrs["egress-cidrs"] = "10.0.0.1/16"
+	rel := s.setupRelation(c, "54.1.2.3")
+	w, err := firewaller.NewIngressAddressWatcher(s.st, rel, "django")
+	c.Assert(err, jc.ErrorIsNil)
+	defer statetesting.AssertStop(c, w)
+	wc := statetesting.NewStringsWatcherC(c, nopSyncStarter{}, w)
+
+	// Initial event.
+	wc.AssertChange()
+	wc.AssertNoChange()
+
+	rel.ruw.changes <- params.RelationUnitsChange{
+		Changed: map[string]params.UnitSettings{
+			"django/0": {},
+		},
+	}
+	wc.AssertChange("10.0.0.1/16")
+	wc.AssertNoChange()
+
+	// Change user configured egress addresses.
+	s.st.configAttrs["egress-cidrs"] = "192.168.0.1/16"
+	s.st.modelWatcher.changes <- struct{}{}
+	wc.AssertChange("192.168.0.1/16")
+	wc.AssertNoChange()
+
+	// Reset user configured egress addresses.
+	s.st.configAttrs["egress-cidrs"] = ""
+	s.st.modelWatcher.changes <- struct{}{}
+	wc.AssertChange("54.1.2.3/32")
+	wc.AssertNoChange()
+
+	// A not found unit doesn't trigger an event.
+	rel.ruw.changes <- params.RelationUnitsChange{
+		Changed: map[string]params.UnitSettings{
+			"unknown/0": {},
+		},
+	}
+	wc.AssertNoChange()
+}
diff --git a/apiserver/firewaller/mock_test.go b/apiserver/firewaller/mock_test.go
@@ -15,8 +15,10 @@ import (
 	"github.com/juju/juju/apiserver/common/cloudspec"
 	"github.com/juju/juju/apiserver/firewaller"
 	"github.com/juju/juju/apiserver/params"
+	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/network"
 	"github.com/juju/juju/state"
+	coretesting "github.com/juju/juju/testing"
 )
 
 type mockcloudSpecAPI struct {
@@ -27,7 +29,6 @@ type mockcloudSpecAPI struct {
 type mockState struct {
 	// TODO - implement when remaining firewaller tests become unit tests
 	state.ModelMachinesWatcher
-	state.ModelAccessor
 
 	testing.Stub
 	modelUUID      string
@@ -37,6 +38,8 @@ type mockState struct {
 	machines       map[string]*mockMachine
 	relations      map[string]*mockRelation
 	subnetsWatcher *mockStringsWatcher
+	modelWatcher   *mockNotifyWatcher
+	configAttrs    map[string]interface{}
 }
 
 func newMockState(modelUUID string) *mockState {
@@ -48,9 +51,19 @@ func newMockState(modelUUID string) *mockState {
 		machines:       make(map[string]*mockMachine),
 		remoteEntities: make(map[names.Tag]string),
 		subnetsWatcher: newMockStringsWatcher(),
+		modelWatcher:   newMockNotifyWatcher(),
+		configAttrs:    coretesting.FakeConfig(),
 	}
 }
 
+func (st *mockState) WatchForModelConfigChanges() state.NotifyWatcher {
+	return st.modelWatcher
+}
+
+func (st *mockState) ModelConfig() (*config.Config, error) {
+	return config.New(config.UseDefaults, st.configAttrs)
+}
+
 func (st *mockState) ModelUUID() string {
 	return st.modelUUID
 }
@@ -159,6 +172,24 @@ func (w *mockStringsWatcher) Changes() <-chan []string {
 	return w.changes
 }
 
+func newMockNotifyWatcher() *mockNotifyWatcher {
+	w := &mockNotifyWatcher{changes: make(chan struct{}, 1)}
+	// Initial event
+	w.changes <- struct{}{}
+	go w.doneWhenDying()
+	return w
+}
+
+type mockNotifyWatcher struct {
+	mockWatcher
+	changes chan struct{}
+}
+
+func (w *mockNotifyWatcher) Changes() <-chan struct{} {
+	w.MethodCall(w, "Changes")
+	return w.changes
+}
+
 type mockApplication struct {
 	testing.Stub
 	name  string

diff --git a/environs/config/config.go b/environs/config/config.go
@@ -5,6 +5,7 @@ package config
 
 import (
 	"fmt"
+	"net"
 	"os"
 	"strings"
 	"time"
@@ -152,6 +153,10 @@ const (
 	// UpdateStatusHookInterval is how often to run the update-status hook.
 	UpdateStatusHookInterval = "update-status-hook-interval"
 
+	// EgressCidrs are the source addresses from which traffic from this model
+	// originates if the model is deployed such that NAT or similar is in use.
+	EgressCidrs = "egress-cidrs"
+
 	//
 	// Deprecated Settings Attributes
 	//
@@ -349,6 +354,7 @@ var defaultConfigValues = map[string]interface{}{
 	"test-mode":                false,
 	TransmitVendorMetricsKey:   true,
 	UpdateStatusHookInterval:   DefaultUpdateStatusHookInterval,
+	EgressCidrs:                "",
 
 	// Image and agent streams and URLs.
 	"image-stream":       "released",
@@ -516,6 +522,15 @@ func Validate(cfg, old *Config) error {
 		}
 	}
 
+	if v, ok := cfg.defined[EgressCidrs].(string); ok && v != "" {
+		addresses := strings.Split(v, ",")
+		for _, addr := range addresses {
+			if _, _, err := net.ParseCIDR(strings.TrimSpace(addr)); err != nil {
+				return errors.Annotatef(err, "invalid egress address: %v", addr)
+			}
+		}
+	}
+
 	// Check the immutable config values.  These can't change
 	if old != nil {
 		for _, attr := range immutableAttributes {
@@ -969,6 +984,22 @@ func (c *Config) UpdateStatusHookInterval() time.Duration {
 	return val
 }
 
+// EgressCidrs are the source addresses from which traffic from this model
+// originates if the model is deployed such that NAT or similar is in use.
+func (c *Config) EgressCidrs() []string {
+	raw := c.asString(EgressCidrs)
+	if raw == "" {
+		return []string{}
+	}
+	// Value has already been validated.
+	rawAddr := strings.Split(raw, ",")
+	result := make([]string, len(rawAddr))
+	for i, addr := range rawAddr {
+		result[i] = strings.TrimSpace(addr)
+	}
+	return result
+}
+
 // UnknownAttrs returns a copy of the raw configuration attributes
 // that are supposedly specific to the environment type. They could
 // also be wrong attributes, though. Only the specific environment
@@ -1077,6 +1108,7 @@ var alwaysOptional = schema.Defaults{
 	MaxStatusHistoryAge:          schema.Omit,
 	MaxStatusHistorySize:         schema.Omit,
 	UpdateStatusHookInterval:     schema.Omit,
+	EgressCidrs:                  schema.Omit,
 }
 
 func allowEmpty(attr string) bool {
@@ -1463,4 +1495,9 @@ data of the store. (default false)`,
 		Type:        environschema.Tstring,
 		Group:       environschema.EnvironGroup,
 	},
+	EgressCidrs: {
+		Description: "Source address(es) for traffic originating from this model",
+		Type:        environschema.Tstring,
+		Group:       environschema.EnvironGroup,
+	},
 }
diff --git a/environs/config/config_test.go b/environs/config/config_test.go
@@ -1124,6 +1124,13 @@ func (s *ConfigSuite) TestUpdateStatusHookIntervalConfigValue(c *gc.C) {
 	c.Assert(cfg.UpdateStatusHookInterval(), gc.Equals, 30*time.Minute)
 }
 
+func (s *ConfigSuite) TestEgressCidrs(c *gc.C) {
+	cfg := newTestConfig(c, testing.Attrs{
+		"egress-cidrs": "10.0.0.1/32, 192.168.1.1/16",
+	})
+	c.Assert(cfg.EgressCidrs(), gc.DeepEquals, []string{"10.0.0.1/32", "192.168.1.1/16"})
+}
+
 func (s *ConfigSuite) TestSchemaNoExtra(c *gc.C) {
 	schema, err := config.Schema(nil)
 	c.Assert(err, gc.IsNil)