Add ipa_pwpolicy module

[adralioh]

SUMMARY

Add a module for modifying FreeIPA password policies

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

ipa_pwpolicy

ADDITIONAL INFORMATION

The module is based heavily on the existing IPA modules, and extends the base IPA arguments/documentation

A couple considerations:

• The arguments all use the raw LDAP name, which are pretty long, eg krbpwdfailurecountinterval. I added shorter aliases based on the ipa command that are easier to use.

  I did it this way because this is how the existing IPA modules do it, but I can change the default name to the shorter versions if preferred.

• All of the arguments are strings instead of integers because they're passed directly to the IPA web API, which stores them as strings. I can switch them to integers if preferred.

[ansibullbot]

cc @Akasurde @Nosmoht @fxfitz
click here for bot help

[felixfontein]

Please note that all new modules and plugins are required to have tests (unit and/or integration tests).

[felixfontein]

Thanks for your PR! The module looks pretty polished. I'm not familiar with FreeIPA or the ipa_* modules, so there's not much I can say except that :)

[adralioh]

I added unit tests

ready_for_review

[felixfontein]

Cool, looks great! :)

@Akasurde @Nosmoht @fxfitz as maintainers of related modules, can you please take a look?

[Andersson007]

@adralioh hi, thanks for the new module!
General things like the documentation, examples, etc. LGTM
would be good to get feedback from mentioned people

[Andersson007]

Suggested change

	aliases: ["group", "name"]
	aliases: ["group", "name"]

i would avoid aliases which is ok to add sometimes (but not initially), e.g. when a typo or something. So i'd choose one best option for every parameter.

[Andersson007]

folks from freeipa team hinted that there's a special repo https://github.com/freeipa/ansible-freeipa/
and it looks like there's a similar module already implemented https://github.com/freeipa/ansible-freeipa/blob/master/plugins/modules/ipapwpolicy.py
@adralioh did you see that? Maybe better just to add missed functionality (e.g. return value, maybe some options) to the existing one?

[Andersson007]

we're now discussing in IRC ansible-community the possibility of deprecating ipa related stuff in community.general and redirecting people to the special repo

[adralioh]

The module doesn't do anything that the one in the ansible-freeipa repo doesn't do as far as I can tell. I wanted to add the module here because it was the only IPA functionality I needed in my playbooks that's currently missing from community.general.

Was there a consensus on whether or not to deprecate the IPA modules? If they're not going to be deprecated, I think it would still be worth adding this module.

[ansibullbot]

