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

Migrate oauth to GitHub app #155

Merged
merged 63 commits into from May 19, 2020
Merged

Migrate oauth to GitHub app #155

merged 63 commits into from May 19, 2020

Conversation

jimallman
Copy link
Member

This branch also incorporates selected improvements from easter-fix, all now conforming to our usual deployment methods (no manual tweaking required).

This reverts commit a74348e.
merging easter-fix into migrate-oauth-to-github-app for full testing on dev* servers
This prevents errors when (e.g.) sending a CORS preflight 'OPTONS'
request to fetch cached API responses. Test: If this patch is not
applied, no tree will appear in our synth-tree viewer webapp.
This prevents errors when authorizing to a GitHub app (or OAuth app)
on proxied web2py servers, as is common in our setup.

Test: If this patch is not applied, we'll be unable to successfully
login to the synth-tree viewer or curation app. (This error usually
manifests as a mismatched-uri, even when the requested and registered
URLs are identical.)
Happily newer versions of web2py correctly handle non-ASCII names when
importing a python module.
rsync -p -e "${SSH}" "$OPENTREE_GH_IDENTITY" "$OT_USER@$OPENTREE_HOST":.ssh/opentree
${SSH} "$OT_USER@$OPENTREE_HOST" chmod 600 .ssh/opentree
rsync -p -e "${SSH}" "$OPENTREE_GH_IDENTITY" "$OT_USER@$OPENTREE_HOST":.ssh/opentree || exit 1
${SSH} "$OT_USER@$OPENTREE_HOST" chmod 600 .ssh/opentree || exit 1
fi

# Try to place an OAuth token for GitHub API by bot user 'opentreeapi'
tokenfile=${OPENTREE_SECRETS}/OPENTREEAPI_OAUTH_TOKEN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about what should be happening here. I should have the OPENTREE_OAUTH_TOKEN saved in .ssh/opentree/ on my local machine (https://github.com/OpenTreeOfLife/germinator/pull/155/files#diff-028ee22cd145bc1227616bfad2602bdeR90), and then it gets copied over? Is this how it should still work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - following conversation with Mark I believe we are using a keypair generated directly on phylsystemapi/devphylesystemapi for the pushes to phylesystem. So I think we don't need this bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.





# ---------- Redis for caching ---------
#This is in the default venv
REDIS_WITH_VERSION="redis-2.10.6"
REDIS_WITH_VERSION="redis-3.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change, but based on experiments on devphylesystemapi needs to be reverted to redis==2.10.6. I don't recollect why I changed it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So are we running two versions of redis, one inside and one outside of the python virtual environment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good q.. and one that I don't honestly understand the answer to!
on dev phylsesystem-api redis-cli 3.0.0 is installed from releases, but we were running into celery/celery#5175 until @mtholder ran venv/bin/pip install redis==2.10.6
But AFAIK redis 3.0.0 is still getting called by https://github.com/OpenTreeOfLife/germinator/blob/migrate-oauth-to-github-app/deploy/setup/install-api.sh#L264 I may be wrong about that part though! Mark might be able to clarify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is running. Here is what I suspect: the redis server can probably be running 2.10.6 or 3.0.0.
However, celery has be running through python2.7 (because we are using web2py). It imports the redis python package to interact with the redis server. I think that the means that we need to be using a not-the-latest version of the redis python package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right - that sounds correct to me!

@snacktavish
Copy link
Member

OK! I re-deployed devphylesystemapi with this branch, and all seems good! Currently requires deployer to place opentree-api.pem and set git config, but can make a new branch to add that later once this PR is merged.
@jimallman do you want to resolve the merge conflict? I think I know which logic is new/correct, but I'm not 100%.

@jimallman
Copy link
Member Author

Sure, I'll take a look...

@snacktavish snacktavish merged commit 78c8002 into master May 19, 2020
@jimallman
Copy link
Member Author

As it turns out, there was some non-trivial picking and choosing here. I kept the switch from
branch migrate-oauth-to-github-app, but the logging and other updated sweeper options from master.

@jimallman jimallman deleted the migrate-oauth-to-github-app branch October 27, 2021 16:23
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

4 participants