Fix V22ManifestListTemplate cast when pulling an oci index manifest #3974

rquinio · 2023-03-30T10:48:07Z

Since there's no more downcast in PullBaseImageStep of manifest lists actual implementation, use the parent class in the unit tests.
Also update jib-core gradle example to compile with 0.23.0.

I was able to reproduce via a gradle build using jib-core (from: gcr.io/distroless/java17-debian11:nonroot@sha256:5154596f58d81687d66c8302face284198caf65033cde6bf6667c488b21745e6) and verify the fix works. Though I'm not sure why the issue wasn't caught by the new integration test cases added in #3965 ?

Before filing a pull request, make sure to do the following:

Create a new issue at https://github.com/GoogleContainerTools/jib/issues/new/choose.
Ensure that your implementation plan is approved by the team.
Verify that integration tests and unit tests are passing after the change.
Address all checkstyle issues. Refer to the style guide.

This helps to reduce the chance of having a pull request rejected.

emmileaf

Just one small typo typo fix, otherwise LGTM. Thank you for the contribution!

On your note about integration tests, I think the ones added in #3965 didn't catch this issue because it is in the caching logic and will not be reached (and fail) on the first time pulling. It would be nice to have additional coverage in the integration tests for this (perhaps by having a test using OciIndexTemplate that pulls twice and forces cache with Djib.alwaysCacheBaseImage=true?), but not a blocker for merging this fix.

jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStepTest.java

rquinio · 2023-04-03T19:42:24Z

Regarding the integration test I've realized ManifestPullerIntegrationTest only tests the RegistryClient, not the actual Jib build steps.
I think it's missing a case in JibIntegrationTest that would pull an oci index.
Today it's targeting only docker.io busybox image which is a V22 Manifest list.
I'll have a look later in the week.

Since there's no more downcast in PullBaseImageStep of manifest lists actual implementation, use the parent class in the unit tests. Add an integration-test case of building a multi-platform image from a base oci index manifest. Also update jib-core gradle example to compile with 0.23.0.