search

operator-framework / operator-courier

Build, verify and push operators
Apache License 2.0
41 stars 53 forks source link

Remove SemVer requirement on nested folders #116

Closed kevinrizza closed 5 years ago

kevinrizza commented 5 years ago

Problem: Technically, folder names need not match the version name in a manifest directory. Enforcing this is just convention. We should remove this.

Solution: Remove the function and lambda from the verified_manifest class that generates the dictionary needed to parse these files.

Additionally, update the tests to include a case when the folders are not named with semver.

Also fix some directories that cannot be parsed. An artifact of this pr is that flat directories now cannot include folders that should be ignored. This will be addressed in a separate pr.

jeremy-wl commented 5 years ago

/lgtm

MartinBasti commented 5 years ago

Should be this change reflected in flatten command?

kevinrizza commented 5 years ago

Should be this change reflected in flatten command?

@MartinBasti @jeremylinlin

Agreed. The problem is that this change was a strategic one to get one operator unblocked. I don't think this is actually the right way to implement this change because it also introduced the problem that now, if a random unrelated folder is in the same folder as the manifests, it will attempt to parse whatever junk is in there and break. What we actually need to do is replace the semver check with a check to see if the contents of each folder include data for a valid manifest.

We also need to do this kind of checking before we push. Ideally all the noise gets filtered in all cases and all we verify and push are files related to the manifests. Since this required some more effort and thought than what was done here, we opted for the minimum viable change for now and we will revisit this soon.

ralphbean commented 5 years ago

Hm, that operator is still blocked - since we call flatten on it in OMPS and that's exactly the case we're trying to unblock here.