Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: update integration tests that expect docker manifest format from distroless image #3965

Merged
merged 7 commits into from
Mar 24, 2023

Conversation

emmileaf
Copy link
Contributor

@emmileaf emmileaf commented Mar 23, 2023

Fixes #3963 to unblock CI (See issue for more error details and discussion).

Changes in this PR to address manifest format change in latest distroless image:

  • Updates BlobCheckerIntegrationTest and BlobPullerIntegrationTest by using a fixed older version of the distroless image
  • Removes use of “latest” tag in ManifestPullerIntegrationTest, and extends it to specifically cover OCI format using pinned image versions

@emmileaf emmileaf force-pushed the integration-test-update-image branch from 465278a to ab1697a Compare March 23, 2023 17:19
Assert.assertEquals(2, manifestTargeted.getSchemaVersion());
// Ensures call to image at KNOWN_OCI_INDEX_SHA returns an OCI index
OciIndexTemplate manifestListTargeted =
registryClient.pullManifest(KNOWN_OCI_INDEX_SHA, OciIndexTemplate.class).getManifest();
Copy link
Contributor Author

@emmileaf emmileaf Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alt: instead of KNOWN_OCI_INDEX_SHA, this newly added test can keep using the "latest" tag (which keeps it susceptible for breakage but able to detect future changes in this image, as noted in #3963 (comment)).

@emmileaf emmileaf marked this pull request as ready for review March 23, 2023 19:34
@emmileaf emmileaf requested review from a team and mpeddada1 March 23, 2023 19:36
Copy link
Contributor

@mpeddada1 mpeddada1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @emmileaf! Left some minor comments but otherwise LGTM!

Comment on lines 141 to 149
Assert.assertEquals(2, manifestListTargeted.getSchemaVersion());
Assert.assertTrue((manifestListTargeted.getManifests().size() > 0));

// Generic call to image at KNOWN_OCI_INDEX_SHA, should also return an OCI index
ManifestTemplate manifestListGeneric =
registryClient.pullManifest(KNOWN_OCI_INDEX_SHA).getManifest();
Assert.assertEquals(2, manifestListGeneric.getSchemaVersion());
MatcherAssert.assertThat(manifestListGeneric, CoreMatchers.instanceOf(OciIndexTemplate.class));
Assert.assertTrue(((OciIndexTemplate) manifestListGeneric).getManifests().size() > 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Perhaps we can use Truth here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpeddada1 Thanks for the suggestion! Updated assertions across this file to use Truth.assertThat where applicable.

@@ -80,31 +112,41 @@ public void testPull_v22ManifestList() throws IOException, RegistryException {
RegistryClient.factory(EventHandlers.NONE, "gcr.io", "distroless/base", httpClient)
.newRegistryClient();

// Ensure ":latest" is a manifest list
// Ensures call to image at KNOWN_MANIFEST_LIST_SHA returns a manifest list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: how about we say something like "at the specified sha`? This can prevent the comment from becoming stale if the variable name changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, updated!

@sonarcloud
Copy link

sonarcloud bot commented Mar 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix integration tests that use latest distroless image and expect docker manifest format
2 participants