Skip to content

Commit

Permalink
Have the Webhook react to pod creation/update only
Browse files Browse the repository at this point in the history
This was already working almost out-of-the-box, just had to:

- Change the webhook config so it watches pods instead of deployments
- Grant some extra ClusterRole permissions
- Add the piece that figures what's the OwnerReference and add the label
for it

Fixes #2342 and #1751

Signed-off-by: Alejandro Pedraza <[email protected]>
  • Loading branch information
alpeb committed Mar 11, 2019
1 parent 8f6c63d commit 2ab8edd
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 12 deletions.
3 changes: 3 additions & 0 deletions chart/templates/proxy_injector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ rules:
- apiGroups: [""]
resources: ["namespaces"]
verbs: ["get"]
- apiGroups: ["", "extensions"]
resources: ["replicationcontrollers", "replicasets"]
verbs: ["get"]
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
3 changes: 3 additions & 0 deletions cli/cmd/testdata/install_no_init_container_auto_inject.golden
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,9 @@ rules:
- apiGroups: [""]
resources: ["namespaces"]
verbs: ["get"]
- apiGroups: ["", "extensions"]
resources: ["replicationcontrollers", "replicasets"]
verbs: ["get"]
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
3 changes: 3 additions & 0 deletions cli/cmd/testdata/install_output.golden
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,9 @@ rules:
- apiGroups: [""]
resources: ["namespaces"]
verbs: ["get"]
- apiGroups: ["", "extensions"]
resources: ["replicationcontrollers", "replicasets"]
verbs: ["get"]
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ webhooks:
caBundle: {{ .CABundle }}
rules:
- operations: [ "CREATE" , "UPDATE" ]
apiGroups: ["apps", "extensions"]
apiVersions: ["v1", "v1beta1", "v1beta2"]
resources: ["deployments"]`
apiGroups: [""]
apiVersions: ["v1"]
resources: ["pods"]`
55 changes: 50 additions & 5 deletions controller/proxy-injector/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package injector

import (
"fmt"
"strings"

"github.com/linkerd/linkerd2/pkg/config"
"github.com/linkerd/linkerd2/pkg/inject"
"github.com/linkerd/linkerd2/pkg/k8s"
"github.com/linkerd/linkerd2/pkg/version"
log "github.com/sirupsen/logrus"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -130,11 +133,20 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission

p.AddCreatedByPodAnnotation(fmt.Sprintf("linkerd/proxy-injector %s", version.Version))

// When adding workloads through `kubectl apply` the spec template labels are
// automatically copied to the workload's main metadata section.
// This doesn't happen when adding labels through the webhook. So we manually
// add them to remain consistent.
conf.AddRootLabels(p)
if refs := conf.GetOwnerReferences(); len(refs) > 0 {
// assuming just one owner reference
k, v, err := w.parentRefLabel(request.Namespace, refs[0])
if err != nil {
return nil, err
}
p.AddPodLabel(k, v)
} else {
// When adding workloads through `kubectl apply` the spec template labels are
// automatically copied to the workload's main metadata section.
// This doesn't happen when adding labels through the webhook. So we manually
// add them to remain consistent.
conf.AddRootLabels(p)
}

patchJSON, err := p.Marshal()
if err != nil {
Expand All @@ -148,3 +160,36 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission

return admissionResponse, nil
}

func (w *Webhook) parentRefLabel(ns string, ref metav1.OwnerReference) (string, string, error) {
var key string
switch strings.ToLower(ref.Kind) {
case k8s.Deployment:
key = k8s.ProxyDeploymentLabel
case k8s.ReplicationController:
key = k8s.ProxyReplicationControllerLabel
rs, err := w.client.CoreV1().ReplicationControllers(ns).Get(ref.Name, v1.GetOptions{})
if err != nil {
return "", "", err
}
for _, ref := range rs.OwnerReferences {
return w.parentRefLabel(ns, ref)
}
case k8s.ReplicaSet:
key = k8s.ProxyReplicaSetLabel
rs, err := w.client.ExtensionsV1beta1().ReplicaSets(ns).Get(ref.Name, v1.GetOptions{})
if err != nil {
return "", "", err
}
for _, ref := range rs.OwnerReferences {
return w.parentRefLabel(ns, ref)
}
case k8s.Job:
key = k8s.ProxyJobLabel
case k8s.DaemonSet:
key = k8s.ProxyDaemonSetLabel
case k8s.StatefulSet:
key = k8s.ProxyStatefulSetLabel
}
return key, ref.Name, nil
}
140 changes: 140 additions & 0 deletions controller/proxy-injector/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,146 @@ func TestShouldInject(t *testing.T) {
})
}

