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

reapplying CRD yaml with conversion webhook enabled causes unexpected CRD resourceVersion update #1684

Closed
matthchr opened this issue Sep 18, 2020 · 1 comment · Fixed by #1699
Labels
kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@matthchr
Copy link
Contributor

What broke

kubectl apply -f of a Kubebuilder generated CRD with conversion webhook enabled causes resourceVersion of CRD to be updated (which triggers watches to expire, which triggers re-list + reconciliation of all CRs of that CRD type).

What did I expect to happen?

resourceVersion shouldn't be changed.

The reason resourceVersion was changed is because of this section of the enablewebhook_patch template.

What ends up happening is:

  1. Kustomize generates a CRD which includes the following snippet:
webhookClientConfig:
      # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank,
      # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager)
      caBundle: Cg==
  1. That CRD is initially applied (so caBundle is Cg==).
  2. cert-manager sees the cert-manager.io/inject-ca-from annotation and updates caBundle to the actual ca value.
  3. kubectl apply -f of the CRD happens again some time later (for example due to a GitOps tool like flux, or just because somebody is reapplying CRDs for whatever reason).
  4. The caBundle is reset to Cg== (causing a resourceVersion change).
  5. cert-manager reapplys the caBundle (causing another resourceVersion change).

Suggested solution

If instead of having a default of caBundle: Cg==, Kubebuilder generated patches that omitted caBundle entirely, this workflow would work much better, as per the documented behavior of apply and how it considers last-applied-configuration.

The comment:

  # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank,
  # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager)

doesn't seem correct anyway since as far as I can tell this field has been optional since Kubernetes 1.13

I've tested this manually and confirmed that if I remove caBundle: Cg== from the yaml and reapply it on the CRD, it is accepted and subsequent reapplies don't change the resourceVersion.

  • go version: 1.13
  • kubebuilder version: 2.3.1
  • Kubernetes version: 1.16.13

/kind bug

@DirectXMan12
Copy link
Contributor

Yeah, I think that comment predated 1.13, possibly. the fix sounds good, plus it'll be more correct with SSA (since we don't want to own that field anyway).

matthchr added a commit to matthchr/azure-service-operator that referenced this issue Sep 24, 2020
  - This prevents resourceVersion change when CRD is reapplied.
  - See kubernetes-sigs/kubebuilder#1684 for
    more details.
  - Fixes Azure#1261.
matthchr added a commit to Azure/azure-service-operator that referenced this issue Sep 24, 2020
- This prevents resourceVersion change when CRD is reapplied.
- See kubernetes-sigs/kubebuilder#1684 for
  more details.
- Fixes #1261.
matthchr added a commit to matthchr/kubebuilder that referenced this issue Sep 29, 2020
matthchr added a commit to matthchr/kubebuilder that referenced this issue Sep 29, 2020
@camilamacedo86 camilamacedo86 added this to the v3+ plugin milestone Oct 20, 2020
@camilamacedo86 camilamacedo86 modified the milestones: v3+ plugin, v3.0.0 Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants