allegroai / clearml-server-k8s

ClearML Server for Kubernetes Clusters
Other
21 stars 26 forks source link

Support for apiserver.conf via Helm #1

Open Shaked opened 4 years ago

Shaked commented 4 years ago

Hello,

I have installed trains-server with Helm.

I was wondering, what would be the best practice to set /opt/trains/config/apiserver.conf?

Ideally, in most of my apps I'd just mount it as a volume either as a ConfigMap or if from cloud then I'd use Azure KeyVault (or any of the equivalent).

I'm not sure how to do it with trains-server though. Any suggestions?

Thank you, Shaked

bmartinn commented 4 years ago

@Shaked can I assume you mean from security perspective, i.e. fixed user/pass list in the apiserver.conf ?

If this is the case, I would suggest to have the entire section stored in Azure KeyVault (json encode it, or with any other string serialization function).

Then when you spin the machine just add it to the base apiserver.conf. Notice that the apiserver.conf is a HOCON standard config file, meaning you can safely append sections at the end of the file, it will not break the parser

Shaked commented 4 years ago

@bmartinn

@Shaked can I assume you mean from security perspective, i.e. fixed user/pass list in the apiserver.conf ?

Yes exactly!

If this is the case, I would suggest to have the entire section stored in Azure KeyVault (json encode it, or with any other string serialization function).

Then when you spin the machine just add it to the base apiserver.conf. Notice that the apiserver.conf is a HOCON standard config file, meaning you can safely append sections at the end of the file, it will not break the parser

I usually use Azure KeyVault by using FlexVolume. So ideally I'd just save the entire content of apiserver.conf and add a FlexVolume. Is there another way to do that? How would you load from Azure KeyVault and append it to the apiserver.conf using Helm?

(EDIT: if I choose to do it with FlexVolume, then I'd have to change the helm chart, which I rather not to)

Thank you for helping! Shaked

bmartinn commented 4 years ago

I have to admit, I was not aware of FlexVolume. What I had in mind was writing a setup script accessing the Azure KeyVault with REST API, which is a bit of work... :)

After a brief review of FlexVolume, I have to say, it looks like the easiest way to go about it.

That said, I can't think of a way to do that without changing the helm chart... I guess you'll have to fork the repository...

Do you see other options?

Shaked commented 4 years ago

@bmartinn

Yea it is the easiest I have found but I thought maybe you know another way I wasn't familiar with :)

I don't mind forking it and submitting a PR.

What I usually do is setting my volumes through values.yaml, meaning that the deployment.yaml template contains:

....
       containers:
          {{- with .Values.volumeMounts }}
          volumeMounts:
            {{- toYaml . | nindent 12 }}
          {{- end }}
      {{- with .Values.volumes }}
      volumes:
        {{- toYaml . | nindent 8 }}
      {{- end }}
...

Then values.yaml would contain something like:

volumeMounts:
      - name: secret-vol
        mountPath: /app/secrets
        readOnly: true

      - name: shared-files
        mountPath: /var/www/shared
volumes:
  - name: nginx-config-volume
    configMap:
      name: nginx-config
      items:
      - key: config
        path: site.conf

  # Create the shared files volume to be used in both pods
  - name: shared-files
    emptyDir: {}

  - name: secret-vol
    flexVolume:
      driver: "azure/kv"
      secretRef:
        name: credentialkey
      options:
        usepodidentity: "false"
        resourcegroup: "resourcegroup"
        keyvaultname: "vaultsname"
        keyvaultobjectnames: "config-in-azkeyvault;other-in-keyvault;yet-in-keyvault"
        keyvaultobjectaliases: "config.json;other.conf;yet.yaml"
        keyvaultobjecttypes: "secret;secret;secret"
        subscriptionid: "SUBID"
        tenantid: "TENID"

Then config.json, other.conf and yet.yaml will be mounted directly to /app/secrets while those who don't need the FlexVolume, can just create their own values.yaml and mount a ConfigMap instead.

What do you think?

bmartinn commented 4 years ago

Yes! please submit a PR :smile:

I think that we need to set an OS environment to tell the trains-apiserver to look for an additional folder with the apiserver.conf file, something like TRAINS_CONFIG_DIR=/app/secrets (I'll look into it later and update here)

EDIT: we should set TRAINS_CONFIG_DIR=/app/secrets and this will cause the trains-apiserver to load the configuration file /app/secrets/apiserver.conf

So my thinking is that I would like to control whether FlexVolume is used, with an external variable outside of the helm values.yaml / deployment.yaml

This way, I'll end up with something like: helm install allegroai/trains-server-chart --namespace=trains --name trains-server --set use_secrets_flexvolume=1

How does that sound?

Shaked commented 4 years ago

I like the idea of controlling it via cli params, so that there won't be a need to create a new values.yaml file. However, the only thing with FlexVolume is that you have to set your own subscriptionId, tenantId, resourceGroup, keyvaultname and the secretRef. So, it would have to be more like:

helm install allegroai/trains-server-chart --namespace=trains --name trains-server --set use_secrets_flexvolume=1 --set flexvolume_subscriptionid=.... --set flexvolume_tenantid=...

bmartinn commented 4 years ago

Sounds about right to me :1st_place_medal: That's awesome ! Can't wait for the PR :)

p.s. We are not strict with PRs, any contribution is appreciated, and if things get too complicated we will just generalize post-merge.

Shaked commented 4 years ago

@bmartinn I'm on it!

