test/ads: fix test flakyness

PR openservicemesh#3803 introduced a mapset (backed by golang map) to slice conversion, which does not guarantee same output ordering for the same input, thus was making a unit test fail from time to time. This commit forces output of the slice to be alphabetically ordered, ensuring determinism of the output slice for the same input mapset. Signed-off-by: Eduard Serra <[email protected]>
eduser25 · Aug 16, 2021 · f844e88 · f844e88
1 parent 94f6260
commit f844e88
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 20 deletions.
diff --git a/pkg/envoy/ads/response_test.go b/pkg/envoy/ads/response_test.go
@@ -143,7 +143,7 @@ var _ = Describe("Test ADS response functions", func() {
 			mockCertManager.EXPECT().IssueCertificate(gomock.Any(), certDuration).Return(certPEM, nil).Times(1)
 
 			// Set subscribed resources for SDS
-			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore"))
+			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore", "root-cert-https:default/bookstore"))
 
 			err := s.sendResponse(proxy, &server, nil, mockConfigurator, envoy.XDSResponseOrder...)
 			Expect(err).To(BeNil())
@@ -180,25 +180,25 @@ var _ = Describe("Test ADS response functions", func() {
 			Expect(err).To(BeNil())
 			Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.ServiceCertType,
+				CertType: secrets.RootCertTypeForMTLSInbound,
 			}.String()))
 
-			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
+			serverRootCertTypeForHTTPS := xds_auth.Secret{}
 			tmpResource = (*actualResponses)[4].Resources[1]
-			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound)
+			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
+			Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForMTLSInbound,
+				CertType: secrets.RootCertTypeForHTTPS,
 			}.String()))
 
-			serverRootCertTypeForHTTPS := xds_auth.Secret{}
+			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
 			tmpResource = (*actualResponses)[4].Resources[2]
-			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS)
+			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{
+			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForHTTPS,
+				CertType: secrets.ServiceCertType,
 			}.String()))
 		})
 	})
@@ -229,7 +229,7 @@ var _ = Describe("Test ADS response functions", func() {
 			mockCertManager.EXPECT().IssueCertificate(gomock.Any(), certDuration).Return(certPEM, nil).Times(1)
 
 			// Set subscribed resources
-			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore"))
+			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore", "root-cert-https:default/bookstore"))
 
 			err := s.sendResponse(proxy, &server, nil, mockConfigurator, envoy.TypeSDS)
 			Expect(err).To(BeNil())
@@ -255,25 +255,25 @@ var _ = Describe("Test ADS response functions", func() {
 			Expect(err).To(BeNil())
 			Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.ServiceCertType,
+				CertType: secrets.RootCertTypeForMTLSInbound,
 			}.String()))
 
-			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
+			serverRootCertTypeForHTTPS := xds_auth.Secret{}
 			tmpResource = sdsResponse.Resources[1]
-			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound)
+			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
+			Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForMTLSInbound,
+				CertType: secrets.RootCertTypeForHTTPS,
 			}.String()))
 
-			serverRootCertTypeForHTTPS := xds_auth.Secret{}
+			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
 			tmpResource = sdsResponse.Resources[2]
-			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS)
+			err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{
+			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
 				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForHTTPS,
+				CertType: secrets.ServiceCertType,
 			}.String()))
 		})
 	})

diff --git a/pkg/envoy/ads/stream.go b/pkg/envoy/ads/stream.go
@@ -2,6 +2,7 @@ package ads
 
 import (
 	"context"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -285,6 +286,7 @@ func getRequestedResourceNamesSet(discoveryRequest *xds_discovery.DiscoveryReque
 }
 
 // getResourceSliceFromMapset is a helper to convert a mapset of resource names to a string slice
+// return slice is alphabetically ordered to ensure output determinism for a given input
 func getResourceSliceFromMapset(resourceMap mapset.Set) []string {
 	resourceSlice := []string{}
 	it := resourceMap.Iterator()
@@ -297,6 +299,7 @@ func getResourceSliceFromMapset(resourceMap mapset.Set) []string {
 		}
 		resourceSlice = append(resourceSlice, resString)
 	}
+	sort.Strings(resourceSlice)
 	return resourceSlice
 }