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

New idview management module. #1134

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Conversation

t-woerner
Copy link
Member

There is a new idview management module placed in the plugins folder:

plugins/modules/ipaidview.py

The idview module allows to ensure presence and absence of idviews and idview host members.

Here is the documentation for the module:

README-idview.md

New example playbooks have been added:

playbooks/idview/idview-absent.yml
playbooks/idview/idview-host-applied.yml
playbooks/idview/idview-host-unapplied.yml
playbooks/idview/idview-present.yml

New tests for the module can be found at:

tests/idview/test_idview.yml
tests/idview/test_idview_client_context.yml

@rjeffman rjeffman linked an issue Aug 28, 2023 that may be closed by this pull request
@t-woerner t-woerner force-pushed the new_idview_module branch 2 times, most recently from 5c501bc to 1657069 Compare August 30, 2023 12:06
Copy link
Collaborator

@varunmylaraiah varunmylaraiah left a comment

Choose a reason for hiding this comment

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

LGTM.
It would be helpful if we added an example for the domain_resolution_order variable.

@t-woerner t-woerner force-pushed the new_idview_module branch 2 times, most recently from b08a8d1 to e7b1f29 Compare September 4, 2023 12:41
Copy link
Collaborator

@varunmylaraiah varunmylaraiah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rjeffman rjeffman left a comment

Choose a reason for hiding this comment

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

Even with my comments, this PR looks good to me.

All comments I added can be discussed/addressed in the future, if we decide so.

README-idview.md Outdated Show resolved Hide resolved
README-idview.md Outdated Show resolved Hide resolved
# But as we create the intersection with the list of hosts of
# the idview, we emulate the correct behaviour. But this means
# that there is no general idview_unapply like in the cli.
commands.append([None, "idview_unapply", {"host": host_del}])
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about "unapplying" a host within the ipahost plugin?

I'm thinking something like:

ipahost:
   host: myhost
   state: idview_unapplied

I know it is a kind of behavior we still don't have in ansbile-freeipa, but it looks to me that "unapply" can be something defined from the host point of view.

I wouldn't change this plugin, but would provide ipahost a way to unapply from an idview.

(This would be a new PR.)

Copy link
Member Author

Choose a reason for hiding this comment

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

The information is stored in the idview. It would require to have additional queries per host for idviews.

Maybe it would be possible to additionally have a generiac unapply for hosts as we have in the command line.

There is a new idview management module placed in the plugins folder:

    plugins/modules/ipaidview.py

The idview module allows to ensure presence and absence of idviews and
idview host members.

Here is the documentation for the module:

    README-idview.md

New example playbooks have been added:

    playbooks/idview/idview-absent.yml
    playbooks/idview/idview-host-applied.yml
    playbooks/idview/idview-host-unapplied.yml
    playbooks/idview/idview-present.yml

New tests for the module can be found at:

    tests/idview/test_idview.yml
    tests/idview/test_idview_client_context.yml
Copy link
Member

@rjeffman rjeffman left a comment

Choose a reason for hiding this comment

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

LGTM.

@rjeffman rjeffman merged commit be9a2db into freeipa:master Sep 6, 2023
26 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFE] Missing module to manage ID views
3 participants