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

Do not forget CreatedBy in mutate.Canonical #978

Merged
merged 4 commits into from
Nov 9, 2021
Merged

Conversation

hermanbanken
Copy link
Contributor

@hermanbanken hermanbanken commented Apr 7, 2021

Warning: breaking change.

Kaniko uses this project and uses mutate.Canonical when the --reproducible flag is set. This is advertised as resetting the dates, so that the build becomes reproducible. However, it also resets the CreatedBy field, due to improper copying.

Example docker history <img> without --reproducible:

IMAGE          CREATED         CREATED BY                                      SIZE      COMMENT
0c0d4077e9d6   292 years ago   COPY debug-page /app/debug-page                 1.51kB
<missing>      292 years ago   COPY --from=builder-dev /app/dist /app/dist     196kB
<missing>      292 years ago   COPY --from=builder-prod /app/node_modules /…   47.9MB
<missing>      292 years ago   COPY package.json config-localdev.json confi…   5.43kB
<missing>      292 years ago   WORKDIR /app                                    0B
<missing>      292 years ago   RUN apk add --update     libc6-compat     &&…   621kB
<missing>      8 months ago    /bin/sh -c #(nop)  CMD ["node"]                 0B
<missing>      8 months ago    /bin/sh -c #(nop)  ENTRYPOINT ["docker-entry…   0B
<missing>      8 months ago    /bin/sh -c #(nop) COPY file:238737301d473041…   116B
<missing>      8 months ago    /bin/sh -c apk add --no-cache --virtual .bui…   7.62MB
<missing>      8 months ago    /bin/sh -c #(nop)  ENV YARN_VERSION=1.22.4      0B
<missing>      8 months ago    /bin/sh -c addgroup -g 1000 node     && addu…   76.1MB
<missing>      8 months ago    /bin/sh -c #(nop)  ENV NODE_VERSION=12.18.3     0B
<missing>      11 months ago   /bin/sh -c #(nop)  CMD ["/bin/sh"]              0B
<missing>      11 months ago   /bin/sh -c #(nop) ADD file:b91adb67b670d3a6f…   5.61MB

Example docker history <img> with --reproducible:

IMAGE          CREATED         CREATED BY   SIZE      COMMENT
d6b5217acfb2   292 years ago                1.51kB
<missing>      292 years ago                196kB
<missing>      292 years ago                47.9MB
<missing>      292 years ago                5.43kB
<missing>      292 years ago                0B
<missing>      292 years ago                621kB
<missing>      292 years ago                116B
<missing>      292 years ago                7.62MB
<missing>      292 years ago                76.1MB
<missing>      292 years ago                5.61MB

@jonjohnsonjr
Copy link
Collaborator

due to improper copying

I'd argue that CreatedBy also hinders reproducibility.

@hermanbanken
Copy link
Contributor Author

hermanbanken commented Apr 13, 2021

One of the reasons we are using immutable docker images (by Kaniko) is to know when something unexpectedly changes in the images. Without code changes, the docker image ID should remain stable. However, sometimes the docker image ID changes, and you need to diff (using container-diff/some other solution).

Example of a diff I generated:


Layer summary:

- 1.	 "sha256:d6265d920ff75dd0ec52a5735eec41a803867f03c17d1ed10bb25bba8a31dfb3" (3.0 MB, unknown command)
- 2.	 "sha256:6fcf011df71d08253ad5cf3810b0461c1318545e631c990c93608c7885c8b4d7" (28 MB, unknown command)
- 3.	 "sha256:5c45ea255d9b6f0373ca6066d419ec87b603d7c8c6eff26c9a1f344a63b9ab36" (2.7 MB, unknown command)
+ 1.	 "sha256:b9fe0d8cf57b207134de9c7ae39bf820e03e2cfdebd3ca287fd862f9c8a12dd0" (3.0 MB, unknown command)
+ 2.	 "sha256:b6f5f5ad9c6df58ef04c31944af6a7a574705b47aa1d2e1680030805212df2df" (28 MB, unknown command)
+ 3.	 "sha256:9124915e14c5817c1e7c0593635c9bfbeffd408fc00b1eef0593c5faad5471a5" (2.7 MB, unknown command)
  4.	 "sha256:352b2d896b75b90825aed17300926907f8346467a4a2e8a20567a1bb7162ff37" (279 B, unknown command)
