1Password / connect-helm-charts

Official 1Password Helm Charts
https://developer.1password.com
MIT License
90 stars 73 forks source link

Incorrect default operator.pollingInterval value #146

Closed JoshCooley-alto closed 1 year ago

JoshCooley-alto commented 1 year ago

Your environment

Chart Version: 1.10.0

Helm Version: 3.11.3

Kubernetes Version: 1.23

What happened?

The POLLING_INTERVAL environment is set to 10 on the onepassword-connect-operator pod.

What did you expect to happen?

The polling interval should be set to 600 per the chart README.

Steps to reproduce

  1. Install operator helm chart, without specifying a value for operator.pollingInterval
  2. Observe the helm chart has a value of 10 for its pollingInterval
    $ helm get values connect --all --namespace secrets-management --output json | jq .operator.pollingInterval
    10
  3. Observe the running container has a value of 10 for its POLLING_INTERVAL environment variable.
    $ kubectl get pod --namespace secrets-management --selector name=onepassword-connect --output json | jq '.items[].spec.containers[].env[] | select(.name == "POLLING_INTERVAL")'
    {
    "name": "POLLING_INTERVAL",
    "value": "10"
    }

Notes & Logs

Here is where the default chart value is getting set: https://github.com/1Password/connect-helm-charts/blob/0267c6837112fef4c5ec1c784e787caf04109e38/charts/connect/values.yaml#L174

The default value should be 600 per the chart README here: https://github.com/1Password/connect-helm-charts/blob/0267c6837112fef4c5ec1c784e787caf04109e38/charts/connect/README.md?plain=1#L109

The operator README also states that the default value should 600 here: https://github.com/1Password/onepassword-operator/blob/fe930fef052d71516854568bf1042cb93af594a1/README.md?plain=1#L89

The default for the helm chart and the operator itself should not necessarily be same, but the fact that there's agreement between their READMEs suggests to me that this is a mistake in the default value, rather than a mistake in the helm chart README.

JoshCooley-alto commented 1 year ago

PR here: https://github.com/1Password/connect-helm-charts/pull/147

jillianwilson commented 1 year ago

Thanks for making the PR! I've gone ahead and approved it :)