Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
Merge pull request #2633 from draychev/small-refactor-to-sds-tests-re…
Browse files Browse the repository at this point in the history 
…move-services

sds: Remove unused services list from SDS struct
  • Loading branch information
@draychev
draychev authored Feb 26, 2021
2 parents 3ea56ba + 1d3c76a commit 3f6428e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 39 deletions.
22 changes: 7 additions & 15 deletions pkg/envoy/sds/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ import (
func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, request *xds_discovery.DiscoveryRequest, cfg configurator.Configurator, certManager certificate.Manager) (*xds_discovery.DiscoveryResponse, error) {
log.Debug().Msgf("Composing SDS Discovery Response for Envoy with certificate SerialNumber=%s on Pod with UID=%s", proxy.GetCertificateSerialNumber(), proxy.GetPodUID())

svcList, err := meshCatalog.GetServicesFromEnvoyCertificate(proxy.GetCertificateCommonName())
if err != nil {
log.Error().Err(err).Msgf("Error getting services associated with Envoy with certificate SerialNumber=%s on Pod with UID=%s",
proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
return nil, err
}

// OSM currently relies on kubernetes ServiceAccount for service identity
svcAccount, err := catalog.GetServiceAccountFromProxyCertificate(proxy.GetCertificateCommonName())
if err != nil {
Expand All @@ -36,18 +29,17 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, request
return nil, err
}

sdsImpl := newSDSImpl(proxy, meshCatalog, certManager, cfg, svcList, svcAccount)
sdsImpl := newSDSImpl(proxy, meshCatalog, certManager, cfg, svcAccount)
return sdsImpl.createDiscoveryResponse(request)
}

func newSDSImpl(proxy *envoy.Proxy, meshCatalog catalog.MeshCataloger, certManager certificate.Manager, cfg configurator.Configurator, proxyServices []service.MeshService, svcAccount service.K8sServiceAccount) *sdsImpl {
func newSDSImpl(proxy *envoy.Proxy, meshCatalog catalog.MeshCataloger, certManager certificate.Manager, cfg configurator.Configurator, svcAccount service.K8sServiceAccount) *sdsImpl {
impl := &sdsImpl{
proxy: proxy,
meshCatalog: meshCatalog,
certManager: certManager,
cfg: cfg,
svcAccount: svcAccount,
proxyServices: proxyServices,
proxy: proxy,
meshCatalog: meshCatalog,
certManager: certManager,
cfg: cfg,
svcAccount: svcAccount,
}

return impl
Expand Down
24 changes: 6 additions & 18 deletions pkg/envoy/sds/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestGetRootCert(t *testing.T) {
type testCase struct {
name string
sdsCert envoy.SDSCert
proxyService service.MeshService
proxySvcAccount service.K8sServiceAccount
prepare func(d *dynamicMock)

Expand All @@ -50,7 +49,6 @@ func TestGetRootCert(t *testing.T) {
Name: "ns-1/service-1",
CertType: envoy.RootCertTypeForMTLSInbound,
},
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -76,7 +74,6 @@ func TestGetRootCert(t *testing.T) {
Name: "ns-2/service-2",
CertType: envoy.RootCertTypeForMTLSOutbound,
},
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -102,7 +99,6 @@ func TestGetRootCert(t *testing.T) {
Name: "ns-2/service-2",
CertType: envoy.RootCertTypeForMTLSOutbound,
},
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand Down Expand Up @@ -137,10 +133,9 @@ func TestGetRootCert(t *testing.T) {
certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", uuid.New().String(), "sa-1", "ns-1"))
certSerialNumber := certificate.SerialNumber("123456")
s := &sdsImpl{
proxyServices: []service.MeshService{tc.proxyService},
svcAccount: tc.proxySvcAccount,
proxy: envoy.NewProxy(certCommonName, certSerialNumber, nil),
certManager: mockCertManager,
svcAccount: tc.proxySvcAccount,
proxy: envoy.NewProxy(certCommonName, certSerialNumber, nil),
certManager: mockCertManager,

// these points to the dynamic mocks which gets updated for each test
meshCatalog: d.mockCatalog,
Expand Down Expand Up @@ -208,7 +203,6 @@ func TestGetSDSSecrets(t *testing.T) {

type testCase struct {
name string
proxyService service.MeshService
proxySvcAccount service.K8sServiceAccount
prepare func(d *dynamicMock)

Expand All @@ -230,7 +224,6 @@ func TestGetSDSSecrets(t *testing.T) {
// Test case 1: root-cert-for-mtls-inbound requested -------------------------------
{
name: "test root-cert-for-mtls-inbound cert type request",
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -255,7 +248,6 @@ func TestGetSDSSecrets(t *testing.T) {
// Test case 2: root-cert-for-mtls-outbound requested -------------------------------
{
name: "test root-cert-for-mtls-outbound cert type request",
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -281,7 +273,6 @@ func TestGetSDSSecrets(t *testing.T) {
// Test case 3: root-cert-for-https requested -------------------------------
{
name: "test root-cert-https cert type request",
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -301,7 +292,6 @@ func TestGetSDSSecrets(t *testing.T) {
// Test case 4: service-cert requested -------------------------------
{
name: "test root-cert-https cert type request",
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: func(d *dynamicMock) {
Expand All @@ -321,7 +311,6 @@ func TestGetSDSSecrets(t *testing.T) {
// Test case 5: invalid cert type requested -------------------------------
{
name: "test root-cert-https cert type request",
proxyService: service.MeshService{Name: "service-1", Namespace: "ns-1"},
proxySvcAccount: service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"},

prepare: nil,
Expand Down Expand Up @@ -356,10 +345,9 @@ func TestGetSDSSecrets(t *testing.T) {
certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", uuid.New().String(), "sa-1", "ns-1"))
certSerialNumber := certificate.SerialNumber("123456")
s := &sdsImpl{
proxyServices: []service.MeshService{tc.proxyService},
svcAccount: tc.proxySvcAccount,
proxy: envoy.NewProxy(certCommonName, certSerialNumber, nil),
certManager: mockCertManager,
svcAccount: tc.proxySvcAccount,
proxy: envoy.NewProxy(certCommonName, certSerialNumber, nil),
certManager: mockCertManager,

// these points to the dynamic mocks which gets updated for each test
meshCatalog: d.mockCatalog,
Expand Down
11 changes: 5 additions & 6 deletions pkg/envoy/sds/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ var (

// sdsImpl is the type that implements the internal functionality of SDS
type sdsImpl struct {
proxy *envoy.Proxy
proxyServices []service.MeshService
svcAccount service.K8sServiceAccount
meshCatalog catalog.MeshCataloger
cfg configurator.Configurator
certManager certificate.Manager
proxy *envoy.Proxy
svcAccount service.K8sServiceAccount
meshCatalog catalog.MeshCataloger
cfg configurator.Configurator
certManager certificate.Manager
}

0 comments on commit 3f6428e

Please sign in to comment.