Merge pull request juju#17608 from cderici/merge-3.4-3.5-20240627

juju#17608

Bringing:

- juju#17594 
- juju#17597 

No conflicts

apiserver/facades/agent/uniter/uniter.go

@@ -2681,6 +2681,11 @@ func (u *UniterAPI) commitHookChangesForOneUnit(unitTag names.UnitTag, changes p
 				return apiservererrors.ErrPerm
 			}
 
+			// ICMP is not supported on CAAS models.
+			if u.m.Type() == state.ModelTypeCAAS && r.Protocol == "icmp" {
+				return errors.NotSupportedf("protocol icmp on caas models")
+			}
+
 			// Pre-2.9 clients (using V15 or V16 of this API) do
 			// not populate the new Endpoint field; this
 			// effectively opens the port for all endpoints and

apiserver/facades/agent/uniter/uniter_test.go

@@ -4648,6 +4648,30 @@ func (s *uniterNetworkInfoSuite) TestCommitHookChanges(c *gc.C) {
 	c.Assert(appCfg, gc.DeepEquals, map[string]interface{}{"app_data": "updated"}, gc.Commentf("application data not updated by leader unit"))
 }
 
+func (s *uniterNetworkInfoSuite) TestCommitHookChangesOpenPortsICMPNotSupported(c *gc.C) {
+	_, cm, _, unit := s.setupCAASModel(c, true)
+
+	b := apiuniter.NewCommitHookParamsBuilder(unit.UnitTag())
+
+	b.OpenPortRange("db", network.MustParsePortRange("icmp"))
+	req, _ := b.Build()
+
+	s.State = cm.State()
+	s.authorizer = apiservertesting.FakeAuthorizer{Tag: unit.Tag()}
+	uniterAPI, err := uniter.NewUniterAPI(s.facadeContext())
+	c.Assert(err, jc.ErrorIsNil)
+
+	result, err := uniterAPI.CommitHookChanges(req)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(result, gc.DeepEquals, params.ErrorResults{
+		Results: []params.ErrorResult{
+			{Error: &params.Error{
+				Message: "protocol icmp on caas models not supported",
+				Code:    "not supported"}},
+		},
+	})
+}
+
 func (s *uniterNetworkInfoSuite) TestCommitHookChangesWhenNotLeader(c *gc.C) {
 	s.addRelationAndAssertInScope(c)
 

apiserver/facades/controller/crosscontroller/crosscontroller_test.go

@@ -1,37 +1,38 @@
 // Copyright 2017 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package crosscontroller_test
+package crosscontroller
 
 import (
 	"errors"
 
+	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/apiserver/common"
-	"github.com/juju/juju/apiserver/facades/controller/crosscontroller"
+	"github.com/juju/juju/controller"
+	"github.com/juju/juju/core/network"
 	"github.com/juju/juju/rpc/params"
 	"github.com/juju/juju/state"
-	coretesting "github.com/juju/juju/testing"
 )
 
 var _ = gc.Suite(&CrossControllerSuite{})
 
 type CrossControllerSuite struct {
-	coretesting.BaseSuite
+	testing.IsolationSuite
 
 	resources                *common.Resources
 	watcher                  *mockNotifyWatcher
 	localControllerInfo      func() ([]string, string, error)
 	watchLocalControllerInfo func() state.NotifyWatcher
-	api                      *crosscontroller.CrossControllerAPI
+	api                      *CrossControllerAPI
 
 	publicDnsAddress string
 }
 
 func (s *CrossControllerSuite) SetUpTest(c *gc.C) {
-	s.BaseSuite.SetUpTest(c)
+	s.IsolationSuite.SetUpTest(c)
 	s.resources = common.NewResources()
 	s.AddCleanup(func(*gc.C) { s.resources.StopAll() })
 	s.localControllerInfo = func() ([]string, string, error) {
@@ -40,7 +41,7 @@ func (s *CrossControllerSuite) SetUpTest(c *gc.C) {
 	s.watchLocalControllerInfo = func() state.NotifyWatcher {
 		return s.watcher
 	}
-	api, err := crosscontroller.NewCrossControllerAPI(
+	api, err := NewCrossControllerAPI(
 		s.resources,
 		func() ([]string, string, error) { return s.localControllerInfo() },
 		func() (string, error) { return s.publicDnsAddress, nil },
@@ -49,14 +50,14 @@ func (s *CrossControllerSuite) SetUpTest(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	s.api = api
 	s.watcher = newMockNotifyWatcher()
-	s.AddCleanup(func(*gc.C) { s.watcher.Stop() })
+	s.AddCleanup(func(*gc.C) { _ = s.watcher.Stop() })
 }
 
 func (s *CrossControllerSuite) TestControllerInfo(c *gc.C) {
 	results, err := s.api.ControllerInfo()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, params.ControllerAPIInfoResults{
-		[]params.ControllerAPIInfoResult{{
+		Results: []params.ControllerAPIInfoResult{{
 			Addresses: []string{"addr1", "addr2"},
 			CACert:    "ca-cert",
 		}},
@@ -68,7 +69,7 @@ func (s *CrossControllerSuite) TestControllerInfoWithDNSAddress(c *gc.C) {
 	results, err := s.api.ControllerInfo()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, params.ControllerAPIInfoResults{
-		[]params.ControllerAPIInfoResult{{
+		Results: []params.ControllerAPIInfoResult{{
 			Addresses: []string{"publicDNSaddr", "addr1", "addr2"},
 			CACert:    "ca-cert",
 		}},
@@ -82,7 +83,7 @@ func (s *CrossControllerSuite) TestControllerInfoError(c *gc.C) {
 	results, err := s.api.ControllerInfo()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, params.ControllerAPIInfoResults{
-		[]params.ControllerAPIInfoResult{{
+		Results: []params.ControllerAPIInfoResult{{
 			Error: &params.Error{Message: "nope"},
 		}},
 	})
@@ -93,7 +94,7 @@ func (s *CrossControllerSuite) TestWatchControllerInfo(c *gc.C) {
 	results, err := s.api.WatchControllerInfo()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, params.NotifyWatchResults{
-		[]params.NotifyWatchResult{{
+		Results: []params.NotifyWatchResult{{
 			NotifyWatcherId: "1",
 		}},
 	})
@@ -107,9 +108,51 @@ func (s *CrossControllerSuite) TestWatchControllerInfoError(c *gc.C) {
 	results, err := s.api.WatchControllerInfo()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, params.NotifyWatchResults{
-		[]params.NotifyWatchResult{{
+		Results: []params.NotifyWatchResult{{
 			Error: &params.Error{Message: "nope"},
 		}},
 	})
 	c.Assert(s.resources.Get("1"), gc.IsNil)
 }
+
+type stubControllerInfoGetter struct{}
+
+func (stubControllerInfoGetter) APIHostPortsForClients() ([]network.SpaceHostPorts, error) {
+	return []network.SpaceHostPorts{{
+		network.SpaceHostPort{
+			SpaceAddress: network.SpaceAddress{
+				MachineAddress: network.MachineAddress{
+					Value: "10.1.2.3",
+					Scope: network.ScopeCloudLocal,
+				},
+				SpaceID: "0",
+			},
+			NetPort: 50000,
+		},
+		network.SpaceHostPort{
+			SpaceAddress: network.SpaceAddress{
+				MachineAddress: network.MachineAddress{
+					Value: "host-name",
+					Scope: network.ScopePublic,
+				},
+				SpaceID: "0",
+			},
+			NetPort: 50000,
+		},
+	}}, nil
+}
+
+func (stubControllerInfoGetter) ControllerConfig() (controller.Config, error) {
+	return map[string]interface{}{
+		"ca-cert": "ca-cert",
+	}, nil
+}
+
+func (s *CrossControllerSuite) TestGetControllerInfo(c *gc.C) {
+	addrs, cert, err := controllerInfo(stubControllerInfoGetter{})
+	c.Assert(err, jc.ErrorIsNil)
+
+	// Public address is sorted first.
+	c.Check(addrs, jc.DeepEquals, []string{"host-name:50000", "10.1.2.3:50000"})
+	c.Check(cert, gc.Equals, "ca-cert")
+}

apiserver/facades/controller/crosscontroller/package_test.go

@@ -1,14 +1,50 @@
 // Copyright 2017 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package crosscontroller_test
+package crosscontroller
 
 import (
 	"testing"
 
 	gc "gopkg.in/check.v1"
+	"gopkg.in/tomb.v2"
 )
 
 func TestAll(t *testing.T) {
 	gc.TestingT(t)
 }
+
+type mockNotifyWatcher struct {
+	tomb    tomb.Tomb
+	changes chan struct{}
+}
+
+func newMockNotifyWatcher() *mockNotifyWatcher {
+	w := &mockNotifyWatcher{changes: make(chan struct{}, 1)}
+	w.tomb.Go(func() error {
+		<-w.tomb.Dying()
+		return nil
+	})
+	return w
+}
+
+func (w *mockNotifyWatcher) Stop() error {
+	w.Kill()
+	return w.Wait()
+}
+
+func (w *mockNotifyWatcher) Wait() error {
+	return w.tomb.Wait()
+}
+
+func (w *mockNotifyWatcher) Kill() {
+	w.tomb.Kill(nil)
+}
+
+func (w *mockNotifyWatcher) Err() error {
+	return w.tomb.Err()
+}
+
+func (w *mockNotifyWatcher) Changes() <-chan struct{} {
+	return w.changes
+}

apiserver/facades/controller/crosscontroller/register.go

@@ -8,8 +8,9 @@ import (
 
 	"github.com/juju/errors"
 
-	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/facade"
+	"github.com/juju/juju/controller"
+	"github.com/juju/juju/core/network"
 )
 
 // Register is called to expose a package of facades onto a given registry.
@@ -26,7 +27,7 @@ func newStateCrossControllerAPI(ctx facade.Context) (*CrossControllerAPI, error)
 	return NewCrossControllerAPI(
 		ctx.Resources(),
 		func() ([]string, string, error) {
-			return common.StateControllerInfo(st)
+			return controllerInfo(st)
 		},
 		func() (string, error) {
 			config, err := st.ControllerConfig()
@@ -38,3 +39,37 @@ func newStateCrossControllerAPI(ctx facade.Context) (*CrossControllerAPI, error)
 		st.WatchAPIHostPortsForClients,
 	)
 }
+
+// controllerInfoGetter indirects state for retrieving information
+// required for cross-controller communication.
+type controllerInfoGetter interface {
+	APIHostPortsForClients() ([]network.SpaceHostPorts, error)
+	ControllerConfig() (controller.Config, error)
+}
+
+// controllerInfo retrieves information required to communicate
+// with this controller - API addresses and the CA cert.
+func controllerInfo(st controllerInfoGetter) ([]string, string, error) {
+	apiHostPorts, err := st.APIHostPortsForClients()
+	if err != nil {
+		return nil, "", errors.Trace(err)
+	}
+
+	var addrs []string
+	for _, hostPorts := range apiHostPorts {
+		ordered := hostPorts.HostPorts().PrioritizedForScope(network.ScopeMatchPublic)
+		for _, addr := range ordered {
+			if addr != "" {
+				addrs = append(addrs, addr)
+			}
+		}
+	}
+
+	controllerConfig, err := st.ControllerConfig()
+	if err != nil {
+		return nil, "", errors.Trace(err)
+	}
+
+	caCert, _ := controllerConfig.CACert()
+	return addrs, caCert, nil
+}