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

Commit

Permalink
sds: Adding more unit tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Delyan Raychev <delyan.raychev@microsoft.com>
  • Loading branch information
@draychev
draychev committed Feb 26, 2021
1 parent 3f6428e commit 4a584c6
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 44 deletions.
57 changes: 20 additions & 37 deletions pkg/envoy/sds/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import (
xds_auth "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
xds_discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"

"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/any"

"github.com/openservicemesh/osm/pkg/catalog"
"github.com/openservicemesh/osm/pkg/certificate"
Expand All @@ -24,71 +22,56 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, request
// OSM currently relies on kubernetes ServiceAccount for service identity
svcAccount, err := catalog.GetServiceAccountFromProxyCertificate(proxy.GetCertificateCommonName())
if err != nil {
log.Error().Err(err).Msgf("Error retrieving ServiceAccount for Envoy with certificate SerialNumber=%s on Pod with UID=%s",
proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
log.Error().Err(err).Msgf("Error retrieving ServiceAccount for Envoy with certificate SerialNumber=%s on Pod with UID=%s", proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
return nil, err
}

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, svcAccount service.K8sServiceAccount) *sdsImpl {
impl := &sdsImpl{
proxy: proxy,
s := &sdsImpl{
meshCatalog: meshCatalog,
certManager: certManager,
cfg: cfg,
svcAccount: svcAccount,
}

return impl
}

func (s *sdsImpl) createDiscoveryResponse(request *xds_discovery.DiscoveryRequest) (*xds_discovery.DiscoveryResponse, error) {
// Resources corresponding to SDS secrets returned as a part of the DiscoveryResponse
var resources []*any.Any
discoveryResponse := &xds_discovery.DiscoveryResponse{
TypeUrl: string(envoy.TypeSDS),
}

// The DiscoveryRequest contains the requested certs
requestedCerts := request.ResourceNames

log.Trace().Msgf("Creating SDS response for request for ResourceNames (certificates) %+v from Envoy with certificate SerialNumber=%s on Pod with UID=%s", requestedCerts, s.proxy.GetCertificateSerialNumber(), s.proxy.GetPodUID())
log.Trace().Msgf("Creating SDS response for request for ResourceNames (certificates) %+v from Envoy with certificate SerialNumber=%s on Pod with UID=%s", requestedCerts, proxy.GetCertificateSerialNumber(), proxy.GetPodUID())

// 1. Issue a service certificate for this proxy
// OSM currently relies on kubernetes ServiceAccount for service identity
si := identity.GetKubernetesServiceIdentity(s.svcAccount, identity.ClusterLocalTrustDomain)
cert, err := s.certManager.IssueCertificate(certificate.CommonName(si), s.cfg.GetServiceCertValidityPeriod())
si := identity.GetKubernetesServiceIdentity(svcAccount, identity.ClusterLocalTrustDomain)
cert, err := certManager.IssueCertificate(certificate.CommonName(si), cfg.GetServiceCertValidityPeriod())
if err != nil {
log.Error().Err(err).Msgf("Error issuing a certificate for proxy with certificate SerialNumber=%s", s.proxy.GetCertificateSerialNumber())
log.Error().Err(err).Msgf("Error issuing a certificate for proxy with certificate SerialNumber=%s", proxy.GetCertificateSerialNumber())
return nil, err
}

// 2. Create SDS secret resources based on the requested certs in the DiscoveryRequest
// request.ResourceNames is expected to be a list of either "service-cert:namespace/service" or "root-cert:namespace/service"
for _, envoyProto := range s.getSDSSecrets(cert, requestedCerts) {
for _, envoyProto := range s.getSDSSecrets(cert, requestedCerts, proxy) {
marshalledSecret, err := ptypes.MarshalAny(envoyProto)
if err != nil {
log.Error().Err(err).Msgf("Error marshaling Envoy secret %s for proxy with certificate SerialNumber=%s on Pod with UID=%s", envoyProto.Name, s.proxy.GetCertificateSerialNumber(), s.proxy.GetPodUID())
log.Error().Err(err).Msgf("Error marshaling Envoy secret %s for proxy with certificate SerialNumber=%s on Pod with UID=%s", envoyProto.Name, proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
}

resources = append(resources, marshalledSecret)
discoveryResponse.Resources = append(discoveryResponse.Resources, marshalledSecret)
}

return &xds_discovery.DiscoveryResponse{
TypeUrl: string(envoy.TypeSDS),
Resources: resources,
}, nil
return discoveryResponse, nil
}

func (s *sdsImpl) getSDSSecrets(cert certificate.Certificater, requestedCerts []string) []*xds_auth.Secret {
func (s *sdsImpl) getSDSSecrets(cert certificate.Certificater, requestedCerts []string, proxy *envoy.Proxy) (certs []*xds_auth.Secret) {
// requestedCerts is expected to be a list of either of the following:
// - "service-cert:namespace/service"
// - "root-cert-for-mtls-outbound:namespace/service"
// - "root-cert-for-mtls-inbound:namespace/service"
// - "root-cert-for-https:namespace/service"

var envoySecrets []*xds_auth.Secret

// The Envoy makes a request for a list of resources (aka certificates), which we will send as a response to the SDS request.
for _, requestedCertificate := range requestedCerts {
sdsCert, err := envoy.UnmarshalSDSCert(requestedCertificate)
Expand All @@ -97,32 +80,32 @@ func (s *sdsImpl) getSDSSecrets(cert certificate.Certificater, requestedCerts []
continue
}

log.Debug().Msgf("Envoy with certificate SerialNumber=%s on Pod with UID=%s requested %s", s.proxy.GetCertificateSerialNumber(), s.proxy.GetPodUID(), requestedCertificate)
log.Debug().Msgf("Envoy with certificate SerialNumber=%s on Pod with UID=%s requested %s", proxy.GetCertificateSerialNumber(), proxy.GetPodUID(), requestedCertificate)

switch sdsCert.CertType {
// A service certificate is requested
case envoy.ServiceCertType:
envoySecret, err := getServiceCertSecret(cert, requestedCertificate)
if err != nil {
log.Error().Err(err).Msgf("Error creating cert %s for Envoy with xDS Certificate SerialNumber=%s on Pod with UID=%s",
requestedCertificate, s.proxy.GetCertificateSerialNumber(), s.proxy.GetPodUID())
requestedCertificate, proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
continue
}
envoySecrets = append(envoySecrets, envoySecret)
certs = append(certs, envoySecret)

// A root certificate used to validate a service certificate is requested
case envoy.RootCertTypeForMTLSInbound, envoy.RootCertTypeForMTLSOutbound, envoy.RootCertTypeForHTTPS:
envoySecret, err := s.getRootCert(cert, *sdsCert)
if err != nil {
log.Error().Err(err).Msgf("Error creating cert %s for Envoy with xDS Certificate SerialNumber=%s on Pod with UID=%s",
requestedCertificate, s.proxy.GetCertificateSerialNumber(), s.proxy.GetPodUID())
requestedCertificate, proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
continue
}
envoySecrets = append(envoySecrets, envoySecret)
certs = append(certs, envoySecret)
}
}

return envoySecrets
return certs
}

// getServiceCertSecret creates the struct with certificates for the service, which the
Expand Down
66 changes: 61 additions & 5 deletions pkg/envoy/sds/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,75 @@ import (
"fmt"
"testing"

xds_discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
"github.com/golang/mock/gomock"
"github.com/google/uuid"
tassert "github.com/stretchr/testify/assert"
testclient "k8s.io/client-go/kubernetes/fake"

"github.com/openservicemesh/osm/pkg/catalog"
"github.com/openservicemesh/osm/pkg/certificate"
"github.com/openservicemesh/osm/pkg/certificate/providers/tresor"
"github.com/openservicemesh/osm/pkg/configurator"
"github.com/openservicemesh/osm/pkg/envoy"
"github.com/openservicemesh/osm/pkg/service"
)

// TestNewResponse sets up a fake kube client, then a pod and makes an SDS request,
// and finally verifies the response from sds.NewResponse().
func TestNewResponse(t *testing.T) {
assert := tassert.New(t)

// Setup a fake Kube client. We use this to create a full simulation of creating a pod with
// the required xDS Certificate, properly formatted CommonName etc.
fakeKubeClient := testclient.NewSimpleClientset()

// We deliberately set the namespace and service accounts to random values
// to ensure no hard-coded values sneak in.
namespace := uuid.New().String()
serviceAccount := uuid.New().String()

// This is the thing we are going to be requesting (pretending that the Envoy is requesting it)
request := &xds_discovery.DiscoveryRequest{
TypeUrl: string(envoy.TypeSDS),
ResourceNames: []string{
envoy.SDSCert{Name: serviceAccount, CertType: envoy.ServiceCertType}.String(),
envoy.SDSCert{Name: serviceAccount, CertType: envoy.RootCertTypeForMTLSInbound}.String(),
envoy.SDSCert{Name: serviceAccount, CertType: envoy.RootCertTypeForHTTPS}.String(),
},
}

stop := make(chan struct{})
defer close(stop)

// The Common Name of the xDS Certificate (issued to the Envoy on the Pod by the Injector) will
// have be prefixed with the ID of the pod. It is the first chunk of a dot-separated string.
podID := uuid.New().String()

certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", podID, serviceAccount, namespace))
certSerialNumber := certificate.SerialNumber("123456")
goodProxy := envoy.NewProxy(certCommonName, certSerialNumber, nil)

badProxy := envoy.NewProxy("-certificate-common-name-is-invalid-", "-cert-serial-number-is-invalid-", nil)

cfg := configurator.NewConfigurator(fakeKubeClient, stop, namespace, "-the-config-map-name-")
certManager := tresor.NewFakeCertManager(cfg)
meshCatalog := catalog.NewFakeMeshCatalog(fakeKubeClient)

// ----- Test with a rogue proxy (does not belong to the mesh)
actualSDSResponse, err := NewResponse(meshCatalog, badProxy, request, cfg, certManager)
assert.Equal(err, catalog.ErrInvalidCertificateCN, "Expected a different error!")
assert.Nil(actualSDSResponse)

// ----- Test with an properly configured proxy
actualSDSResponse, err = NewResponse(meshCatalog, goodProxy, request, cfg, certManager)
assert.Equal(err, nil, fmt.Sprintf("Error evaluating sds.NewResponse(): %s", err))
assert.NotNil(actualSDSResponse)
assert.Equal(len(actualSDSResponse.Resources), 3)
assert.Equal(actualSDSResponse.TypeUrl, "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret")
}

func TestGetRootCert(t *testing.T) {
assert := tassert.New(t)
mockCtrl := gomock.NewController(t)
Expand Down Expand Up @@ -130,11 +187,8 @@ func TestGetRootCert(t *testing.T) {
tc.prepare(&d)
}

certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", uuid.New().String(), "sa-1", "ns-1"))
certSerialNumber := certificate.SerialNumber("123456")
s := &sdsImpl{
svcAccount: tc.proxySvcAccount,
proxy: envoy.NewProxy(certCommonName, certSerialNumber, nil),
certManager: mockCertManager,

// these points to the dynamic mocks which gets updated for each test
Expand Down Expand Up @@ -181,6 +235,7 @@ func TestGetServiceCert(t *testing.T) {
sdsSecret, err := getServiceCertSecret(mockCertificater, tc.certName)

assert.Equal(err != nil, tc.expectError)
assert.NotNil(sdsSecret)
assert.Equal(sdsSecret.GetTlsCertificate().GetCertificateChain().GetInlineBytes(), tc.certChain)
assert.Equal(sdsSecret.GetTlsCertificate().GetPrivateKey().GetInlineBytes(), tc.privKey)
})
Expand Down Expand Up @@ -346,16 +401,17 @@ func TestGetSDSSecrets(t *testing.T) {
certSerialNumber := certificate.SerialNumber("123456")
s := &sdsImpl{
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,
cfg: d.mockConfigurator,
}

proxy := envoy.NewProxy(certCommonName, certSerialNumber, nil)

// test the function
sdsSecrets := s.getSDSSecrets(d.mockCertificater, tc.requestedCerts)
sdsSecrets := s.getSDSSecrets(d.mockCertificater, tc.requestedCerts, proxy)
assert.Len(sdsSecrets, tc.expectedSecretCount)

if tc.expectedSecretCount <= 0 {
Expand Down
2 changes: 0 additions & 2 deletions pkg/envoy/sds/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"github.com/openservicemesh/osm/pkg/catalog"
"github.com/openservicemesh/osm/pkg/certificate"
"github.com/openservicemesh/osm/pkg/configurator"
"github.com/openservicemesh/osm/pkg/envoy"
"github.com/openservicemesh/osm/pkg/logger"
"github.com/openservicemesh/osm/pkg/service"
)
Expand All @@ -16,7 +15,6 @@ var (

// sdsImpl is the type that implements the internal functionality of SDS
type sdsImpl struct {
proxy *envoy.Proxy
svcAccount service.K8sServiceAccount
meshCatalog catalog.MeshCataloger
cfg configurator.Configurator
Expand Down

0 comments on commit 4a584c6

Please sign in to comment.