Merge pull request TheThingsNetwork#1760 from TheThingsIndustries/qui…

…ckfix/auto-discovery

Use gRPC discovery for source/target NS/AS dialing

pkg/deviceclaimingserver/grpc.tti.go

@@ -223,11 +223,11 @@ func (s *endDeviceClaimingServer) Claim(ctx context.Context, req *ttnpb.ClaimEnd
 	sourceCallOpts := []grpc.CallOption{
 		grpc.PerRPCCredentials(sourceMD),
 	}
-	sourceDialOpts := append(rpcclient.DefaultDialOptions(sourceCtx), grpc.WithBlock(), grpc.FailOnNonTempDialError(true))
-	if tlsConfig, err := s.DCS.GetTLSClientConfig(sourceCtx); err == nil {
-		sourceDialOpts = append(sourceDialOpts, discover.WithTransportCredentials(credentials.NewTLS(tlsConfig))...)
-	} else if s.DCS.AllowInsecureForCredentials() {
-		sourceDialOpts = append(sourceDialOpts, discover.WithInsecure()...)
+	sourceDialOpts := append([]grpc.DialOption(nil), rpcclient.DefaultDialOptions(sourceCtx)...)
+	sourceDialOpts = append(sourceDialOpts, grpc.WithBlock(), grpc.FailOnNonTempDialError(true))
+	sourceTLSConfig, err := s.DCS.GetTLSClientConfig(sourceCtx)
+	if err != nil {
+		return nil, err
 	}
 
 	// Validate that the authorized application API key has enough rights to read and delete the device.
@@ -304,12 +304,11 @@ func (s *endDeviceClaimingServer) Claim(ctx context.Context, req *ttnpb.ClaimEnd
 	}
 
 	// Get source end device from Network Server and Application Server.
-	var sourceNSConn *grpc.ClientConn
 	var sourceNSClient ttnpb.NsEndDeviceRegistryClient
 	if sourceDev.NetworkServerAddress != "" {
 		logger := logger.WithField("network_server_address", sourceDev.NetworkServerAddress)
 		logger.Debug("Get source end device from Network Server")
-		sourceNSConn, err = grpc.DialContext(sourceCtx, sourceDev.NetworkServerAddress, sourceDialOpts...)
+		sourceNSConn, err := discover.DialContext(sourceCtx, sourceDev.NetworkServerAddress, credentials.NewTLS(sourceTLSConfig), sourceDialOpts...)
 		if err != nil {
 			logger.WithError(err).Warn("Failed to dial source Network Server")
 			return nil, err
@@ -334,17 +333,12 @@ func (s *endDeviceClaimingServer) Claim(ctx context.Context, req *ttnpb.ClaimEnd
 	if sourceDev.ApplicationServerAddress != "" {
 		logger := logger.WithField("application_server_address", sourceDev.ApplicationServerAddress)
 		logger.Debug("Get source end device from Application Server")
-		var sourceASConn *grpc.ClientConn
-		if sourceDev.ApplicationServerAddress == sourceDev.NetworkServerAddress {
-			sourceASConn = sourceNSConn
-		} else {
-			sourceASConn, err = grpc.DialContext(sourceCtx, sourceDev.ApplicationServerAddress, sourceDialOpts...)
-			if err != nil {
-				logger.WithError(err).Warn("Failed to dial source Application Server")
-				return nil, err
-			}
-			defer sourceASConn.Close()
+		sourceASConn, err := discover.DialContext(sourceCtx, sourceDev.ApplicationServerAddress, credentials.NewTLS(sourceTLSConfig), sourceDialOpts...)
+		if err != nil {
+			logger.WithError(err).Warn("Failed to dial source Application Server")
+			return nil, err
 		}
+		defer sourceASConn.Close()
 		sourceASClient = ttnpb.NewAsEndDeviceRegistryClient(sourceASConn)
 		sourceASDev, err := sourceASClient.Get(sourceCtx, &ttnpb.GetEndDeviceRequest{
 			EndDeviceIdentifiers: *sourceIDs,
@@ -366,26 +360,24 @@ func (s *endDeviceClaimingServer) Claim(ctx context.Context, req *ttnpb.ClaimEnd
 	targetCallOpts := []grpc.CallOption{
 		targetForwardAuth,
 	}
-	targetDialOpts := append(rpcclient.DefaultDialOptions(targetCtx), grpc.WithBlock(), grpc.FailOnNonTempDialError(true))
-	if tlsConfig, err := s.DCS.GetTLSClientConfig(targetCtx); err == nil {
-		targetDialOpts = append(targetDialOpts, discover.WithTransportCredentials(credentials.NewTLS(tlsConfig))...)
-	} else if s.DCS.AllowInsecureForCredentials() {
-		targetDialOpts = append(targetDialOpts, discover.WithInsecure()...)
+	targetDialOpts := append([]grpc.DialOption(nil), rpcclient.DefaultDialOptions(targetCtx)...)
+	targetDialOpts = append(targetDialOpts, grpc.WithBlock(), grpc.FailOnNonTempDialError(true))
+	targetTLSConfig, err := s.DCS.GetTLSClientConfig(targetCtx)
+	if err != nil {
+		return nil, err
 	}
 	var targetNSConn *grpc.ClientConn
 	if req.TargetNetworkServerAddress != "" {
-		targetNSConn, err = grpc.DialContext(targetCtx, req.TargetNetworkServerAddress, targetDialOpts...)
+		targetNSConn, err = discover.DialContext(targetCtx, req.TargetNetworkServerAddress, credentials.NewTLS(targetTLSConfig), targetDialOpts...)
 		if err != nil {
 			logger.WithError(err).Warn("Failed to dial target Network Server")
 			return nil, err
 		}
 		defer targetNSConn.Close()
 	}
 	var targetASConn *grpc.ClientConn
-	if req.TargetApplicationServerAddress == req.TargetNetworkServerAddress {
-		targetASConn = targetNSConn
-	} else if req.TargetApplicationServerAddress != "" {
-		targetASConn, err = grpc.DialContext(targetCtx, req.TargetApplicationServerAddress, targetDialOpts...)
+	if req.TargetApplicationServerAddress != "" {
+		targetASConn, err = discover.DialContext(targetCtx, req.TargetApplicationServerAddress, credentials.NewTLS(targetTLSConfig), targetDialOpts...)
 		if err != nil {
 			logger.WithError(err).Warn("Failed to dial target Application Server")
 			return nil, err

pkg/deviceclaimingserver/grpc.tti_test.go

@@ -20,6 +20,7 @@ import (
 	"go.thethings.network/lorawan-stack/pkg/events"
 	"go.thethings.network/lorawan-stack/pkg/log"
 	"go.thethings.network/lorawan-stack/pkg/rpcmetadata"
+	"go.thethings.network/lorawan-stack/pkg/rpcmiddleware/discover"
 	"go.thethings.network/lorawan-stack/pkg/rpcserver"
 	"go.thethings.network/lorawan-stack/pkg/tenant"
 	"go.thethings.network/lorawan-stack/pkg/ttipb"
@@ -1548,6 +1549,7 @@ func TestClaim(t *testing.T) {
 
 			dcs.AddContextFiller(func(ctx context.Context) context.Context {
 				ctx = test.ContextWithT(ctx, t)
+				ctx = discover.WithTLSFallback(ctx, false)
 				ctx = rights.NewContext(ctx, rights.Rights{
 					ApplicationRights: tc.ApplicationRights,
 				})

pkg/rpcmiddleware/discover/discover.go

@@ -74,11 +74,44 @@ func WithTransportCredentials(creds credentials.TransportCredentials) []grpc.Dia
 	}
 }
 
-// WithInsecure returns a DialOption which disables transport security and discovers the default insecure listen port if
-// not specified in the dial target.
+// WithInsecure returns gRPC dial options which disable transport security and discovers the default insecure listen
+// port if not specified in the dial target.
 func WithInsecure() []grpc.DialOption {
 	return []grpc.DialOption{
 		grpc.WithInsecure(),
 		grpc.WithContextDialer(resolver(false)),
 	}
 }
+
+type tlsFallbackKeyType struct{}
+
+var tlsFallbackKey tlsFallbackKeyType
+
+// WithTLSFallback returns a derived context which is configured to fall back to the given TLS setting if discovery
+// fails.
+func WithTLSFallback(parent context.Context, tls bool) context.Context {
+	return context.WithValue(parent, tlsFallbackKey, tls)
+}
+
+// DialOptions discovers gRPC dial options based on the given target. This includes whether or not transport level
+// security is enabled and service port discovery.
+func DialOptions(ctx context.Context, target string, creds credentials.TransportCredentials) ([]grpc.DialOption, error) {
+	// TODO: Discover through SRV records and cache result (https://github.com/TheThingsNetwork/lorawan-stack/issues/138)
+	if val, ok := ctx.Value(tlsFallbackKey).(bool); ok && !val {
+		return WithInsecure(), nil
+	}
+	return WithTransportCredentials(creds), nil
+}
+
+// DialContext creates a client connection to the given target. It uses DialOptions to discover gRPC dial options for
+// the target.
+func DialContext(ctx context.Context, target string, creds credentials.TransportCredentials, opts ...grpc.DialOption) (*grpc.ClientConn, error) {
+	discoveredOpts, err := DialOptions(ctx, target, creds)
+	if err != nil {
+		return nil, err
+	}
+	allOpts := make([]grpc.DialOption, 0, len(opts)+len(discoveredOpts))
+	allOpts = append(allOpts, opts...)
+	allOpts = append(allOpts, discoveredOpts...)
+	return grpc.DialContext(ctx, target, allOpts...)
+}