Bug 52847 - UCR Scripts: Variables passed are not "layer aware"
UCR Scripts: Variables passed are not "layer aware"
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UCR
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0
Assigned To: Philipp Hahn
Dirk Wiesenthal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2021-02-24 20:56 CET by Dirk Wiesenthal
Modified: 2021-05-25 16:02 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Wiesenthal univentionstaff 2021-02-24 20:56:42 CET
You may define scripts that are triggered when a UCRV changes.

The documentation says, it is called with argv[1] == "generate" and stdin.read() == """key1@%@old@%@new
key2@%@old@%@new
key3@%@old@%@new"""

But if you do that:

ucr set --force key1='forced'
ucr set key1=old
ucr set key1=new

you get this for your script:

key1@%@old@%@new

Both values are wrong. In fact, there was no change at all, why bother triggering?

But in general, the values passed are not layer aware. This is also the case if you unset a variable that has a default value. The new value is considered None, although it should be the default value.

See Bug#52846 why this leads to no stdin at all.

If you really want to work with the values in the script, you cannot rely on the lines passed in stdin.
Comment 1 Philipp Hahn univentionstaff 2021-03-04 11:22:43 CET
f73f77ea3a..e589e6b7b5
[5.0-0] e589e6b7b5 fix[ucr]: Call handlers only for changed variables
 base/univention-config-registry/debian/changelog                              |  6 ++++++
 base/univention-config-registry/python/univention/config_registry/frontend.py | 13 ++++++++-----
 base/univention-config-registry/tests/test_frontend.py                        |  8 ++++++--
 3 files changed, 20 insertions(+), 7 deletions(-)

Package: univention-config-registry
Version: 15.0.7-3A~5.0.0.202103040928
Branch: ucs_5.0-0

QA:
ucr set test/key=init
ucr set --forced test/key=forced
ucr set test/key=normal
OK: module
OK: script
Comment 2 Dirk Wiesenthal univentionstaff 2021-03-11 04:14:36 CET
I still get changes that I would consider wrong:

ucr set key1=normal
ucr set --force key1='forced'

This will give a UCR module the following:
1. {"key1": (None, "normal")}
2. {"key1": (None, "forced")}

2. I would expect ("normal", "forced"). Likewise if I now unset the --forced value: I get ("forced", None) instead of ("forced", "normal").


Apart from that:
Setting key1 without --force will now not trigger anything -> OK
Comment 3 Philipp Hahn univentionstaff 2021-03-14 07:53:41 CET
"Brown paper bug": Used `old_value` instead of `value`.

[5.0-0] 6535f14089 fix[ucr]: Call handlers with old visible value
 base/univention-config-registry/debian/changelog                |  6 +++++
 .../python/univention/config_registry/frontend.py               |  4 +--
 base/univention-config-registry/tests/test_frontend.py          | 38 +++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

Adding a proper test-case required some refacturing and modifications to the existing tests, "now better than ever".

[5.0-0] 35d8acbda6 test[ucr]: Change FORCED to LDAP layer
 base/univention-config-registry/tests/conftest.py      |  6 +++---
 base/univention-config-registry/tests/test_backend.py  | 20 ++++++++++----------
 base/univention-config-registry/tests/test_frontend.py | 18 +++++++++---------
 base/univention-config-registry/tests/test_handler.py  |  4 ++--
 4 files changed, 24 insertions(+), 24 deletions(-)

[5.0-0] 6628c8e4c2 test[ucr]: Make test layer configurable
 base/univention-config-registry/tests/conftest.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

[5.0-0] c629657b00 refactor[ucr]: asciify UCR variable
 .../python/univention/config_registry/handler.py                | 37 +++++++++++++----------------
 base/univention-config-registry/tests/test_handler.py           | 16 ++++++-------
 2 files changed, 23 insertions(+), 30 deletions(-)

[5.0-0] 9cc04702f0 refactor[ucr]: asciify
 base/univention-config-registry/python/univention/config_registry/misc.py | 13 +++++++++++--
 base/univention-config-registry/tests/test_misc.py                        |  9 +++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

[5.0-0] 465afa11ef refactor[ucr]: Split run_filter
 .../python/univention/config_registry/handler.py                     | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

[5.0-0] 119b5fa0fc fix[ucr]: PEP 484 type annotation
 .../python/univention/config_registry/backend.py                | 56 ++++++-----------------------
 .../python/univention/config_registry/frontend.py               |  6 ++--
 .../python/univention/config_registry/handler.py                |  4 +--
 3 files changed, 16 insertions(+), 50 deletions(-)