@!UNKNOWN @Akasurde @Alb0t @AnderEnder @ApsOps @AugustusKling @Aversiste @ColOfAbRiX @DBendit @DavidWittman @Deepakkothandan @DenBeke @Dorn- @FlossWare @GR360RY @GwenaelPellenArkeup @InTheCloudDan @JayKayy @Jmainguy @JoergFiedler @KKoukiou @KellerFuchs @KimNorgaard @L2G @LinusU @Lunik @MacLemon @MatrixCrawler @MeganLiu @Mogztter @MorrisA @NickatEpic @NilashishC @nitaco @Nosmoht @Qalthos @QijunPan @QuentinBrosse @RickS-C137 @Rylon @Sajna-Shetty @SamyCoenen @Sedward @Shaps @Slezhuk @Smithx10 @Spredzy @Tatsh @ThePixelDeveloper @ThomasSteinbach @timothyvandenbrande @TommyLike @Tomorrow9 @UnderGreen @WojciechowskiPiotr @Wolfant @aajdinov @aalexmonteiro @abarbare @abellotti @abulimov @adamgoossens @adamvaughan @adejoux @adriane-cardozo @adrianmoisey @agaffney @agmezr @agronholm @ahtik @aimonb @akostyuk @akshay196 @albertomurillo @alcamie101 @alikins @alxgu @aminvakil @amit0701 @andreparames @andrew-d @andsens @andyhky @andytom @angristan @angstwad @antoinell @archf @ares @arturaz @azaghal @b6d @bachradsusi @baldwinSPC @bannaych @barnabycourt @barryib @bbyhuy @bcoca @bendoh @bennojoy @berenddeboer @berendt @bgaifullin @bgurney-rh @bhcopeland @bigmstone @billdodd @bincyber @bjolivot @bleader @bpennypacker @brampling @bregman-arie @brian-brazil @briceburg @brontitall @bsanders @bushvin @bvitnik @bwhaley @caphrim007 @carchi8py @catcombo @cben @cheese @chouseknecht @chris93111 @chrishoffman @chrisisbeef @claco @clc-runner @clementtrebuchet @cloudnull @cmprescott @colin-nolan @commel @coreywan @cove @cprh @cwollinger @d-little @danieljaouen @danielmellado @danihodovic @dankeder @dareko @dariko @davx8342 @dcermak @dch @decentral1se @derez @dermute @devyanikota @dinoocch @dirtyharrycallahan @displague @dj-wasabi @djmattyg007 @dkorn @dmtrs @dnix101 @dohoangkhiem @dougluce @dprts @drcapulet @drew-russell @drewkerrigan @drybjed @dschep @dstoflet @dusdanig @ebirn @edevenport @edisonxiang @eest @ehelms @eikef @ekohl @elad661 @elasticdog @enriclluelles @erjohnso @erydo @eryx12o45 @evertmulder @evgkrsk @evrardjp @fabulops @farhan7500 @fcuny @fgbulsoni @fishman @flaper87 @flynn1973 @fraff @freesky-edward @fxfitz @ganeshrn @garbled1 @gautamphegde @genegr @getjack @gforster @giovannisciortino @giuseppe @glitchcrab @groks @gtanzillo @guillaume_ro_fr @haad @hannseman @hekonsek @helldorado @hkariti @hnakamur @hnanni @hogarthj @hryamzik @huaweicloud @hulquest @hwDCN @ilicmilan @imjoseangel @indrajitr @inetfuture @ivanvanderbyl @jagadeeshnv @jails @jairojunior @jamescassell @jasperla @jbenden @jbscalia @jcftang @jcgruenhage @jdauphant @jensdepuydt @jerome-quere @jhoekx @jirutka @jlaska @jle64 @johanwiren @john-westcott-iv @jose-delarosa @joshainglis @joshuaconner @jparrill @jpdasma @jpmens @jscalia @jsmartin @jsumners @jtyr @justjais @jwitko @kahowell @kairoaraujo @kamsz @karmab @kassiansun @kavu @kbrebanov @keachi @keitwb @kenichi-ogawa-1988 @kevensen @kindermoumoute @kostiantyn-nemchenko @krisvasudevan @krsacme @kubevirt @kunkku @kyleabenson @lekum @levonet @lionmax @lmprice @lonico @lrupp @lukasbestle @m-yosefpor @machacekondra @madhav-bharadwaj @magnus919 @makaimc @manojmeda @marc-sensenich @markuman @marns93 @martinm82 @marvin-sinister @marwatk @matbu @matburt @mator @mattjeffery @mattupstate @matze @mavit @maxamillion @mcltn @mcodd @meerkampdvv @mekanix @melodous @mgedmin @mgruener @mheap @mmazur @molekuul @mpdehaan @mraineri @mross22 @mscherer @mulby @mwarkentin @n0trax @n0ts @nalsaber @nasx @nate-kingsley @natefoo @navalkp @nbuchwitz @ndavison @ndclt @ndswartz @nerzhul @neuroid @nibalizer @nitzmahone @niuzhenguo @noles @noseka1 @nurfet-becirevic @obirvalger @oboukili @obourdon @ogenstad @olsaki @oolongbrothers @opoplawski @opslounge @orgito @ovcharenko @overhacked @parfeon @pascalheraud @pb8226 @pcgentry @pertoft @phumpal @pilou- @pkliczewski @pmakowski @pmarkham @prabhosa @precurse @privateip @pubnub @pyykkis @quentinsf @quidame @raekins @rajeevarakkal @rambleraptor @ramondelafuente @ramooncamacho @ravibhure @remixtj @remyleone @ribbons @ricardogpsf @rmcintosh @robinro @robwagner33 @rohitChaware @rosmo @rosowiecki @rsmontero @russoz @rvalle @ryansb @sac @samdoran @saranyasridharan @sayap @scathatheworm @schmots1 @scodeman @scottanderson42 @sdodsley @seandst @sebasmannem @sermilrod @sganesh-infoblox @sgargan @shane-walker @shirou @sieben @sile16 @sivel @sjaiswal @skinp @skornehl @slok @sluther @sm4rk0 @smashwilson @smbambling @snopoke @softzilla @soodpr @srvg @ssbarnea @steamx @stearz @stpierre @strk @stygstra @stympy @supertom @suprememoocow @sysadmind @szinck @t0mk @t794104 @tacatac @talzur @tarka @tastychutney @tbielawa @tbouvet @tchernomax @tcraxs @tdtrask @teebes @tgoetheyn @thaumos @the-maldridge @thoiberg @timuster @tintoy @tksmd @tmiotto @tmshn @toabctl @tomasg2012 @tonyseek @treyperry @trishnaguha @troy2914 @tumbl3w33d @turb @tuxillo @tyouxa @tzure @ujwalkomarla @usawa @ushuz @vaygr @vcarceler @vedit @verkaufer @vexata @vfauth @vincentvdk @vmalloc @vritant @waheedi @walbert947 @wbrefvem @weaselkeeper @willthames @willybarro @wltjr @wopfel @wrouesnel @wtcross @xcambar @xen0l @xiaozhu36 @xorel @xprazak2 @xuxiaowei0512 @yaacov @yanzhangi @yeukhon @zanssa @zbal @zengchen1024 @zfil @zgalor @zhhuta @zhongjun2 @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

