Knife is unable to search for attributes with forward slashes

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 0.10.4, 11.0.0, 11.0.4-server
  • Fix Version/s: 11.0.6-server
  • Component/s: Chef Server, Knife
  • Labels:
    None

Description

If you create an attribute such as mount:/foo, you are unable to search for it with knife without the star '*' wildcard.

Repro steps:
1. Create a node attribute with the name mount, and put /foo in it
2. Try searching for the node by the attribute. I tried a few different ways, only using the '*' wildcard worked:

$ knife search node "mount:*foo"
1 items found
$ knife search node "mount:?foo"
ERROR: knife search failed: invalid search query: 'mount:?foo'
$ knife search node "mount:/foo"
ERROR: knife search failed: invalid search query: 'mount:/foo'
$ knife search node "mount:\/foo"
ERROR: knife search failed: invalid search query: 'mount:\/foo'

Related ticket: http://help.opscode.com/discussions/questions/746-how-to-search-for-mount

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mike Javorski added a comment -

I've run into this issue as well. It turns out to be related to the query parsing that is done in the Chef API server prior to sending to Solr. As a result, tt effects all Chef search usage – via knife search as well as search(). Reading the Solr docs as well as the Chef docs I have come to the following conclusions:

  • '/' is a valid character with respect to Solr's query system
  • '/' is not expressly forbidden in the Chef documentation
  • '/' is not a special character, and thus escaping it is not required
  • '?' IS a special character, and is expressly forbidden for usage at the beginning of a search string (see http://wiki.opscode.com/display/chef/Search#Search-WildcardMatches

I've successfully patched my own chef server to support '/' as a valid character. I am mulling over what is needed to submit this as a pull-request for the chef project.

Show
Mike Javorski added a comment - I've run into this issue as well. It turns out to be related to the query parsing that is done in the Chef API server prior to sending to Solr. As a result, tt effects all Chef search usage – via knife search as well as search(). Reading the Solr docs as well as the Chef docs I have come to the following conclusions: '/' is a valid character with respect to Solr's query system '/' is not expressly forbidden in the Chef documentation '/' is not a special character, and thus escaping it is not required '?' IS a special character, and is expressly forbidden for usage at the beginning of a search string (see http://wiki.opscode.com/display/chef/Search#Search-WildcardMatches I've successfully patched my own chef server to support '/' as a valid character. I am mulling over what is needed to submit this as a pull-request for the chef project.
Hide
Permalink
Mike Javorski added a comment -

Unfortunately, due to the legal doc signatures required, it may be some time before my employer is able to sign a corporate contributor agreement, which I believe may be required due to IP rights. If someone not so impaired would be able to contribute a fix for the issue, that would be appreciated I'm sure.

It's just a matter of updating start_letter, end_letter and valid_letter rules in lucene.treetop

Show
Mike Javorski added a comment - Unfortunately, due to the legal doc signatures required, it may be some time before my employer is able to sign a corporate contributor agreement, which I believe may be required due to IP rights. If someone not so impaired would be able to contribute a fix for the issue, that would be appreciated I'm sure. It's just a matter of updating start_letter, end_letter and valid_letter rules in lucene.treetop
Hide
Permalink
Mike Javorski added a comment -

Pull request is: https://github.com/opscode/chef/pull/308

Show
Mike Javorski added a comment - Pull request is: https://github.com/opscode/chef/pull/308
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Mike, this looks good, but could you add a unit test for this use case and then re-resolve please?

Show
Bryan McLellan [Opscode] added a comment - Mike, this looks good, but could you add a unit test for this use case and then re-resolve please?
Hide
Permalink
Mike Javorski added a comment - - edited

Bryan:

While I can appreciate the desire to test special cases, I do not believe one applies in this instance. The change was simply to include a character which was missed in the permitted character list.

It would seem to be require an integration test where you would confirm that a query using the character fails to produce errors through all the systems. That seems very excessive though. I would welcome suggestions as to how I can provide what you are looking for, but at the moment, I am at a loss.

Show
Mike Javorski added a comment - - edited Bryan: While I can appreciate the desire to test special cases, I do not believe one applies in this instance. The change was simply to include a character which was missed in the permitted character list. It would seem to be require an integration test where you would confirm that a query using the character fails to produce errors through all the systems. That seems very excessive though. I would welcome suggestions as to how I can provide what you are looking for, but at the moment, I am at a loss.
Hide
Permalink
Mike Javorski added a comment -

I had given up on the ruby Chef fix due to lack of response, but I've made a fix for erchef (chef_index) which has the same issue.

Pull Request: https://github.com/opscode/chef_index/pull/7

Show
Mike Javorski added a comment - I had given up on the ruby Chef fix due to lack of response, but I've made a fix for erchef (chef_index) which has the same issue. Pull Request: https://github.com/opscode/chef_index/pull/7
Hide
Permalink
Mike Javorski added a comment -

I can't seem to work out how to mark this as resolved but not closed. Apologies if closing it was the wrong action.

Show
Mike Javorski added a comment - I can't seem to work out how to mark this as resolved but not closed. Apologies if closing it was the wrong action.
Hide
Permalink
Seth Falcon added a comment -

This is awesome. Thanks for putting together a nice PR. We will do some review and testing and get this merged as soon as we can.

Show
Seth Falcon added a comment - This is awesome. Thanks for putting together a nice PR. We will do some review and testing and get this merged as soon as we can.
Hide
Permalink
Seth Falcon added a comment -

Merged to chef_index repo in 639be0930245586f9ff6bb845b51d4891dc38b83

Show
Seth Falcon added a comment - Merged to chef_index repo in 639be0930245586f9ff6bb845b51d4891dc38b83

People

  • Assignee:
    Mike Javorski
    Reporter:
    Jessica Bourne
Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: