Client pods being deployed irrespective of values

[CptQuint]

The chart values file sets certain client values by default meaning the client deployment is always deployed irrespective of the values set by the operator.

[RazcoDev]

We need to understand what should be the default here.
We want to provide values file that will deploy OPAL fully and functioning, although it can be confusing when deploying only the server or the client, like happens to you.

In the meanwhile, I guess that you solved it by override the default values, right ?

[theeheng]

Hi @RazcoDev, I have tested trying to deploy with setting the client: null and it throws couple of errors on the following file:

• opal-helm-chart\templates\tests\e2e.yaml
• opal-helm-chart\templates\cm.yaml

Basically, both files just need to add a condition check {{- if .Values.client }} and it solve the issue:

• opal-helm-chart\templates\tests\e2e.yaml

{{- if .Values.client }}
{{- $url := printf "http://%v:%v/v1/data" (include "opal.clientName" .) .Values.client.opaPort -}}
apiVersion: v1
kind: Pod
metadata:
  name: opal-e2e
  annotations:
    "helm.sh/hook": test
    "helm.sh/hook-delete-policy": before-hook-creation
spec:
  restartPolicy: Never
  containers:
  - name: e2e
    image: "apteno/alpine-jq:2021-04-25"
    command:
    - '/bin/sh'
    - '-c'
    - |
      set -x
      set -e

      sleep 15

      [ "$(curl -s {{ $url }} | jq '.result.users | keys | length')" -eq 3 ]
      [ "$(curl -s {{ $url }} | jq '.result.role_permissions | keys | length')" -eq 3 ]
{{- end }}

• opal-helm-chart\templates\cm.yaml

{{- if .Values.e2e }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: policy-repo-data
data:
  upd.sh: |
    {{- .Files.Get "test/e2e/upd.sh" | nindent 4 }}
  data.json: |
    {{- .Files.Get "test/e2e/data.json" | nindent 4 }}
  rbac.rego: |
    {{- .Files.Get "test/e2e/rbac.rego" | nindent 4 }}
{{- end }}
---
{{- if .Values.client }}
{{- if .Values.client.opaStartupData }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: opa-startup-data
data:
  {{- range $name, $value := .Values.client.opaStartupData }}
  {{ $name }}: |
    {{ $value |  nindent 4 }}
  {{- end }}
{{- end }}
{{- end }}

Would you able to create a PR and merge it to the repo? cheers

[RazcoDev]

Hey @theeheng ,
Sounds good.
I'll handle this shortly.

Thanks for the suggestions.

[theeheng]

@RazcoDev thanks for merging the fix to master branch, I'm just wondering if any chance you would able to create a new release tag 0.0.12 for this change?

[obsd]

Hi @theeheng I just released 👍
Have a great week

[maurice-freitag]

For me the client is still being deployed. Is there something wrong with my config?

values.yaml:

opal:
  client: null
  server:
  [...]

Chart.yaml:

dependencies:
  - name: opal
    version: "~0.0.12"
    repository: "https://permitio.github.io/opal-helm-chart"
    alias: opal

[theeheng]

@maurice-freitag This is the sample value file i use :

values.test.yaml

imageRegistry: docker.io
postgresImageRegistry: docker.io
imagePullSecrets: [ ]

server:
  port: 7002
  policyRepoUrl: ""
  policyRepoSshKey: null
  policyRepoClonePath: null
  policyRepoMainBranch: null
  pollingInterval: 30
  dataConfigSources:
    external_source_url: "https://your-api.com/path/to/api/endpoint"
  broadcastUri: "redis://xxxxxxx:6380"
  broadcastPgsql: false
  uvicornWorkers: 4
  replicas: 1
  extraEnv:
    "OPAL_REPO_WATCHER_ENABLED" : false
  secrets:
    - auth-opal-server-secrets

client: null

and it works when i use helm install command :

helm install auth opal-helm-chart/ --values opal-helm-chart/values.test.yaml

here is what i get when I try run kubectl get pods :

NAME                           READY   STATUS    RESTARTS   AGE
auth-server-6b59765954-dfk5x   1/1     Running   0          19s

[maurice-freitag]

I finally got around to this issue. The client is still there if you use the chart as a subchart instead of installing it directly. There is a known issue in Helm where null values don't get properly passed on to subcharts. Here's a minimal repo reproducing the issue.

This makes it difficult to use the chart as the unwanted client is not properly configured and fills the server logs with invalid requests. My suggestion is to add a parameter client.enabled and use it to conditionally render the client template.

[RazcoDev]

Hey @maurice-freitag ,
Added this mechanism to enable/disable the client and server.
Thanks for bringing the attention on that Helm issue.