Skip to content

Commit

Permalink
Replace curlimages/curl with extension-init (#108)
Browse files Browse the repository at this point in the history 
* Replace `curlimages/curl` with `extension-init`

Replaced curlimages/curl docker image in the namespace-metadata Job with linkerd's extension-init image, to avoid all the OS luggage included in the former, which generates CVE alerts.
  • Loading branch information
@alpeb
alpeb authored Sep 25, 2023
1 parent ef8986a commit 364ca6e
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 23 deletions.
12 changes: 6 additions & 6 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ Alternatively, you can download the CLI directly via the
To install the linkerd-smi Helm chart, run:

helm repo add l5d-smi https://linkerd.github.io/linkerd-smi
helm install linkers-smi -n --create-namespace l5d-smi/linkerd-smi
helm install linkerd-smi -n --create-namespace l5d-smi/linkerd-smi

## Compatibility matrix

| linkerd-smi | linkerd stable | linkerd edge |
| ----------- | ----------------- | ------------------------- |
| v0.1.0 | 2.11 and previous | edge-21.12.1 and previous |
| v0.2.0 | 2.12.0 and later | edge-21.12.2 and later |
| v0.2.1 | 2.12.0 and later | edge-21.12.2 and later |
| linkerd-smi | linkerd stable | linkerd edge |
| ---------------- | ----------------- | ------------------------- |
| v0.1.0 | 2.11 and previous | edge-21.12.1 and previous |
| v0.2.0 | 2.12.0 and later | edge-21.12.2 and later |
| v0.2.1 and later | 2.12.0 and later | edge-21.12.2 and later |

## License

Expand Down
5 changes: 5 additions & 0 deletions charts/linkerd-smi/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ Kubernetes: `>=1.16.0-0`
| adaptor.nodeSelector | object | `{}` | Node selector for the adaptor instance |
| adaptor.tolerations | list | `[]` | Tolerations for the adaptor instance |
| clusterDomain | string | `"cluster.local"` | Kubernetes DNS Domain name to use |
| linkerdNamespace | string | `"linkerd"` | Namespace of the Linkerd core control-plane install |
| namespaceMetadata.image.name | string | `"extension-init"` | Docker image name for the namespace-metadata instance |
| namespaceMetadata.image.pullPolicy | string | `"IfNotPresent"` | Pull policy for the namespace-metadata instance |
| namespaceMetadata.image.registry | string | `"cr.l5d.io/linkerd"` | Docker registry for the namespace-metadata instance |
| namespaceMetadata.image.tag | string | `"v0.1.0"` | Docker image tag for the namespace-metadata instance |

----------------------------------------------
Autogenerated from chart metadata using [helm-docs v1.4.0](https://github.com/norwoodj/helm-docs/releases/v1.4.0)
21 changes: 20 additions & 1 deletion charts/linkerd-smi/templates/namespace-metadata-rbac.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,23 @@ subjects:
- kind: ServiceAccount
name: namespace-metadata
namespace: {{.Release.Namespace}}

---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
namespace: {{ .Values.linkerdNamespace }}
labels:
linkerd.io/extension: smi
annotations:
"helm.sh/hook": post-install
"helm.sh/hook-weight": "0"
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
name: smi-namespace-metadata-linkerd-config
roleRef:
kind: Role
name: ext-namespace-metadata-linkerd-config

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 4, 2024 •
edited
Loading

the ext-namespace-metadata-linkerd-config role only exists in linkerd's control plane namespace (typically linkerd), not in the smi namespace.

same goes for the linkerd-config config map itself:

$ kubectl logs -n linkerd-smi job/namespace-metadata
Found 7 pods, using pod/namespace-metadata-hrs5m
2024-02-04T15:31:31.347511Z  INFO linkerd_extension_init: patching namespace linkerd-smi
Error: ApiError: configmaps "linkerd-config" is forbidden: User "system:serviceaccount:linkerd-smi:namespace-metadata" cannot get resource "configmaps" in API group "" in the namespace "linkerd": Forbidden (ErrorResponse { status: "Failure", message: "configmaps \"linkerd-config\" is forbidden: User \"system:serviceaccount:linkerd-smi:namespace-metadata\" cannot get resource \"configmaps\" in API group \"\" in the namespace \"linkerd\"", reason: "Forbidden", code: 403 })

Caused by:
    configmaps "linkerd-config" is forbidden: User "system:serviceaccount:linkerd-smi:namespace-metadata" cannot get resource "configmaps" in API group "" in the namespace "linkerd": Forbidden

since the smi-adaptor is supposed to be deployed in a different namespace from linkerd's control plane (so that proxy injection can be done, which is disabled in linkerd's control plane namespace), relying on resources that can only be found in linkerd's control plane namespace is kinda broken. 😞

by the way, if the ext-namespace-metadata-linkerd-config role is supposed to be a generic mechanism for allowing linkerd components in other namespaces to read the linkerd-config config map, then this must be a cluster role, not a role. and instead of a role binding you must use a cluster role binding to bind to that cluster role.

i can make the namespace-metadata job work by adding the following rbac config:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: ext-namespace-metadata-linkerd-config
rules:
- apiGroups:
  - ""
  resourceNames:
  - linkerd-config
  resources:
  - configmaps
  verbs:
  - get

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: smi-namespace-metadata-linkerd-config
  labels:
    linkerd.io/extension: smi
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: ext-namespace-metadata-linkerd-config
subjects:
- kind: ServiceAccount
  name: namespace-metadata
  namespace: linkerd-smi

...but this definitely isn't great!

plus i don't understand why the namespace-metadata job needs access to the linkerd-config config map in the first place. all the job does is label the smi namespace.

since i'm creating the linkerd-smi namespace elsewhere with the correct labels and annotations, i don't need this job at all.

so might i suggest that you make this job optional with something along the lines of

namespaceMetadata:
  enabled: false

This comment has been minimized.

Sign in to view

Copy link
@alpeb

alpeb Feb 6, 2024

Author Member

relying on resources that can only be found in linkerd's control plane namespace is kinda broken. 😞

Note that this role is deployed in linkerd's namespace, and it's bound to the namespace-metadata ServiceAccount in the linkerd-smi namespace. How is this broken?

plus i don't understand why the namespace-metadata job needs access to the linkerd-config config map in the first place. all the job does is label the smi namespace.

It reads linkerd-config to know whether linkerd was installed with the option --linkerd-cni-enabled, in which case the namespace's pod-security.kubernetes.io/enforce annotation is set to restricted.

so might i suggest that you make this job optional with something along the lines of

Agreed, we should add that in.

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 6, 2024 •
edited
Loading

this role is deployed in linkerd's namespace

ah. i see. my mistake.

i was kustomizeing over the resources with

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: linkerd-smi
...

so the RoleBinding didn't end up in the linkerd namespace but in the linkerd-smi namespace. 🤦

ignore me. i wasn't paying attention. 🙂

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 6, 2024 •
edited
Loading

how would you feel about adding

namespace: {{ .Release.Namespace }}

to all the resources that are not put into the {{ .Values.linkerdNamespace }} namespace?

we use helm only to render out manifests and then kubectl apply them in a deploy pipeline. it's a fairly common pattern that i've seen used elsewhere.

and since we're deploying resources to many different namespaces we rely on either helm adding the namespace config or kustomize to fix that (if the helm chart omits it).

in this case, we can't use the above kustomization to set the namespace because that leads to everything ending up in the linkerd-smi namespace.

i mean, we can target individual resources with the kustomization but then we have to update it every time new resources come around.

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 6, 2024

ok. i went with this for now:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: linkerd-smi
resources:
- manifest.yaml
transformers:
- |-
  apiVersion: builtin
  kind: PatchTransformer
  metadata:
    name: fix-namespace
  patch: |-
    - op: replace
      path: /metadata/namespace
      value: linkerd
  target:
    group: rbac.authorization.k8s.io
    kind: RoleBinding
    name: smi-namespace-metadata-linkerd-config

☝️ that spits out manifests with the correct namespaces so we can just kubectl apply them.

This comment has been minimized.

Sign in to view

Copy link
@alpeb

alpeb Feb 6, 2024

Author Member

I see... yes we'd be open to that change 👍

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 6, 2024

thanks for your help with this.

keep up the great work!!! i'm a big fan of linkerd!

This comment has been minimized.

Sign in to view

Copy link
@sdickhoven

sdickhoven Feb 7, 2024

by the way, i noticed this:

$ kubectl label ns linkerd-smi --overwrite pod-security.kubernetes.io/enforce=restricted
Warning: existing pods in namespace "linkerd-smi" violate the new PodSecurity enforce level "restricted:latest"
Warning: namespace-metadata-6d4kz (and 1 other pod): allowPrivilegeEscalation != false, unrestricted capabilities, runAsNonRoot != true, seccompProfile

i.e. the Job that labels the namespace with

pod-security.kubernetes.io/enforce: restricted

(assuming that cni is enabled) makes it so that nothing works anymore. 😞

https://github.com/linkerd/linkerd-extension-init/blob/release/v0.1.0/src/main.rs#L156-L165

should be easy to fix by adding the appropriate securityContext here:

https://github.com/linkerd/linkerd-smi/blob/v0.2.6/charts/linkerd-smi/templates/namespace-metadata.yaml#L13
https://github.com/linkerd/linkerd-smi/blob/v0.2.6/charts/linkerd-smi/templates/adaptor.yaml#L23

i'm using baseline pss for now... 🙂

This comment has been minimized.

Sign in to view

Copy link
@alpeb

alpeb Feb 8, 2024

Author Member

Good catch! I've just pushed #159 to address that before releasing a new version 😅
apiGroup: rbac.authorization.k8s.io
subjects:
- kind: ServiceAccount
name: namespace-metadata
namespace: {{.Release.Namespace}}
25 changes: 9 additions & 16 deletions charts/linkerd-smi/templates/namespace-metadata.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ kind: Job
metadata:
annotations:
"helm.sh/hook": post-install
"helm.sh/hook-weight": "0"
"helm.sh/hook-weight": "1"
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
labels:
app.kubernetes.io/name: namespace-metadata
Expand All @@ -22,19 +22,12 @@ spec:
serviceAccountName: namespace-metadata
containers:
- name: namespace-metadata
image: curlimages/curl:7.78.0
command: ["/bin/sh"]
image: {{.Values.namespaceMetadata.image.registry}}/{{.Values.namespaceMetadata.image.name}}:{{.Values.namespaceMetadata.image.tag}}
imagePullPolicy: {{.Values.namespaceMetadata.image.pullPolicy }}
args:
- -c
- |
ops=''
token=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token)
ns=$(curl -kfv -H "Authorization: Bearer $token" \
"https://kubernetes.default.svc/api/v1/namespaces/{{.Release.Namespace}}")
if echo "$ns" | grep -vq 'labels'; then
ops="$ops{\"op\": \"add\",\"path\": \"/metadata/labels\",\"value\": {}},"
fi
ops="$ops{\"op\": \"add\", \"path\": \"/metadata/labels/linkerd.io~1extension\", \"value\": \"smi\"}"
curl -kfv -XPATCH -H "Content-Type: application/json-patch+json" -H "Authorization: Bearer $token" \
-d "[$ops]" \
"https://kubernetes.default.svc/api/v1/namespaces/{{.Release.Namespace}}?fieldManager=kubectl-label"
- --extension
- smi
- --namespace
- {{.Release.Namespace}}
- --linkerd-namespace
- {{.Values.linkerdNamespace}}
14 changes: 14 additions & 0 deletions charts/linkerd-smi/values.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# -- Kubernetes DNS Domain name to use
clusterDomain: cluster.local

# -- Namespace of the Linkerd core control-plane install
linkerdNamespace: linkerd

# SMI Adaptor configuration
adaptor:
image:
Expand All @@ -19,3 +22,14 @@ adaptor:
nodeSelector: {}
# -- Tolerations for the adaptor instance
tolerations: []

namespaceMetadata:
image:
# -- Docker registry for the namespace-metadata instance
registry: cr.l5d.io/linkerd
# -- Docker image name for the namespace-metadata instance
name: extension-init
# -- Docker image tag for the namespace-metadata instance
tag: v0.1.0
# -- Pull policy for the namespace-metadata instance
pullPolicy: IfNotPresent

0 comments on commit 364ca6e

Please sign in to comment.