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

755 folder permissions stomped on my lambda... #64

Closed
jjmartin opened this issue Jan 19, 2016 · 30 comments
Closed

755 folder permissions stomped on my lambda... #64

jjmartin opened this issue Jan 19, 2016 · 30 comments

Comments

@jjmartin
Copy link

#62

This fix (i think) broke my AWS lambda function. With 3.0.2 it works fine but with 3.1.0, the AWS lambda isn't able to load external modules.
(it could be something else but we have explicitly tested with the only change being the version number of gulp-zip)

I'm not saying this is a bug in gulp-zip - but i would like to know if there are options to set it back to the permissions i think were working. if you think it was some other change between 3.0.2 and 3.1.0, i'd be glad to explore that too.

Thanks for your help.

@sindresorhus
Copy link
Owner

It was just changed to correctly preserve directory permissions: 864a8fd

@pe8ter
Copy link

pe8ter commented Jan 19, 2016

Are you saying you don't believe this to be a bug?

Edit: I see what's happening. Before this update, the bad permissions just happened to work for our project. After the update, the permissions were being set correctly and those correct permissions were breaking our app.

@t1st3
Copy link
Contributor

t1st3 commented Jan 19, 2016

@jjmartin @pe8ter I just tested on a Fedora Linux with various sets of permissions and multiple nested files and folders, and I can't reproduce the problem. All the permissions are kept in the zip file as they were before using the plugin.

@t1st3
Copy link
Contributor

t1st3 commented Jan 20, 2016

@pe8ter so, to you, this issue may be closed?

@pe8ter
Copy link

pe8ter commented Jan 20, 2016

Yes, but it's not my Issue to say so. Thanks for timely responses!

@t1st3
Copy link
Contributor

t1st3 commented Jan 20, 2016

@pe8ter you're welcome! Sure, the issue is not yours, but yet, 1 of 2 issuers are OK with the module's behavior!

@jjmartin
Copy link
Author

maybe it wasn't the permissions on ours that was causing the problem - all the issues we had with our deployment were coming from windows - they all work correctly with 3.0.2 but failed with 3.1.0...

we aren't setting any special permissions on the folders, its a newly created folder that gets zipped. and its hard to tell exactly what the problem is from the AWS lambda side, we have re-downloaded the zip from amazon and not had any problems opening it or running the node scripts inside of them.

@pe8ter
Copy link

pe8ter commented Jan 20, 2016

@jjmartin We were building on a Windows box too. Time to update!

@jjmartin
Copy link
Author

time to update what tho - i am not sure what i need to be doing differently... i can accept that we are doing something wrong but its pretty default behavior...

gulp.task('zip', ['npm', 'subfolders'], function () {    
    return gulp.src([distributionFolder + '**/*'])    
        .pipe(zip(butils.getZipName()))    
        .pipe(gulp.dest(distributionFolder));    
});

@pe8ter
Copy link

pe8ter commented Jan 21, 2016

This update means it's time for us to not build on a Windows box and deploy to a Linix machine. Hard to say much else without details of your CI.

@jjmartin
Copy link
Author

well that is unacceptable... one of the whole points of my migrating my deployment to gulp was so that it would be cross platform. (was previously using a powershell script).

My build server actually is linux and i believe i still get the issue.

@t1st3
Copy link
Contributor

t1st3 commented Jan 21, 2016

@jjmartin After a few new tests, I could reproduce your problem.

I assume your variable distributionFolder is a folder path, like this one my-folder/my-sub, without any slash at the end, so the glob you're having looks like this: my-folder/my-sub**/*.
Using this glob, you'll get permission errors (I don't know if it's intended, or if it's a bug).

Try to add a slash after the folder name, like this one my-folder/my-sub/**/*, and you'll get a ZIP without any permission errors (but you will lose the top-level folder my-sub in that ZIP).

@jjmartin
Copy link
Author

ok thanks - i will give that a try ...
that seems like a workaround or fix that i can live with.

@jjmartin
Copy link
Author

actually after getting a chance to look back at my code that distribution folder does have a trailing slash so it looks somthing like '../myproject-dist/' and the final glob looks like '../myproject-dist/*/' which i think is about what you are saying it should look like, but we did get the issue of Lambda not being able to read the node_modules folder.

is there some windows setting i can put on the folders as i create them that would make the gulp-zip 3.1.0 make the folders 775 again? (or appear a such to the linux container that lambda runs in? )

@jjmartin
Copy link
Author

ok after watching a debug and reading a bunch - i think i understand the underlying problem. Windows is ALWAYS going to return 16822 for its stat.mode on a directory. when translated to octal we get 40666 - (directory)read/write for owner, group and others (no execute)

I'm assuming that the node_modules of my deployment should have execute permissions (otherwise i wouldn't be having this problem)

would you accept a merge request that included an option to force set a mode on the zipped files/folders? (or left them unset)

i think thats the only solution that i would be able to continue to use gulp-zip to do this - i think i'd rather move to my own use of yazl or another library rather than remained pinned to a specific version.

@sindresorhus
Copy link
Owner

@jjmartin yazl seems to default to 040775 for directories, so not sure why gulp differs. This should really be brought up on gulp issue tracker, and not here. All this plugin does is pass the values from gulp to yazl. Not interested in having an option. You can use https://github.com/sindresorhus/gulp-chown for that.

@jjmartin
Copy link
Author

ok - i'll look into the gulp-chown - that is totally a fine solution -

yazl did do 775 by default (this is what was set in 3.0.2), but since you added your own mode setting - you aren't using the default anymore - you are using what is coming off of the node file.stat.mode which in this case of folders is the 16822.

@jjmartin
Copy link
Author

just in case anyone else is finding this - i think what i actually want is your other project https://github.com/sindresorhus/gulp-chmod

@timdp
Copy link

timdp commented Mar 3, 2016

We ran into a related issue, which I believe might be the underlying cause of some of the symptoms described in this thread. I just wanted to document it here.

We're creating zip files on Windows and unzipping them on Linux. (Actually, AWS Elastic Beanstalk is doing the latter for us.) The code that creates the zip files used to be fairly trivial:

return gulp.src('build/**/*')
  .pipe(zip('package.zip'))
  .pipe(gulp.dest('dist'))

The thing is that the glob also matches directories, which makes gulp-zip create explicit directory entries in the zip file, copying their permissions. Unlike Linux, Windows doesn't require that directories be executable in order to allow access, meaning that the directory entries in the zip file won't be marked as executable. Consequently, unzipping package.zip on Linux will render every subdirectory inaccessible. We verified this by opening the archive in 7-Zip File Manager and checking the Attributes column, but there are of course many other ways.

The easiest fix we could think of is passing {nodir: true} to gulp.src:

return gulp.src('dist/**/*', {nodir: true})

This will only pass files to gulp-zip, keeping it from creating explicit directory entries, and instead creating the directories implicitly upon unzipping files contained inside them. Of course, this means that directory permissions won't be preserved, which may not be what you're after. (I didn't check, but I'm pretty sure the default directory permissions from umask will be applied in this case.) Also, empty directories won't be created, but you can easily work around that with an empty file.

Of course, as Sindre already mentioned, gulp-chmod and/or gulp-chown will work as well, but you'll need to combined it with gulp-filter to only modify directories, which could become tedious. Alternatively, I guess you could also pipe the files through an object stream yourself, check if they're directories, and change their permissions.

@sindresorhus
Copy link
Owner

Open an issue/PR on https://github.com/gulpjs/vinyl-fs. This plugin is just passing the permissions it get's from there.

@timdp
Copy link

timdp commented Mar 4, 2016

@sindresorhus Do you think that's relevant though? I don't think vinyl-fs is doing something wrong per se. It's just that the permissions on Windows and Linux don't align, so it kinda makes sense that you have to do some additional work if you really want to preserve them.

@timdp
Copy link

timdp commented Mar 4, 2016

By which I don't mean that gulp-zip should take care of that for you. An option like autoMakeDirectoriesExecutable would probably save a lot of people a bit of code, but it's far from generic and it opens a can of woms.

northerncodemky pushed a commit to northerncodemky/aws-lambda-gulp-tasks that referenced this issue Mar 30, 2016
@jjmartin
Copy link
Author

jjmartin commented Apr 4, 2016

just as a followup for people finding this... this was my workaround: using the gulp-tap:

gulp.task('zip', ['npm', 'subfolders'], function() {
    return gulp.src([distributionFolder + '**/*'])
        .pipe(tap(function(file) {
            if (file.isDirectory()) {
                file.stat.mode = parseInt('40777', 8);
            }
        }))
        .pipe(zip(distroZip))
        .pipe(gulp.dest(distributionFolder));
});

@softwaredude
Copy link

As another follow-up for those finding this: Thank you, @timdp! Your idea to use the {nodir: true} option to gulp.src worked for me in my situation of using gulp-zip to create a PhoneGap Build package on Windows and having the upload to PGB fail in weird, nondeterministic ways.

@orangewise
Copy link

@jjmartin, your tap workaround saved my day!

@mbruning24
Copy link

@jjmartin 3.5 hours of struggling with this today and BOOM your gulp-tap solution works. I REALLY need to get a linux machine so I don't have to deal with windows-related issues...

@jmahmud
Copy link

jmahmud commented Sep 16, 2016

This was a great suggestion @jjmartin !! Many thanks for the workaround.

@gpierrick
Copy link

@jjmartin not sure what your tap workaround does but it saved me from hours of figuring it out! thanks

@tstanev
Copy link

tstanev commented Dec 22, 2016

@timdp suggestion to add {nodir: true} also works when deploying a zip from Windows to AWS Lambda. Without it, Lambda has a problem unzipping zips created by gulp-zip on Windows.

@Spetnik
Copy link

Spetnik commented Jul 26, 2017

The suggestion from @timdp to add {nodir: true} worked for me as well. To prevent future complaints/"me toos" would you accept a PR to note this in the documentation?

jamesgawn added a commit to jamesgawn/node-aws-lambda that referenced this issue Nov 5, 2017
There's a known issue with gulp-zip that means that failing to use the nodir: true parameter means that the zip file doesn't extract correctly once uploaded to AWS Lambda. The change ensures this works as expected. See the thread here: sindresorhus/gulp-zip#64
AlekseyMartynov added a commit to AlekseyMartynov/pace-calculator that referenced this issue Feb 9, 2019
ataylorme added a commit to ataylorme/wprig that referenced this issue Sep 16, 2019
See wprig#594. gulp `src` passes directories by default. However, on Windows and Linux the permissions are different so if the prod bundle is created on a Windows machine and deployed to a Linux server there are issues. Setting `nodir: true` in gulp `src` is a workaround suggested [here](sindresorhus/gulp-zip#64 (comment)).
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

No branches or pull requests