1Password / connect-helm-charts

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

CRDs do not get upgraded #100

Open onedr0p opened 2 years ago

onedr0p commented 2 years ago

Hi 👋🏼 The way crds are handled in this repo Helm does not upgrade them when the user upgrades the chart.

https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

There is no support at this time for upgrading or deleting CRDs using Helm. This was an explicit decision after much community discussion due to the danger for unintentional data loss. Furthermore, there is currently no community consensus around how to handle CRDs and their lifecycle. As this evolves, Helm will add support for those use cases.

What I've seen most chart developers do it have a installCRDs option in the helm values and template them out, take a look at cert-manager and rook-ceph charts for an example on how they handle CRDs.

I would suggest a option in the values.yaml and to move the crds folder into the templates folder and wrap it in an if statement.

operator:
  create: false
  installCRDs: true
  autoRestart: false
john-yacuta-submittable commented 2 years ago

+1

I am currently in the process of deploying 1Password Connect and Operator into our infrastructure using:

operator:
  create: true

However, I would like to use the latest Connect and Operator to leverage the latest feature: "set type on OnePasswordItem resources, which can be any of the Kubernetes secret types" as discussed here.

I have tried upgrading it via the values.yaml by setting under operator version: "1.5.0" like below but there is an error: unknown field "type" in com.onepassword.v1.OnePasswordItem which I believe is related to the CRDs not upgrading. I am using GitOps: ArgoCD + Helm to deploy.

john-yacuta-submittable commented 2 years ago

I managed to upgrade 1Password Connect Server to 1.5.4 and Operator to 1.5.0 using the latest chart revision 1.8.0 that released 2 days ago. I went from revision 1.7.1 to 1.8.0 using ArgoCD App template + Helm source.

However, I am getting the error from the operator when trying to take advantage of the new type feature and doing type: kubernetes.io/dockerconfigjson. I already tried redeploying 1Password and the same issue occurs.

Error: "error":"Failed to retrieve item: need at least version 1.3.0 of Connect for this function, detected version 1.2.0 (or earlier). Please update your Connect server"

john-yacuta-submittable commented 2 years ago

I created a new issue to track my issue: https://github.com/1Password/connect-helm-charts/issues/105

jillianwilson commented 1 year ago

Thanks for raising this issue. @onedr0p feel free to make a PR with the suggested changes

matthiasbaldi commented 1 year ago

Edit: Ignore that :wink: I messed up the version from the helm chart with the operator itself...

~I bring that up again.~ ~Today I patched to the current chart version 1.7.1 and suddenly I had issues to apply new 1PasswordItems because of the error onepassword.com/v1, Kind=OnePasswordItem): .type: field not declared in schema.~

~After trying some stuff I decided to run kubectl apply -f https://raw.githubusercontent.com/1Password/connect-helm-charts/main/charts/connect/crds/onepassworditem-crd.yaml manually. Since then it looks good again.~

~Is there an issue in the current chart release? Or do we had some fancy issues on our cluster? We had no issues in earlier versions like 1.5.x or 1.6.x.~