Fixes for run image extension #1134

natalieparellano · 2023-06-28T17:04:41Z

When pulling remote image data during restore, fail if the remote image is not found
When validating dockerfiles during generate, set extend to true if there are any instructions (vs more than one instruction)
Update matching logic when considering if two image names are equivalent to ignore the digest portion of the reference if present (for the purpose of selecting data from run.toml to add to the lifecycle metadata label during export i.e., “run image for rebase”)
During export, continue to use run image identifier (which could be a digest reference or daemon image ID) as the run image reference instead of falling back to image name when exporting to a daemon
When finding the run image info during export, use the run image "image" (name) in analyzed.toml as the search key, because the run image "reference" could be a daemon image ID or include the digest, which isn't helpful when retrieving image names that are supposed to float

natalieparellano · 2023-06-28T21:02:42Z

Will undraft once tests are added

buildpack/dockerfile.go

natalieparellano · 2023-06-30T14:59:45Z

First fix: pack v0.30.0-pre2 has a bug where registry credentials for the run image are not provided to the restorer, we can see the failure here with broken pack and fixed lifecycle (this branch):

     |         ===> RESTORING
     |         Running the 'restorer' on OS 'linux' with:
     |         Container Settings:
     |           Args: '/cnb/lifecycle/restorer -cache-dir /cache -log-level debug'
     |           System Envs: 'CNB_USER_ID=2222 CNB_GROUP_ID=3333 CNB_PLATFORM_API=0.12'
     |           Image: 'index.docker.io/library/lifecycle:fdf2f9f8'
     |           User: 'root'
     |           Labels: 'map[author:pack]'
     |         Host Settings:
     |           Binds: 'pack-cache-some-org_vvpmfybuie_latest-bff67ad6bc90.build:/cache pack-cache-some-org_vvpmfybuie_latest-bff67ad6bc90.kaniko:/kaniko pack-layers-wrvbsrdokl:/layers pack-app-luykxtohnj:/workspace'
     |           Network Mode: 'host'
     |         [restorer] Starting restorer...
     |         [restorer] Parsing inputs...
     |         [restorer] Ensuring privileges...
     |         [restorer] Executing command...
     |         [restorer] Pulling run image metadata for localhost:55012/pack-test/run...
     |         [restorer] ERROR: failed to pull run image: failed to get remote image
     |         ERROR: failed to build: executing lifecycle: failed with status code: 1

With fixed pack (buildpacks/pack#1815) and fixed lifecycle:

     |         ===> RESTORING
     |         Running the 'restorer' on OS 'linux' with:
     |         Container Settings:
     |           Args: '/cnb/lifecycle/restorer -cache-dir /cache -log-level debug'
     |           System Envs: 'CNB_USER_ID=2222 CNB_GROUP_ID=3333 CNB_REGISTRY_AUTH=<redacted> CNB_PLATFORM_API=0.12'
     |           Image: 'index.docker.io/library/lifecycle:fdf2f9f8'
     |           User: 'root'
     |           Labels: 'map[author:pack]'
     |         Host Settings:
     |           Binds: 'pack-cache-some-org_swikudtwom_latest-7835b39bb382.build:/cache pack-cache-some-org_swikudtwom_latest-7835b39bb382.kaniko:/kaniko pack-layers-vrugmvjkcd:/layers pack-app-sxgvmonswt:/workspace'
     |           Network Mode: 'host'
     |         [restorer] Starting restorer...
     |         [restorer] Parsing inputs...
     |         [restorer] Ensuring privileges...
     |         [restorer] Executing command...
     |         [restorer] Pulling run image metadata for localhost:55013/pack-test/run...
     |         [restorer] Saving image metadata to /kaniko/cache/base/sha256:9ea9f8ce440b399eecd28af397e493e52b96bf87c2d5b04b048c2ca3261789aa...
     |         [restorer] Updating run image info in analyzed metadata...
     |         [restorer] Run image info in analyzed metadata was: 
     |         [restorer] {"Reference":"10eab44d6000fc0cd8f6651c4665c509815eddfea8f12d4659be0d7e292e3131","Image":"localhost:55013/pack-test/run","Extend":true,"target":{"os":"linux","arch":"amd64"}}
     |         [restorer] Run image info in analyzed metadata is: 
     |         [restorer] {"Reference":"localhost:55013/pack-test/run@sha256:9ea9f8ce440b399eecd28af397e493e52b96bf87c2d5b04b048c2ca3261789aa","Image":"localhost:55013/pack-test/run","Extend":true,"target":{"os":"linux","arch":"amd64"}}

Signed-off-by: Natalie Arellano <narellano@vmware.com>

…ructions (vs more than one instruction) Signed-off-by: Natalie Arellano <narellano@vmware.com>

…ent to ignore the digest portion of the reference if present (for the purpose of selecting data from run.toml to add to the lifecycle metadata label i.e., “run image for rebase”) Signed-off-by: Natalie Arellano <narellano@vmware.com>

Don't print `%!s(<nil>)` if nil is provided to the "parse maybe" function Signed-off-by: Natalie Arellano <narellano@vmware.com>

…a digest reference or daemon image ID) instead of falling back to image name when exporting to a daemon. Previously, the digest reference was incorrect which caused the daemon not to find the image. But when provided a correct digest reference the daemon can still find it. Signed-off-by: Natalie Arellano <narellano@vmware.com>

