Skip to content

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

Merged
emmileaf merged 7 commits intomasterfrom
integration-test-update-image
Mar 24, 2023
Merged

ci: update integration tests that expect docker manifest format from distroless image#3965
emmileaf merged 7 commits intomasterfrom
integration-test-update-image

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.

.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!

@sonarqubecloud
Copy link

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