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

Infoblox - Correct GetObject query #2890

Merged

Conversation

skalpin
Copy link
Contributor

@skalpin skalpin commented Jul 18, 2022

Description

The ibclient.GetObject will return all the objects associated with the query. This is an issue when external-dns has identified 1 record to remove. The object being created retrieves many more records than expected and so external-dns will delete all the matching records.

This change adds one "name" filter to the query parameters so that only the expected record is removed from infoblox

Checklist

  • Unit tests updated
  • End user documentation updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 18, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: skalpin / name: Steve Kalpin (59f4f1c)

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 18, 2022
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 18, 2022
@skalpin
Copy link
Contributor Author

skalpin commented Jul 18, 2022

/assign @njuettner

Copy link
Member

@njuettner njuettner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 18, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: njuettner, skalpin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2022
@k8s-ci-robot k8s-ci-robot merged commit 3c1e72f into kubernetes-sigs:master Aug 18, 2022
@skalpin skalpin deleted the correct_infoblox_deletes branch August 25, 2022 20:12
@RichieSams
Copy link
Contributor

Can we get a release for this fix and a huge warning on all releases before this fix?
We got hit by this issue, and external-dns deleted 4000 DNS records that it didn't own....

@torbendury
Copy link

+1, we need a hotfix release for this. Completely messed up our infoblox.

@RichieSams
Copy link
Contributor

@seanmalloy @njuettner @Raffo Do you all have any ETA for when a new release will be created with this fix?

@RichieSams
Copy link
Contributor

@zreigz Do you know when the next release will be? This fix is sorely needed. As it stands, we can't upgrade external-dns due to this issue.

@zreigz
Copy link
Contributor

zreigz commented Sep 28, 2022

@zreigz Do you know when the next release will be? This fix is sorely needed. As it stands, we can't upgrade external-dns due to this issue.

sorry, I am the wrong person for this

@RichieSams
Copy link
Contributor

sorry, I am the wrong person for this

Ah apologies. I saw your name in the commits. Thanks for your other work! :)

RichieSams added a commit to RichieSams/external-dns that referenced this pull request Oct 7, 2022
With respect to them having a huge bug for the Infoblox provider. See kubernetes-sigs#2890
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants