pantheon-systems / quicksilver-pushback

Push any commits made on the Pantheon dashboard back to the original GitHub repository.
MIT License
14 stars 13 forks source link

Quicksilver Pushback Fails on BitBucket because the metadata URL is HTTP not HTTPS #12

Closed ataylorme closed 5 years ago

ataylorme commented 5 years ago

First I ran into Error parsing Bitbucket URL from Build Metadata because the BitBucket repo URL in build-metadata.json was http and not https.

push-back.php checks strictly for https with BitBucket but is more flexible with GitLab.

I'm not sure if we should make BitBucket more flexible here or modify Build Tools to ensure https only is written to build-metadata.json.

Once the was resolved, I ran into fatal: could not read Username for 'https://bitbucket.org': No such device or address in the pushback process when git add . is run.

@aaronbauman have you been using Quicksilver Pushback with BitBucket successfully? If so, I'm curious what is different in your configuration than mine.

aaronbauman commented 5 years ago

Do you have "user" defined in private://.build-secrets/tokens.json?

My tokens.json looks like this, with USERNAME and PASSWORD replaced appropriately: { "user": "USERNAME", "pass": "PASSWORD", "token": "dummy" }

ataylorme commented 5 years ago

Ah, I authed with a token so mine looks like {"token":"REDACTED_TOKEN_VALUE"}

aaronbauman commented 5 years ago

yeah, bitbucket cloud rest api doesn't support access tokens, so you have to use username + password

ataylorme commented 5 years ago

Okay, that seems like a documentation issue then, alerting folks if they use a token Quicksilver pushback won't work. Maybe warn in the README and during build:project:create (if users have a saved token or enter a token instead of username/password)

Alternatively, we could remove token support and only support username/password for Build Tools.

@greg-1-anderson I'm curious what your thoughts are.

aaronbauman commented 5 years ago

Aren't we prompting for bb username/password already during build:project:create? Would it be possible to inject these into tokens.json?

ataylorme commented 5 years ago

I have a saved token from months ago. I will clear that out and test build:project:create again to see what is prompted

ataylorme commented 5 years ago

Odd, when I look in ~/.terminus/cache/build-tools/ I see files for BITBUCKET_USER and BITBUCKET_PASS, neither of which have the entire token.

tree ~/.terminus/cache/build-tools/                                                                                             
├── <redacted_email>-ADMIN_EMAIL
├── <redacted_email>-ADMIN_PASSWORD
├── <redacted_email>-BITBUCKET_PASS
├── <redacted_email>-BITBUCKET_USER
├── <redacted_email>-CIRCLE_TOKEN
├── <redacted_email>-GITHUB_TOKEN
├── <redacted_email>-GITLAB_TOKEN
├── <redacted_email>-GIT_USER_EMAIL
└── <redacted_email>-SITE_NAME

Digging into the code for build:project:create it seems to set the token, derived from the username and password, in tokens.json. See https://github.com/pantheon-systems/terminus-build-tools-plugin/blob/master/src/Commands/ProjectCreateCommand.php#L348:L350

Perhaps we should do as you suggest, and store user and pass entries as well

aaronbauman commented 5 years ago

Opened https://github.com/pantheon-systems/terminus-build-tools-plugin/pull/268 to address this.

ataylorme commented 5 years ago

@aaronbauman I can confirm this is working once I set user and pass in tokens.json. See https://bitbucket.org/ataylorme/example-wp-composer-pipelines-test-2019-09-10/pull-requests/2/wip-test-quicksilver-pushback/diff#Lweb/quicksilver-pushback-test.txtT2

ataylorme commented 5 years ago

https://github.com/pantheon-systems/terminus-build-tools-plugin/pull/268 makes it so tokens.json has entries for token, user and pass with BitBucket.

Quicksilver pushback still fails because of the url value in build-metadata.json starting with http://bitbucket.org and not https

ataylorme commented 5 years ago

Perhaps the BitBucket provider should use alterBuildMetadata to ensure that the url is https.

ataylorme commented 5 years ago

Actually, build:project:create set url to git@bitbucket.org:ataylorme/example-wp-composer-pipelines-test-2019-09-11.git

So I'm thinking it gets changed when a new push from BitBucket happens.

ataylorme commented 5 years ago

Yup - [notice] Set build-metadata.json to {"url":"http://bitbucket.org/$BITBUCKET_USER/...

aaronbauman commented 5 years ago

Looks like this is happening during initialization on bitbucket. messageagency___pipe-test___Pipelines_—_Bitbucket

ataylorme commented 5 years ago

@aaronbauman I opened https://github.com/pantheon-systems/terminus-build-tools-plugin/pull/270 to force https in build-metadata.json and am going to test that to see if it helps

ataylorme commented 5 years ago

Hmm, the changes in #270 didn't affect the url value in build-metadata.json sent to Pantheon. I'm going to bed now but will look at this again tomorrow

ataylorme commented 5 years ago

I think the alter method I added is failing because getBuildMetadata is passing $repositoryDir not $buildMetadata to alterBuildMetadata.

@rvtraveller has the GitLab alterBuildMetadata method been working for you?

ataylorme commented 5 years ago

I think the alter method I added is failing because getBuildMetadata is passing $repositoryDir not $buildMetadata to alterBuildMetadata.

Changing that didn't help. I've reverted and am going to test forcing https in getBuildMetadata directly.

rvtraveller commented 5 years ago

Interestingly, I haven't run into the problem I was trying to solve with all that but looking at it now, I am not sure why it is working.

Digging deeper...

rvtraveller commented 5 years ago

So, GitLab changed the way they test merge requests which means git -C $repositoryDir rev-parse --abbrev-ref HEAD always returns HEAD and mitigates what we are trying to work around. So...it works for GitLab but not because of the change here we introduced.

rvtraveller commented 5 years ago

I'm surprised changing the variable passed though didn't work. Looking at it, that seems to be what did the trick...

ataylorme commented 5 years ago

I'm surprised changing the variable passed though didn't work. Looking at it, that seems to be what did the trick...

yeah, me too. Any concerns with forcing https on url in getBuildMetadata directly? The API for all 3 providers support HTTPS. I'm not sure why we would want/need HTTP instead of HTTPS

rvtraveller commented 5 years ago

The only concern I have is if there are individuals with self-hosted GitLab who are not using https (maybe because they are on a private network?).

ataylorme commented 5 years ago

push-back.php checks strictly for https with BitBucket but is more flexible with GitLab.

Maybe the solution here is to allow http for BitBucket in Quicksilver pushback or to force https in Quicksilver pushback, but not in build-metadata.json.

rvtraveller commented 5 years ago

I think that seems reasonable (allowing for http in pushback)

ataylorme commented 5 years ago

I think that seems reasonable (allowing for http in pushback)

Do you think that is the better approach than forcing https in pushback?