Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SDS v2 API #4444

Merged
merged 2 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions cmd/spire-agent/cli/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 0 additions & 14 deletions cmd/spire-agent/cli/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {},
Expand Down Expand Up @@ -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)
},
},
{
Expand Down
6 changes: 0 additions & 6 deletions conf/agent/agent_full.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion doc/spire_agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
1 change: 0 additions & 1 deletion pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions pkg/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 0 additions & 6 deletions pkg/agent/endpoints/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
18 changes: 0 additions & 18 deletions pkg/agent/endpoints/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -102,7 +86,6 @@ func New(c Config) *Endpoints {
log: c.Log,
metrics: c.Metrics,
workloadAPIServer: workloadAPIServer,
sdsv2Server: sdsv2Server,
sdsv3Server: sdsv3Server,
healthServer: healthServer,
}
Expand All @@ -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)

Expand Down
54 changes: 0 additions & 54 deletions pkg/agent/endpoints/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/endpoints/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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))
Expand Down
Loading