Next major version changes

[Phil-Friderici]

Collection of planned changes for next major version

[Phil-Friderici]

$computers_ou defaults to -c UNSET in exec {vasinst} resource which looks wrong.
Double check with VAS people.

[Phil-Friderici]

Change all parameter default values from 'UNSET' to undef.
will break backward compatibilty

[Phil-Friderici]

upm-computerou-attr should be unset by default, see #110

[anders-larsson]

Maybe we should consider to change default values for group-update-mode and root-update-mode to force-if-missing instead of none. This is default according to vas.conf's man pages.

...
group-update-mode = < force | force-if-missing | none >
   Default value: force-if-missing
...
root-update-mode = <force | force-if-missing | none>
   Default value: force-if-missing
...

[Phil-Friderici]

From @anders-larsson in #139 (comment)

IMHO: We should be able to remove all _hiera_merge parameters. This can be handled directly in hiera1 instead of in the Puppet module in newer versions of Hiera.

[anders-larsson]

@Phil-Friderici
Just realised that we have made a major release since this issue was created. I think most things mentioned here should be fixed already.

$computers_ou defaults to -c UNSET in exec {vasinst} resource which looks wrong. Double check with VAS people.

This one is interesting. It's set to undef now by default. However there is no check to verify whether is set or not. Finally during vas install we use -c ${computers_ou}. If it is undef wouldn't that become -c . This must be a syntax error?

Maybe we should consider to change default values for group-update-mode and root-update-mode to force-if-missing instead of none. This is default according to vas.conf's man pages.

...
group-update-mode = < force | force-if-missing | none >
   Default value: force-if-missing
...
root-update-mode = <force | force-if-missing | none>
   Default value: force-if-missing
...

{group,user}-update-mode is not done. We still might want it.

---

I just realised in api_fetch we uses OpenSSL.OpenSSL::SSL::VERIFY_NONE. We really do not want this. Currently working on a refactor of the API function to new format. Will include a parameter to handle it, default would be true though. This would probably require a new major release?

EDIT: -c is not required. However currently it is broken in the module. Can't see that it would accept -c (no arguments to it

[Phil-Friderici]

Well, after the major release is before the major release ;)

{group,user}-update-mode is not done. We still might want it.

Currently working on a refactor of the API function to new format. Will include a parameter to handle it, default would be true though. This would probably require a new major release?

Both good arguments for the next major release.
Should I help somehow ?

[anders-larsson]

Right now I don't think any help is needed. Thanks though!

Should we make another major release then which includes:

• Update default value for update-mode stuff
• Either parameter regarding TLS validation or TLS validation always enabled

[Phil-Friderici]

Can't we wait for the breaking changes to become complete and release them in one go (and one major upgrade) ?
Otherwise version numbers are cheap !

[anders-larsson]

Do we have anything else we want to break?

[Phil-Friderici]

Currently I have nothing in mind, but I did not dive in deeply.

[anders-larsson]

Refresh facts and make facts which are currently either comma- or space-separated to be actual arrays instead? That is, make use of data structures. This would be breaking.

[Phil-Friderici]

Refresh facts and make facts which are currently either comma- or space-separated to be actual arrays instead? That is, make use of data structures. This would be breaking.

That's a great idea to be integrated into a breaking change ! (making notes in my brain)