Skip to content

Commit

Permalink
juju: use new DNS caching functionality
Browse files Browse the repository at this point in the history 
We rely on the new API client functionality to do our
DNS resolving for us, and store any resulting cached
names in the controllers.yaml entry.

The UnresolvedAPIEndpoints field is now redundant
and is replaced by the strictly advisory DNSCache
field. The APIEndpoints field now always contains all
the host names returned from the Login call with
the exception of addresses deemed unusable
(e.g. link-local addresses).

We also change rpcreflect.Value.FindMethod to accept any
version so that it's more useful for testing - this makes it
possible to server limited portions of the juju API from
a mocked API type without implementing a custom FindMethod.
And, in passing, fix rpc.NewConn so that it works with a nil obververFactory
as documented.

QA check that API connections still work OK, particularly on
MAAS setups where some of the returned host names will
not resolve on a client.

Also, check that:

	juju register jaas
	juju list-models

does not produce the "cannot validate certificate for because it
doesn't contain any IP SANs" error.

Fixes https://bugs.launchpad.net/juju/+bug/1692905
  • Loading branch information
@rogpeppe
rogpeppe committed Jun 2, 2017
1 parent 6d55ec8 commit 69ea61e
Show file tree
Hide file tree
Showing 20 changed files with 510 additions and 670 deletions.
43 changes: 9 additions & 34 deletions api/apiclient_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"gopkg.in/juju/worker.v1"

