Skip to content

Commit

Permalink
remote-build: Use personal access tokens (#4270)
Browse files Browse the repository at this point in the history
The initial mechanism used by Launchpad to grant temporary access tokens
to `snapcraft remote-build` was a bit of a hack.  They were essentially
pure bearer tokens (implemented as macaroons) that weren't recorded
anywhere on the Launchpad side, which meant that there was no mechanism
for revoking them.  We mitigated this with a hardcoded expiry time, but
it wasn't great.

We now have a new "personal access tokens" mechanism, which is much
better.  These tokens show up in a "Manage access tokens" page on the
repository and can be revoked by the owner; they have more specific
control of the access scopes they grant, and the caller can specify an
expiry time.  The old mechanism is now deprecated, and we intend to
remove it after a suitable transition period.

This commit changes `snapcraft remote-build` to use the new mechanism.
The unit tests can essentially only confirm that it's passing the
expected arguments to the Launchpad API, so I've also run an end-to-end
manual test against Launchpad production to make sure that those
arguments have the intended effect.

We can use a very tight expiry time here, since it only has to last for
long enough for the `git push` process to _start_.  It's OK to increase
it a little if needed, but it's generally good for token lifetimes to be
short where possible.

Signed-off-by: Colin Watson <cjwatson@canonical.com>
  • Loading branch information
cjwatson committed Jul 12, 2023
1 parent 4d0d751 commit 7c80a05
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 16 deletions.
19 changes: 13 additions & 6 deletions snapcraft_legacy/internal/remote_build/_launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import os
import shutil
import time
from datetime import datetime
from datetime import datetime, timedelta, timezone
from typing import Any, Dict, List, Optional, Sequence, Type
from urllib.parse import unquote, urlsplit

Expand Down Expand Up @@ -460,11 +460,20 @@ def has_outstanding_build(self) -> bool:
snap = self._get_snap()
return snap is not None

def push_source_tree(self, repo_dir: str) -> str:
"""Push source tree to Launchpad, returning URL."""
def push_source_tree(self, repo_dir: str) -> None:
"""Push source tree to Launchpad."""
git_handler = self._gitify_repository(repo_dir)
lp_repo = self._create_git_repository(force=True)
token = lp_repo.issueAccessToken()
# This token will only be used once, immediately after issuing it,
# so it can have a short expiry time. It's not a problem if it
# expires before the build completes, or even before the push
# completes.
date_expires = datetime.now(timezone.utc) + timedelta(minutes=1)
token = lp_repo.issueAccessToken(
description=f"snapcraft remote-build for {self._build_id}",
scopes=["repository:push"],
date_expires=date_expires.isoformat(),
)

url = self.get_git_https_url(token=token)
stripped_url = self.get_git_https_url(token="<token>")
Expand All @@ -478,5 +487,3 @@ def push_source_tree(self, repo_dir: str) -> str:
command = error.command.replace(token, "<token>") # type: ignore
exit_code = error.exit_code # type: ignore
raise errors.LaunchpadGitPushError(command=command, exit_code=exit_code)

return url
33 changes: 23 additions & 10 deletions tests/legacy/unit/remote_build/test_launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import textwrap
from datetime import datetime, timedelta, timezone
from unittest import mock

import fixtures
Expand Down Expand Up @@ -452,16 +453,28 @@ def test_git_repository_creation(self):
def test_push_source_tree(self):
source_testdir = self.useFixture(TestDir())
repo_dir = source_testdir.path

self.lpc.push_source_tree(repo_dir)

self.mock_git_class.assert_has_calls == [
mock.call(
"https://user:access-token@git.launchpad.net/~user/+git/id/",
"HEAD:master",
force=True,
)
]
now = datetime.now(timezone.utc)

with mock.patch(
"snapcraft_legacy.internal.remote_build._launchpad.datetime"
) as mock_datetime:
mock_datetime.now = lambda tz: now
self.lpc.push_source_tree(repo_dir)

self.lpc._lp.git_repositories._git.issueAccessToken_mock.assert_called_once_with(
description="snapcraft remote-build for id",
scopes=["repository:push"],
date_expires=(now + timedelta(minutes=1)).isoformat(),
)
self.mock_git_class.assert_has_calls(
[
mock.call().push(
"https://user:access-token@git.launchpad.net/~user/+git/id/",
"HEAD:master",
force=True,
)
]
)

def test_push_source_tree_error(self):
self.mock_git_class.return_value.push.side_effect = (
Expand Down

0 comments on commit 7c80a05

Please sign in to comment.