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

Use new approach to validate whether the ipa-ca DNS record is complete #274

Merged
merged 2 commits into from Oct 17, 2022

Conversation

rcritten
Copy link
Collaborator

@rcritten rcritten commented Jul 6, 2022

Related to the ticket:

Use new approach to validate whether the ipa-ca DNS record is complete

The previous method counted the number of servers with CA's and
expected an identical count of servers in ipa-ca, for each of the
A and AAAA types.

If one server had only A or AAAA records then this count could be
off and issue a spurious warning.

Instead get the list of A and AAAA records for servers with a CA
and compare the IP addresses to those of the A and AAAA records
of ipa-ca. Return a warning if any are missing or not expected
(e.g. a server was removed but remains in ipa-ca).

https://github.com/freeipa/freeipa-healthcheck/issues/270

Signed-off-by: Rob Crittenden <rcritten@redhat.com>

I ran into this while testing the above patch so went ahead and included the fix:

Use exceptions to indicate parsing errors, not a return value

The validation in parse_options() retured a 1 on failure.
Raise an exception instead and expect the caller to handle it.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>

The validation in parse_options() retured a 1 on failure.
Raise an exception instead and expect the caller to handle it.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
@rcritten
Copy link
Collaborator Author

rcritten commented Jul 6, 2022

Need to address the lint items and just found that the IPv6 address of hidden servers is being reported as unexpected. It isn't in the ipa-ca dig output though.

@rcritten
Copy link
Collaborator Author

rcritten commented Jul 6, 2022

The bad ipa-ca record is not a healthcheck issue. I opened https://pagure.io/freeipa/issue/9195

The previous method counted the number of servers with CA's and
expected an identical count of servers in ipa-ca, for each of the
A and AAAA types.

If one server had only A or AAAA records then this count could be
off and issue a spurious warning.

Instead get the list of A and AAAA records for servers with a CA
and compare the IP addresses to those of the A and AAAA records
of ipa-ca. Return a warning if any are missing or not expected
(e.g. a server was removed but remains in ipa-ca).

freeipa#270

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Copy link
Contributor

@flo-renaud flo-renaud left a comment

Choose a reason for hiding this comment

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

Hi @rcritten
thanks for the patch, LGTM. I tested various situations where the CA servers have only ipv4 / only ipv6 / both addresses and the new code properly handles the cases. Also works with hidden servers.

@flo-renaud flo-renaud self-assigned this Oct 17, 2022
@flo-renaud flo-renaud added the ack label Oct 17, 2022
@rcritten
Copy link
Collaborator Author

Thanks for the review!

@rcritten rcritten merged commit a00e029 into freeipa:master Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants