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

Zip file created with a default (file) mode causes corrupted Azure function #802

Closed
romanofski opened this issue Dec 27, 2023 · 4 comments · Fixed by #804
Closed

Zip file created with a default (file) mode causes corrupted Azure function #802

romanofski opened this issue Dec 27, 2023 · 4 comments · Fixed by #804
Assignees
Labels
P2 An issue that should be worked on when time is available

Comments

@romanofski
Copy link

Disclaimer: I'm not sure whether this is a bug in rules_pkg or really a problem in Azure. However I thought I better file it regardless in case others stumble over this since the "workaround" is easy. So feel free to close.

We're using the packaging rules to package up Azure Function Apps. The mode used to create a zipfile.ZipInfo object in the zip file created, causes a broken Azure Function App deployment. The main problem seems to be the file mode which differs significantly when used zipfile.ZipInfo.from_file:

Env details

OS: GNU/Linux - Ubuntu 22.04 running in WSL2
Python: 3.11.0

Problem description

The filemode set with the current code:

> <ZipInfo filename='dist/main.js' filemode='?rw-r--r--' file_size=3408 compress_size=0>
> entry_info.external_attr
27525120
> oct(entry_info.external_attr >> 16)
'0o644'

The filemode when the info is constructed with zipfile.ZipInfo.from_file:

> <ZipInfo filename='dist/main.js' filemode='-rw-r--r--' file_size=3408 compress_size=0>
> oct(info.external_attr >> 16)
'0o100644'

You'll notice the ? at the beginning of the zipinfo for the file if constructed with the current default mode.

Reproducer

I can reproduce this 100% with access to deploying a function app. I see if I can file a bug somewhere in Azure too. A minimal reproducer is to create a zip file with the make_zipinfo method from build_zip.py using a 16bit? file mode (0o644).

Workaround

The workaround I currently use is to set an explicit file mode:

pkg_files(
    name = "pkg_zip_contents",
    srcs = [
        ":zip_contents",
    ],
    strip_prefix = strip_prefix.from_pkg(),
    attributes = pkg_attributes(
        mode = '100644',
    ),
)

It seems using a 32bit file mode (sorry only stumbled over it in a Stackoverflow post about git file modes is not causing this confusing behaviour in deployment.

@aiuto
Copy link
Collaborator

aiuto commented Jan 2, 2024

It looks like we should be adding the file mode bits in addition to permissions.
That seems like an easy change to build_zip.py

@aiuto aiuto added the P2 An issue that should be worked on when time is available label Jan 2, 2024
aiuto added a commit to aiuto/rules_pkg that referenced this issue Jan 2, 2024
This should not be needed, but it seems to be for some Azure users.

Fixes bazelbuild#802
@aiuto
Copy link
Collaborator

aiuto commented Jan 2, 2024

Please patch and try #804.
It's not clear if there needs to be a special bit for plain files on Windows too

@romanofski
Copy link
Author

@aiuto thanks for the quick reply. I'll double check what happens on Windows.

@romanofski
Copy link
Author

This works for me. Baseline without the patch:

>>> import zipfile
>>> foo = zipfile.ZipFile('test.zip')
>>> foo.infolist()[0]
<ZipInfo filename='.funcignore' compress_type=deflate filemode='?rw-r--r--' file_size=105 compress_size=82>

Then I used a local_override to grab the patch:

local_path_override(module_name = "rules_pkg", path = "rules_pkg")

Created the zip again and now I get the proper file attributes:

>>> foo = zipfile.ZipFile('test.zip')
>>> foo.infolist()[0]
<ZipInfo filename='.funcignore' compress_type=deflate filemode='-rw-r--r--' file_size=105 compress_size=82>

aiuto added a commit that referenced this issue Jan 10, 2024
* Explicitly set the FILE bit in zip external attributes.

This should not be needed, but it seems to be for some Azure users.

Fixes #802

* remove a comment that proved unneeded
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 An issue that should be worked on when time is available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants