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

[bitnami/external-dns] External-DNS Chart Broken with 7.3.3 due to removal of protected CRD group annotation #25967

Open
pinkfloydx33 opened this issue May 17, 2024 · 46 comments · May be fixed by #29266
Assignees
Labels
external-dns tech-issues The user has a technical issue about an application

Comments

@pinkfloydx33
Copy link

pinkfloydx33 commented May 17, 2024

Name and Version

bitnami/external-dns

What architecture are you using?

None

What steps will reproduce the bug?

Attempt to upgrade or install the external-dns chart at version 7.3.3. In our case, an automated upgrade via Flux has started spamming alerts across all of our environments where minor/patch upgrades are automatically performed.

What is the expected behavior?

Helm chart upgrades/installs

What do you see instead?

Installation fails with:

Helm upgrade failed: cannot patch "dnsendpoints.externaldns.k8s.io" with kind CustomResourceDefinition: CustomResourceDefinition.apiextensions.k8s.io "dnsendpoints.externaldns.k8s.io" is invalid: metadata.annotations[api-approved.kubernetes.io]: Required value: protected groups must have approval annotation "api-approved.kubernetes.io", see kubernetes/enhancements#1111

Additional information

This is because the automated upgrade has removed the protected annotation on the CRDs:

This annotation is required on CRDs if the group is k8s.io, kubernetes.io, or ends with .k8s.io, .kubernetes.io which is applicable here because the CRD group is apiextensions.k8s.io.

This change needs to be reverted or else the chart is unusable.

@pinkfloydx33 pinkfloydx33 added the tech-issues The user has a technical issue about an application label May 17, 2024
@github-actions github-actions bot added the triage Triage is needed label May 17, 2024
@carrodher
Copy link
Member

Thank you for bringing this issue to our attention. We appreciate your involvement! If you're interested in contributing a solution, we welcome you to create a pull request. The Bitnami team is excited to review your submission and offer feedback. You can find the contributing guidelines here.

Your contribution will greatly benefit the community. Feel free to reach out if you have any questions or need assistance.

@pinkfloydx33
Copy link
Author

@carrodher I wouldn't know where to begin as the original change was made by some form of automation, which tells me that it too would need to be fixed. I also unfortunately need approval from my company before contributing, even to tools we use, and that would likely take more time than others broken by this change would appreciate.

@andeke07
Copy link

I have added the annotation to the pull request on the original external-dns project, I believe this is where the Bitnami automation gets its information from so if this gets approved hopefully the next release will contain it again?

@andeke07
Copy link

This has been fixed now with kubernetes-sigs/external-dns@f46676f

I don't know the next steps though, I suppose a release needs to be done and then the bitnami chart updated to refer to the new release?

@rouke-broersma
Copy link
Contributor

I think the release of such updates is automated, not sure how they are triggered but I expect that it will come within a reasonable time.

@carrodher
Copy link
Member

carrodher commented May 21, 2024

Our automation looks for new releases at https://github.com/kubernetes-incubator/external-dns. When a new release is cut there, our automated test & release process is triggered. As part of that process, the upstream CRDs are compared and the ones in the Bitnami chart are updated to match the upstream, i.e 375ee3b

@herrbpl
Copy link

herrbpl commented May 22, 2024

Still broken in 7.5.0

@andeke07
Copy link

external-dns hasn't issued a new release yet so there's nothing for the new chart to go off of

@raiomarco
Copy link

