Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: remove pkg_tar workaround from temurin archive #1582

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

thesayyn
Copy link
Collaborator

@thesayyn thesayyn commented May 1, 2024

Removes the workaround for executable files in temurin_archive and replaces it with tar from bazel-lib. This PR introduces a new system dependency awk for the build. @loosebazooka is awk installed on cloudbuild runners?

Some diff is expected, because tar rule preserves both numeric and non-numeric owner for files and defaults to file mode on disk.

Also bazel-contrib/bazel-lib#829 landed, i will remove genrule once there's a new release of bazel-lib.

@thesayyn thesayyn requested a review from loosebazooka May 1, 2024 20:13
@thesayyn thesayyn force-pushed the fix_temurin_archive branch from 5d4c6c3 to 047dc86 Compare May 1, 2024 20:17
Copy link
Contributor

github-actions bot commented May 1, 2024

🌳 🔄 Image Check
This pull request doesn't make any changes to the images. 👍
You can check the details in the report here

@loosebazooka
Copy link
Member

loosebazooka commented May 6, 2024

@thesayyn
seeing directories with odd permissions (from the diff report)

File     usr/lib/jvm/temurin21_jre_arm64/conf/security/policy/                                                Mode 0x1ed                                                    Mode 0x0

Current 0755
New 0000

which looks incorrect to me. Is that just what's provided by the temurin archive? But I can't seem to figure out what's going on here.

@loosebazooka
Copy link
Member

found some other issues here. Duplicate entries of the same file (ex:/usr) wont load up in docker.

@thesayyn thesayyn force-pushed the fix_temurin_archive branch 2 times, most recently from aa64c7a to 737b1d0 Compare August 2, 2024 04:51
@thesayyn
Copy link
Collaborator Author

thesayyn commented Aug 2, 2024

@loosebazooka okay i fixed the permission issues after a lengthy debug session;

rest of the diff is harmless;


🚧 Diffing gcr.io/distroless/java21-debian12:latest-amd64 against localhost:4564/stage/java21-debian12:latest-amd64

localhost:4564/stage/java21-debian12@sha256:9eb1a61b2196de0aa2a8984d986a4995d8b8c86306fe1a860d88ee72817e6694
TYPE     NAME                    INPUT-0                                                       INPUT-1
Layer    ctx:/layers-27/layer    length mismatch (328 vs 329)                                  
Layer    ctx:/layers-27/layer    name "usr/" appears 1 times in input 0, 2 times in input 1    
Layer    ctx:/layers-27/layer    name "usr/" appears 1 times in input 0, 2 times in input 1  

@loosebazooka loosebazooka mentioned this pull request Aug 6, 2024
@thesayyn thesayyn force-pushed the fix_temurin_archive branch from 737b1d0 to c5d75fa Compare August 8, 2024 17:20
@loosebazooka
Copy link
Member

I'll check the awk dependency.

@loosebazooka
Copy link
Member

kk awk appears to be on the cloud-build bazel image. So we should be good there.

@thesayyn
Copy link
Collaborator Author

this should be ready to land then?

@loosebazooka loosebazooka merged commit 9250016 into main Aug 12, 2024
10 checks passed
@loosebazooka loosebazooka deleted the fix_temurin_archive branch August 12, 2024 20:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants