physera / onelogin-aws-cli

Assume an AWS Role and cache credentials using Onelogin
MIT License
67 stars 32 forks source link

PEX compiler breaks build #41

Open drewsonne opened 6 years ago

drewsonne commented 6 years ago

The 'dist' stage of the pex compilation is breaking the build. This breaking feature was added in #30.

I have spent a couple of days trying to fix this issue, and the problems seems to be that as lxml relies on libxml (used for the SAML assertion parsing), it must have specific wheels for each os you wish to install on. Furthermore, as lxml only distributes with manylinux1 wheels and pex does not support them, I can not see a way forward.

If this was tested/built inside a docker container, I would like to suggest that the dockerimage @mumoshu used before the build be made a part of the travis build process.

I can appreciate the wish for a binary in docker (as the example in the PR provides), but if python3 is installed anyway as mentioned in #30, what is the advantage of: curl -o onelogin-aws-login <onelogin-aws-login release> && chmod +x onelogin-aws-login over pip3 install onelogin-aws-cli?

In regards to alternatives to fixing this, I have built a homebrew formula which wraps this install in a hidden python3 virtualenv for mac osx, and I can try and get it into homebrew core, but that is out of my control to an extent. On the linux side, a deb/rpm/apk could be built, if pex can't be fixed, and this is a feature which is needed.

The PR probably should have had master merged/rebased back into the branch to run the travis build, which would have caught this.This is blocking the config updates (as their build now fails and tests can not be run) I made in prep for a daemon and which would have caught the breaking, so I would like to get it fixed.

If there is no way forward in fixing this, is there a possibility for the PR to be rolled back, so as not to block other work?

@mumoshu do you have any insight into how to fix the lxml issue:

Could not satisfy all requirements for lxml==3.8.0:
    lxml==3.8.0(from: onelogin_aws_cli->onelogin)
/home/travis/.travis/job_stages: line 60: dist/onelogin-aws-login: No such file or directory
slycoder commented 6 years ago

I've gone ahead and reverted #30 for the time being to unblock things.

@mumoshu , if you have suggestions for a fix we can try landing again.

mumoshu commented 6 years ago

@drewsonne @slycoder Sorry for breaking things, and thanks for your efforts :bow:

if python3 is installed anyway as mentioned in #30, what is the advantage of...

My take is: Less chances to polluting existing python envs to break either onelogin-aws-cli or other existing python-based tools in people's dev machines.

I really wish we could bundle literally everything into the pex binary. However, even if some dependencies leaked out of the pex binary, it seems to ease installation (for me).

WDYT?

a homebrew formula which wraps this install in a hidden python3 virtualenv for mac osx

This does sound like a great idea for macOS users 👍

Anyway, I'll keep thinking what I can do for this topic. I'm ok with any way as long as it eases installation, prevents e.g. my colleagues from breaking their python envs 😉 For example, if it simply turns out to be not a real issue, just a documentation issue, making it a server-side app with a thin client cli(plus extra benefit of proper source ip restriction), etc., that's also ok. Any input is welcomed!

drewsonne commented 6 years ago

@mumoshu I'm with you on the pex binary. I just wanted to understand motivations as it broke the build :-)

You can see the build logs in travis for what happened, but the long story short is that it can't find a wheel dependency to build for all 3 environments, because lxml wheels are OS/platform specific. So you would need to download pip download... the wheel for each of those env's (which I tried).

Then I found that lxml is only distributed as manylinux1, which apparently pex doesn't support (see above).

I think the main problem is that wheel names (eg, manylinux1... ) does not match the OS name linux_i386

drewsonne commented 6 years ago

I can't create a core homebrew, as:

$ brew audit --new-formula onelogin-aws-cli
onelogin-aws-cli:
  * GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)

@slycoder it could be hosted on a custom homebrew tap, but not sure how invested you guys are in this. :-) Would need a repo, github.com/physera/homebrew-tap, and I can add the formula we use internally in our company.

after that,

brew tap physera/tap
brew install onelogin-aws-cli

and that would only work for OSX.

Could probably build that into the build process to make it a bit easier.

drewsonne commented 6 years ago

... or get your friends and family to all star/watch this repo :-)

drewsonne commented 6 years ago

In relation to this, we (@beamly) are hosting a homebrew tap for this. I'm happy to move this to @physera if you would like, but it would be another step in the release process.

https://github.com/beamly/homebrew-tap/blob/master/onelogin-aws-cli.rb

slycoder commented 6 years ago

Nice, we don't have any homebrew taps set up so you're more than welcome to host it if you've already got it set up =).