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

Mlo 133 create admission controller #906

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ pip install -r requirements-test.txt
pytest -vv tests
```

NOTE:
if you are running tests on a macOS,
you should install the coreutils, so tests which are using the `du` cmd will work properly:

```shell
brew install coreutils
```

### Running integration tests locally
Make sure you have docker installed.
To build docker image where test run, you should set the following ENV variables:
Expand Down
18 changes: 18 additions & 0 deletions charts/platform-storage/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@ release: {{ .Release.Name | quote }}
value: {{ .Values.permissionForgettingInterval | quote }}
- name: NP_STORAGE_API_KEEP_ALIVE_TIMEOUT
value: {{ .Values.keepAliveTimeout | quote }}
- name: NP_STORAGE_API_K8S_API_URL
value: https://kubernetes.default:443
- name: NP_STORAGE_API_K8S_AUTH_TYPE
value: token
- name: NP_STORAGE_API_K8S_CA_PATH
value: {{ include "platformStorage.kubeAuthMountRoot" . }}/ca.crt
- name: NP_STORAGE_API_K8S_TOKEN_PATH
value: {{ include "platformStorage.kubeAuthMountRoot" . }}/token
- name: NP_STORAGE_API_K8S_NS
value: {{ .Values.kube.namespace | default "default" | quote }}
- name: NP_STORAGE_ADMISSION_CONTROLLER_TLS_KEY
value: {{ .Values.admissionController.tlsKey}}
- name: NP_STORAGE_ADMISSION_CONTROLLER_TLS_CERT
value: {{ .Values.admissionController.tlsCert}}
{{ include "platformStorage.env.s3" . }}
{{- if .Values.sentry }}
- name: SENTRY_DSN
Expand Down Expand Up @@ -126,3 +140,7 @@ app: {{ include "platformStorage.name" . }}
release: {{ .Release.Name }}
service: platform-storage-metrics
{{- end -}}

{{- define "platformStorage.kubeAuthMountRoot" -}}
{{- printf "/var/run/secrets/kubernetes.io/serviceaccount" -}}
{{- end -}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ .Values.admissionController.app_name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: {{ .Values.admissionController.app_name }}
name: {{ include "platformStorage.fullname" . }}-injector

let's use this name for new resources, you can define a template for it

namespace: platform
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not define namespace, helm by default installs resources in the namespace where helm release is created

labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
service: {{ .Values.admissionController.app_name }}
spec:
replicas: 1
selector:
matchLabels:
app: {{ .Values.admissionController.app_name }}
template:
metadata:
labels:
app: {{ .Values.admissionController.app_name }}
spec:
serviceAccountName: {{ .Values.admissionController.app_name }}
containers:
- name: admission-controller
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
command:
- platform-storage-admission-controller
ports:
- containerPort: {{ .Values.admissionController.service.port }}
name: http
protocol: TCP
env:
- name: SERVER_PORT
value: {{ .Values.admissionController.service.port | quote }}
{{- include "platformStorage.env" . | nindent 10 }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
69 changes: 69 additions & 0 deletions charts/platform-storage/templates/admission-controller-rbac.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Values.admissionController.app_name }}
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}

---

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ .Values.admissionController.app_name }}
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get", "list", "watch"]

---

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ .Values.admissionController.app_name }}
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
subjects:
- kind: ServiceAccount
name: {{ .Values.admissionController.app_name }}
namespace: platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
namespace: platform
namespace: {{ .Release.Namespace }}

roleRef:
kind: Role
apiGroup: rbac.authorization.k8s.io
name: {{ .Values.admissionController.app_name }}

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ .Values.admissionController.app_name}}
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
rules:
- apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["persistentvolumes"]
verbs: ["get", "list", "watch"]

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ .Values.admissionController.app_name}}
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
subjects:
- kind: ServiceAccount
name: {{ .Values.admissionController.app_name}}
namespace: platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
namespace: platform
namespace: {{ .Release.Namespace }}

roleRef:
kind: ClusterRole
name: {{ .Values.admissionController.app_name}}
apiGroup: rbac.authorization.k8s.io
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: v1
kind: Service
metadata:
name: {{ .Values.admissionController.app_name }}-svc
namespace: platform
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove namespace

labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
spec:
selector:
app: {{ .Values.admissionController.app_name }}
ports:
- name: https
port: 443
targetPort: 8080
protocol: TCP
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: platform-storage-mutating-webhook
annotations:
"helm.sh/hook": post-install,post-upgrade
labels:
{{- include "platformStorage.labels.standard" . | nindent 4 }}
webhooks:
- name: pod-volume-injector.apolo.us
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
clientConfig:
caBundle: {{ .Values.admissionController.caBundle }}
service:
namespace: platform
name: {{ .Values.admissionController.app_name }}-svc
path: /admission-controller/mutate
rules:
- operations: ["CREATE"]
apiGroups: ["*"]
apiVersions: ["*"]
resources: ["pods"]
failurePolicy: Fail
objectSelector:
matchLabels:
platform.apolo.us/storage-injection-webhook: "enabled"
Copy link
Contributor

@zubenkoivan zubenkoivan Feb 19, 2025

Choose a reason for hiding this comment

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

Suggested change
platform.apolo.us/storage-injection-webhook: "enabled"
platform.apolo.us/inject-storage: "true"

wdyt?

1 change: 1 addition & 0 deletions charts/platform-storage/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ spec:
app: {{ include "platformStorage.name" . }}
release: {{ .Release.Name }}
service: platform-storage
platform.apolo.us/storage-injection-webhook: "enabled"
{{- if .Values.secrets }}
annotations:
checksum/secret: {{ include (print $.Template.BasePath "/secrets.yaml") . | sha256sum }}
Expand Down
24 changes: 24 additions & 0 deletions charts/platform-storage/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ service:

storages: []

kube:
namespace: "default"

sentry:
appName: platform-storage
sampleRate: 0.002
Expand Down Expand Up @@ -82,3 +85,24 @@ metrics:

serviceMonitor:
enabled: true


admissionController:
app_name: "storage-admission-controller"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove it and use generated name

tlsKey:
valueFrom:
secretKeyRef:
name: platform-storage-ac-certs
key: tls.key
tlsCert:
valueFrom:
secretKeyRef:
name: platform-storage-ac-certs
key: tls.crt
caBundle:
valueFrom:
secretKeyRef:
name: platform-storage-ac-certs
key: ca.bundle
service:
port: 8080
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ select = [
ignore = [
"A003", # Class attribute "..." is shadowing a Python builtin
"N818",
"PT005"
]

[tool.ruff.lint.isort]
Expand Down
8 changes: 7 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ install_requires =
aiodns==3.2.0
aiofiles==24.1.0
aiohttp==3.10.10
aiorwlock==1.5.0
apolo-kube-client==25.2.6
cbor==1.0.0
cchardet==2.1.7
cchardet==2.2.0a2
fastapi==0.115.8
neuro-admin-client==25.1.1
neuro-auth-client==22.6.1
Expand All @@ -43,6 +45,7 @@ console_scripts =
platform-storage-api = platform_storage_api.api:main
platform-storage-metrics = platform_storage_api.metrics:main
platform-storage-worker = platform_storage_api.worker:main
platform-storage-admission-controller = platform_storage_api.admission_controller.__main__:main

[options.extras_require]
dev =
Expand Down Expand Up @@ -122,3 +125,6 @@ ignore_missing_imports = true

[mypy-botocore.*]
ignore_missing_imports = true

[mypy-apolo_kube_client.*]
ignore_missing_imports = true
Empty file.
69 changes: 69 additions & 0 deletions src/platform_storage_api/admission_controller/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import asyncio
import logging
import os
import ssl
import tempfile
from base64 import b64decode
from typing import cast

from aiohttp import web
from neuro_logging import init_logging, setup_sentry

from platform_storage_api.admission_controller.app import create_app
from platform_storage_api.config import AdmissionControllerTlsConfig, Config


logger = logging.getLogger(__name__)


def main() -> None:
init_logging()
config = Config.from_environ()
logging.info("Loaded config: %r", config)

setup_sentry(
health_check_url_path="/api/v1/ping",
ignore_errors=[web.HTTPNotFound],
)

loop = asyncio.get_event_loop()
app = loop.run_until_complete(create_app(config))

context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)

crt_file = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix='.crt')
Copy link
Contributor

@zubenkoivan zubenkoivan Feb 19, 2025

Choose a reason for hiding this comment

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

You can use it also as a context manager instead and pass delete=True, delete_on_close=False

key_file = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix='.key')

tls_config = cast(
AdmissionControllerTlsConfig,
config.admission_controller_tls_config
)
try:
# extract certificates from the env and store in a temp files
crt_file.write(b64decode(tls_config.tls_cert).decode())
key_file.write(b64decode(tls_config.tls_key).decode())
crt_file.close()
key_file.close()

context.load_cert_chain(
certfile=crt_file.name,
keyfile=key_file.name,
)

web.run_app(
app,
host=config.server.host,
port=config.server.port,
ssl_context=context,
)

except Exception as e:
logger.exception("Unhandled error")
raise e
finally:
os.unlink(crt_file.name)
os.unlink(key_file.name)


if __name__ == "__main__":
main()
Loading