runtime-tools icon indicating copy to clipboard operation
runtime-tools copied to clipboard

Problematic dependencies for downstream projects

Open klihub opened this issue 1 year ago • 13 comments

During the review process of the recently merged Kubernetes Dynamic Resource Allocation feature it was pointed out that runtime-tools has a few problems to vendor for otherwise potential downstream projects.

The first problem is the lack of actively tagged releases, which I guess should be relatively easy to solve. The primary concern with the lack of recent tags is that "it's not clear if HEAD of the repo is always expected to be production-worthy".

The remaining problems are related to dependencies which are problematic to inherit downstream, either because of licensing choices (not a CNCF-approved license), or lack of proper maintenance. Those dependencies are considered problematic even if they are isolated within runtime-tools well enough that vendoring runtime-tools typically does not pull any of the actual code from the dependencies downstream. The list of these packages is the following, together with a brief description of the main issues and some potential remedies:

  • github.com/hashicorp/go-multierror:
    • MPL license not in the CNCF license allowlist
    • if not part of the publicly visible API, could be probably dropped/replaced with some work (we've done something similar recently)
  • github.com/hashicorp/errwrap:
    • MPL license, not in the CNCF license allowlist
    • if not part of the publicly visible API, could be probably dropped/replaced with some work
  • github.com/mndrix/tap-go:
    • no releases/tags
    • no changes since 2017
    • repo marked as archived / read-only
    • maintenance status is unclear
    • this is so little code, and in the PD, that it could be probably simply copied over as an internal package
  • github.com/xeipuuv/gojsonschema:
    • no updates since 2020
    • many open issues, some of which indicate lack of maintenance and correctness (fails tests on current go versions) issues
  • github.com/xeipuuv/gojsonpointer:
    • same as for gojsonschema
  • github.com/xeipuuv/gojsonreference:
    • same as for gojsonschema

Folks who contribute to DRA and CDI would be willing to do all the heavy lifting if we can agree what lifting with be acceptable. The hashicorp and tap-go bits look fairly straightforward.

The xeipuuv bits are less so, mostly because there does not seem to be an easy replacement/alternative implementation for JSON-Schema based verification. One possible solution would be to simply split out the JSON Schema validation function from validate into a new repo, say opencontainers/[runtime]-spec-validation. This would be a backward-incompatible change as anyone using schema-based validation would need then to start importing that repo.

klihub avatar Nov 17 '22 16:11 klihub

/cc @zvonkok

zvonkok avatar Nov 18 '22 10:11 zvonkok

/cc @mrunalp @kolyshkin

klihub avatar Nov 21 '22 20:11 klihub

@giuseppe PTAL

rhatdan avatar Nov 21 '22 21:11 rhatdan

@rhatdan @giuseppe have you had a chance to look into this issue? As @klihub mentioned we'll try to do most of the heavy lifting from our side but would require the buy-in from the maintainers.

elezar avatar Nov 29 '22 15:11 elezar

as much as I like your plan, I am not a maintainer to ack it: https://github.com/opencontainers/runtime-tools/blob/master/MAINTAINERS

I think it is better if any maintainer will ack it first

giuseppe avatar Nov 29 '22 17:11 giuseppe

Thanks @kolyshkin since you're the latest addition approved by @vbatts and @tianon would you take a look at the proposal?

elezar avatar Nov 29 '22 18:11 elezar

I am :+1: on attempting to fix these issues. Thanks @klihub!

mrunalp avatar Dec 01 '22 00:12 mrunalp

Just an update/note related to eliminating github.com/hashicorp/go-multierror from dependencies: golang 1.20 has now support for wrapping multiple errors. This hopefully removes the need to rely on an external or internal multierror-like package altogether.

klihub avatar Feb 07 '23 15:02 klihub

I willing to help with this work. I see that go version is 1.16 here, and for multierror dependency dropping we could use go's native support for multi error wrapping but that requires bumping the version to at least 1.20. Would folks here be okay with that change as well? /cc @mrunalp @kolyshkin

fmuyassarov avatar Mar 01 '23 09:03 fmuyassarov

/assign

fmuyassarov avatar Mar 29 '23 08:03 fmuyassarov