- 5.	 "sha256:acc317ca5d3dc70baebf4dee20b06c282b517e69cf55d47dbde3383c2e2af0fd" (304 kB, unknown command)
+ 5.	 "sha256:c771136239e56052c58d2970534d1e893d1234fd6a529a0c105026781ef1e707" (304 kB, unknown command)
  6.	 "sha256:d037238c5194b3d60328227610cdee20899a59e3860b520fcb08676f9564550b" (107 B, unknown command)
  7.	 "sha256:7ac3e71826182d75e43a533d93a115e7163bf359da921dd1fdb740caffc9e9a9" (2.5 kB, unknown command)
  8.	 "sha256:99d3966c25dcfb60fdde7e8d08950ab486837f0d750275231768d8f9260384af" (1.1 kB, unknown command)
- 9.	 "sha256:a3542aeb5cb163ba4bed62679a5f6e0cb7a730a0628e209479eb466e88aa0b3c" (9.7 MB, unknown command)
- 10.	 "sha256:cd94f87f291b5fef97014f172adfefd24d217fcc521bd00e91344d4dfdebe1fa" (37 kB, unknown command)
+ 9.	 "sha256:79611e1daadc504fd0956c1f0f4950e180447ab5270b7f27327f9347d30af331" (9.7 MB, unknown command)
+ 10.	 "sha256:c03df33169d4eddbcc0e8fe949e5dca83aba29980c0585f15e96ec93a24e94e5" (37 kB, unknown command)

Layer 1

Highlighted content changes:

etc/alpine-release
- 3.11.9
+ 3.11.10

Directories with changes:

  • lib/apk/db
  • usr/bin
  • bin
  • etc

Layer 2
... cut for brevity ...

Layer 3
... cut for brevity ...

etc.


Next to this use case - which I agree, is very tailored to us; though I plan on open-sourcing this as a tool/GH Action - there are many people that which to inspect the images before they run them using docker history <img>. I think there should be an option.

On the other hand, it is understandable that we don't want to introduce this breaking change in go-containerregistry. I can also close this issue here, and continue in Kaniko to create a non-breaking change by using a different method than mutate.Canonical, but it would help that project if there was an option exposed for mutate.Canonical to keep the history. I think it would also help other people using go-containerregistry discover the actual behavior.

Maybe it would be sufficient to add documentation to this mutate.Canonical method (that it renders docker history <img> unusable)? I had quite the journey discovering why this history was stripped, and would like to save others that work.

@jonjohnsonjr
Copy link
Collaborator

Given how difficult debugging this would be otherwise, I think I'm okay with this change.

Does kaniko attempt to make the CreatedBy text reproducible, at least? If not, this is kind of moot.

On the other hand, it is understandable that we don't want to introduce this breaking change in go-containerregistry.

I think it's probably fine to merge this. If you're really serious about reproducibility, you're probably pinning a specific version of your build tool.

@jonjohnsonjr
Copy link
Collaborator

Does kaniko attempt to make the CreatedBy text reproducible, at least?

@tejal29 does this seem reasonable to you?

@github-actions
Copy link

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@hermanbanken
Copy link
Contributor Author

@tejal29 does this seem reasonable to you?

Hi @tejal29 is this reasonable or do you prefer an alternative route?

@github-actions
Copy link

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@hermanbanken
Copy link
Contributor Author

hermanbanken commented Oct 20, 2021

@tejal29 @jonjohnsonjr how to move forward? If this is undesirable, please let me know or close the issue.

For us this is still a problem (that we've been ignoring).

@imjasonh
Copy link
Collaborator

imjasonh commented Nov 9, 2021

I think it's probably fine to merge this. If you're really serious about reproducibility, you're probably pinning a specific version of your build tool.

@jonjohnsonjr do you still feel comfortable with this change? If so I think we should just merge it.

@imjasonh imjasonh merged commit 3cd0cb5 into google:main Nov 9, 2021
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.

None yet

3 participants