fix(oci): oci downloader unavailable missing WithBundleParserOpts method #6571
Conversation
There was a problem hiding this comment.
Thanks for working on this @slonka! Instead of introducing an interface if we just implemented WithBundleParserOpts in the no OCI downloader this should work, correct? If yes, we can just do that and look into adding a new interface if needed in the future. WDYT?
Also can you please look into the test failures. Thanks.
Yeah, it's not strictly needed but it might show up in some IDEs that a method is not implemented on this struct. A definite guard is what I've added in CI. I'll remove and we can add it in the future. |
…navailable Signed-off-by: slonka <slonka@users.noreply.github.com>
slonka
force-pushed
the
fix-oci-download-unavailable
branch
from
677fdd8
to
3f7850f
Compare
|
@ashutosh-narkar squashed, can this be merged? |
|
@ashutosh-narkar do you think we could get this released soon-ish? |
|
@slonka we have a monthly release cadence. We'll have an OPA release end of this month. |
Why the changes in this PR are needed?
In #6159 I introduced a disabled OCI downloader to get rid of dependency on containerd to minimise the CVE surface. https://github.com/open-policy-agent/opa/pull/6521/files#diff-f808512360f39e5c4f67342ca24d84117be5403637b032d67e82df3a425cdde9R139 broke our builds adding a method only to one of the implementations.
What are the changes in this PR?
In this PR I added an interface (should show up in IDE) and a CI check (makes sure building
make GO_TAGS="-tags=opa_no_oci" buildworks) to see if everything builds withGO_TAGS="-tags=opa_no_oci".Notes to assist PR review:
There supposed to be a long term solution for (getting rid of containerd) this but it seems that it was not implemented.
Further comments:
Hope you don't mind me doing a PR - if you want to move this to an issue and discuss - feel free to do so, or tell me to do it :)