Package: univention-config-registry
Version: 15.0.7-5A~5.0.0.202103140750
Comment 4 Philipp Hahn univentionstaff 2021-03-16 07:34:09 CET
Still not correct
Comment 6 Philipp Hahn univentionstaff 2021-03-16 18:45:27 CET
[5.0-0] 0f613f600c fix[ucr]: Call handlers with old&new visible value
 base/univention-config-registry/debian/changelog                       |  6 ++++++
 .../python/univention/config_registry/frontend.py                      | 14 ++++++++++----
 base/univention-config-registry/tests/test_frontend.py                 | 22 ++++++++++++++--------
 3 files changed, 30 insertions(+), 12 deletions(-)

[5.0-0] cff4b65193 fix[ucr]: Change layer priorities
 base/univention-config-registry/python/univention/config_registry/backend.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

[5.0-0] 74545cd352 test[ucr]: Convert ucs-test to unit-test: bool
 base/univention-config-registry/tests/test_backend.py | 16 ++++++---
 test/ucs-test/tests/03_ucr/70_ucr_true_false_values   | 70 ---------------------------------------
 2 files changed, 12 insertions(+), 74 deletions(-)

[5.0-0] 5f3fe4f2b1 test[ucr]: Convert ucs-test to unit-test: search
 base/univention-config-registry/tests/test_frontend.py | 27 ++++++++++++--------
 test/ucs-test/tests/03_ucr/62ucr_search                | 51 --------------------------------------
 2 files changed, 17 insertions(+), 61 deletions(-)

[5.0-0] 986a570c00 test[ucr]: Convert ucs-test to unit-test: keys
 base/univention-config-registry/tests/test_misc.py |  8 +++++-
 test/ucs-test/tests/03_ucr/60check_ucr_variables   | 60 ------------------------------------------
 test/ucs-test/tests/03_ucr/65ucr_checkdots         | 53 -------------------------------------
 3 files changed, 7 insertions(+), 114 deletions(-)

[5.0-0] 0e8d13d89a test[ucr]: Convert ucs-test to unit-test: conditional
 base/univention-config-registry/tests/test_frontend.py |  2 ++
 test/ucs-test/tests/03_ucr/53ucs_set_layer             | 61 --------------------------------------
 2 files changed, 2 insertions(+), 61 deletions(-)

[5.0-0] 90297e831b test[ucr]: Convert ucs-test to unit-test: layering
 base/univention-config-registry/tests/test_backend.py | 15 ++++++++
 test/ucs-test/tests/03_ucr/50layer-priority           | 72 ---------------------------------------
 2 files changed, 15 insertions(+), 72 deletions(-)

[5.0-0] 1ecd785fbe test[ucr]: Shellcheck ucs-test/03_ucr/*
 test/ucs-test/tests/03_ucr/35check-ucr-templates      | 1 +
 test/ucs-test/tests/03_ucr/36check-modified-templates | 8 +++++---
 test/ucs-test/tests/03_ucr/50layer-priority           | 3 ++-
 test/ucs-test/tests/03_ucr/51file_permissions         | 4 +++-
 test/ucs-test/tests/03_ucr/52atomic_ucr               | 1 +
 test/ucs-test/tests/03_ucr/53ucs_set_layer            | 2 ++
 test/ucs-test/tests/03_ucr/60check_ucr_variables      | 1 +
 test/ucs-test/tests/03_ucr/60ucr_dump                 | 6 ++++--
 test/ucs-test/tests/03_ucr/61ucr_set                  | 5 +++--
 test/ucs-test/tests/03_ucr/62ucr_search               | 4 +++-
 ...
 16 files changed, 47 insertions(+), 28 deletions(-)

[5.0-0] 6e9fd5d595 fix[ucr]: typing error in handler_get
 base/univention-config-registry/python/univention/config_registry/frontend.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

[5.0-0] aa1ae4de9a fix[ucr]: more PEP 484 typing hints
 base/univention-config-registry/python/univention/config_registry/backend.py  | 12 ++++++------
 base/univention-config-registry/python/univention/config_registry/frontend.py |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

Package: univention-config-registry
Version: 15.0.7-7A~5.0.0.202103161838

OK: ucs-test -E dangerous -c -s ucr -F raw
Comment 7 Dirk Wiesenthal univentionstaff 2021-04-15 10:39:08 CEST
Okay, all combinations I could think of now give reasonable arguments for UCR modules.
Comment 8 Florian Best univentionstaff 2021-05-25 16:02:26 CEST
UCS 5.0 has been released:
 https://docs.software-univention.de/release-notes-5.0-0-en.html
 https://docs.software-univention.de/release-notes-5.0-0-de.html

If this error occurs again, please use "Clone This Bug".