[felixfontein]

Since there hasn't really been any discussions, it might be better to add the module here for now. @Andersson007 what do you think?

[Andersson007]

Since there hasn't really been any discussions, it might be better to add the module here for now. @Andersson007 what do you think?

Ok

[Andersson007]

@adralioh thanks for the work! Let's continue here then.
my comment about aliases is still relevant.

[adralioh]

@Andersson007 Regarding the aliases, I'm not sure which ones are the best option to keep.

The long names that I have as the default option name, eg krbpwdfailurecountinterval, are the raw LDAP name as returned by the IPA API.
The shorter aliases, eg failinterval, are the names used on the commandline by the ipa command.

The other IPA modules mostly only use the LDAP name, but they're not as long as the ones in this module, so are more manageable.

Also, for reference, the module in the ansible-freeipa repo has aliases for both names.

I'm leaning towards removing the shorter aliases so it's consistent with the other modules, but I wanted to see what you thought.

[Andersson007]

@adralioh IMO we could use the short forms because as i can see they don't intersect with each other but we could put the long forms to descriptions, e.g. (very roughly):

    maxlife:
        description: Maximum password lifetime (in days). Corresponds to the C(krbmaxpwdlife) raw LDAP name.

I think maxpwdlife would sound better.

(edited: I did the typo:), sorry)

[Andersson007]

@adralioh IMO we could use the short forms because as i can see they don't intersect with each other but we could put the long forms to descriptions, e.g. (very roughly):

    maxlife:
        description: Maximum password lifetime (in days). Corresponds to the C(krbmaxpwdlife) raw LDAP name.

I think maxpwdlife would sound better.

, provided that to know the raw names can be important for anyone. If not, i'd leave only the short forms

[adralioh]

@Andersson007 Thanks, I changed them to short names, and removed mention of the LDAP long names since they probably aren't important.

I renamed some of the short names to be more descriptive, such as maxlife to maxpwdlife like you mentioned, and maxfail to maxfailcount.

I also improved the description of some of the options so that they're better worded.

[Andersson007]

@adralioh thanks for the new module!
@felixfontein thanks for reviewing!

[Andersson007]

@adralioh i'll add you as a maintainer to the special BOTMETA.yml file next week to keep you informed about related issues / PRs