So... there's a fix? a workaround? if not, what version of the chart should i use? i tried the 7.5.2 but it didn't work :(

@cheinema
Copy link

@raiomarco Since the problem seems to have occurred starting with chart version 7.3.3, v7.3.2 should be the last stable version for now. We are still waiting for a new release in https://github.com/kubernetes-sigs/external-dns to include the fix.

@MaxAnderson95
Copy link

@cheinema Unless I'm missing something, the v0.14.2 release appears to have the fix. Is there anything else that needs to be done before merging the fix into the chart?

@rouke-broersma
Copy link
Contributor

@cheinema Unless I'm missing something, the v0.14.2 release appears to have the fix. Is there anything else that needs to be done before merging the fix into the chart?

You're looking at the external dns helm chart managed by external dns. This is not the source of crds for the bitnami chart.

@MaxAnderson95
Copy link

@rouke-broersma I knew I was missing something! Thanks.

@Atoms
Copy link
Contributor

Atoms commented Jun 14, 2024

so seems crd cannot be updated manually, as there is ci pipeline which allows only bitnami bot to update crd. and there is no release from external-dns side which would include api-approved annotation.

@pinkfloydx33
Copy link
Author

This chart is effectively broken for now... Does anyone have a way to workaround it or are we SOL for now?

@rouke-broersma
Copy link
Contributor

This chart is effectively broken for now... Does anyone have a way to workaround it or are we SOL for now?

I think you could simply deploy the required resources yourself (updated crd, clusterRole on crd, clusterRoleBinding to serviceaccount from chart): https://github.com/search?q=repo%3Abitnami%2Fcharts%20path%3A%2F%5Ebitnami%5C%2Fexternal-dns%5C%2Ftemplates%5C%2F%2F%20.Values.crd.create&type=code

That should be sufficient until upstream releases a new version.

@pinkfloydx33
Copy link
Author

Ok thanks. We use Flux for management, I'm sure theres a way to do that, just haven't looked into it yet. Hopefully the upstream fixes it soon...

@hawkesn
Copy link

hawkesn commented Jun 19, 2024

Still broken with 7.5.7

@andeke07
Copy link

@hawkesn the chart relies on external-dns coming out with another release which they haven't done in a month: https://github.com/kubernetes-sigs/external-dns/releases

The next time external-dns release, it will have the fix, which will then get embedded in this chart.

Copy link

github-actions bot commented Jul 5, 2024

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

@github-actions github-actions bot added the stale 15 days without activity label Jul 5, 2024
@MaxAnderson95
Copy link

Commenting to keep this issue open

@pinkfloydx33
Copy link
Author

@ruifung thanks for that. I had been thinking about patching with Flux but wasn't sure how it would handle patching CRDs. It does the trick though!

@mpechner-akasa
Copy link

Seems to be fixed here: kubernetes-sigs/external-dns#4488
I guess wait until bitnami updated teir chart, or we all just move to the external-dns chart.

@venkatamutyala
Copy link

Seems to be fixed here: kubernetes-sigs/external-dns#4488 I guess wait until bitnami updated teir chart, or we all just move to the external-dns chart.

I think external-dns needs to still release the fix and then bitnami will pick it up automatically. The PR you shared was merged on May 20th but the last release was May 16th.

@mpechner-akasa
Copy link

Seems to be fixed here: kubernetes-sigs/external-dns#4488 I guess wait until bitnami updated teir chart, or we all just move to the external-dns chart.

I think external-dns needs to still release the fix and then bitnami will pick it up automatically. The PR you shared was merged on May 20th but the last release was May 16th.

Then bitnami is broken, has been broken, and should not have updated to external-dns 0.14.2.

@rouke-broersma
Copy link
Contributor

The crds are an addon feature that is disabled by default and is most likely not widely used in production scenarios. It's not required for the core functionality of external dns. It's annoying that it hasn't been fixed yet but it's hardly the end of the world. The external dns chart with it's support for specific providers out of the box is imo way more convenient than the upstream chart.

@venkatamutyala
Copy link

The crds are an addon feature that is disabled by default and is most likely not widely used in production scenarios. It's not required for the core functionality of external dns. It's annoying that it hasn't been fixed yet but it's hardly the end of the world. The external dns chart with it's support for specific providers out of the box is imo way more convenient than the upstream chart.

I can't speak for everyone but I'm using this resource across 10+ production clusters.

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint

I also see others on github have been toggling the crd.create=true as well:

https://github.com/search?q=external-dns+crd.create%3Dtrue&type=code

I think this needs to get fixed properly. I just don't know how given the lack of traction in getting a release cut here:

kubernetes-sigs/external-dns#4657 (comment)

@javsalgar javsalgar changed the title External-DNS Chart Broken with 7.3.3 due to removal of protected CRD group annotation [bitnami/external-dns] External-DNS Chart Broken with 7.3.3 due to removal of protected CRD group annotation Aug 13, 2024
@javsalgar
Copy link
Contributor

Let me check with the team. Maybe, for the time being, we can disable the automation for this specific asset and re-enable it once they cut the release.

@fmulero fmulero self-assigned this Aug 13, 2024
@fmulero
Copy link
Collaborator

fmulero commented Aug 13, 2024

This problem comes from kubernets-sig/external-dns There are several issues about this:

The issue was fixed this PR but there is no new releases with that change.

From the bitnami side, we update automatically the CRDs and the containers based on new releases. We shouldn't use main branch for the CRDs as source of truth because this and the containers could be not properly aligned. Until there's an upstream release, you can use kustomize (or similar tools) to apply the changes in the Bitnami chart, sth like:

helm template external-dns oci://registry-1.docker.io/bitnamicharts/external-dns --set crd.create=true (...) | kubectl apply -k (...)

@rouke-broersma
Copy link
Contributor

This problem comes from kubernets-sig/external-dns There are several issues about this:

The issue was fixed this PR but there is no new releases with that change.

From the bitnami side, we update automatically the CRDs and the containers based on new releases. We shouldn't use main branch for the CRDs as source of truth because this and the containers could be not properly aligned. Until there's an upstream release, you can use kustomize (or similar tools) to apply the changes in the Bitnami chart, sth like:

helm template external-dns oci://registry-1.docker.io/bitnamicharts/external-dns --set crd.create=true (...) | kubectl apply -k (...)

I don't have this capability in my environment, I can only perform helm deployments. This does not solve my problem.
I agree that you should not rely on main for the crds, however the bitnami chart crds have been broken for months now.

Would it instead be possible to temporarily use the latest helm chart release as the source of the crd instead? The helm chart has been updated with the fix to the crd.

@juan131
Copy link
Contributor

juan131 commented Aug 13, 2024

There's a PR attempt to address this, see #27434

fmulero added a commit to gnom4ik/charts that referenced this issue Aug 13, 2024
Signed-off-by: Fran Mulero <fmulero@vmware.com>
fmulero added a commit to gnom4ik/charts that referenced this issue Aug 13, 2024
Signed-off-by: Fran Mulero <fmulero@vmware.com>
fmulero added a commit that referenced this issue Aug 13, 2024
* Update crd.yaml
CRD doesn't work, this solving the problem ) 

* chore(CRD): Disable auto-update due to #25967

Signed-off-by: Fran Mulero <fmulero@vmware.com>

---------

Signed-off-by: Aleksei Pashkin <jr.pashkin@gmail.com>
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
Signed-off-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
Signed-off-by: Fran Mulero <fmulero@vmware.com>
Co-authored-by: Bitnami Containers <bitnami-bot@vmware.com>
Co-authored-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
Co-authored-by: juan131 <jariza@vmware.com>
Co-authored-by: Fran Mulero <fmulero@vmware.com>
@venkatamutyala
Copy link

venkatamutyala commented Aug 15, 2024

Looks like we may have a fix. Anyone roll out >= 8.3.5 in production yet? :)

