Skip to content

Commit

Permalink
fix: disallow injecting proxy sidecar in pods with hostNetwork: true (
Browse files Browse the repository at this point in the history
#1090)

Signed-off-by: Anish Ramasekar <[email protected]>
  • Loading branch information
aramase committed Aug 23, 2023
1 parent b68e331 commit 4a889b7
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 7 deletions.
4 changes: 4 additions & 0 deletions docs/book/src/known-issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,7 @@ az rest --method post --uri "${URI}" --body "${BODY}" --headers "Content-Type=ap
To protect the stability of the system and prevent custom admission controllers from impacting internal services in the kube-system, namespace AKS has an Admissions Enforcer, which automatically excludes kube-system and AKS internal namespaces. Refer to [doc](https://docs.microsoft.com/en-us/azure/aks/faq#can-admission-controller-webhooks-impact-kube-system-and-internal-aks-namespaces) for more details.
If you're deploying a pod in the `kube-system` namespace of an AKS cluster and need the environment variables, projected service account token volume injected by the Azure Workload Identity Mutating Webhook, add the `"admissions.enforcer/disabled": "true"` label or annotation in the [MutatingWebhookConfiguration](https://github.com/Azure/azure-workload-identity/blob/8644a217f09902fa1ac63e05cf04d9a3f3f1ebc3/deploy/azure-wi-webhook.yaml#L206-L235).

## Proxy sidecar not injected into pods that have `hostNetwork: true`

The proxy sidecar modifies the `iptables` rules to redirect traffic to the Azure Instance Metadata Service (IMDS) endpoint to the proxy sidecar. This is not supported when `hostNetwork: true` is set on the pod as it will modify the host's `iptables` rules which will impact other pods running on the same host.
8 changes: 8 additions & 0 deletions pkg/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
}

if shouldInjectProxySidecar(pod) {
// if the pod has hostNetwork set to true, we cannot inject the proxy sidecar
// as it'll end up modifying the network stack of the host and affecting other pods
if pod.Spec.HostNetwork {
err := errors.New("hostNetwork is set to true, cannot inject proxy sidecar")
logger.Error("failed to inject proxy sidecar", err)
return admission.Errored(http.StatusBadRequest, err)
}

proxyPort, err := getProxyPort(pod)
if err != nil {
logger.Error("failed to get proxy port", err)
Expand Down
86 changes: 79 additions & 7 deletions pkg/webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"reflect"
"strconv"
"strings"
"testing"

admissionv1 "k8s.io/api/admission/v1"
Expand All @@ -26,12 +27,13 @@ var (
serviceAccountTokenExpiry = MinServiceAccountTokenExpiration
)

func newPod(name, namespace, serviceAccountName string, labels map[string]string) *corev1.Pod {
func newPod(name, namespace, serviceAccountName string, labels, annotations map[string]string, hostNetwork bool) *corev1.Pod {
return &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: labels,
Name: name,
Namespace: namespace,
Labels: labels,
Annotations: annotations,
},
Spec: corev1.PodSpec{
ServiceAccountName: serviceAccountName,
Expand All @@ -47,12 +49,13 @@ func newPod(name, namespace, serviceAccountName string, labels map[string]string
Image: "image",
},
},
HostNetwork: hostNetwork,
},
}
}

func newPodRaw(name, namespace, serviceAccountName string, labels map[string]string) []byte {
pod := newPod(name, namespace, serviceAccountName, labels)
func newPodRaw(name, namespace, serviceAccountName string, labels, annotations map[string]string, hostNetwork bool) []byte {
pod := newPod(name, namespace, serviceAccountName, labels, annotations, hostNetwork)
raw, err := json.Marshal(pod)
if err != nil {
panic(err)
Expand Down Expand Up @@ -785,7 +788,7 @@ func TestHandle(t *testing.T) {
Version: "v1",
Kind: "Pod",
},
Object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", test.serviceAccountName, test.podLabels)},
Object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", test.serviceAccountName, test.podLabels, nil, false)},
Namespace: "ns1",
Operation: admissionv1.Create,
},
Expand Down Expand Up @@ -1268,3 +1271,72 @@ func TestGetProxyPort(t *testing.T) {
})
}
}

func TestHandleError(t *testing.T) {
serviceAccounts := []client.Object{}
for _, name := range []string{"default", "sa"} {
serviceAccounts = append(serviceAccounts, &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "ns1",
Annotations: map[string]string{
ClientIDAnnotation: "clientID",
ServiceAccountTokenExpiryAnnotation: "4800",
},
},
})
}

decoder, _ := atypes.NewDecoder(runtime.NewScheme())

tests := []struct {
name string
object runtime.RawExtension
clientObjects []client.Object
expectedErr string
}{
{
name: "pod has host network",
object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", "sa",
map[string]string{UseWorkloadIdentityLabel: "true"}, map[string]string{InjectProxySidecarAnnotation: "true"}, true)},
clientObjects: serviceAccounts,
expectedErr: "hostNetwork is set to true, cannot inject proxy sidecar",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if err := registerMetrics(); err != nil {
t.Fatalf("failed to register metrics: %v", err)
}

m := &podMutator{
client: fake.NewClientBuilder().WithObjects(test.clientObjects...).Build(),
reader: fake.NewClientBuilder().WithObjects().Build(),
config: &config.Config{TenantID: "tenantID"},
decoder: decoder,
}

req := atypes.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
Kind: metav1.GroupVersionKind{
Group: "",
Version: "v1",
Kind: "Pod",
},
Object: test.object,
Namespace: "ns1",
Operation: admissionv1.Create,
},
}

resp := m.Handle(context.Background(), req)
if resp.Allowed {
t.Fatalf("expected to be denied")
}
if !strings.Contains(resp.Result.Message, test.expectedErr) {
t.Fatalf("expected error to contain: %v, got: %v", test.expectedErr, resp.Result.Message)
}
})
}
}

0 comments on commit 4a889b7

Please sign in to comment.