search

Ericsson / puppet-module-vas

Puppet module to manage DELL Authentication Services previously known as VAS or Quest Authentication Services
Apache License 2.0
4 stars 26 forks source link

Next major version changes #103

Open Phil-Friderici opened 8 years ago

Phil-Friderici commented 8 years ago

Collection of planned changes for next major version

Phil-Friderici commented 8 years ago

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

Phil-Friderici commented 8 years ago

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

Phil-Friderici commented 7 years ago

upm-computerou-attr should be unset by default, see https://github.com/Ericsson/puppet-module-vas/pull/110

anders-larsson commented 5 years ago

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 commented 2 years ago

From @anders-larsson in https://github.com/Ericsson/puppet-module-vas/pull/139#issuecomment-1141153201

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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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

Should we make another major release then which includes:

Phil-Friderici commented 9 months ago

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 commented 9 months ago

Do we have anything else we want to break?

Phil-Friderici commented 9 months ago

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

anders-larsson commented 9 months ago

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 commented 9 months ago

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)