Project

General

Profile

Actions

User story #7006

closed

Parameter validation in Rudder should accept rudder variables ( ${ } )

Added by Nicolas CHARLES over 8 years ago. Updated almost 4 years ago.

Status:
Released
Priority:
N/A
Category:
Web - Config management
Target version:
UX impact:
Suggestion strength:
Require - I need this to use Rudder as I intend
User visibility:
Getting started - demo | first install | Technique editor and level 1 Techniques
Effort required:
Very Small
Name check:
Fix check:
Checked
Regression:

Description

The fileds with regexp validation are validating exactly the text we pass. However, they do not accept the ${ }, which could contain proper value. It simply prevent us from using variablles altogether


Subtasks 1 (0 open1 closed)

Bug #17307: Improve parsing of non rudder variable in directives fields.ReleasedFrançois ARMANDActions

Related issues 4 (0 open4 closed)

Related to Rudder - Bug #16725: Directive 'Processes' Number of processes cannot be a propertyRejectedActions
Related to Rudder - Bug #11449: Allow variable as directive parameters in regexp validated inputs ReleasedFrançois ARMANDActions
Related to Rudder - Bug #17334: Property name aren't limited to asciiReleasedVincent MEMBRÉActions
Related to Rudder - Bug #17842: it's impossible to use parameters in technique editor when there are constraints as it doesn't match the regexResolvedActions
Actions #1

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.11.13 to 2.11.14
Actions #2

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.11.14 to 2.11.15
Actions #3

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.11.15 to 2.11.16
Actions #4

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.11.16 to 2.11.17
Actions #5

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.11.17 to 2.11.18
Actions #6

Updated by Vincent MEMBRÉ about 8 years ago

  • Target version changed from 2.11.18 to 2.11.19
Actions #7

Updated by Vincent MEMBRÉ about 8 years ago

  • Target version changed from 2.11.19 to 2.11.20
Actions #8

Updated by Vincent MEMBRÉ almost 8 years ago

  • Target version changed from 2.11.20 to 2.11.21
Actions #9

Updated by Vincent MEMBRÉ almost 8 years ago

  • Target version changed from 2.11.21 to 2.11.22
Actions #10

Updated by Vincent MEMBRÉ almost 8 years ago

  • Target version changed from 2.11.22 to 2.11.23
Actions #11

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 2.11.23 to 2.11.24
Actions #12

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 2.11.24 to 308
Actions #13

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 308 to 3.1.14
Actions #14

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.14 to 3.1.15
Actions #15

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.15 to 3.1.16
Actions #16

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.16 to 3.1.17
Actions #17

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.17 to 3.1.18
Actions #18

Updated by Vincent MEMBRÉ about 7 years ago

  • Target version changed from 3.1.18 to 3.1.19
Actions #19

Updated by Benoît PECCATTE about 7 years ago

  • Tracker changed from Bug to User story
  • Subject changed from Regex validation in Rudder does not accept rudder variables ( ${ } ) to Regex validation in Rudder should accept rudder variables ( ${ } )
Actions #20

Updated by Vincent MEMBRÉ almost 7 years ago

  • Target version changed from 3.1.19 to 3.1.20
Actions #21

Updated by Vincent MEMBRÉ almost 7 years ago

  • Target version changed from 3.1.20 to 3.1.21
Actions #22

Updated by Vincent MEMBRÉ almost 7 years ago

  • Target version changed from 3.1.21 to 3.1.22
Actions #23

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.22 to 3.1.23
Actions #24

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.23 to 3.1.24
Actions #25

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.24 to 3.1.25
Actions #26

Updated by Benoît PECCATTE over 6 years ago

  • Target version changed from 3.1.25 to 4.1.9
Actions #27

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 4.1.9 to 4.1.10
Actions #28

Updated by Benoît PECCATTE about 6 years ago

  • Target version changed from 4.1.10 to Ideas (not version specific)
Actions #29

Updated by Nicolas CHARLES about 4 years ago

  • Related to Bug #16725: Directive 'Processes' Number of processes cannot be a property added
Actions #30

Updated by François ARMAND about 4 years ago

We should just accept parameter variable, always. Of course they can break afterward, but here we are preventing valid, common usage. Validation (client side validation) should always be just an advice.

We can't either really try to wait for generation and fail there, because some variables can be only defined on agent.

So, I would say:
1/ authorize parameter in regex check
2/ if possible, for variable whose value is known at generation, add back the regex check post expansion.

Another question: how does it works for "can not be empty" variable? Of course, technicaly "${something}" is not the empty string, but its expansion may be. Do we check post expansion correctly? If not, we should (and the regex case is even more inconsistant)

Actions #31

Updated by François ARMAND about 4 years ago

  • Suggestion strength set to Want - This would make my life a lot easier but I can manage without
  • User visibility set to Getting started - demo | first install | Technique editor and level 1 Techniques
  • Effort required set to Very Small
Actions #32

Updated by Alexis Mousset almost 4 years ago

  • Target version changed from Ideas (not version specific) to 6.1.0~beta2
  • Suggestion strength changed from Want - This would make my life a lot easier but I can manage without to Require - I need this to use Rudder as I intend

Let's do this in 6.1, now we have properties everywhere.

Actions #33

Updated by François ARMAND almost 4 years ago

  • Status changed from New to In progress
  • Assignee set to François ARMAND
Actions #34

Updated by François ARMAND almost 4 years ago

  • Related to Bug #11449: Allow variable as directive parameters in regexp validated inputs added
Actions #35

Updated by François ARMAND almost 4 years ago

  • Subject changed from Regex validation in Rudder should accept rudder variables ( ${ } ) to Parameter validation in Rudder should accept rudder variables ( ${ } )

The regex part was done in #11449 but it can happen in any non string comparator, like #11449 for integer (but also boolean, etc).

Actions #36

Updated by François ARMAND almost 4 years ago

Actually, boolean can't be an interpolated variable, because we translate them into checkbox in the UI, and checkbox need to know if it's true or false to display correctly. We could use a special widget that let user enter a value, but it becomes complicated and will be for an other time (I'm not even sure we want to go that way).

Actions #39

Updated by François ARMAND almost 4 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from François ARMAND to Vincent MEMBRÉ
  • Pull Request set to https://github.com/Normation/rudder/pull/2942
Actions #40

Updated by François ARMAND almost 4 years ago

So, to sum, we will be able to have parameter where regex or int are awaited (and string of course), but not boolean because boolean need to be known when the directive is configured to display the checkbox state (and there's no way to enter the property in all cases).

We kept the old behavior, so any ${...} is authorized (to allow cfengine/etc node side expansion), but if the variable is a rudder one (ie: ${rudder.*} or ${node.properties[...]} we DO check that the variable is ok (is defined for rudder ones, is well formed for node properties - we can't check for existence, of course, since we are in a directive, not in nodes).

The corresponding PR also change a technical backend point: the parser lib used to parse properties was switched to a new one: "fastparse".

Actions #41

Updated by Vincent MEMBRÉ almost 4 years ago

  • Target version changed from 6.1.0~beta2 to 6.1.0~beta3
Actions #42

Updated by François ARMAND almost 4 years ago

  • Related to Bug #17334: Property name aren't limited to ascii added
Actions #43

Updated by François ARMAND almost 4 years ago

  • Status changed from Pending technical review to Pending release
Actions #44

Updated by François ARMAND almost 4 years ago

  • Fix check set to To do
Actions #45

Updated by Vincent MEMBRÉ almost 4 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 6.1.0~beta3 which was released today.

Actions #46

Updated by François ARMAND almost 4 years ago

  • Fix check changed from To do to Checked
Actions #47

Updated by François ARMAND over 3 years ago

  • Related to Bug #17842: it's impossible to use parameters in technique editor when there are constraints as it doesn't match the regex added
Actions

Also available in: Atom PDF