@rouke-broersma
Copy link
Contributor

I updated without issues

@javsalgar
Copy link
Contributor

Thanks for letting us know! Can we close this issue then?

@juan131 juan131 removed the triage Triage is needed label Aug 16, 2024
Copy link

github-actions bot commented Sep 1, 2024

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

@github-actions github-actions bot added the stale 15 days without activity label Sep 1, 2024
@venkatamutyala
Copy link

@javsalgar I think we need to revert the change once the upstream chart has been released?

@carrodher carrodher assigned javsalgar and unassigned carrodher and fmulero Sep 2, 2024
@javsalgar
Copy link
Contributor

Correct, when upstream releases, we can re-enable it.

@github-actions github-actions bot removed the stale 15 days without activity label Sep 3, 2024
@venkatamutyala
Copy link

@javsalgar it is time. :) A new release was dropped yesterday and appears to have the fix:

https://github.com/kubernetes-sigs/external-dns/releases/tag/v0.15.0

fix: re-add api-approved.kubernetes.io annotation by @morremeyer in https://github.com/kubernetes-sigs/external-dns/pull/4488

@javsalgar
Copy link
Contributor

Thanks for letting us know! I created a PR ⏫

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-dns tech-issues The user has a technical issue about an application
Projects
None yet
Development

Successfully merging a pull request may close this issue.