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

ipa_dnszone: add PTR synchronization support for dnszones #3374

Merged

Conversation

ahambrick
Copy link
Contributor

SUMMARY

Fixes #1687

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

IPA

ADDITIONAL INFORMATION

Allows support for the idnsallowsyncptr in the IPA REST API:

{
    "id": 0, 
    "method": "dnszone_mod/1", 
    "params": [
        [
            "foo.bar"
        ], 
        {
            "idnsallowsyncptr": true, 
            "version": "2.237"
        }
    ]
}

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request identity module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Sep 14, 2021
@ahambrick
Copy link
Contributor Author

First time contributor. Feedback greatly appreciated here. :)

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Here are some first comments.

changelogs/fragments/3374-add-ipa-ptr-sync-support.yml Outdated Show resolved Hide resolved
plugins/modules/identity/ipa/ipa_dnszone.py Outdated Show resolved Hide resolved
plugins/modules/identity/ipa/ipa_dnszone.py Outdated Show resolved Hide resolved
plugins/modules/identity/ipa/ipa_dnszone.py Outdated Show resolved Hide resolved
plugins/modules/identity/ipa/ipa_dnszone.py Outdated Show resolved Hide resolved
plugins/modules/identity/ipa/ipa_dnszone.py Show resolved Hide resolved
@felixfontein felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Sep 15, 2021
@felixfontein felixfontein changed the title Add PTR synchronization support for dnszones ipa_dnszone: add PTR synchronization support for dnszones Sep 20, 2021
@ansibullbot

This comment has been minimized.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed ci_verified Push fixes to PR branch to re-run CI labels Sep 20, 2021
@ansibullbot

This comment has been minimized.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 20, 2021
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Sep 28, 2021
@felixfontein
Copy link
Collaborator

@ahambrick ping, are you still working on this?

needs_info

@ansibullbot ansibullbot added the needs_info This issue requires further information. Please answer any outstanding questions label Oct 27, 2021
@ahambrick
Copy link
Contributor Author

Hey @felixfontein Yeah - I'm still tracking this on the side. I'll review my commits and remind myself where things stand.

@ansibullbot ansibullbot removed the needs_info This issue requires further information. Please answer any outstanding questions label Nov 4, 2021
@ansibullbot

This comment has been minimized.

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! and removed stale_ci CI is older than 7 days, rerun before merging labels Dec 17, 2021
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Dec 21, 2021
@ahambrick
Copy link
Contributor Author

@felixfontein
I wanted to ask about the "items" vs. "itens". In keeping w/ the rest of the code in this file, I opted for "itens" but can start steering towards "items".

@felixfontein
Copy link
Collaborator

@ahambrick to me it looks like itens is a typo and should always have been items. Also a search for 'itens' on the web didn't indicate this is a special term I'm not aware of. So I'd tend to use items instead in new code.

@ahambrick
Copy link
Contributor Author

@felixfontein Thanks for the input - changes made/tested. Hoping this can maybe make 4.3.0?

@felixfontein
Copy link
Collaborator

@ahambrick 4.3.0 sounds good (I also suggested that in https://github.com/ansible-collections/community.general/pull/3374/files#r715330667). Could you please look at the remaining open suggestions?

ahambrick and others added 2 commits December 26, 2021 14:59
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Dec 26, 2021
@felixfontein
Copy link
Collaborator

@justchris1 can you also take a look? I think it looks good, but I don't use IPA and this module :)

@justchris1
Copy link
Contributor

LGTM. shipit

@felixfontein felixfontein merged commit 84e45c2 into ansible-collections:main Dec 27, 2021
@patchback
Copy link

patchback bot commented Dec 27, 2021

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/84e45c2cc0ba4aa096aba0c58649d1fdfefad3ff/pr-3374

Backported as #3950

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Dec 27, 2021
patchback bot pushed a commit that referenced this pull request Dec 27, 2021
* Add PTR synchronization support for dnszones

* Add changelog fragment

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Update to reflect proper module name.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Add period.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Remove requires comment.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Change type to boolean in following with API docs

* Tested with needed changes made.

* Fix documentation to max implementation

* Check for specific params; allow for modifications if needed

* Add PTR synchronization support for dnszones

* Add changelog fragment

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Update to reflect proper module name.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove trailing whitespace

* Make use of full search and compare params

* Fix formatting errors

* Move the change flag outside of module check

* Fix itens typo to items

* Update dynamicupdate to a boolean

* Remove unnecessary flags and options

* Minor comment changes

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Anne-Marie Lee <alee@datainterfuse.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 84e45c2)
@felixfontein
Copy link
Collaborator

@ahambrick thanks for implementing this!
@justchris1 thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Dec 27, 2021
)

* Add PTR synchronization support for dnszones

* Add changelog fragment

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Update to reflect proper module name.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Add period.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Remove requires comment.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Change type to boolean in following with API docs

* Tested with needed changes made.

* Fix documentation to max implementation

* Check for specific params; allow for modifications if needed

* Add PTR synchronization support for dnszones

* Add changelog fragment

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Update to reflect proper module name.

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove trailing whitespace

* Make use of full search and compare params

* Fix formatting errors

* Move the change flag outside of module check

* Fix itens typo to items

* Update dynamicupdate to a boolean

* Remove unnecessary flags and options

* Minor comment changes

* Update changelogs/fragments/3374-add-ipa-ptr-sync-support.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/identity/ipa/ipa_dnszone.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Anne-Marie Lee <alee@datainterfuse.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 84e45c2)

Co-authored-by: Annie Lee <hambriak@gmail.com>
@ahambrick ahambrick deleted the ipa_dnszone_ptr_sync_support branch January 7, 2022 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request identity module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: community.general.ipa_dnszone support for automatic record PTR synchronization
4 participants