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

Commit

Permalink
catalog/injector: Use uuid.UUID type for proxyUUID variable; converge…
Browse files Browse the repository at this point in the history 
… on naming (#1899)
  • Loading branch information
@draychev
draychev authored Oct 23, 2020
1 parent 202121c commit 0f5bbc2
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 63 deletions.
3 changes: 2 additions & 1 deletion pkg/catalog/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

mapset "github.com/deckarep/golang-set"
"github.com/google/uuid"
target "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/access/v1alpha2"
spec "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/specs/v1alpha3"
split "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/split/v1alpha2"
Expand Down Expand Up @@ -138,7 +139,7 @@ type disconnectedProxy struct {

// certificateCommonNameMeta is the type that stores the metadata present in the CommonName field in a proxy's certificate
type certificateCommonNameMeta struct {
ProxyID string
ProxyUUID uuid.UUID
ServiceAccount string
Namespace string
}
Expand Down
25 changes: 16 additions & 9 deletions pkg/catalog/xds_certificates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package catalog
import (
"strings"

"github.com/google/uuid"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"

Expand Down Expand Up @@ -85,22 +86,22 @@ func GetPodFromCertificate(cn certificate.CommonName, kubecontroller k8s.Control
return nil, err
}

log.Trace().Msgf("Looking for pod with label %q=%q", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyID)
log.Trace().Msgf("Looking for pod with label %q=%q", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyUUID)
podList := kubecontroller.ListPods()
var pods []v1.Pod
for _, pod := range podList {
if pod.Namespace != cnMeta.Namespace {
continue
}
for labelKey, labelValue := range pod.Labels {
if labelKey == constants.EnvoyUniqueIDLabelName && labelValue == cnMeta.ProxyID {
if labelKey == constants.EnvoyUniqueIDLabelName && labelValue == cnMeta.ProxyUUID.String() {
pods = append(pods, *pod)
}
}
}

if len(pods) == 0 {
log.Error().Msgf("Did not find pod with label %s = %s in namespace %s", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyID, cnMeta.Namespace)
log.Error().Msgf("Did not find pod with label %s = %s in namespace %s", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyUUID, cnMeta.Namespace)
return nil, errDidNotFindPodForCertificate
}

Expand All @@ -109,12 +110,12 @@ func GetPodFromCertificate(cn certificate.CommonName, kubecontroller k8s.Control
// This is a limitation we set in place in order to make the mesh easy to understand and reason about.
// When a pod belongs to more than one service XDS will not program the Envoy proxy, leaving it out of the mesh.
if len(pods) > 1 {
log.Error().Msgf("Found more than one pod with label %s = %s in namespace %s; There should be only one!", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyID, cnMeta.Namespace)
log.Error().Msgf("Found more than one pod with label %s = %s in namespace %s; There should be only one!", constants.EnvoyUniqueIDLabelName, cnMeta.ProxyUUID, cnMeta.Namespace)
return nil, errMoreThanOnePodForCertificate
}

pod := pods[0]
log.Trace().Msgf("Found pod %s for proxyID %s", pod.Name, cnMeta.ProxyID)
log.Trace().Msgf("Found pod %s for proxyID %s", pod.Name, cnMeta.ProxyUUID)

// Ensure the Namespace encoded in the certificate matches that of the Pod
if pod.Namespace != cnMeta.Namespace {
Expand Down Expand Up @@ -156,14 +157,20 @@ func getCertificateCommonNameMeta(cn certificate.CommonName) (*certificateCommon
if len(chunks) < 3 {
return nil, errInvalidCertificateCN
}
proxyUUID, err := uuid.Parse(chunks[0])
if err != nil {
log.Error().Err(err).Msgf("Error parsing %s into uuid.UUID", chunks[0])
return nil, err
}

return &certificateCommonNameMeta{
ProxyID: chunks[0],
ProxyUUID: proxyUUID,
ServiceAccount: chunks[1],
Namespace: chunks[2],
}, nil
}

// NewCertCommonNameWithProxyID returns a newly generated CommonName for a certificate of the form: <ProxyID>.<serviceAccount>.<namespace>
func NewCertCommonNameWithProxyID(proxyUUID, serviceAccount, namespace string) certificate.CommonName {
return certificate.CommonName(strings.Join([]string{proxyUUID, serviceAccount, namespace}, constants.DomainDelimiter))
// NewCertCommonNameWithProxyID returns a newly generated CommonName for a certificate of the form: <ProxyUUID>.<serviceAccount>.<namespace>
func NewCertCommonNameWithProxyID(proxyUUID uuid.UUID, serviceAccount, namespace string) certificate.CommonName {
return certificate.CommonName(strings.Join([]string{proxyUUID.String(), serviceAccount, namespace}, constants.DomainDelimiter))
}
52 changes: 26 additions & 26 deletions pkg/catalog/xds_certificates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var _ = Describe("Test XDS certificate tooling", func() {
kubeClient := testclient.NewSimpleClientset()

mc := NewFakeMeshCatalog(kubeClient)
cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", tests.EnvoyUID, tests.BookstoreServiceAccountName, tests.Namespace))
cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", tests.ProxyUUID, tests.BookstoreServiceAccountName, tests.Namespace))

Context("Test GetServicesFromEnvoyCertificate()", func() {
It("works as expected", func() {
Expand Down Expand Up @@ -74,11 +74,11 @@ var _ = Describe("Test XDS certificate tooling", func() {
It("works as expected", func() {

// Create the POD
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
namespace := uuid.New().String()
podName := uuid.New().String()
newPod := tests.NewPodTestFixture(namespace, podName)
newPod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()
newPod.Labels[tests.SelectorKey] = tests.SelectorValue

_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod, metav1.CreateOptions{})
Expand All @@ -91,7 +91,7 @@ var _ = Describe("Test XDS certificate tooling", func() {
_, err = kubeClient.CoreV1().Services(namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

podCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, tests.BookstoreServiceAccountName, namespace))
podCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, tests.BookstoreServiceAccountName, namespace))
meshServices, err := mc.GetServicesFromEnvoyCertificate(podCN)
Expect(err).ToNot(HaveOccurred())

Expand All @@ -107,7 +107,7 @@ var _ = Describe("Test XDS certificate tooling", func() {

Context("Test GetPodFromCertificate()", func() {
It("works as expected", func() {
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
someOtherEnvoyUID := uuid.New().String()
namespace := uuid.New().String()
mockKubeController := k8s.NewMockController(mockCtrl)
Expand All @@ -123,7 +123,7 @@ var _ = Describe("Test XDS certificate tooling", func() {
Expect(err).ToNot(HaveOccurred())

newPod1 := tests.NewPodTestFixture(namespace, fmt.Sprintf("pod-1-%s", uuid.New()))
newPod1.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod1.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()
_, err = kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

Expand All @@ -137,7 +137,7 @@ var _ = Describe("Test XDS certificate tooling", func() {
Expect(err).ToNot(HaveOccurred())
Expect(len(pods.Items)).To(Equal(3))

newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, tests.BookstoreServiceAccountName, namespace))
newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, tests.BookstoreServiceAccountName, namespace))

mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{&newPod0, &newPod1, &newPod2})
actualPod, err := GetPodFromCertificate(newCN, mockKubeController)
Expand All @@ -151,21 +151,21 @@ var _ = Describe("Test XDS certificate tooling", func() {
Context("Test GetPodFromCertificate()", func() {
It("fails with invalid certificate", func() {
namespace := uuid.New().String()
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
mockKubeController := k8s.NewMockController(mockCtrl)

// Create a pod with the same certificateCN twice
for range []int{0, 1} {
podName := uuid.New().String()
newPod := tests.NewPodTestFixture(namespace, podName)
newPod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()

_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
}

// No service account in this CN
newCN := certificate.CommonName(fmt.Sprintf("%s.%s", envoyUID, namespace))
newCN := certificate.CommonName(fmt.Sprintf("%s.%s", proxyUUID, namespace))
actualPod, err := GetPodFromCertificate(newCN, mockKubeController)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(errInvalidCertificateCN))
Expand All @@ -176,7 +176,7 @@ var _ = Describe("Test XDS certificate tooling", func() {
Context("Test GetPodFromCertificate()", func() {
It("fails with two pods with same cert", func() {
namespace := uuid.New().String()
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
mockKubeController := k8s.NewMockController(mockCtrl)

// Create a pod with the same certificateCN twice
Expand All @@ -185,14 +185,14 @@ var _ = Describe("Test XDS certificate tooling", func() {
podName := uuid.New().String()
newPod := tests.NewPodTestFixture(namespace, podName)
pods = append(pods, &newPod)
newPod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()

_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
}

mockKubeController.EXPECT().ListPods().Return(pods)
newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, tests.BookstoreServiceAccountName, namespace))
newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, tests.BookstoreServiceAccountName, namespace))
actualPod, err := GetPodFromCertificate(newCN, mockKubeController)
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(errMoreThanOnePodForCertificate))
Expand All @@ -203,20 +203,20 @@ var _ = Describe("Test XDS certificate tooling", func() {
Context("Test GetPodFromCertificate()", func() {
It("fails when service account does not match certificate", func() {
namespace := uuid.New().String()
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
randomServiceAccount := uuid.New().String()
mockKubeController := k8s.NewMockController(mockCtrl)

podName := uuid.New().String()
newPod := tests.NewPodTestFixture(namespace, podName)
newPod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()

_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(newPod.Spec.ServiceAccountName).ToNot(Equal(randomServiceAccount))
Expect(newPod.Spec.ServiceAccountName).To(Equal(tests.BookstoreServiceAccountName))

newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, randomServiceAccount, namespace))
newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, randomServiceAccount, namespace))
mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{&newPod})
actualPod, err := GetPodFromCertificate(newCN, mc.kubeController)
Expect(err).To(HaveOccurred())
Expand All @@ -228,18 +228,18 @@ var _ = Describe("Test XDS certificate tooling", func() {
Context("Test GetPodFromCertificate()", func() {
It("fails when namespace does not match certificate", func() {
namespace := uuid.New().String()
envoyUID := uuid.New().String()
proxyUUID := uuid.New()
someOtherRandomNamespace := uuid.New().String()
mockKubeController := k8s.NewMockController(mockCtrl)

podName := uuid.New().String()
newPod := tests.NewPodTestFixture(namespace, podName)
newPod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
newPod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String()

_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, tests.BookstoreServiceAccountName, someOtherRandomNamespace))
newCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, tests.BookstoreServiceAccountName, someOtherRandomNamespace))
mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{&newPod})
actualPod, err := GetPodFromCertificate(newCN, mockKubeController)
Expect(err).To(HaveOccurred())
Expand Down Expand Up @@ -333,17 +333,17 @@ var _ = Describe("Test XDS certificate tooling", func() {

Context("Test getCertificateCommonNameMeta()", func() {
It("parses CN into certificateCommonNameMeta", func() {
proxyID := uuid.New().String()
proxyUUID := uuid.New()
testNamespace := uuid.New().String()
serviceAccount := uuid.New().String()

cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyID, serviceAccount, testNamespace))
cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, serviceAccount, testNamespace))

cnMeta, err := getCertificateCommonNameMeta(cn)
Expect(err).ToNot(HaveOccurred())

expected := &certificateCommonNameMeta{
ProxyID: proxyID,
ProxyUUID: proxyUUID,
ServiceAccount: serviceAccount,
Namespace: testNamespace,
}
Expand All @@ -359,16 +359,16 @@ var _ = Describe("Test XDS certificate tooling", func() {
Context("Test NewCertCommonNameWithProxyID() and getCertificateCommonNameMeta() together", func() {
It("returns the the CommonName of the form <proxyID>.<namespace>", func() {

proxyID := uuid.New().String()
proxyUUID := uuid.New()
serviceAccount := uuid.New().String()
namespace := uuid.New().String()

cn := NewCertCommonNameWithProxyID(proxyID, serviceAccount, namespace)
Expect(cn).To(Equal(certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyID, serviceAccount, namespace))))
cn := NewCertCommonNameWithProxyID(proxyUUID, serviceAccount, namespace)
Expect(cn).To(Equal(certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, serviceAccount, namespace))))

actualMeta, err := getCertificateCommonNameMeta(cn)
expectedMeta := certificateCommonNameMeta{
ProxyID: proxyID,
ProxyUUID: proxyUUID,
ServiceAccount: serviceAccount,
Namespace: namespace,
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const (
KubernetesOpaqueSecretCAExpiration = "expiration"

// EnvoyUniqueIDLabelName is the label applied to pods with the unique ID of the Envoy sidecar.
EnvoyUniqueIDLabelName = "osm-envoy-uid"
EnvoyUniqueIDLabelName = "osm-proxy-uuid"

// TimeDateLayout is the layout for time.Parse used in this repo
TimeDateLayout = "2006-01-02T15:04:05.000Z"
Expand Down
8 changes: 4 additions & 4 deletions pkg/envoy/ads/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ var _ = Describe("Test ADS response functions", func() {
// --- setup
kubeClient := testclient.NewSimpleClientset()
namespace := tests.Namespace
envoyUID := tests.EnvoyUID
proxyUUID := tests.ProxyUUID
serviceName := tests.BookstoreV1ServiceName
serviceAccountName := tests.BookstoreServiceAccountName

labels := map[string]string{constants.EnvoyUniqueIDLabelName: tests.EnvoyUID}
labels := map[string]string{constants.EnvoyUniqueIDLabelName: tests.ProxyUUID}
mc := catalog.NewFakeMeshCatalog(kubeClient)

// Create a Pod
pod := tests.NewPodTestFixture(namespace, fmt.Sprintf("pod-0-%s", uuid.New()))
pod.Labels[constants.EnvoyUniqueIDLabelName] = envoyUID
pod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID
_, err := kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &pod, metav1.CreateOptions{})
It("should have created a pod", func() {
Expect(err).ToNot(HaveOccurred())
Expand All @@ -68,7 +68,7 @@ var _ = Describe("Test ADS response functions", func() {
GinkgoT().Fatalf("Error creating new Bookstire Apex service: %s", err.Error())
}

cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, serviceAccountName, namespace))
cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, serviceAccountName, namespace))
proxy := envoy.NewProxy(cn, nil)

meshService := service.MeshService{
Expand Down
4 changes: 2 additions & 2 deletions pkg/envoy/cds/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var _ = Describe("CDS Response", func() {

Context("Test cds.NewResponse", func() {
It("Returns unique list of clusters for CDS", func() {
proxyUUID := fmt.Sprintf("proxy-0-%s", uuid.New())
proxyUUID := uuid.New()
podName := fmt.Sprintf("pod-0-%s", uuid.New())

// The format of the CN matters
Expand All @@ -57,7 +57,7 @@ var _ = Describe("CDS Response", func() {
{
// Create a pod to match the CN
pod := tests.NewPodTestFixtureWithOptions(tests.Namespace, podName, proxyServiceAccountName)
pod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID // This is what links the Pod and the Certificate
pod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String() // This is what links the Pod and the Certificate
_, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &pod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/envoy/eds/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var _ = Describe("Test EDS response", func() {
// Initialize the proxy service
proxyServiceName := tests.BookbuyerServiceName
proxyServiceAccountName := tests.BookbuyerServiceAccountName
proxyUUID := fmt.Sprintf("proxy-0-%s", uuid.New())
proxyUUID := uuid.New()

// The format of the CN matters
xdsCertificate := certificate.CommonName(fmt.Sprintf("%s.%s.%s.foo.bar", proxyUUID, proxyServiceAccountName, tests.Namespace))
Expand All @@ -46,8 +46,9 @@ var _ = Describe("Test EDS response", func() {
{
// Create a pod to match the CN
podName := fmt.Sprintf("pod-0-%s", uuid.New())

pod := tests.NewPodTestFixtureWithOptions(tests.Namespace, podName, proxyServiceAccountName)
pod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID // This is what links the Pod and the Certificate
pod.Labels[constants.EnvoyUniqueIDLabelName] = proxyUUID.String() // This is what links the Pod and the Certificate
_, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &pod, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
}
Expand All @@ -71,7 +72,7 @@ var _ = Describe("Test EDS response", func() {
It("Correctly returns an error response for endpoints when the proxy isn't associated with a MeshService", func() {
// Initialize the proxy service
proxyServiceAccountName := "non-existent-service-account"
proxyUUID := fmt.Sprintf("non-existent-pod-%s", uuid.New())
proxyUUID := uuid.New()

// The format of the CN matters
xdsCertificate := certificate.CommonName(fmt.Sprintf("%s.%s.%s.foo.bar", proxyUUID, proxyServiceAccountName, tests.Namespace))
Expand Down
Loading

0 comments on commit 0f5bbc2

Please sign in to comment.