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

Lossy numeric LayerContentMetadata #473

Open
joshwlewis opened this issue Jul 19, 2022 · 6 comments
Open

Lossy numeric LayerContentMetadata #473

joshwlewis opened this issue Jul 19, 2022 · 6 comments
Labels
bug Something isn't working upstream issue

Comments

@joshwlewis
Copy link
Member

When using any integer type as part of a Layer's Metadata, that metadata is deemed incompatible when reading from a previous layer. libcnb.rs seems to interpret these fields as Float.

For example, LayerMetadata like this:

#[derive(Deserialize, Serialize, Debug, Clone)]
struct CountingMetadata {
    counter_u64: u64,
    counter_i32: i32,
}

Will always hit the migrate_incompatible_metadata method, reporting the GenericMetadata as Some({"counter_i32": Float(1.0), "counter_u64": Float(1.0)}).

To reproduce:

  1. Clone https://github.com/joshwlewis/sample-counter-buildpack
  2. cargo libcnb package
  3. pack build sample --buildpack target/buildpack/debug/sample-counter. It should build just fine.
  4. pack build sample --buildpack target/buildpack/debug/sample-counter. It will fail to use the previously cached metadata, and that should show in the build logs.
@joshwlewis
Copy link
Member Author

I dug into the cached docker volume. It looks like the values are stored as integers there:

$ docker run -it --rm -v=pack-cache-library_sample_latest-411aa02e5253.build:/tmp/build --entrypoint bash sample
heroku@85a7fa18b759:/workspace$ cat /tmp/build/committed/io.buildpacks.lifecycle.cache.metadata
{"sbom":{"sha":""},"buildpacks":[{"key":"sample-counter","version":"0.1.0","layers":{"counting":{"sha":"sha256:3748ee14800fba3519a14af54fffb9d0cbe8f20e8339f51cca422119dda0ecdf","data":{"counter_i32":1,"counter_u64":1},"build":true,"launch":true,"cache":true}}}]}

@edmorley
Copy link
Member

Adding this to migrate_incompatible_metadata():

        let layer_file = std::fs::read_to_string(_ctx.layers_dir.join("counting.toml")).unwrap();
        println!("Layer file contents: {layer_file}");

...gives:

===> BUILDING
Layer file contents: [metadata]
  counter_i32 = 1.0
  counter_u64 = 1.0

Previous layer data invalid!!!

So the issue must either be on write, or a pack/lifecycle thing.

@edmorley
Copy link
Member

Ok so this is a pack/lifecycle thing. If I read counting.toml to a string in build() before+after the handle_layer() call, I can see that it's written correctly, but on the next build it's been changed even before handle_layer() runs.

@edmorley
Copy link
Member

This is enough to repro (create a skeleton buildpack using pack buildpack new testcase, drop the below in, then update api to 0.6 or newer and stacks to "*" in buildpack.toml):

#!/usr/bin/env bash

set -euo pipefail

layers_dir="${1}"

test_layer_dir="${layers_dir}/test"
test_layer_toml="${test_layer_dir}.toml"

mkdir -p "${test_layer_dir}"

if [[ -f "${test_layer_toml}" ]]; then
    echo "Existing layer TOML contents:"
    cat "${test_layer_toml}"
fi

cat > "${test_layer_toml}" << EOF
[types]
launch = true
build = true
cache = true

[metadata]
version = 1
EOF

...gives on the second (cached) build:

===> RESTORING
Restoring metadata for "testcase:test" from app image
Restoring data for "testcase:test" from cache
===> BUILDING
Existing layer TOML contents:
[metadata]
  version = 1.0

@schneems
Copy link
Contributor

Here's a one stop reproduction script:

mkdir -p /tmp/fc3cf072c65938325d7541f1048f9f97
cd /tmp/fc3cf072c65938325d7541f1048f9f97

mkdir -p bin/

touch bin/build
touch bin/detect

cat > bin/detect <<'EOFF'
#!/usr/bin/env bash

