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) |
Related bugs: |
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.
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
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: /canonical. example. org/artifactory /jammy- golang- backport
- type: apt
components: [main]
suites: [focal]
url: https:/
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...