"github.com/juju/juju/api"
apitesting "github.com/juju/juju/api/testing"
"github.com/juju/juju/apiserver"
"github.com/juju/juju/apiserver/observer"
"github.com/juju/juju/apiserver/observer/fakeobserver"
Expand Down Expand Up @@ -210,7 +211,7 @@ func (s *apiclientSuite) TestDialWebsocketStopsOtherDialAttempts(c *gc.C) {
DialAddressInterval: dialAddressInterval,
DialWebsocket: fakeDialer,
Clock: clock,
IPAddrResolver: fakeResolver{
IPAddrResolver: apitesting.IPAddrResolverMap{
"place1.example": {"0.1.1.1"},
"place2.example": {"0.2.2.2"},
},
Expand Down Expand Up @@ -587,7 +588,7 @@ func (s *apiclientSuite) TestOpenCachesDNS(c *gc.C) {
Timeout: 5 * time.Second,
RetryDelay: 1 * time.Second,
DialWebsocket: fakeDialer,
IPAddrResolver: fakeResolver{
IPAddrResolver: apitesting.IPAddrResolverMap{
"place1.example": {"0.1.1.1"},
},
DNSCache: dnsCache,
Expand Down Expand Up @@ -616,7 +617,7 @@ func (s *apiclientSuite) TestDNSCacheUsed(c *gc.C) {
// then there's a possibility that the resolving will
// happen and a second dial attempt will happen before
// the Open returns, giving rise to a race.
IPAddrResolver: fakeResolver{},
IPAddrResolver: apitesting.IPAddrResolverMap{},
DNSCache: dnsCacheMap{
"place1.example": {"0.1.1.1"},
},
Expand Down Expand Up @@ -645,7 +646,7 @@ func (s *apiclientSuite) TestNumericAddressIsNotAddedToCache(c *gc.C) {
Timeout: 5 * time.Second,
RetryDelay: 1 * time.Second,
DialWebsocket: fakeDialer,
IPAddrResolver: fakeResolver{},
IPAddrResolver: apitesting.IPAddrResolverMap{},
DNSCache: dnsCache,
Clock: &fakeClock{},
})
Expand Down Expand Up @@ -686,7 +687,7 @@ func (s *apiclientSuite) TestFallbackToIPLookupWhenCacheOutOfDate(c *gc.C) {
// Note: zero timeout means each address attempt
// will only try once only.
DialWebsocket: fakeDialer,
IPAddrResolver: fakeResolver{
IPAddrResolver: apitesting.IPAddrResolverMap{
"place1.example": {"0.2.2.2"},
},
DNSCache: dnsCache,
Expand Down Expand Up @@ -742,7 +743,7 @@ func (s *apiclientSuite) TestWithUnresolvableAddr(c *gc.C) {
Timeout: 5 * time.Second,
RetryDelay: 1 * time.Second,
DialWebsocket: fakeDialer,
IPAddrResolver: fakeResolver{},
IPAddrResolver: apitesting.IPAddrResolverMap{},
Clock: &fakeClock{},
})
c.Assert(err, gc.ErrorMatches, `cannot resolve "nowhere.example": mock resolver cannot resolve "nowhere.example"`)
Expand Down Expand Up @@ -771,7 +772,7 @@ func (s *apiclientSuite) TestWithUnresolvableAddrAfterCacheFallback(c *gc.C) {
Timeout: 5 * time.Second,
RetryDelay: 1 * time.Second,
DialWebsocket: fakeDialer,
IPAddrResolver: fakeResolver{
IPAddrResolver: apitesting.IPAddrResolverMap{
"place1.example": {"0.2.2.2"},
},
DNSCache: dnsCache,
Expand Down Expand Up @@ -1019,37 +1020,11 @@ func (c fakeConn) Close() error {
return nil
}

// fakeResolver implements IPAddrResolver
// by looking up the addresses in the map,
// which maps host names to IP addresses.
type fakeResolver map[string][]string

func (r fakeResolver) LookupIPAddr(ctx context.Context, host string) ([]net.IPAddr, error) {
if ip := net.ParseIP(host); ip != nil {
return []net.IPAddr{{IP: ip}}, nil
}
ipStrs := r[host]
if len(ipStrs) == 0 {
return nil, errors.Errorf("mock resolver cannot resolve %q", host)
}
ipAddrs := make([]net.IPAddr, len(ipStrs))
for i, ipStr := range ipStrs {
ip := net.ParseIP(ipStr)
if ip == nil {
panic("invalid IP address: " + ipStr)
}
ipAddrs[i] = net.IPAddr{
IP: ip,
}
}
return ipAddrs, nil
}

// seqResolver returns an implementation of
// IPAddrResolver that maps the given addresses
// to sequential IP addresses 0.1.1.1, 0.2.2.2, etc.
func seqResolver(addrs ...string) api.IPAddrResolver {
r := make(fakeResolver)
r := make(apitesting.IPAddrResolverMap)
for i, addr := range addrs {
host, _, err := net.SplitHostPort(addr)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion api/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ type IPAddrResolver interface {

// DNSCache implements a cache of DNS lookup results.
type DNSCache interface {
// Lookup returns an IP addresses associated
// Lookup returns the IP addresses associated
// with the given host.
Lookup(host string) []string
// Add sets the IP addresses associated with
Expand Down
27 changes: 27 additions & 0 deletions api/testing/fakeserver.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2017 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package testing

import (
"net"

"github.com/juju/juju/rpc"
"github.com/juju/juju/rpc/jsoncodec"
)

// FakeAPIServer returns a net.Conn implementation
// that serves the RPC server defined by the given
// root object (see rpc.Conn.Serve).
func FakeAPIServer(root interface{}) net.Conn {
c0, c1 := net.Pipe()
serverCodec := jsoncodec.NewNet(c1)
serverRPC := rpc.NewConn(serverCodec, nil)
serverRPC.Serve(root, nil)
serverRPC.Start()
go func() {
<-serverRPC.Dead()
serverRPC.Close()
}()
return c0
}
41 changes: 41 additions & 0 deletions api/testing/resolver.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2017 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package testing

import (
"context"
"net"

"github.com/juju/errors"

"github.com/juju/juju/api"
)

var _ api.IPAddrResolver = IPAddrResolverMap(nil)

// IPAddrResolverMap implements IPAddrResolver by looking up the
// addresses in the map, which maps host names to IP addresses. The
// strings in the value slices should be valid IP addresses.
type IPAddrResolverMap map[string][]string

func (r IPAddrResolverMap) LookupIPAddr(ctx context.Context, host string) ([]net.IPAddr, error) {
if ip := net.ParseIP(host); ip != nil {
return []net.IPAddr{{IP: ip}}, nil
}
ipStrs := r[host]
if len(ipStrs) == 0 {
return nil, errors.Errorf("mock resolver cannot resolve %q", host)
}
ipAddrs := make([]net.IPAddr, len(ipStrs))
for i, ipStr := range ipStrs {
ip := net.ParseIP(ipStr)
if ip == nil {
panic("invalid IP address: " + ipStr)
}
ipAddrs[i] = net.IPAddr{
IP: ip,
}
}
return ipAddrs, nil
}
4 changes: 4 additions & 0 deletions cmd/juju/application/upgradecharm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,10 @@ func (m *mockAPIConnection) Addr() string {
return "0.1.2.3:1234"
}

func (m *mockAPIConnection) IPAddr() string {
return "0.1.2.3:1234"
}

func (m *mockAPIConnection) AuthTag() names.Tag {
return names.NewUserTag("testuser")
}
Expand Down
35 changes: 16 additions & 19 deletions cmd/juju/backups/restore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,11 @@ func (s *restoreSuite) SetUpTest(c *gc.C) {

s.store = jujuclient.NewMemStore()
s.store.Controllers["testing"] = jujuclient.ControllerDetails{
ControllerUUID: "deadbeef-0bad-400d-8000-5b1d0d06f00d",
CACert: testing.CACert,
Cloud: "mycloud",
CloudRegion: "a-region",
APIEndpoints: []string{"10.0.1.1:17777"},
UnresolvedAPIEndpoints: []string{"10.0.1.1:17777"},
ControllerUUID: "deadbeef-0bad-400d-8000-5b1d0d06f00d",
CACert: testing.CACert,
Cloud: "mycloud",
CloudRegion: "a-region",
APIEndpoints: []string{"10.0.1.1:17777"},
}
s.store.CurrentControllerName = "testing"
s.store.Models["testing"] = &jujuclient.ControllerModels{
Expand Down Expand Up @@ -205,12 +204,11 @@ func (s *restoreSuite) TestFailedRestoreReboostrapMaintainsControllerInfo(c *gc.
c.Assert(err, gc.ErrorMatches, "failed")
// The details below are as per what was done in test setup, so no changes.
c.Assert(s.store.Controllers["testing"], jc.DeepEquals, jujuclient.ControllerDetails{
Cloud: "mycloud",
CloudRegion: "a-region",
CACert: testing.CACert,
ControllerUUID: "deadbeef-0bad-400d-8000-5b1d0d06f00d",
APIEndpoints: []string{"10.0.1.1:17777"},
UnresolvedAPIEndpoints: []string{"10.0.1.1:17777"},
Cloud: "mycloud",
CloudRegion: "a-region",
CACert: testing.CACert,
ControllerUUID: "deadbeef-0bad-400d-8000-5b1d0d06f00d",
APIEndpoints: []string{"10.0.1.1:17777"},
})
}

Expand Down Expand Up @@ -248,13 +246,12 @@ func (s *restoreSuite) TestRestoreReboostrapWritesUpdatedControllerInfo(c *gc.C)
c.Assert(err, jc.ErrorIsNil)
c.Assert(boostrapped, jc.IsTrue)
c.Assert(s.store.Controllers["testing"], jc.DeepEquals, jujuclient.ControllerDetails{
Cloud: "mycloud",
CloudRegion: "a-region",
CACert: testing.CACert,
ControllerUUID: "deadbeef-1bad-500d-9000-4b1d0d06f00d",
APIEndpoints: []string{"10.0.0.1:17777"},
UnresolvedAPIEndpoints: []string{"10.0.0.1:17777"},
AgentVersion: version.Current.String(),
Cloud: "mycloud",
CloudRegion: "a-region",
CACert: testing.CACert,
ControllerUUID: "deadbeef-1bad-500d-9000-4b1d0d06f00d",
APIEndpoints: []string{"10.0.0.1:17777"},
AgentVersion: version.Current.String(),
// We won't get correct model and machine counts until
// we connect properly eventually.
ModelCount: nil,
Expand Down
1 change: 0 additions & 1 deletion cmd/juju/commands/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ func (s *BootstrapSuite) run(c *gc.C, test bootstrapTest) testing.Restorer {
controller, err := s.store.ControllerByName(controllerName)
c.Assert(err, jc.ErrorIsNil)
c.Assert(controller.CACert, gc.Not(gc.Equals), "")
c.Assert(controller.UnresolvedAPIEndpoints, gc.DeepEquals, addrConnectedTo)
c.Assert(controller.APIEndpoints, gc.DeepEquals, addrConnectedTo)
c.Assert(utils.IsValidUUIDString(controller.ControllerUUID), jc.IsTrue)
// We don't care about build numbers here.
Expand Down
Loading

0 comments on commit 69ea61e

Please sign in to comment.