set -euo pipefail

exit 0
EOFF


cat > bin/build <<'EOFF'
#!/usr/bin/env bash

set -euo pipefail

layers_dir="${1}"

test_layer_dir="${layers_dir}/test"
test_layer_toml="${test_layer_dir}.toml"

mkdir -p "${test_layer_dir}"

if [[ -f "${test_layer_toml}" ]]; then
    echo "Existing layer TOML contents:"
    cat "${test_layer_toml}"
fi

cat > "${test_layer_toml}" << EOF
[types]
launch = true
build = true
cache = true

[metadata]
version = 1
EOF
EOFF

cat > buildpack.toml <<'EOFF'
api = "0.6"

[buildpack]
id = "sample-counter"
version = "0.1.0"
name = "Sample Counter"
clear-env = true

[[buildpack.licenses]]
type = "BSD-3-Clause"

[[stacks]]
id = "*"
EOFF



chmod +x bin/build
chmod +x bin/detect

Running it twice demonstrates the error ed was seeing:

$ pack build sample --buildpack ./ --path ./
20: Pulling from heroku/buildpacks
Digest: sha256:c49ebf7c333d099ee765a1841acea6d5e5aa0a81a3375e96d7af41c4abc53344
Status: Image is up to date for heroku/buildpacks:20
20-cnb: Pulling from heroku/heroku
Digest: sha256:cb6e1902c1bd7c10e0e0d81d1a6c67a070e7820326b3cd22b11d0847b155ca5a
Status: Image is up to date for heroku/heroku:20-cnb
Previous image with name "sample" not found
===> DETECTING
sample-counter 0.1.0
===> RESTORING
===> BUILDING
===> EXPORTING
Adding layer 'sample-counter:test'
Adding 1/1 app layer(s)
Adding layer 'launcher'
Adding layer 'config'
Adding label 'io.buildpacks.lifecycle.metadata'
Adding label 'io.buildpacks.build.metadata'
Adding label 'io.buildpacks.project.metadata'
no default process type
Saving sample...
*** Images (12e0340bd3e0):
      sample
Adding cache layer 'sample-counter:test'
Successfully built image sample
⛄️ 3.1.2 🚀 /tmp/fc3cf072c65938325d7541f1048f9f97
$ pack build sample --buildpack ./ --path ./
20: Pulling from heroku/buildpacks
Digest: sha256:c49ebf7c333d099ee765a1841acea6d5e5aa0a81a3375e96d7af41c4abc53344
Status: Image is up to date for heroku/buildpacks:20
20-cnb: Pulling from heroku/heroku
Digest: sha256:cb6e1902c1bd7c10e0e0d81d1a6c67a070e7820326b3cd22b11d0847b155ca5a
Status: Image is up to date for heroku/heroku:20-cnb
===> DETECTING
sample-counter 0.1.0
===> RESTORING
Restoring metadata for "sample-counter:test" from app image
Restoring data for "sample-counter:test" from cache
===> BUILDING
Existing layer TOML contents:
[metadata]
  version = 1.0
===> EXPORTING
Reusing layer 'sample-counter:test'
Reusing 1/1 app layer(s)
Reusing layer 'launcher'
Reusing layer 'config'
Adding label 'io.buildpacks.lifecycle.metadata'
Adding label 'io.buildpacks.build.metadata'
Adding label 'io.buildpacks.project.metadata'
no default process type
Saving sample...
*** Images (12e0340bd3e0):
      sample
Reusing cache layer 'sample-counter:test'
Successfully built image sample

Note the version = 1.0 instead of version = 1 as expected.

According to https://toml.io/en/ integers are supported types. I say this is for sure a bug in lifecycle. I'll move to make this bash example an https://codetriage.com/example_app and open an issue against the lifecycle.

@schneems
Copy link
Contributor

Bash example repro https://github.com/schneems/cnb-metadata-integer-bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream issue
Projects
None yet
Development

No branches or pull requests

4 participants