Remove tap from public API and associated test infrastructure (linker…

…d#3240) ### Summary After the addition of the tap APIServer, all the logic related to tap in the public API no longer needs to be there. The servers and clients that are created but not used, as well as all the old testing infrastrucure related to tap can be removed. This deprecates TapByResource and therefore required an update to the protobuf files with `bin/protoc-go.sh`. While the change to deprecate this method was extremely small, a lot of protobuf fils were updated in the process. These changes to the code and protobuf files should probably remain coupled since `TapByResource` is officially deprecated in the public API, but a majority of the additions/deletions are related to those files. This draft passes `go test` as well as a local run of the integration tests. Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
mackjmr · Aug 14, 2019 · cc3c53f · cc3c53f
1 parent 8ef4104
commit cc3c53f
Show file tree

Hide file tree

Showing 21 changed files with 252 additions and 403 deletions.
diff --git a/charts/linkerd2/templates/controller.yaml b/charts/linkerd2/templates/controller.yaml
@@ -80,7 +80,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.{{.Namespace}}.svc.{{.ClusterDomain}}:9090
-        - -tap-addr=linkerd-tap.{{.Namespace}}.svc.{{.ClusterDomain}}:8088
         - -controller-namespace={{.Namespace}}
         - -log-level={{.ControllerLogLevel}}
         image: {{.ControllerImage}}:{{default .LinkerdVersion .ControllerImageVersion}}

diff --git a/cli/cmd/testdata/install_control-plane.golden b/cli/cmd/testdata/install_control-plane.golden
@@ -318,7 +318,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:install-control-plane-version

diff --git a/cli/cmd/testdata/install_default.golden b/cli/cmd/testdata/install_default.golden
@@ -999,7 +999,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:install-control-plane-version

diff --git a/cli/cmd/testdata/install_ha_output.golden b/cli/cmd/testdata/install_ha_output.golden
@@ -1051,7 +1051,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:install-control-plane-version

diff --git a/cli/cmd/testdata/install_ha_with_overrides_output.golden b/cli/cmd/testdata/install_ha_with_overrides_output.golden
@@ -1051,7 +1051,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:install-control-plane-version

diff --git a/cli/cmd/testdata/install_no_init_container.golden b/cli/cmd/testdata/install_no_init_container.golden
@@ -963,7 +963,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:install-control-plane-version

diff --git a/cli/cmd/testdata/install_output.golden b/cli/cmd/testdata/install_output.golden
@@ -997,7 +997,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.Namespace.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.Namespace.svc.cluster.local:8088
         - -controller-namespace=Namespace
         - -log-level=ControllerLogLevel
         image: ControllerImage:ControllerImageVersion

diff --git a/cli/cmd/testdata/upgrade_default.golden b/cli/cmd/testdata/upgrade_default.golden
@@ -1000,7 +1000,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:UPGRADE-CONTROL-PLANE-VERSION

diff --git a/cli/cmd/testdata/upgrade_ha.golden b/cli/cmd/testdata/upgrade_ha.golden
@@ -1052,7 +1052,6 @@ spec:
       - args:
         - public-api
         - -prometheus-url=http://linkerd-prometheus.linkerd.svc.cluster.local:9090
-        - -tap-addr=linkerd-tap.linkerd.svc.cluster.local:8088
         - -controller-namespace=linkerd
         - -log-level=info
         image: gcr.io/linkerd-io/controller:UPGRADE-CONTROL-PLANE-VERSION

diff --git a/controller/api/public/client.go b/controller/api/public/client.go
@@ -97,22 +97,11 @@ func (c *grpcOverHTTPClient) ListServices(ctx context.Context, req *pb.ListServi
 }
 
 func (c *grpcOverHTTPClient) Tap(ctx context.Context, req *pb.TapRequest, _ ...grpc.CallOption) (pb.Api_TapClient, error) {
-	return nil, status.Error(codes.Unimplemented, "Tap is deprecated, use TapByResource")
+	return nil, status.Error(codes.Unimplemented, "Tap is deprecated in public API, use tap APIServer")
 }
 
 func (c *grpcOverHTTPClient) TapByResource(ctx context.Context, req *pb.TapByResourceRequest, _ ...grpc.CallOption) (pb.Api_TapByResourceClient, error) {
-	url := c.endpointNameToPublicAPIURL("TapByResource")
-	httpRsp, err := c.post(ctx, url, req)
-	if err != nil {
-		return nil, err
-	}
-
-	client, err := getStreamClient(ctx, httpRsp)
-	if err != nil {
-		return nil, err
-	}
-
-	return &tapClient{client}, nil
+	return nil, status.Error(codes.Unimplemented, "Tap is deprecated in public API, use tap APIServer")
 }
 
 func (c *grpcOverHTTPClient) Get(ctx context.Context, req *destinationPb.GetDestination, _ ...grpc.CallOption) (destinationPb.Destination_GetClient, error) {
@@ -189,16 +178,6 @@ func (c *grpcOverHTTPClient) endpointNameToPublicAPIURL(endpoint string) *url.UR
 	return c.serverURL.ResolveReference(&url.URL{Path: endpoint})
 }
 
-type tapClient struct {
-	streamClient
-}
-
-func (c tapClient) Recv() (*pb.TapEvent, error) {
-	var msg pb.TapEvent
-	err := protohttp.FromByteStreamToProtocolBuffers(c.reader, &msg)
-	return &msg, err
-}
-
 type destinationClient struct {
 	streamClient
 }

diff --git a/controller/api/public/grpc_server.go b/controller/api/public/grpc_server.go
@@ -13,7 +13,6 @@ import (
 	healthcheckPb "github.com/linkerd/linkerd2/controller/gen/common/healthcheck"
 	configPb "github.com/linkerd/linkerd2/controller/gen/config"
 	discoveryPb "github.com/linkerd/linkerd2/controller/gen/controller/discovery"
-	tapPb "github.com/linkerd/linkerd2/controller/gen/controller/tap"
 	pb "github.com/linkerd/linkerd2/controller/gen/public"
 	"github.com/linkerd/linkerd2/controller/k8s"
 	"github.com/linkerd/linkerd2/pkg/config"
@@ -37,7 +36,6 @@ type APIServer interface {
 
 type grpcServer struct {
 	prometheusAPI         promv1.API
-	tapClient             tapPb.TapClient
 	discoveryClient       discoveryPb.DiscoveryClient
 	destinationClient     destinationPb.DestinationClient
 	k8sAPI                *k8s.API
@@ -62,7 +60,6 @@ const (
 
 func newGrpcServer(
 	promAPI promv1.API,
-	tapClient tapPb.TapClient,
 	discoveryClient discoveryPb.DiscoveryClient,
 	destinationClient destinationPb.DestinationClient,
 	k8sAPI *k8s.API,
@@ -72,7 +69,6 @@ func newGrpcServer(
 
 	grpcServer := &grpcServer{
 		prometheusAPI:         promAPI,
-		tapClient:             tapClient,
 		discoveryClient:       discoveryClient,
 		destinationClient:     destinationClient,
 		k8sAPI:                k8sAPI,
@@ -244,29 +240,11 @@ func (s *grpcServer) Config(ctx context.Context, req *pb.Empty) (*configPb.All,
 }
 
 func (s *grpcServer) Tap(req *pb.TapRequest, stream pb.Api_TapServer) error {
-	return status.Error(codes.Unimplemented, "Tap is deprecated, use TapByResource")
+	return status.Error(codes.Unimplemented, "Tap is deprecated in public API, use tap APIServer")
 }
 
-// Pass through to tap service
 func (s *grpcServer) TapByResource(req *pb.TapByResourceRequest, stream pb.Api_TapByResourceServer) error {
-	tapStream := stream.(tapServer)
-	tapClient, err := s.tapClient.TapByResource(tapStream.Context(), req)
-	if err != nil {
-		log.Errorf("Unexpected error tapping [%v]: %v", req, err)
-		return err
-	}
-	for {
-		select {
-		case <-tapStream.Context().Done():
-			return nil
-		default:
-			event, err := tapClient.Recv()
-			if err != nil {
-				return err
-			}
-			tapStream.Send(event)
-		}
-	}
+	return status.Error(codes.Unimplemented, "Tap is deprecated in public API, use tap APIServer")
 }
 
 // Pass through to Destination service

diff --git a/controller/api/public/grpc_server_test.go b/controller/api/public/grpc_server_test.go
@@ -400,7 +400,6 @@ status:
 				&mProm,
 				nil,
 				nil,
-				nil,
 				k8sAPI,
 				"linkerd",
 				[]string{},
@@ -503,7 +502,6 @@ metadata:
 				&MockProm{},
 				nil,
 				nil,
-				nil,
 				k8sAPI,
 				"linkerd",
 				[]string{},
@@ -534,7 +532,6 @@ func TestConfig(t *testing.T) {
 			&MockProm{},
 			nil,
 			nil,
-			nil,
 			k8sAPI,
 			"linkerd",
 			[]string{},

diff --git a/controller/api/public/http_server.go b/controller/api/public/http_server.go
@@ -9,7 +9,6 @@ import (
 	destinationPb "github.com/linkerd/linkerd2-proxy-api/go/destination"
 	healthcheckPb "github.com/linkerd/linkerd2/controller/gen/common/healthcheck"
 	discoveryPb "github.com/linkerd/linkerd2/controller/gen/controller/discovery"
-	tapPb "github.com/linkerd/linkerd2/controller/gen/controller/tap"
 	pb "github.com/linkerd/linkerd2/controller/gen/public"
 	"github.com/linkerd/linkerd2/controller/k8s"
 	"github.com/linkerd/linkerd2/pkg/prometheus"
@@ -21,17 +20,16 @@ import (
 )
 
 var (
-	statSummaryPath   = fullURLPathFor("StatSummary")
-	topRoutesPath     = fullURLPathFor("TopRoutes")
-	versionPath       = fullURLPathFor("Version")
-	listPodsPath      = fullURLPathFor("ListPods")
-	listServicesPath  = fullURLPathFor("ListServices")
-	tapByResourcePath = fullURLPathFor("TapByResource")
-	selfCheckPath     = fullURLPathFor("SelfCheck")
-	endpointsPath     = fullURLPathFor("Endpoints")
-	edgesPath         = fullURLPathFor("Edges")
-	destGetPath       = fullURLPathFor("DestinationGet")
-	configPath        = fullURLPathFor("Config")
+	statSummaryPath  = fullURLPathFor("StatSummary")
+	topRoutesPath    = fullURLPathFor("TopRoutes")
+	versionPath      = fullURLPathFor("Version")
+	listPodsPath     = fullURLPathFor("ListPods")
+	listServicesPath = fullURLPathFor("ListServices")
+	selfCheckPath    = fullURLPathFor("SelfCheck")
+	endpointsPath    = fullURLPathFor("Endpoints")
+	edgesPath        = fullURLPathFor("Edges")
+	destGetPath      = fullURLPathFor("DestinationGet")
+	configPath       = fullURLPathFor("Config")
 )
 
 type handler struct {
@@ -60,8 +58,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		h.handleListPods(w, req)
 	case listServicesPath:
 		h.handleListServices(w, req)
-	case tapByResourcePath:
-		h.handleTapByResource(w, req)
 	case selfCheckPath:
 		h.handleSelfCheck(w, req)
 	case endpointsPath:
@@ -226,28 +222,6 @@ func (h *handler) handleListServices(w http.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (h *handler) handleTapByResource(w http.ResponseWriter, req *http.Request) {
-	flushableWriter, err := protohttp.NewStreamingWriter(w)
-	if err != nil {
-		protohttp.WriteErrorToHTTPResponse(w, err)
-		return
-	}
-
-	var protoRequest pb.TapByResourceRequest
-	err = protohttp.HTTPRequestToProto(req, &protoRequest)
-	if err != nil {
-		protohttp.WriteErrorToHTTPResponse(w, err)
-		return
-	}
-
-	server := tapServer{streamServer{w: flushableWriter, req: req}}
-	err = h.grpcServer.TapByResource(&protoRequest, server)
-	if err != nil {
-		protohttp.WriteErrorToHTTPResponse(w, err)
-		return
-	}
-}
-
 func (h *handler) handleDestGet(w http.ResponseWriter, req *http.Request) {
 	flushableWriter, err := protohttp.NewStreamingWriter(w)
 	if err != nil {
@@ -315,14 +289,6 @@ func (s streamServer) Send(msg proto.Message) error {
 	return nil
 }
 
-type tapServer struct {
-	streamServer
-}
-
-func (s tapServer) Send(msg *pb.TapEvent) error {
-	return s.streamServer.Send(msg)
-}
-
 type destinationServer struct {
 	streamServer
 }
@@ -352,7 +318,6 @@ func (h *handler) handleEndpoints(w http.ResponseWriter, req *http.Request) {
 func NewServer(
 	addr string,
 	prometheusClient promApi.Client,
-	tapClient tapPb.TapClient,
 	discoveryClient discoveryPb.DiscoveryClient,
 	destinationClient destinationPb.DestinationClient,
 	k8sAPI *k8s.API,
@@ -362,7 +327,6 @@ func NewServer(
 	baseHandler := &handler{
 		grpcServer: newGrpcServer(
 			promv1.NewAPI(prometheusClient),
-			tapClient,
 			discoveryClient,
 			destinationClient,
 			k8sAPI,