Skip to content

Commit

Permalink
Extract CP roots from config map (#25)
Browse files Browse the repository at this point in the history 
When running with the latest Linkerd edge bloud agent is sending a certificate message with the Roots being empty. The reason for that is that in the bcloud agent, we look for the `EnvVar.Value` field rather than the `EnvVar.ValueFrom`. In linkerd/linkerd2#6455 the proxy manifest was changed to load the trust anchors from a config map. This causes the described above problem. 

This can be fixed by making the agent consider this value when inspecting the env vars. It is important to make this backwards compatible - check the `Value` field and only if not present, check the `ValueFrom` one. This might require RBAC changes.

Note to reviewers: 

In order to test, run this agent with the latest stable to ensure the change is backwards compatible. After that you can run it with the latest edge to convince yourself that the problem has been fixed. In both cases you need to make sure that the certificates in the control plane page are populated.

Signed-off-by: Zahari Dichev <[email protected]>
  • Loading branch information
@zaharidichev
zaharidichev authored Aug 6, 2021
1 parent f3ca9dd commit 360fbc0
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 10 deletions.
7 changes: 4 additions & 3 deletions agent/pkg/handler/linkerd_info.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handler

import (
"context"
"time"

"github.com/buoyantio/linkerd-buoyant/agent/pkg/api"
Expand Down Expand Up @@ -43,7 +44,7 @@ func (h *LinkerdInfo) Start() {
for {
select {
case <-ticker.C:
h.handleCertsInfo()
h.handleCertsInfo(context.Background())
case <-h.stopCh:
return
}
Expand All @@ -56,8 +57,8 @@ func (h *LinkerdInfo) Stop() {
close(h.stopCh)
}

func (h *LinkerdInfo) handleCertsInfo() {
certs, err := h.k8s.GetControlPlaneCerts()
func (h *LinkerdInfo) handleCertsInfo(ctx context.Context) {
certs, err := h.k8s.GetControlPlaneCerts(ctx)
if err != nil {
h.log.Errorf("error getting control plane certs: %s", err)
return
Expand Down
33 changes: 29 additions & 4 deletions agent/pkg/k8s/certificates.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package k8s

import (
"context"
"crypto/tls"
"fmt"
"net"
Expand All @@ -11,16 +12,19 @@ import (
l5dk8s "github.com/linkerd/linkerd2/pkg/k8s"
ldTls "github.com/linkerd/linkerd2/pkg/tls"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
)

const (
identityComponentName = "identity"
linkerdNsEnvVarName = "_l5d_ns"
linkerdTrustDomainEnvVarName = "_l5d_trustdomain"
trustRootsConfigMapName = "linkerd-identity-trust-roots"
trustRootsConfigMapKeyName = "ca-bundle.crt"
)

func (c *Client) GetControlPlaneCerts() (*pb.ControlPlaneCerts, error) {
func (c *Client) GetControlPlaneCerts(ctx context.Context) (*pb.ControlPlaneCerts, error) {
identityPod, err := c.getControlPlaneComponentPod(identityComponentName)
if err != nil {
return nil, err
Expand All @@ -31,7 +35,7 @@ func (c *Client) GetControlPlaneCerts() (*pb.ControlPlaneCerts, error) {
return nil, err
}

rootCerts, err := extractRootsCerts(container)
rootCerts, err := c.extractRootsCerts(ctx, container, identityPod.Namespace)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -95,15 +99,36 @@ func getServerName(podsa string, podns string, container *v1.Container) (string,
return fmt.Sprintf("%s.%s.serviceaccount.identity.%s.%s", podsa, podns, l5dns, l5dtrustdomain), nil
}

func extractRootsCerts(container *v1.Container) ([]*pb.CertData, error) {
func (c *Client) extractRootsCerts(ctx context.Context, container *v1.Container, namespace string) ([]*pb.CertData, error) {
for _, ev := range container.Env {
if ev.Name != identity.EnvTrustAnchors {
continue
}
certificates, err := ldTls.DecodePEMCertificates(ev.Value)

rootsValue := ev.Value
if rootsValue == "" {
// in this case we need to check for a reference to a config map
if ev.ValueFrom == nil || ev.ValueFrom.ConfigMapKeyRef == nil {
return nil, fmt.Errorf("neither a Value nor a ConfigMapKeyRef for the %s env var are present on proxy container [%s]", identity.EnvTrustAnchors, container.Name)
}
cmName := ev.ValueFrom.ConfigMapKeyRef.Name
cm, err := c.k8sClient.CoreV1().ConfigMaps(namespace).Get(ctx, cmName, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("cannot obtain config map %s/%s", namespace, cmName)
}

var ok bool
rootsValue, ok = cm.Data[trustRootsConfigMapKeyName]
if !ok {
return nil, fmt.Errorf("config map %s/%s does not have %s key", namespace, cmName, trustRootsConfigMapKeyName)
}
}

certificates, err := ldTls.DecodePEMCertificates(rootsValue)
if err != nil {
return nil, err
}

certsData := make([]*pb.CertData, len(certificates))
for i, crt := range certificates {
encoded := ldTls.EncodeCertificatesPEM(crt)
Expand Down
100 changes: 97 additions & 3 deletions agent/pkg/k8s/certificates_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package k8s

import (
"context"
"crypto/x509"
"errors"
"fmt"
Expand Down Expand Up @@ -317,11 +318,12 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5
fixtures := []*struct {
testName string
container *v1.Container
crtConfigMap *v1.ConfigMap
expectedCerts string
expectedErr error
}{
{
"gets correct cert",
"gets correct cert from env value",
&v1.Container{
Name: l5dk8s.ProxyContainerName,
Env: []v1.EnvVar{
Expand All @@ -331,15 +333,99 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5
},
},
},
nil,
expectedRoots,
nil,
},
{
"gets correct cert from config map",
&v1.Container{
Name: l5dk8s.ProxyContainerName,
Env: []v1.EnvVar{
{
Name: identity.EnvTrustAnchors,
ValueFrom: &v1.EnvVarSource{
ConfigMapKeyRef: &v1.ConfigMapKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: trustRootsConfigMapName,
},
Key: trustRootsConfigMapKeyName,
},
},
},
},
},
&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: trustRootsConfigMapName,
Namespace: "linkerd",
},
Data: map[string]string{
trustRootsConfigMapKeyName: expectedRoots,
},
},
expectedRoots,
nil,
},
{
"no roots",
"errors when config map not present",
&v1.Container{
Name: l5dk8s.ProxyContainerName,
Env: []v1.EnvVar{
{
Name: identity.EnvTrustAnchors,
ValueFrom: &v1.EnvVarSource{
ConfigMapKeyRef: &v1.ConfigMapKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: trustRootsConfigMapName,
},
Key: trustRootsConfigMapKeyName,
},
},
},
},
},
nil,
expectedRoots,
fmt.Errorf("cannot obtain config map linkerd/%s", trustRootsConfigMapName),
},
{
"errors when config map key not present",
&v1.Container{
Name: l5dk8s.ProxyContainerName,
Env: []v1.EnvVar{
{
Name: identity.EnvTrustAnchors,
ValueFrom: &v1.EnvVarSource{
ConfigMapKeyRef: &v1.ConfigMapKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: trustRootsConfigMapName,
},
Key: trustRootsConfigMapKeyName,
},
},
},
},
},
&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: trustRootsConfigMapName,
Namespace: "linkerd",
},
Data: map[string]string{
"nonsense.key": expectedRoots,
},
},
expectedRoots,
fmt.Errorf("config map linkerd/%s does not have %s key", trustRootsConfigMapName, trustRootsConfigMapKeyName),
},
{
"no roots env var",
&v1.Container{
Name: l5dk8s.ProxyContainerName,
Env: []v1.EnvVar{},
},
nil,
"",
fmt.Errorf("could not find env var with name %s on proxy container [linkerd-proxy]", identity.EnvTrustAnchors),
},
Expand All @@ -348,7 +434,15 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5
for _, tc := range fixtures {
tc := tc
t.Run(tc.testName, func(t *testing.T) {
roots, err := extractRootsCerts(tc.container)
c := fakeClient()
if tc.crtConfigMap != nil {
c = fakeClient(tc.crtConfigMap)
}

c.Sync(nil, time.Second)
client := NewClient(c.k8sClient, c.sharedInformers, nil)

roots, err := client.extractRootsCerts(context.Background(), tc.container, "linkerd")
if tc.expectedErr != nil {
if tc.expectedErr.Error() != err.Error() {
t.Fatalf("exepected err %s, got %s", tc.expectedErr, err)
Expand Down

0 comments on commit 360fbc0

Please sign in to comment.