voxpupuli / metadata-json-lint

Tool to check the validity of Puppet metadata.json files
Apache License 2.0
29 stars 27 forks source link

Illegal formed requirement not detected #97

Closed rmestrum closed 6 years ago

rmestrum commented 6 years ago

A version_requirement of a dependency that is illegal formed is not detected.

{
  "name": "test-testmodule",
  "version": "3.0.0",
  "author": "test",
  "summary": "",
  "license": "Apache-2.0",
  "source": "",
  "project_page": "",
  "issues_url": "",
  "dependencies": [
    {
      "name": "herculesteam-augeasproviders_ssh", "version_requirement": ">=2.5.3  <3.0.0"
    }
  ],
  "operatingsystem_support": []
}

Between the lower and upper bound of the version requirement are 2 spaces. When running metadata-json-lint no errors are displayed. So this gem approves the metadata.json file.

When running librarian-puppet the following error occurs: /usr/share/rubygems/rubygems/requirement.rb:101:in `parse': Illformed requirement [">=2.5.3 < 3.0.0"] (Gem::Requirement::BadRequirementError)

ekohl commented 6 years ago

I'm not sure if that's a bug here or in librarian-puppet. https://puppet.com/docs/puppet/5.1/modules_metadata.html#version-specifiers-in-module-metadata doesn't mention spacing at all so that's not very useful. This gem uses semantic_puppet which is also used by puppet internally so I'd consider it the defacto spec. Since that allows it I'm leaning to considering it a bug in librarian-puppet instead.

That said, I'd welcome a PR that warns about this.

rnelson0 commented 6 years ago

@rmestrum Obviously you're using a test module, but have you found that in a real life module on the forge? The concern (among others) with metadata json lint is to make sure that valid json is uploaded that can be used with the forge when someone makes a forge API call to download a module and receive the dependency information properly.

ekohl commented 6 years ago

@rnelson0 it could also be an internal module that's consumed from git.

rmestrum commented 6 years ago

It is indeed a private module that we use and it's consumed from git. The code I provided is only for clarification.

rmestrum commented 6 years ago

I've seen that there's a new version of librarian-puppet. I give that one a try.

rmestrum commented 6 years ago

I've tracked the root cause to the the librarianp gem (https://github.com/voxpupuli/librarian/blob/librarianp/lib/librarian/dependency.rb#L128)

The regexp in this method takes only 1 space. So it't not an issue in the metadata-json-lint gem. Closing this issue.