…port When determining if a provided reference is found in existing metadata, remove its digest - except when setting the new run image "image" in analyzed.toml, because we should always respect what the extension author wrote. Signed-off-by: Natalie Arellano <narellano@vmware.com>

… (name) in analyzed.toml as the search key, because the run image "reference" could be a daemon image ID or include the digest, which isn't helpful when retrieving image names that are supposed to float. Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

joe-kimmel-vmw · 2023-06-30T17:17:22Z

cmd/lifecycle/restorer.go

+		var (
+			remoteRunImage imgutil.Image
+		)
+		runImageName := analyzedMD.RunImageImage() // FIXME: if we have a digest reference available in `Reference` (e.g., in the non-daemon case) we should use it


just verifying you still want this FIXME

This seems to be an edge case and not super important to fix right now

joe-kimmel-vmw · 2023-06-30T17:44:16Z

internal/name/ref_test.go

+				{
+					condition: "has an implicit tag and has a digest",
+					provided:  "some.registry/some-library/some-repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+					does:      "adds an explicit tag and removes the digest",


it's surprising to me that this is the desired behavior?

platform/run_image.go

joe-kimmel-vmw · 2023-06-30T17:48:25Z

platform/run_image.go

+	if err = files.DecodeBuildMetadata(launch.GetMetadataFilePath(inputs.LayersDir), inputs.PlatformAPI, buildMD); err != nil {
+		return files.RunImageForExport{}, err
+	}
+	if len(buildMD.Extensions) > 0 { // FIXME: try to know for sure if extensions were used to switch the run image


how could you know for sure?

Not for sure without some spec changes... but I would rather tackle this in tandem with buildpacks/rfcs#285

…not found when we are only updating the reference and target data in analyzed.toml Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

* When pulling remote image data, fail if the remote image is not found Signed-off-by: Natalie Arellano <narellano@vmware.com> * When validating dockerfiles, set extend to true if there are any instructions (vs more than one instruction) Signed-off-by: Natalie Arellano <narellano@vmware.com> * Update matching logic when considering if two image names are equivalent to ignore the digest portion of the reference if present (for the purpose of selecting data from run.toml to add to the lifecycle metadata label i.e., “run image for rebase”) Signed-off-by: Natalie Arellano <narellano@vmware.com> * Comments and cleanup Don't print `%!s(<nil>)` if nil is provided to the "parse maybe" function Signed-off-by: Natalie Arellano <narellano@vmware.com> * When exporting, continue to use run image identifier (which could be a digest reference or daemon image ID) instead of falling back to image name when exporting to a daemon. Previously, the digest reference was incorrect which caused the daemon not to find the image. But when provided a correct digest reference the daemon can still find it. Signed-off-by: Natalie Arellano <narellano@vmware.com> * Add Contains method to structs that hold run image information for export When determining if a provided reference is found in existing metadata, remove its digest - except when setting the new run image "image" in analyzed.toml, because we should always respect what the extension author wrote. Signed-off-by: Natalie Arellano <narellano@vmware.com> * When finding the run image info for export, use the run image "image" (name) in analyzed.toml as the search key, because the run image "reference" could be a daemon image ID or include the digest, which isn't helpful when retrieving image names that are supposed to float. Signed-off-by: Natalie Arellano <narellano@vmware.com> * Fix detector acceptance and add more logging Signed-off-by: Natalie Arellano <narellano@vmware.com> * Fix: use "image" instead of "reference" and also guard against image not found when we are only updating the reference and target data in analyzed.toml Signed-off-by: Natalie Arellano <narellano@vmware.com> * Add comment Signed-off-by: Natalie Arellano <narellano@vmware.com> --------- Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano commented Jun 29, 2023

View reviewed changes

buildpack/dockerfile.go Show resolved Hide resolved

natalieparellano force-pushed the fix/extensions branch 2 times, most recently from 9fd794a to fdf2f9f Compare June 30, 2023 14:26

natalieparellano added 7 commits June 30, 2023 11:17

When pulling remote image data, fail if the remote image is not found

b698140

Signed-off-by: Natalie Arellano <narellano@vmware.com>

When validating dockerfiles, set extend to true if there are any inst…

13f323f

…ructions (vs more than one instruction) Signed-off-by: Natalie Arellano <narellano@vmware.com>

Comments and cleanup

2afaa5a

Don't print `%!s(<nil>)` if nil is provided to the "parse maybe" function Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the fix/extensions branch from fdf2f9f to 07a7676 Compare June 30, 2023 15:54

natalieparellano requested a review from joe-kimmel-vmw June 30, 2023 15:57

natalieparellano marked this pull request as ready for review June 30, 2023 15:57

natalieparellano requested a review from a team as a code owner June 30, 2023 15:57

natalieparellano mentioned this pull request Jun 30, 2023

Fix: when running restore, provide creds for run image if there are r… buildpacks/pack#1815

Merged

2 tasks

Fix detector acceptance and add more logging

debf8de

Signed-off-by: Natalie Arellano <narellano@vmware.com>

joe-kimmel-vmw reviewed Jun 30, 2023

View reviewed changes

Fix: use "image" instead of "reference" and also guard against image …

eb08a3c

…not found when we are only updating the reference and target data in analyzed.toml Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the fix/extensions branch from 9434a2d to eb08a3c Compare June 30, 2023 18:02

Add comment

ff750f3

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the fix/extensions branch from 60603a3 to ff750f3 Compare June 30, 2023 18:17

natalieparellano added this to the lifecycle 0.17.0 milestone Jul 5, 2023

natalieparellano merged commit 84a94d5 into main Jul 5, 2023

natalieparellano deleted the fix/extensions branch July 5, 2023 20:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for run image extension #1134

Fixes for run image extension #1134

natalieparellano commented Jun 28, 2023 •

edited

Loading

natalieparellano commented Jun 28, 2023

natalieparellano commented Jun 30, 2023

joe-kimmel-vmw Jun 30, 2023

natalieparellano Jul 5, 2023

joe-kimmel-vmw Jun 30, 2023

joe-kimmel-vmw Jun 30, 2023

natalieparellano Jul 5, 2023

Fixes for run image extension #1134

Fixes for run image extension #1134

Conversation

natalieparellano commented Jun 28, 2023 • edited Loading

natalieparellano commented Jun 28, 2023

natalieparellano commented Jun 30, 2023

joe-kimmel-vmw Jun 30, 2023

Choose a reason for hiding this comment

natalieparellano Jul 5, 2023

Choose a reason for hiding this comment

joe-kimmel-vmw Jun 30, 2023

Choose a reason for hiding this comment

joe-kimmel-vmw Jun 30, 2023

Choose a reason for hiding this comment

natalieparellano Jul 5, 2023

Choose a reason for hiding this comment

natalieparellano commented Jun 28, 2023 •

edited

Loading