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

[RFC #0102] Image history metadata #411

Closed
Tracked by #214
nebhale opened this issue Sep 9, 2020 · 14 comments · Fixed by #1099
Closed
Tracked by #214

[RFC #0102] Image history metadata #411

nebhale opened this issue Sep 9, 2020 · 14 comments · Fixed by #1099
Assignees
Labels
good first issue A good first issue to get started with. status/ready type/enhancement New feature or request

Comments

@nebhale
Copy link
Contributor

nebhale commented Sep 9, 2020

Currently, when a layer is created from a Dockerfile, some additional metadata is added to the created image containing the directives that were run to create a particular layer:

image

Since buildpacks don't have "directives", this metadata is blank creating what I call a "black hole" for metadata:
image

The lifecycle should populate whatever metadata is necessary in order to fill in this black hole. As a straw man, I'd suggest each layer getting something like <buildpack name> <buildpack version> – <layer name> (e.g. Paketo BellSoft Liberica Buildpack 3.2.0 – jre) but I'm not really fussed with the actual content of this metadata.

@jromero
Copy link
Member

jromero commented Sep 11, 2020

Was looking a bit at this out of curiosity and found the the field used in the config is under history.created_by and it's described in the spec as "The command which created the layer.".

I wondered if there was any real considerations that would need to be taken into account like something trying to use this to recreate said image and there by needing to be executable. The nebulous description led me to explore certain usages and my ultimate finding was that there really isn't a definitive outcome. I'm only sharing this here in case it may help someone else.

Docker, using docker history cnbs/sample-stack-run:bionic --no-trunc, does current provide executable bits like:

/bin/sh -c mkdir -p /run/systemd && echo 'docker' > /run/systemd/container

as well as proper comments such as:

/bin/sh -c #(nop)  ARG cnb_gid=1000

but then this when using ARGS which I failed to find any real docs into it's format (non-executable?):

|2 cnb_gid=1000 cnb_uid=1000 /bin/sh -c groupadd cnb --gid ${cnb_gid} &&   useradd --uid ${cnb_uid} --gid ${cnb_gid} -m -s /bin/bash cnb

@ddobrin
Copy link

ddobrin commented Mar 25, 2021

This is a question which is being asked by virtually every client of my company after demos.
Using the dive utility is an option, however is something totally different and can't be scripted.
Thank you, it would be very helpful to have

@jchesterpivotal
Copy link
Contributor

jchesterpivotal commented Mar 26, 2021

  1. A quick drive-by obligatory request for JSON formatted information for ease of slurping.
  2. It can possibly be added as a shell comment to avoid execution.

Hence something like:

# {"buildpack_name":"Paketo BellSoft Liberica", "buildpack_version":"3.2.0","layer_name":"jre"}

Or even, alternatively:

echo '{"buildpack_name":"Paketo BellSoft Liberica", "buildpack_version":"3.2.0","layer_name":"jre"}'

@jromero
Copy link
Member

jromero commented Mar 27, 2021

The idea of making it parsable is interesting. I'm not particularly keen on it being JSON because it's not the most human-readable (as presented via docker history or dive). There are other formats that might be a good balance but have their own downfalls.

For example, comma-separated values:

# Paketo BellSoft Liberica,3.2.0,jre

or the distribution(?) spec can define a specific format that is both human readable and parsable based on a format contract.

format # <id>@<version> - <layer>

# Paketo BellSoft Liberica@3.2.0 - jre

@jabrown85
Copy link
Contributor

Does anyone here want to create a draft RFC for discussion? I like the ideas presented here.

@jchesterpivotal
Copy link
Contributor

CSVs are a famous disaster zone and I'm fairly militantly against inventing minilanguages. I see tool robustness as outweighing curiousity in this case. Especially since it's easier to submit PRs for dive et al to pretty-print JSON than to find every parser that inaccurately interprets an unspecified, informal minilanguage.

@natalieparellano natalieparellano added the type/enhancement New feature or request label Apr 14, 2021
@samj1912
Copy link
Member

samj1912 commented Dec 8, 2021

Related - buildpacks/rfcs#194

@natalieparellano
Copy link
Member

natalieparellano commented Feb 25, 2022

Blocking this on buildpacks/rfcs#194

Edit: actually, since the linked RFC is in FCP, let's just call it "ready"

@natalieparellano natalieparellano added status/ready type/enhancement New feature or request and removed type/enhancement New feature or request status/requires-rfc labels Feb 25, 2022
@natalieparellano natalieparellano added this to the lifecycle-0.15.0 milestone Feb 25, 2022
@natalieparellano natalieparellano added the good first issue A good first issue to get started with. label Mar 30, 2022
@natalieparellano natalieparellano changed the title Additional Layer Metadata on Images [RFC #0102] Image history metadata Apr 4, 2022
@samj1912
Copy link
Member

samj1912 commented Apr 4, 2022

I can take this on :)

@natalieparellano
Copy link
Member

Thanks @samj1912 !!!

@samj1912
Copy link
Member

I have slowly been making progress on this (when I get time). I have started with some basic changes to imgutil (to allow setting and getting createdBy arrays through the image interface) and prepopulating this data from base image. Will update with a draft PR once it looks okay on the imgutil side.

@edmorley
Copy link
Contributor

Will this issue cover adding metadata for builders generated by pack builder create too, or do we need a separate one for that?

eg: Currently our builder images show <missing> in docker history:

$ docker history test-builder-22
IMAGE          CREATED        CREATED BY   SIZE      COMMENT
6d4382092962   42 years ago                0B        
<missing>      42 years ago                45B       
<missing>      42 years ago                322B      
<missing>      42 years ago                6.53MB    
<missing>      42 years ago                6.14MB    
<missing>      42 years ago                15.3MB    
<missing>      42 years ago                11MB      
<missing>      42 years ago                13.4kB    
<missing>      42 years ago                32.5kB    
<missing>      42 years ago                5.6kB     
<missing>      42 years ago                10.7MB    
<missing>      42 years ago                4.46kB    
<missing>      42 years ago                25.6MB    
<missing>      42 years ago                15kB      
<missing>      42 years ago                6.01kB    
<missing>      42 years ago                13.4MB    
<missing>      42 years ago                0B        
<missing>      42 years ago                0B        
<missing>      42 years ago                335kB     
<missing>      42 years ago                375MB     
<missing>      42 years ago                1.73kB    
<missing>      42 years ago                543MB     
<missing>      42 years ago                9.14kB    
<missing>      42 years ago                77.8MB    

@natalieparellano
Copy link
Member

@edmorley I think we'll want a separate issue for that on the pack side. This should be relatively easy to implement in pack once buildpacks/imgutil#202 lands.

@edmorley
Copy link
Contributor

edmorley commented Feb 5, 2024

Will this issue cover adding metadata for builders generated by pack builder create too, or do we need a separate one for that?

I've filed:
buildpacks/pack#2052

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good first issue to get started with. status/ready type/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants