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

host: try to resolve FQDN before command execution #5838

Closed
wants to merge 2 commits into from

Conversation

antoniotorresm
Copy link
Contributor

Trying to resolve the FQDN before command execution (during
pre-callback) helps detect cases where the host specified by the user
does not exist, saving execution time. Aside from this, resolving the
FQDN is useful when only the shortname of the host is passed, as this
would cause issues when trying to update the DNS records during
modification of the entry.

Fixes: https://pagure.io/freeipa/issue/8726
Fixes: https://pagure.io/freeipa/issue/8884
Signed-off-by: Antonio Torres antorres@redhat.com

Trying to resolve the FQDN before command execution (during
pre-callback) helps detect cases where the host specified by the user
does not exist, saving execution time. Aside from this, resolving the
FQDN is useful when only the shortname of the host is passed, as this
would cause issues when trying to update the DNS records during
modification of the entry.

Fixes: https://pagure.io/freeipa/issue/8726
Fixes: https://pagure.io/freeipa/issue/8884
Signed-off-by: Antonio Torres <antorres@redhat.com>
@rcritten
Copy link
Contributor

I toyed with a couple of other host-* commands and didn't see any need for the lookup there. Did you come to the same conclusion?

@antoniotorresm
Copy link
Contributor Author

I toyed with a couple of other host-* commands and didn't see any need for the lookup there. Did you come to the same conclusion?

I just added the search where it was being done before. It acts as an existence check so I think it's good to have it on top of the pre-callback function.

@rcritten
Copy link
Contributor

This looks good. Can you add a test to ensure that the shortname use-cases work as expected? The xmlrpc tests would be probably the best place.

Add test to ensure that host-mod resolves the FQDN when passing the
shortname of the host being modified.

Related: https://pagure.io/freeipa/issue/8726
Related: https://pagure.io/freeipa/issue/8884
Signed-off-by: Antonio Torres <antorres@redhat.com>
@rcritten
Copy link
Contributor

LGTM.

Can you set the label for backports? I assume ipa-4-9 is sufficient. You can set ack at the same time if you want, or I can add it later.

@antoniotorresm antoniotorresm added ack Pull Request approved, can be merged ipa-4-9 Mark for backport to ipa 4.9 labels Jun 18, 2021
@rcritten rcritten added the pushed Pull Request has already been pushed label Jun 18, 2021
@rcritten
Copy link
Contributor

master:

  • 3a4939f host: try to resolve FQDN before command execution
  • 3e77d31 ipatests: test host update using shortname

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-9 Mark for backport to ipa 4.9 pushed Pull Request has already been pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants