Remove SDS v2 API (#4444)

The SDS v2 API has been removed for Envoy for several years. We cannot reasonably support it any longer, and we expect most users should no longer be using it anymore. Signed-off-by: Ryan Turner <turner@uber.com>
spiffe · Aug 18, 2023 · 2604652 · 2604652
1 parent e29ddf6
commit 2604652
Show file tree

Hide file tree

Showing 29 changed files with 2 additions and 1,603 deletions.
diff --git a/cmd/spire-agent/cli/run/run.go b/cmd/spire-agent/cli/run/run.go
@@ -107,7 +107,6 @@ type sdsConfig struct {
 	DefaultBundleName           string `hcl:"default_bundle_name"`
 	DefaultAllBundlesName       string `hcl:"default_all_bundles_name"`
 	DisableSPIFFECertValidation bool   `hcl:"disable_spiffe_cert_validation"`
-	EnableDeprecatedv2API       bool   `hcl:"enable_deprecated_v2_api"`
 }
 
 type experimentalConfig struct {
@@ -504,10 +503,6 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
 	ac.JoinToken = c.Agent.JoinToken
 	ac.DataDir = c.Agent.DataDir
 	ac.DefaultSVIDName = c.Agent.SDS.DefaultSVIDName
-	ac.EnableDeprecatedSDSv2API = c.Agent.SDS.EnableDeprecatedv2API
-	if ac.EnableDeprecatedSDSv2API {
-		logger.Warn("The Envoy SDS v2 API is now deprecated in SPIRE and is no longer supported by Envoy. It is recommended that users of the SDS v2 API migrate to the SDS v3 API. The SDS v2 API and this config setting will be removed in a future version.")
-	}
 	ac.DefaultBundleName = c.Agent.SDS.DefaultBundleName
 	ac.DefaultAllBundlesName = c.Agent.SDS.DefaultAllBundlesName
 	if ac.DefaultAllBundlesName == ac.DefaultBundleName {

diff --git a/cmd/spire-agent/cli/run/run_test.go b/cmd/spire-agent/cli/run/run_test.go
@@ -244,18 +244,6 @@ func TestMergeInput(t *testing.T) {
 				require.Equal(t, "foo", c.Agent.SDS.DefaultAllBundlesName)
 			},
 		},
-		{
-			msg: "enable_deprecated_v2_api should be configurable by file",
-			fileInput: func(c *Config) {
-				c.Agent.SDS = sdsConfig{
-					EnableDeprecatedv2API: true,
-				}
-			},
-			cliInput: func(ac *agentConfig) {},
-			test: func(t *testing.T, c *Config) {
-				require.True(t, c.Agent.SDS.EnableDeprecatedv2API)
-			},
-		},
 		{
 			msg:       "disable_spiffe_cert_validation should default value of false",
 			fileInput: func(c *Config) {},
@@ -881,14 +869,12 @@ func TestNewAgentConfig(t *testing.T) {
 				c.Agent.SDS.DefaultBundleName = "DefaultBundleName"
 				c.Agent.SDS.DefaultAllBundlesName = "DefaultAllBundlesName"
 				c.Agent.SDS.DisableSPIFFECertValidation = true
-				c.Agent.SDS.EnableDeprecatedv2API = true
 			},
 			test: func(t *testing.T, c *agent.Config) {
 				assert.Equal(t, c.DefaultSVIDName, "DefaultSVIDName")
 				assert.Equal(t, c.DefaultBundleName, "DefaultBundleName")
 				assert.Equal(t, c.DefaultAllBundlesName, "DefaultAllBundlesName")
 				assert.True(t, c.DisableSPIFFECertValidation)
-				assert.True(t, c.EnableDeprecatedSDSv2API)
 			},
 		},
 		{

diff --git a/conf/agent/agent_full.conf b/conf/agent/agent_full.conf
@@ -76,12 +76,6 @@ agent {
 
     # sds: Optional SDS configuration section.
     # sds = {
-    #     # enable_deprecated_v2_api: Enable deprecated SDS v2 API. It is recommended that
-    #     # users of the SDS v2 API migrate to the SDS v3 API.
-    #     # The SDS v2 API and this config setting will be removed in a future version.
-    #     # Default: false
-    #     # enable_deprecated_v2_api = false
-    #
     #     # default_svid_name: The TLS Certificate resource name to use for the default
     #     # X509-SVID with Envoy SDS. Default: default.
     #     # default_svid_name = "default"

diff --git a/doc/spire_agent.md b/doc/spire_agent.md
@@ -88,7 +88,6 @@ Only one of these three options may be set at a time.
 
 | Configuration                    | Description                                                                                                                                                                         | Default |
 |----------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
-| `enable_deprecated_v2_api`       | Enable deprecated SDS v2 API. It is recommended that users of the SDS v2 API migrate to the SDS v3 API. The SDS v2 API and this config setting will be removed in a future version. | false   |
 | `default_svid_name`              | The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS                                                                                                   | default |
 | `default_bundle_name`            | The Validation Context resource name to use for the default X.509 bundle with Envoy SDS                                                                                             | ROOTCA  |
 | `default_all_bundles_name`       | The Validation Context resource name to use for all bundles (including federated) with Envoy SDS                                                                                    | ALL     |

diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go
@@ -269,7 +269,6 @@ func (a *Agent) newEndpoints(metrics telemetry.Metrics, mgr manager.Manager, att
 		DefaultBundleName:             a.c.DefaultBundleName,
 		DefaultAllBundlesName:         a.c.DefaultAllBundlesName,
 		DisableSPIFFECertValidation:   a.c.DisableSPIFFECertValidation,
-		EnableDeprecatedSDSv2API:      a.c.EnableDeprecatedSDSv2API,
 		AllowUnauthenticatedVerifiers: a.c.AllowUnauthenticatedVerifiers,
 		AllowedForeignJWTClaims:       a.c.AllowedForeignJWTClaims,
 		TrustDomain:                   a.c.TrustDomain,

diff --git a/pkg/agent/config.go b/pkg/agent/config.go
@@ -36,9 +36,6 @@ type Config struct {
 	// The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS
 	DefaultSVIDName string
 
-	// Enable deprecated Envoy SDS v2 API
-	EnableDeprecatedSDSv2API bool
-
 	// If true, the agent will bootstrap insecurely with the server
 	InsecureBootstrap bool
 

diff --git a/pkg/agent/endpoints/config.go b/pkg/agent/endpoints/config.go
@@ -3,14 +3,12 @@ package endpoints
 import (
 	"net"
 
-	discovery_v2 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2"
 	secret_v3 "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3"
 	"github.com/sirupsen/logrus"
 	workload_pb "github.com/spiffe/go-spiffe/v2/proto/spiffe/workload"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	healthv1 "github.com/spiffe/spire/pkg/agent/api/health/v1"
 	attestor "github.com/spiffe/spire/pkg/agent/attestor/workload"
-	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv2"
 	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv3"
 	"github.com/spiffe/spire/pkg/agent/endpoints/workload"
 	"github.com/spiffe/spire/pkg/agent/manager"
@@ -41,9 +39,6 @@ type Config struct {
 	// Disable custom Envoy SDS validator
 	DisableSPIFFECertValidation bool
 
-	// Enable deprecated envoy SDS v2 API
-	EnableDeprecatedSDSv2API bool
-
 	AllowUnauthenticatedVerifiers bool
 
 	AllowedForeignJWTClaims []string
@@ -53,7 +48,6 @@ type Config struct {
 	// Hooks used by the unit tests to assert that the configuration provided
 	// to each handler is correct and return fake handlers.
 	newWorkloadAPIServer func(workload.Config) workload_pb.SpiffeWorkloadAPIServer
-	newSDSv2Server       func(sdsv2.Config) discovery_v2.SecretDiscoveryServiceServer
 	newSDSv3Server       func(sdsv3.Config) secret_v3.SecretDiscoveryServiceServer
 	newHealthServer      func(healthv1.Config) grpc_health_v1.HealthServer
 }
diff --git a/pkg/agent/endpoints/endpoints.go b/pkg/agent/endpoints/endpoints.go
@@ -5,12 +5,10 @@ import (
 	"errors"
 	"net"
 
-	discovery_v2 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2"
 	secret_v3 "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3"
 	"github.com/sirupsen/logrus"
 	workload_pb "github.com/spiffe/go-spiffe/v2/proto/spiffe/workload"
 	healthv1 "github.com/spiffe/spire/pkg/agent/api/health/v1"
-	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv2"
 	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv3"
 	"github.com/spiffe/spire/pkg/agent/endpoints/workload"
 	"github.com/spiffe/spire/pkg/common/api/middleware"
@@ -29,7 +27,6 @@ type Endpoints struct {
 	log               logrus.FieldLogger
 	metrics           telemetry.Metrics
 	workloadAPIServer workload_pb.SpiffeWorkloadAPIServer
-	sdsv2Server       discovery_v2.SecretDiscoveryServiceServer
 	sdsv3Server       secret_v3.SecretDiscoveryServiceServer
 	healthServer      grpc_health_v1.HealthServer
 
@@ -47,11 +44,6 @@ func New(c Config) *Endpoints {
 			return workload.New(c)
 		}
 	}
-	if c.newSDSv2Server == nil {
-		c.newSDSv2Server = func(c sdsv2.Config) discovery_v2.SecretDiscoveryServiceServer {
-			return sdsv2.New(c)
-		}
-	}
 	if c.newSDSv3Server == nil {
 		c.newSDSv3Server = func(c sdsv3.Config) secret_v3.SecretDiscoveryServiceServer {
 			return sdsv3.New(c)
@@ -76,14 +68,6 @@ func New(c Config) *Endpoints {
 		TrustDomain:                   c.TrustDomain,
 	})
 
-	sdsv2Server := c.newSDSv2Server(sdsv2.Config{
-		Attestor:          attestor,
-		Manager:           c.Manager,
-		DefaultSVIDName:   c.DefaultSVIDName,
-		DefaultBundleName: c.DefaultBundleName,
-		Enabled:           c.EnableDeprecatedSDSv2API,
-	})
-
 	sdsv3Server := c.newSDSv3Server(sdsv3.Config{
 		Attestor:                    attestor,
 		Manager:                     c.Manager,
@@ -102,7 +86,6 @@ func New(c Config) *Endpoints {
 		log:               c.Log,
 		metrics:           c.Metrics,
 		workloadAPIServer: workloadAPIServer,
-		sdsv2Server:       sdsv2Server,
 		sdsv3Server:       sdsv3Server,
 		healthServer:      healthServer,
 	}
@@ -120,7 +103,6 @@ func (e *Endpoints) ListenAndServe(ctx context.Context) error {
 	)
 
 	workload_pb.RegisterSpiffeWorkloadAPIServer(server, e.workloadAPIServer)
-	discovery_v2.RegisterSecretDiscoveryServiceServer(server, e.sdsv2Server)
 	secret_v3.RegisterSecretDiscoveryServiceServer(server, e.sdsv3Server)
 	grpc_health_v1.RegisterHealthServer(server, e.healthServer)
 

diff --git a/pkg/agent/endpoints/endpoints_test.go b/pkg/agent/endpoints/endpoints_test.go
@@ -8,16 +8,13 @@ import (
 	"time"
 
 	"github.com/armon/go-metrics"
-	api_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
-	discovery_v2 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2"
 	discovery_v3 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
 	secret_v3 "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3"
 	"github.com/sirupsen/logrus"
 	"github.com/sirupsen/logrus/hooks/test"
 	workload_pb "github.com/spiffe/go-spiffe/v2/proto/spiffe/workload"
 	healthv1 "github.com/spiffe/spire/pkg/agent/api/health/v1"
 	"github.com/spiffe/spire/pkg/agent/api/rpccontext"
-	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv2"
 	"github.com/spiffe/spire/pkg/agent/endpoints/sdsv3"
 	"github.com/spiffe/spire/pkg/agent/endpoints/workload"
 	"github.com/spiffe/spire/pkg/agent/manager"
@@ -98,33 +95,6 @@ func TestEndpoints(t *testing.T) {
 				}},
 			},
 		},
-		{
-			name: "sds v2 api has peertracker attestor plumbed",
-			do: func(t *testing.T, conn *grpc.ClientConn) {
-				sdsClient := discovery_v2.NewSecretDiscoveryServiceClient(conn)
-				_, err := sdsClient.FetchSecrets(ctx, &api_v2.DiscoveryRequest{})
-				require.NoError(t, err)
-			},
-			expectedLogs: []spiretest.LogEntry{
-				logEntryWithPID(logrus.InfoLevel, "Success",
-					"method", "FetchSecrets",
-					"service", "SDS.v2",
-				),
-			},
-			expectedMetrics: []fakemetrics.MetricItem{
-				// Global connection counter and then the increment/decrement of the connection gauge
-				{Type: fakemetrics.IncrCounterType, Key: []string{"sds_api", "connection"}, Val: 1},
-				{Type: fakemetrics.SetGaugeType, Key: []string{"sds_api", "connections"}, Val: 1},
-				{Type: fakemetrics.SetGaugeType, Key: []string{"sds_api", "connections"}, Val: 0},
-				// Call counter
-				{Type: fakemetrics.IncrCounterWithLabelsType, Key: []string{"rpc", "sds", "v2", "fetch_secrets"}, Val: 1, Labels: []metrics.Label{
-					{Name: "status", Value: "OK"},
-				}},
-				{Type: fakemetrics.MeasureSinceWithLabelsType, Key: []string{"rpc", "sds", "v2", "fetch_secrets", "elapsed_time"}, Val: 0, Labels: []metrics.Label{
-					{Name: "status", Value: "OK"},
-				}},
-			},
-		},
 		{
 			name: "sds v3 api has peertracker attestor plumbed",
 			do: func(t *testing.T, conn *grpc.ClientConn) {
@@ -173,7 +143,6 @@ func TestEndpoints(t *testing.T) {
 				DefaultAllBundlesName:       "DefaultAllBundlesName",
 				DisableSPIFFECertValidation: true,
 				AllowedForeignJWTClaims:     tt.allowedClaims,
-				EnableDeprecatedSDSv2API:    true,
 
 				// Assert the provided config and return a fake Workload API server
 				newWorkloadAPIServer: func(c workload.Config) workload_pb.SpiffeWorkloadAPIServer {
@@ -188,17 +157,6 @@ func TestEndpoints(t *testing.T) {
 					return FakeWorkloadAPIServer{Attestor: attestor}
 				},
 
-				// Assert the provided config and return a fake SDS server
-				newSDSv2Server: func(c sdsv2.Config) discovery_v2.SecretDiscoveryServiceServer {
-					attestor, ok := c.Attestor.(PeerTrackerAttestor)
-					require.True(t, ok, "attestor was not a PeerTrackerAttestor wrapper")
-					assert.Equal(t, FakeManager{}, c.Manager)
-					assert.Equal(t, "DefaultSVIDName", c.DefaultSVIDName)
-					assert.Equal(t, "DefaultBundleName", c.DefaultBundleName)
-					assert.True(t, c.Enabled)
-					return FakeSDSv2Server{Attestor: attestor}
-				},
-
 				// Assert the provided config and return a fake SDS server
 				newSDSv3Server: func(c sdsv3.Config) secret_v3.SecretDiscoveryServiceServer {
 					attestor, ok := c.Attestor.(PeerTrackerAttestor)
@@ -276,18 +234,6 @@ func (s FakeWorkloadAPIServer) FetchJWTSVID(ctx context.Context, _ *workload_pb.
 	return &workload_pb.JWTSVIDResponse{}, nil
 }
 
-type FakeSDSv2Server struct {
-	Attestor PeerTrackerAttestor
-	*discovery_v2.UnimplementedSecretDiscoveryServiceServer
-}
-
-func (s FakeSDSv2Server) FetchSecrets(ctx context.Context, _ *api_v2.DiscoveryRequest) (*api_v2.DiscoveryResponse, error) {
-	if err := attest(ctx, s.Attestor); err != nil {
-		return nil, err
-	}
-	return &api_v2.DiscoveryResponse{}, nil
-}
-
 type FakeSDSv3Server struct {
 	Attestor PeerTrackerAttestor
 	*secret_v3.UnimplementedSecretDiscoveryServiceServer

diff --git a/pkg/agent/endpoints/metrics.go b/pkg/agent/endpoints/metrics.go
@@ -31,7 +31,7 @@ func (m *connectionMetrics) Preprocess(ctx context.Context, _ string, _ interfac
 		case middleware.WorkloadAPIServiceName:
 			workloadAPITelemetry.IncrConnectionCounter(m.metrics)
 			workloadAPITelemetry.SetConnectionTotalGauge(m.metrics, atomic.AddInt32(&m.workloadAPIConns, 1))
-		case middleware.EnvoySDSv2ServiceName, middleware.EnvoySDSv3ServiceName:
+		case middleware.EnvoySDSv3ServiceName:
 			sdsAPITelemetry.IncrSDSAPIConnectionCounter(m.metrics)
 			sdsAPITelemetry.SetSDSAPIConnectionTotalGauge(m.metrics, atomic.AddInt32(&m.sdsAPIConns, 1))
 		case middleware.DelegatedIdentityServiceName:
@@ -51,7 +51,7 @@ func (m *connectionMetrics) Postprocess(ctx context.Context, _ string, _ bool, _
 		switch names.RawService {
 		case middleware.WorkloadAPIServiceName:
 			workloadAPITelemetry.SetConnectionTotalGauge(m.metrics, atomic.AddInt32(&m.workloadAPIConns, -1))
-		case middleware.EnvoySDSv2ServiceName, middleware.EnvoySDSv3ServiceName:
+		case middleware.EnvoySDSv3ServiceName:
 			sdsAPITelemetry.SetSDSAPIConnectionTotalGauge(m.metrics, atomic.AddInt32(&m.sdsAPIConns, -1))
 		case middleware.DelegatedIdentityServiceName:
 			adminapi.SetDelegatedIdentityAPIConnectionGauge(m.metrics, atomic.AddInt32(&m.delegatedIdentityAPIConns, -1))