address comments

Signed-off-by: Sean Teeling <[email protected]>

Author: steeling

pkg/debugger/proxy.go

@@ -58,8 +58,8 @@ func (ds DebugConfig) printProxies(w http.ResponseWriter) {
 }
 
 func (ds DebugConfig) getConfigDump(uuid string, w http.ResponseWriter) {
-	proxy, ok := ds.proxyRegistry.GetConnectedProxy(uuid)
-	if !ok {
+	proxy := ds.proxyRegistry.GetConnectedProxy(uuid)
+	if proxy != nil {
 		msg := fmt.Sprintf("Proxy for UUID %s not found, may have been disconnected", uuid)
 		log.Error().Msg(msg)
 		if _, err := w.Write([]byte(msg)); err != nil {

pkg/envoy/ads/stream.go

@@ -335,7 +335,7 @@ func isCNforProxy(proxy *envoy.Proxy, cn certificate.CommonName) bool {
 func getCertificateCommonNameMeta(cn certificate.CommonName) (envoy.ProxyKind, uuid.UUID, identity.ServiceIdentity, error) {
 	// XDS cert CN is of the form <proxy-UUID>.<kind>.<proxy-identity>.<namespace>.<trust-domain>
 	chunks := strings.SplitN(cn.String(), constants.DomainDelimiter, 5)
-	if len(chunks) < 3 {
+	if len(chunks) < 4 {
 		return "", uuid.UUID{}, "", errInvalidCertificateCN
 	}
 	proxyUUID, err := uuid.Parse(chunks[0])
@@ -345,6 +345,15 @@ func getCertificateCommonNameMeta(cn certificate.CommonName) (envoy.ProxyKind, u
 		return "", uuid.UUID{}, "", err
 	}
 
+	switch {
+	case chunks[1] == "":
+		return "", uuid.UUID{}, "", errInvalidCertificateCN
+	case chunks[2] == "":
+		return "", uuid.UUID{}, "", errInvalidCertificateCN
+	case chunks[3] == "":
+		return "", uuid.UUID{}, "", errInvalidCertificateCN
+	}
+
 	return envoy.ProxyKind(chunks[1]), proxyUUID, identity.New(chunks[2], chunks[3]), nil
 }
 

pkg/envoy/ads/stream_test.go

@@ -4,9 +4,6 @@ import (
 	"fmt"
 	"testing"
 
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-
 	xds_discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
 	"github.com/google/uuid"
 	tassert "github.com/stretchr/testify/assert"
@@ -85,42 +82,50 @@ func TestMapsetToSliceConvFunctions(t *testing.T) {
 	assert.False(findSliceElem(nameSlice, "D"))
 }
 
-var _ = Describe("Test ADS response functions", func() {
-	defer GinkgoRecover()
-	Context("Test getCertificateCommonNameMeta()", func() {
-		It("parses CN into certificateCommonNameMeta", func() {
-			proxyUUID := uuid.New()
-			testNamespace := uuid.New().String()
-			serviceAccount := uuid.New().String()
-
-			cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s.%s.cluster.local", proxyUUID, envoy.KindSidecar, serviceAccount, testNamespace))
+func TestGetCertificateCommonNameMeta(t *testing.T) {
+	testCases := []struct {
+		name     string
+		uuid     uuid.UUID
+		identity identity.ServiceIdentity
+		// trustDomain string
+		err error
+	}{
+		{
+			name:     "valid cn",
+			uuid:     uuid.New(),
+			identity: identity.New("foo", "bar"),
+		},
+		{
+			name:     "invalid uuid",
+			uuid:     uuid.Nil,
+			identity: identity.New("foo", "bar"),
+		},
+		{
+			name:     "invalid identity",
+			uuid:     uuid.New(),
+			identity: identity.New("foo", ""),
+			err:      errInvalidCertificateCN,
+		},
+		{
+			name: "no identity",
+			uuid: uuid.New(),
+			err:  errInvalidCertificateCN,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert := tassert.New(t)
+			cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", tc.uuid, envoy.KindSidecar, tc.identity))
 
 			kind, uuid, si, err := getCertificateCommonNameMeta(cn)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(kind).To(Equal(envoy.KindSidecar))
-			Expect(uuid).To(Equal(proxyUUID))
-			Expect(si).To(Equal(identity.New(serviceAccount, testNamespace)))
-		})
 
-		It("parses CN into certificateCommonNameMeta", func() {
-			_, _, _, err := getCertificateCommonNameMeta("a")
-			Expect(err).To(HaveOccurred())
-		})
-	})
+			assert.Equal(tc.err, err)
 
-	Context("Test NewXDSCertCommonName() and getCertificateCommonNameMeta() together", func() {
-		It("returns the identifier of the form <proxyID>.<kind>.<service-account>.<namespace>", func() {
-			proxyUUID := uuid.New()
-			serviceAccount := uuid.New().String()
-			namespace := uuid.New().String()
-
-			cn := envoy.NewXDSCertCommonName(proxyUUID, envoy.KindSidecar, serviceAccount, namespace)
-
-			actualKind, actualUUID, actualSI, err := getCertificateCommonNameMeta(cn)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(actualKind).To(Equal(envoy.KindSidecar))
-			Expect(actualUUID).To(Equal(proxyUUID))
-			Expect(actualSI).To(Equal(identity.New(serviceAccount, namespace)))
+			if err == nil {
+				assert.Equal(envoy.KindSidecar, kind)
+				assert.Equal(tc.uuid, uuid)
+				assert.Equal(tc.identity, si)
+			}
 		})
-	})
-})
+	}
+}