Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disallow injecting proxy sidecar in pods with hostNetwork: true #1090

Merged
merged 2 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
}
})
}
}