// All the cases are tested for full coverage purposes, but the ReplicaSet
// one is the only interesting one, where we actually look into the ReplicaSet's
// ownerReference
func TestParentRefLabel(t *testing.T) {
t.Run("by checking annotations", func(t *testing.T) {
testCases := []struct {
k8sConfigs []string
ownerRef metav1.OwnerReference
expectedLabelKey string
expectedLabelValue string
}{
{
k8sConfigs: []string{`
apiVersion: apps/v1beta1
kind: Deployment
metadata:
name: emoji-dep
namespace: emojivoto
`,
},
ownerRef: metav1.OwnerReference{
Kind: "Deployment",
Name: "emoji-dep",
},
expectedLabelKey: k8s.ProxyDeploymentLabel,
expectedLabelValue: "emoji-dep",
},
{
k8sConfigs: []string{`
apiVersion: v1
kind: ReplicationController
metadata:
labels:
app: emoji-svc
name: emoji-rc
namespace: emojivoto
`,
},
ownerRef: metav1.OwnerReference{
Kind: "ReplicationController",
Name: "emoji-rc",
},
expectedLabelKey: k8s.ProxyReplicationControllerLabel,
expectedLabelValue: "emoji-rc",
},
{
k8sConfigs: []string{`
apiVersion: extensions/v1beta1
kind: ReplicaSet
metadata:
labels:
app: emoji-svc
name: emoji-rs
namespace: emojivoto
ownerReferences:
- apiVersion: apps/v1
kind: Deployment
name: emoji
`,
},
ownerRef: metav1.OwnerReference{
Kind: "ReplicaSet",
Name: "emoji-rs",
},
expectedLabelKey: k8s.ProxyDeploymentLabel,
expectedLabelValue: "emoji",
},
{
k8sConfigs: []string{`
apiVersion: batch/v1
kind: Job
metadata:
name: emoji-job
namespace: emojivoto
`,
},
ownerRef: metav1.OwnerReference{
Kind: "Job",
Name: "emoji-job",
},
expectedLabelKey: k8s.ProxyJobLabel,
expectedLabelValue: "emoji-job",
},
{
k8sConfigs: []string{`
apiVersion: apps/v1beta2
kind: DaemonSet
metadata:
name: emoji-ds
namespace: emojivoto
`,
},
ownerRef: metav1.OwnerReference{
Kind: "DaemonSet",
Name: "emoji-ds",
},
expectedLabelKey: k8s.ProxyDaemonSetLabel,
expectedLabelValue: "emoji-ds",
},
{
k8sConfigs: []string{`
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: emoji-sts
namespace: emojivoto
`,
},
ownerRef: metav1.OwnerReference{
Kind: "StatefulSet",
Name: "emoji-sts",
},
expectedLabelKey: k8s.ProxyStatefulSetLabel,
expectedLabelValue: "emoji-sts",
},
}

for _, tt := range testCases {
fakeClient, _, err := k8s.NewFakeClientSets(tt.k8sConfigs...)
if err != nil {
t.Fatalf("Error instantiating client: %s", err)
}
webhook, err := NewWebhook(fakeClient, "emojivoto", false, true)
if err != nil {
t.Fatalf("Error instantiating Webhook: %s", err)
}
k, v, err := webhook.parentRefLabel("emojivoto", tt.ownerRef)
if err != nil {
t.Fatalf("Error building parent label: %s", err)
}
if k != tt.expectedLabelKey {
t.Fatalf("Expected label key to be \"%s\", got \"%s\"", tt.expectedLabelKey, k)
}
if v != tt.expectedLabelValue {
t.Fatalf("Expected label value to be \"%s\", got \"%s\"", tt.expectedLabelValue, v)
}
}
})
}

func getFakeReq(b []byte) *admissionv1beta1.AdmissionRequest {
return &admissionv1beta1.AdmissionRequest{
Kind: metav1.GroupVersionKind{Kind: "Deployment"},
Expand Down
6 changes: 5 additions & 1 deletion pkg/inject/inject.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ func (conf *ResourceConfig) parse(bytes []byte) error {
return nil
}

func (conf *ResourceConfig) GetOwnerReferences() []metav1.OwnerReference {
return conf.podMeta.OwnerReferences
}

func (conf *ResourceConfig) complete(template *v1.PodTemplateSpec) {
conf.podSpec = &template.Spec
conf.podMeta = objMeta{&template.ObjectMeta}
Expand Down Expand Up @@ -552,7 +556,7 @@ func (conf *ResourceConfig) injectObjectMeta(patch *Patch) {
}

for k, v := range conf.podLabels {
patch.addPodLabel(k, v)
patch.AddPodLabel(k, v)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/inject/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (p *Patch) addVolume(volume *corev1.Volume) {
})
}

func (p *Patch) addPodLabel(key, value string) {
func (p *Patch) AddPodLabel(key, value string) {
p.patchOps = append(p.patchOps, &patchOp{
Op: "add",
Path: p.patchPathPodLabels + "/" + escapeKey(key),
Expand Down
2 changes: 1 addition & 1 deletion pkg/inject/patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestPatch(t *testing.T) {
actual.addVolumeRoot()
actual.addVolume(trustAnchors)
actual.addVolume(secrets)
actual.addPodLabel(k8sPkg.ControllerNSLabel, controllerNamespace)
actual.AddPodLabel(k8sPkg.ControllerNSLabel, controllerNamespace)
actual.addPodAnnotation(k8sPkg.CreatedByAnnotation, createdBy)

expected := NewPatchDeployment()
Expand Down
6 changes: 5 additions & 1 deletion pkg/inject/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ type Report struct {
func newReport(conf *ResourceConfig) Report {
var name string
if m := conf.podMeta.ObjectMeta; m != nil {
name = m.Name
if m.Name != "" {
name = m.Name
} else if m.GenerateName != "" {
name = m.GenerateName
}
}
return Report{
Kind: strings.ToLower(conf.meta.Kind),
Expand Down

0 comments on commit 2ab8edd

Please sign in to comment.