Merge ~jugmac00/lpci:pass-in-credentials into lpci:main

Proposed by Jürgen Gmach
Status: Merged
Merged at revision: 19ac8f544ea983fef7a91b9bab4783a0d6dd4f53
Proposed branch: ~jugmac00/lpci:pass-in-credentials
Merge into: lpci:main
Prerequisite: ~jugmac00/lpci:add-additional-apt-repositories
Diff against target: 462 lines (+302/-4)
7 files modified
NEWS.rst (+2/-0)
docs/cli-interface.rst (+21/-0)
docs/configuration.rst (+5/-2)
lpcraft/commands/run.py (+41/-2)
lpcraft/commands/tests/test_run.py (+230/-0)
setup.cfg (+1/-0)
tox.ini (+2/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+425865@code.staging.launchpad.net

Commit message

Add cli option to pass in a configuration file for credentials

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :
Download full text (3.3 KiB)

I don't really like the fact that this can only allow for a single set of credentials; that seems like an interface smell. I'd also like us to think ahead a bit more: CI environments typically have some way to store secrets safely and pass them into build jobs, and isn't this just a particular kind of secret? It's true that we're under some time pressure, but I don't want that to cause us to write ourselves into a corner where the syntax of `.launchpad.yaml` is concerned, so let's take some time to think through this.

My suggestion on https://code.launchpad.net/~jugmac00/lpcraft/+git/lpcraft/+merge/425829 might help a certain amount here, because then we wouldn't be confined to purely textual substitutions, but we could parse the URL properly and insert the username and password into it using the normal `urllib.parse` functions.

But more broadly, I think we need support for passing in more than one kind of secret, and it would be wise for the actual text of the secrets not to be on the lpcraft command line (as written here it might require extra code in launchpad-buildd to redact it properly, and the command line is just a very leaky sort of place for secrets generally - it's normally better to pass them via files).

How about a `--secrets` option which takes the path to a YAML file, which would contain a mapping of names to scalar values? launchpad-buildd would then take the contents of that file as an XML-RPC parameter (encoded for safe transport over XML-RPC), write it out in such a way as to avoid it showing up in the build log, and pass that option to lpcraft.

For the configuration file syntax, again, we should think ahead to the point where we'll probably want to make secrets accessible to `run` (we can't effectively hide these particular credentials from `run` anyway, since it's running as root inside a container where we've written the credentials to `/etc/apt/sources.list` or similar). I'd prefer to avoid `<...>` for that, since that means something quite different in shell. A natural approach would be to use shell variables: for instance, imagine being able to configure a repository with `MY_SECRET=value`, and then you could evaluate `$MY_SECRET` in a `run` command. We don't have to get all the way there in this branch, but I'd like this to be compatible with that sort of approach in the future.

On the other hand, expanding `$`-prefixed shell variables in a URL is a bit weird. Fortunately, if we've thought ahead enough for the secrets to be a dict mapping keys to scalar values, we have some flexibility here. How about having a secret called something like `apt:canonical.example.org`, which would contain credentials applied to all `package-repositories` entries with that hostname? You'd then write something like this (following Snapcraft's syntax):

    package-repositories:
      - type: apt
        components: [main]
        suites: [focal]
        url: https://canonical.example.org/artifactory/jammy-golang-backport

lpcraft would parse the URL, notice that has the hostname `canonical.example.org` and that it has an `apt:{hostname}` secret matching that, and fill in the credentials.

What do you think? I'm not 100% we...

Read more...

review: Needs Information
Revision history for this message
Jürgen Gmach (jugmac00) wrote (last edit ):

I had a quick look what github does:

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-secrets

They are using Jinja-like placeholders - the placeholders are defined separately per repository - this also seems a viable way.

About URL parsing... there is not much we need to do - as we use a pydantic validator, it automatically parses the URL and it should be straightforward to use something like `url.user = "username"; url.password = "secret"` to add the credentials to the URL object, and then just use `str(url)` to build it again.

I have this morning off but I'll explore these options, but I think we need to discuss this again before implementing the one or the other option.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

Oh, the documentation should also explain that `package-repositories.url` is rendered using Jinja2.

Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Thanks, Colin! That is a good idea.

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches