Skip to content

(PUP-6723) collector_transformer: negation query for array type properties is broken#5290

Closed
isaiahfrantz wants to merge 4 commits intopuppetlabs:masterfrom
isaiahfrantz:master
Closed

(PUP-6723) collector_transformer: negation query for array type properties is broken#5290
isaiahfrantz wants to merge 4 commits intopuppetlabs:masterfrom
isaiahfrantz:master

Conversation

@isaiahfrantz
Copy link

(PUP-6723) collector_transformer: negation query for array type properties is broken

Only in the == case is the query property getting tested to see if it is an array which then uses the compare_operator.include? method. The != case only uses compare_operator.equals which makes it so you cant do negated queries on array type properties like tags :(

With this test code:

$ cat t.pp
@notify {'This is a test 1':
tag => 'one'
}
@notify {'This is a test 2':
tag => 'two'
}
@notify {'This is a test 3':
tag => ['one', 'two']
}
@notify {'This is a test 4':
tag => 'three'
}

Adding these at the end would all work as expected

Notify <| tag = 'one' |>
Notify <| tag = 'two' |>
Notify <| tag = 'one' or tag = 'two' |>
Notify <| tag = 'one' and tag = 'two' |>

The negated versions would not

Notify <| tag != 'one' |>
Notify <| tag != 'two' |>
Notify <| !tag = 'one' or !tag = 'two' |>
Notify <| !tag = 'one' and !tag = 'two' |>

The first should only return 2 and 4 but also returns 3:

$ puppet apply t.pp
Notice: Compiled catalog for defiant.cequintecid.com in environment production in 0.08 seconds
Notice: This is a test 2
Notice: /Stage[main]/Main/Notify[This is a test 2]/message: defined 'message' as 'This is a test 2'
Notice: This is a test 3
Notice: /Stage[main]/Main/Notify[This is a test 3]/message: defined 'message' as 'This is a test 3'
Notice: This is a test 4
Notice: /Stage[main]/Main/Notify[This is a test 4]/message: defined 'message' as 'This is a test 4'
Notice: Applied catalog in 0.03 seconds

The second should only return 1 and 4 but also returns 3:

$ puppet apply t.pp
Notice: Compiled catalog for defiant.cequintecid.com in environment production in 0.07 seconds
Notice: This is a test 1
Notice: /Stage[main]/Main/Notify[This is a test 1]/message: defined 'message' as 'This is a test 1'
Notice: This is a test 3
Notice: /Stage[main]/Main/Notify[This is a test 3]/message: defined 'message' as 'This is a test 3'
Notice: This is a test 4
Notice: /Stage[main]/Main/Notify[This is a test 4]/message: defined 'message' as 'This is a test 4'
Notice: Applied catalog in 0.03 seconds

The "or" case should only return 1,2, and 4. It should not return 3 but does:

$ puppet apply t.pp
Notice: Compiled catalog for defiant.cequintecid.com in environment production in 0.08 seconds
Notice: This is a test 1
Notice: /Stage[main]/Main/Notify[This is a test 1]/message: defined 'message' as 'This is a test 1'
Notice: This is a test 2
Notice: /Stage[main]/Main/Notify[This is a test 2]/message: defined 'message' as 'This is a test 2'
Notice: This is a test 3
Notice: /Stage[main]/Main/Notify[This is a test 3]/message: defined 'message' as 'This is a test 3'
Notice: This is a test 4
Notice: /Stage[main]/Main/Notify[This is a test 4]/message: defined 'message' as 'This is a test 4'
Notice: Applied catalog in 0.03 seconds

The "and" case should only return 4. It should not return 3 but it does:

$ puppet apply t.pp
Notice: Compiled catalog for defiant.cequintecid.com in environment production in 0.07 seconds
Notice: This is a test 3
Notice: /Stage[main]/Main/Notify[This is a test 3]/message: defined 'message' as 'This is a test 3'
Notice: This is a test 4
Notice: /Stage[main]/Main/Notify[This is a test 4]/message: defined 'message' as 'This is a test 4'
Notice: Applied catalog in 0.03 seconds

@isaiahfrantz isaiahfrantz changed the title treat resource properties that are arrays properly in the != case like they are in the == case (PUP-6723) collector_transformer: negation query for array type properties is broken Sep 22, 2016
@puppetcla
Copy link

Waiting for CLA signature by @isaiahfrantz

@isaiahfrantz - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppet.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppet.com/community/trivial_patch_exemption.html

@puppetcla
Copy link

CLA signed by all contributors.

@hlindberg
Copy link
Contributor

Thanks for the contribution and discussion on the ticket. As noted there, I will close this in favor for instead adding the ability to use the in operator in queries.

@hlindberg hlindberg closed this Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants