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.

Sign up for GitHub

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

feat: add wiz manifests #8679

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from
Open

feat: add wiz manifests #8679

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ec72710
feat: add wiz manifests
Dec 19, 2024
de2a398
feat: update permissions for sensor
Dec 19, 2024
a5b89dc
feat: add wiz manifests
Dec 19, 2024
c307aaf
feat: add wiz manifests
Dec 19, 2024
0cdad7a
Update secrets namespace + cleanup trailing curly brackets
zaklawrencea Dec 19, 2024
500acb7
fix: minor changes
Dec 20, 2024
38944d1
Merge branch 'vthupili' of https://github.com/zalando-incubator/kuber…
Dec 20, 2024
4fda53d
fix: minor changes
Dec 20, 2024
a65b322
enable wiz to exclude from node ready event
Dec 20, 2024
eb25c7f
Fix typos
katyanna Dec 20, 2024
ebb156a
Fix missing }
katyanna Dec 20, 2024
3630e94
Update cluster/manifests/deletions.yaml
demonCoder95 Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cluster/config-defaults.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1194,3 +1194,8 @@ role_sync_controller_enabled: "true"
{{ else }}
role_sync_controller_enabled: "false"
{{ end }}

#Wiz Configs
wiz_enable_runtime_monitoring_daemonset: "false"
wiz_adapter_cpu: "300m"
wiz_adapter_memory: "300Mi"
56 changes: 56 additions & 0 deletions cluster/manifests/deletions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,59 @@ post_apply:
- name: kube-janitor
kind: ClusterRoleBinding
{{- end }}
{{- if eq .Cluster.ConfigItems.wiz_enable_runtime_monitoring_daemonset "true"}
katyanna marked this conversation as resolved.
Show resolved Hide resolved
- name: wiz-sensor
kind: ServiceAccount
namespace: wiz
- name: wiz-sensor-apikey
kind: Secret
namespace: wiz
- name: wiz-sensor-imagepullkey
kind : Secret
namespace: wiz
- name: wiz-sensor
kind : DaemonSet
namespace: wiz
- name: wiz-sensor
kind : ClusterRole
namespace: wiz
- name: wiz-sensor
kind : ClusterRoleBinding
namespace: wiz
- name: wiz-broker
kind : ServiceAccount
namespace: wiz
- name: wiz-cluster-reader
kind : ServiceAccount
namespace: wiz
- name: wiz-auto-modify-connector
kind : ServiceAccount
namespace: wiz
- name: wiz-connector-connector
kind : Secret
namespace: wiz
- name: wiz-cluster-reader-token
kind : Secret
namespace: wiz
- name: wiz-api-token
kind : Secret
namespace: wiz
- name: wiz-auto-modify-connector
kind : Role
namespace: wiz
- name: wiz-auto-modify-connector
kind : RoleBinding
namespace: wiz
- name: wiz-kubernetes-connector-create-connector
kind : Job
namespace: wiz
- name: wiz-kubernetes-connector-delete-connector
kind : Job
namespace: wiz
- name: wiz-connector-agent
kind : Deployment
namespace: wiz
- name: wiz-cluster-reader
kind : ClusterRoleBinding
namespace: wiz
{{- end }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Missing empty line at the end 😅

39 changes: 39 additions & 0 deletions cluster/manifests/wiz/connector/clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{{ if eq .Cluster.ConfigItems.wiz_enable_runtime_monitoring_daemonset "true"}}
# ---
# # We are using ClusterRole readonly created by default in the cluster instead of creating new one provided by wiz
# # Source: wiz-kubernetes-integration/charts/wiz-kubernetes-connector/templates/service-account-cluster-reader.yaml
# apiVersion: rbac.authorization.k8s.io/v1
# kind: ClusterRole
# metadata:
# name: wiz-cluster-reader
# labels:
# helm.sh/chart: wiz-kubernetes-connector-3.1.1
# app.kubernetes.io/name: wiz-kubernetes-connector
# app.kubernetes.io/instance: wiz-connector
# app.kubernetes.io/version: "2.5"
# app.kubernetes.io/managed-by: Helm
# rules:
# - apiGroups: ["*"]
# resources: ["*"]
# verbs: ["get", "list", "watch"]
Comment on lines +5 to +18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep this manifest here? I think the comment is sufficient to communicate that we don't need a global reader role 🤔

---
# Source: wiz-kubernetes-integration/charts/wiz-kubernetes-connector/templates/service-account-cluster-reader.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: wiz-cluster-reader
labels:
helm.sh/chart: wiz-kubernetes-connector-3.1.1
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
Comment on lines +25 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these labels important for the Wiz deployment? I think we might need to remove the ones related to helm, also we need to provide the Zalando environment labels like application: foo and component: bar.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a similar concern was raised by Martin in another manifest, so let's fix the labels for all manifests 🙂

  1. Drop the helm related labels.
  2. Add application and component labels.

roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: readonly # readonly role created by default in out kubernetes environment
subjects:
- kind: ServiceAccount
name: wiz-cluster-reader
namespace: "wiz"
{{end}}
81 changes: 81 additions & 0 deletions cluster/manifests/wiz/connector/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
{{ if eq .Cluster.ConfigItems.wiz_enable_runtime_monitoring_daemonset "true"}}
---
# Source: wiz-kubernetes-integration/charts/wiz-kubernetes-connector/charts/wiz-broker/templates/wiz-broker-deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: wiz-connector-agent
namespace: "wiz"
labels:
helm.sh/chart: wiz-broker-2.1.0
app.kubernetes.io/name: wiz-broker
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
vinaythupili marked this conversation as resolved.
Show resolved Hide resolved
application: "wiz"
component: "connector"
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/name: wiz-broker
app.kubernetes.io/instance: wiz-connector
template:
metadata:
annotations:
rollme: "Cd4Gg"
Comment on lines +25 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this annotation? Is it to control rolling updates? We already have the CLM perform updates upon manifest changes made in the repository.

labels:
helm.sh/chart: wiz-broker-2.1.0
app.kubernetes.io/name: wiz-broker
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
spec:
serviceAccountName: wiz-broker
securityContext:
runAsNonRoot: true
runAsUser: 1000
volumes:
- name: connector-data
secret:
secretName: wiz-connector-connector
items:
- key: connectorData
path: data
containers:
- name: wiz-broker
securityContext:
allowPrivilegeEscalation: false
runAsNonRoot: true
runAsUser: 1000
image: "wiziopublic.azurecr.io/wiz-app/wiz-broker:2.5"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our environment all images need to come from container-registry.zalando.net.

Copy link
Member

@demonCoder95 demonCoder95 Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to create an internal repository that "republishes" this public image to our internal ECR registry and then use the link as Martin suggests here. I communicated in our internal chat thread.

imagePullPolicy: Always
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imagePullPolicy should be IfNotPresent.

Images pulled are always cached by the runtime and reusing the cached image also results in faster pod startup times. This is only set to Always in cases where you expect frequent image updates, which is not the case for our infrastructure.

volumeMounts:
- name: connector-data
mountPath: /etc/connectorData
readOnly: true
args: [
/etc/connectorData/data
]
Comment on lines +57 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a lot cleaner to specify a list with - instead of square brackets like this, like

args:
  - /etc/connectorData/data

We use the same format in all of the repository, e-g see: https://github.com/zalando-incubator/kubernetes-on-aws/blob/dev/cluster/manifests/role-sync-controller/cronjob.yaml#L31

env:
- name: LOG_LEVEL
value: info
- name: WIZ_ENV
value:
Comment on lines +63 to +64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to create an empty environment variable?

- name: WIZ_CLIENT_ID
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientId
- name: WIZ_CLIENT_TOKEN
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientToken
- name: TARGET_IP
value: kubernetes.default.svc.cluster.local
- name: TARGET_PORT
value: "443"
resources:
null
{{end}}
171 changes: 171 additions & 0 deletions cluster/manifests/wiz/connector/job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
{{ if eq .Cluster.ConfigItems.wiz_enable_runtime_monitoring_daemonset "true"}}
---
# Source: wiz-kubernetes-integration/charts/wiz-kubernetes-connector/templates/job-create-connector.yaml
apiVersion: batch/v1
kind: Job
metadata:
name: wiz-kubernetes-connector-create-connector
namespace: "wiz"
labels:
helm.sh/chart: wiz-kubernetes-connector-3.1.1
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
application: "wiz"
component: "connector"
annotations:
"helm.sh/hook": pre-install,pre-upgrade
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
rollme.wizApiTokenHash: ce8124bc1b0fbc0cb5cd47338ca0c7d5f5446d79936e443a201d96b192a7bd65
rollme.proxyHash: 9aa53d69075371b3fa23ebeea2fd2416ea81fb533499d071ca2d576f17c7c886
rollme.brokerHash: 115ba85431eeaf8db3ff2173aee02d16e67df1555d5e1ef74cfa7ac0d812cab2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values look like they are computed based on the configuration value in order to trigger an update when they change. Having these values static here doesn't provide any value.

We do have similar functionality which can be used if it's really needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented the same on the rollme annotation on the connector deployment manifest above. It appears this is used to keep track of changes in the deployment manifests, not in the configuration. So, we probably don't need this whole setup anyway. Maybe it's a helm thing? CLM does this job for us.


spec:
ttlSecondsAfterFinished: 60
manualSelector: true
selector:
matchLabels:
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
Comment on lines +29 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably need to change these labels to application: wiz and component: connector everywhere. This is how we manage applications and components in our infrastructure.

backoffLimit: 1
template:
metadata:
labels:

helm.sh/chart: wiz-kubernetes-connector-3.1.1
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
spec:
serviceAccountName: wiz-auto-modify-connector
restartPolicy: "Never"
securityContext:
runAsNonRoot: true
runAsUser: 1000
containers:
- name: wiz-connector-creator
securityContext:
allowPrivilegeEscalation: false
runAsNonRoot: true
runAsUser: 1000
image: "wiziopublic.azurecr.io/wiz-app/wiz-broker:2.5"
imagePullPolicy: Always
Comment on lines +53 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments here to update the image URL and imagePullPolicy.

command:
- "wiz-broker"
args:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove empty line

- create-kubernetes-connector
- --api-server-endpoint
- "https://kubernetes.default.svc.cluster.local"
- --secrets-namespace
- "wiz"
- --service-account-token-secret-name
- "wiz-cluster-reader-token"
- --output-secret-name
- "wiz-connector-connector"
- --is-on-prem=true
- --service-type
- "Kubernetes"
- --wait=true
env:
- name: LOG_LEVEL
value: info
- name: WIZ_CLIENT_ID
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientId
optional: false
- name: WIZ_CLIENT_TOKEN
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientToken
optional: false
- name: WIZ_ENV
value:
resources:
null
---
# Source: wiz-kubernetes-integration/charts/wiz-kubernetes-connector/templates/job-delete-connector.yaml
apiVersion: batch/v1
kind: Job
metadata:
name: wiz-kubernetes-connector-delete-connector
namespace: "wiz"
labels:
helm.sh/chart: wiz-kubernetes-connector-3.1.1
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
application: "wiz"
component: "connector"
annotations:
"helm.sh/hook": pre-delete
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
rollme.proxyHash: 9aa53d69075371b3fa23ebeea2fd2416ea81fb533499d071ca2d576f17c7c886
rollme.brokerHash: 115ba85431eeaf8db3ff2173aee02d16e67df1555d5e1ef74cfa7ac0d812cab2

spec:
ttlSecondsAfterFinished: 60
manualSelector: true
selector:
matchLabels:
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
backoffLimit: 1
template:
metadata:
labels:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove empty line.

helm.sh/chart: wiz-kubernetes-connector-3.1.1
app.kubernetes.io/name: wiz-kubernetes-connector
app.kubernetes.io/instance: wiz-connector
app.kubernetes.io/version: "2.5"
app.kubernetes.io/managed-by: Helm
spec:
serviceAccountName: wiz-auto-modify-connector
restartPolicy: "Never"
securityContext:
runAsNonRoot: true
runAsUser: 1000
containers:
- name: wiz-connector-delete
securityContext:
allowPrivilegeEscalation: false
runAsNonRoot: true
runAsUser: 1000
image: "wiziopublic.azurecr.io/wiz-app/wiz-broker:2.5"
imagePullPolicy: Always
Comment on lines +141 to +142
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same suggestion here to update this

command: ["/bin/sh", "-c"]
args:
- >
wiz-broker delete-kubernetes-connector
--input-secrets-namespace
"default"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this namespaces should be wiz and not default.

I'm looking at the secrets.yaml file below and it's deploying the secret in the wiz namespace.

--input-secret-name
"wiz-connector-connector"
|| true
env:
- name: LOG_LEVEL
value: info
- name: WIZ_CLIENT_ID
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientId
optional: false
- name: WIZ_CLIENT_TOKEN
valueFrom:
secretKeyRef:
name: wiz-api-token
key: clientToken
optional: false
- name: WIZ_ENV
value: ""
Comment on lines +167 to +168
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need an empty env var?

resources:
null
{{end}}
Loading