So currently I have this:

helm template ./trains-server-chart --values ./trains-server-chart/values.yaml --set use_secrets_flexvolume=1 --set fv.secretRef=creds --set fv.usepodidentity=false --set fv.resourcegroup=stage-general --set fv.keyvaultname=test --set fv.keyvaultobjectnames="apiserver_conf;main_yaml" --set fv.keyvaultobjectaliases="apiserver.conf;main.yaml" --set fv.keyvaultobjecttypes="secret;secret" --set fv.subscriptionid=123 --set fv.tenantid=456

will result with:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app.kubernetes.io/name: apiserver
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/part-of: trains-server
    app.kubernetes.io/managed-by: Tiller
    app.kubernetes.io/version: 0.12.1-182
    helm.sh/chart: trains-server-chart-0.12.1_1
  name: apiserver
  namespace: trains
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: apiserver
      app.kubernetes.io/instance: release-name
      app.kubernetes.io/part-of: trains-server
      app.kubernetes.io/managed-by: Tiller
  minReadySeconds: 20
  progressDeadlineSeconds: 30
  strategy:
    type: Recreate
  template:
    metadata:
      labels:
        app.kubernetes.io/name: apiserver
        app.kubernetes.io/instance: release-name
        app.kubernetes.io/part-of: trains-server
        app.kubernetes.io/managed-by: Tiller
        app.kubernetes.io/version: 0.12.1-182
        helm.sh/chart: trains-server-chart-0.12.1_1
    spec:
      affinity:
        nodeAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            nodeSelectorTerms:
              - matchExpressions:
                  - key: app
                    operator: In
                    values:
                    - trains
      containers:
      - image: allegroai/trains-server:0.12.1-182
        name: apiserver
        resources:
          requests:
            memory: "50M"
            cpu: "200m"
          limits:
            memory: "100M"
            cpu: "400m"
        env:
        - name: ELASTIC_SERVICE_HOST
          value: elasticsearch-service
        - name: MONGODB_SERVICE_HOST
          value: mongo-service
        - name: REDIS_SERVICE_HOST
          value: redis

        - name: TRAINS_CONFIG_DIR
          value: /opt/trains/secrets

        args:
        - apiserver
        volumeMounts:

            - mountPath: /opt/trains/secrets
              name: apiserver-azurekeyvault

            - mountPath: /var/log/trains
              name: apiserver-hostpath0
            - mountPath: /opt/trains/config
              name: apiserver-hostpath1

      restartPolicy: Always
      nodeSelector:
        app: trains
      volumes:
        - hostPath:
            path: /opt/trains/logs
          name: apiserver-hostpath0
        - hostPath:
            path: /opt/trains/config
          name: apiserver-hostpath1

        - name: apiserver-azurekeyvault
          flexVolume:
          driver: "azure/kv"
          secretRef:
              name: creds
          options:
              usepodidentity: "false"
              resourcegroup: "stage-general"
              keyvaultname: "test"
              keyvaultobjectnames: "apiserver_conf;main_yaml"
              keyvaultobjectaliases: "apiserver.conf;main.yaml"
              keyvaultobjecttypes: "secret;secret"
              subscriptionid: "123"
              tenantid: "456"

One thing I'm not sure about is why even changing /opt/trains/config to /opt/trains/secrets? We could just set:

- name: TRAINS_CONFIG_DIR
   value: /opt/trains/config

And then in the volumeMounts

        {{ if .Values.use_secrets_flexvolume }}
            - mountPath: /opt/trains/secrets
              name: apiserver-azurekeyvault
       {{ else }}
            - mountPath: /opt/trains/config
               name: apiserver-hostpath1
        {{ end }}

Then you either load your configuration the same as you do today, or everything comes from Azure KeyVault.

Of course we can have both running together i.e /opt/trains/config AND /opt/trains/secrets but it means that it won't be enough to have only one ENV var and the app will have to check for another ENV var i.e TRAINS_CONFIG_DIR will always exist and TRAINS_SECRETS_DIR might be empty.

If you ask me, I'm in favour of the above condition (either you load config from Azure KeyVault or you mount it same as today).

What do you think?

bmartinn commented 4 years ago

1) Looks great! 2) Actually the server already supports it :) TRAINS_CONFIG_DIR=/opt/trains/config:/opt/trains/secrets Will first read configuration file from /opt/trains/config then merge /opt/trains/secrets to them. Meaning you can have base configuration in one place and then extend them with the Azure KeyVault, only holding the login credentials section 3) We could have default: TRAINS_CONFIG_DIR=/opt/trains/config FlexVolume: TRAINS_CONFIG_DIR=/opt/trains/config:/opt/trains/secrets It might be easier for people to migrate to FlexVolume this way... But if you choose to opt for one folder, I'm okay with it as well.

Thoughts?

Shaked commented 4 years ago

@bmartinn

Sounds great.

I have pushed my changes here. Is it enough for a PR?

Thanks! Shaked

bmartinn commented 4 years ago

Thanks @Shaked ! It all looks good to me :) btw: Why did you move the Volume mounts to the "values.yaml" ? convenience / design / readability? (I do not have an opinion on the matter, I'm just curious)

Shaked commented 4 years ago

@bmartinn

Great!

I was thinking that it would be easier to differentiate between the template and the values.

Ideally I would have put FlexVolume there as well, but it over complicates things.

bmartinn commented 4 years ago

Makes sense. I'll make sure we test the PR as soon as we can :)