| -@gerrit:opendev.org- Zuul merged on behalf of Ian Wienand: [zuul/zuul-jobs] 851269: upload-git-mirror: no_log around key writing https://review.opendev.org/c/zuul/zuul-jobs/+/851269 | 08:48 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Ian Wienand: [zuul/zuul-jobs] 851288: linters: standardise on newline at end of file https://review.opendev.org/c/zuul/zuul-jobs/+/851288 | 09:06 | |
| @bingberg:matrix.org | Does anyone know why we do this check? | 10:58 |
|---|---|---|
| ``` | ||
| if not source_context.trusted: | ||
| if project.canonical_name != \ | ||
| source_context.project_canonical_name: | ||
| raise ProjectNotPermittedError() | ||
| ``` | ||
| from | ||
| https://opendev.org/zuul/zuul/src/commit/68684d519e9f906afd899a37f9e3da92b61f914d/zuul/configloader.py#L1123 | ||
| Why do we do this check, what does this protect from? | ||
| -@gerrit:opendev.org- Dr. Jens Harbott proposed: [zuul/nodepool] 852264: Add validation check for diskimages in openstack https://review.opendev.org/c/zuul/nodepool/+/852264 | 12:11 | |
| @fungicide:matrix.org | > <@bingberg:matrix.org> Does anyone know why we do this check? | 13:14 |
| > | ||
| > ``` | ||
| > if not source_context.trusted: | ||
| > if project.canonical_name != \ | ||
| > source_context.project_canonical_name: | ||
| > raise ProjectNotPermittedError() | ||
| > ``` | ||
| > | ||
| > from | ||
| > | ||
| > https://opendev.org/zuul/zuul/src/commit/68684d519e9f906afd899a37f9e3da92b61f914d/zuul/configloader.py#L1123 | ||
| > | ||
| > Why do we do this check, what does this protect from? | ||
| that safety check was introduced by https://review.opendev.org/470442 in order to make sure that an untrusted project couldn't define another project in its configuration, since that would cross a security boundary. zuul does not assume that the maintainers of one project implicitly trust the maintainers of any other project within the tenant | ||
| @fungicide:matrix.org | except for trusted config projects of course, as their name implies | 13:15 |
| @bingberg:matrix.org | I see, what would be the consequences of not having that check? The source of the project is available through setting `required-projects` on a job, so for us being in the same tenant already implicitly grants a degree of trust (atleast read access level to all projects in the tenant) | 13:45 |
| @bingberg:matrix.org | what extra privileges would be given by allowing extension of the project from an untrusted project? the ability to extend the jobs required for a pipeline? | 13:46 |
| @bingberg:matrix.org | and as an aside, would it be possible to give a project a trusted source context, without turning it into config_project? | 13:55 |
| @fungicide:matrix.org | > <@bingberg:matrix.org> I see, what would be the consequences of not having that check? The source of the project is available through setting `required-projects` on a job, so for us being in the same tenant already implicitly grants a degree of trust (atleast read access level to all projects in the tenant) | 14:18 |
| bingberg: the trust boundary in this case is that the maintainers of project A may not trust the maintainers of project B to add jobs or other configuration to project A. unlike trusted config projects, configuration changes proposed for untrusted projects can be run speculatively without first merging them, and this includes the ability to depend on proposed changes for other untrusted projects and incorporate them in the build. configuration from trusted config projects is excluded from being speculatively run in order to prevent attacks which could disclose secrets for an untrusted project (by first proposing a change to a trusted config project, which alters how the secret is handled, then proposing a change to the untrusted project where that secret is configured depending on that new but not merged behavior change) | ||
| @fungicide:matrix.org | there are a number of similar sorts of attacks you could orchestrate if untrusted projects were allowed to define things for other projects than themselves | 14:20 |
| @fungicide:matrix.org | keep in mind that the trust model zuul is built around is that users of a tenant have read access to repositories in that tenant and can propose changes for any of them, but only the maintainers of each project or of config projects get to decide what configuration is committed and used for their specific project | 14:21 |
| @fungicide:matrix.org | you can, of course, further restrict access in whatever code review platform you're using, but zuul at least tries not to break the above assumptions | 14:24 |
| @fungicide:matrix.org | probably worth pointing out, the first use case zuul was built for, and so the origin of a lot of its security design, is as a public-facing ci system for code hosting providers, where the people whose code is developed there may have little or no relationship with nor trust in one another | 14:26 |
| @jim:acmegating.com | that sounds like something that might only apply to public systems, but in fact a lot of private enterprises appreciate the access control model too. | 14:40 |
| @fungicide:matrix.org | yep, many large organizations in effect operate their own multi-departmental hosting and ci, internal-facing but still with very similar trust boundaries | 14:41 |
| @fungicide:matrix.org | zuul also treats what you would consider read access as fairly distinct from being able to make decisions as to what is used to gate a project's changes or otherwise add things to a project without that project's maintainers consenting. that's basically the role that trusted config projects play | 14:44 |
| @fungicide:matrix.org | its "secrets" implementation is a good indicator of that... the idea is that sensitive information needed for running jobs should not be available to anyone even though they may have read access to the repository in which it resides, and only the maintainers of the project get to choose how that sensitive data is used in their jobs | 14:46 |
| @bingberg:matrix.org | I see, well that explains the background quite a bit. | 14:46 |
| While looking at possible modifications to allow us to do perform what we are looking at, is there any reason why a config project can not have a branch specifier on its project definitions, i.e.: | ||
| ``` | ||
| // some config project | ||
| - project: | ||
| name: dep | ||
| <Pipeline configuration A> | ||
| branch: master | ||
| - project: | ||
| name: dep | ||
| <Pipeline configuration B> | ||
| branch: release/alpha | ||
| ``` | ||
| Where the `dep` project would be extended with the A configuration if running on `master` and the B configuration if running on `release/alpha` simular to how the `dep` project can be extended with an unconditional configuration today. | ||
| @fungicide:matrix.org | branch specifiers don't apply to project definitions, you would put those in the job definitions, but yes defining branch-specific jobs and job variants in trusted config projects is the recommended way to do that | 14:47 |
| @fungicide:matrix.org | happy to get you links to lots of examples | 14:49 |
| @bingberg:matrix.org | we're more interested in configuring the trigger ability rather than the job itself, we have subservient project dep1, dep2 etc where any changes need to pass integration tests in any consuming projects con1, con2, etc | 14:51 |
| @bingberg:matrix.org | so the testsuite for con1 might need to be run in both master and release/alpha for any changes to dep1, but the test suite for con2 is only needed for changes to dep2 in release/alpha but not in master | 14:52 |
| @fungicide:matrix.org | an example of doing it with a job variant, which is what we tend to mostly do in config projects, is: https://opendev.org/openstack/project-config/src/branch/master/zuul.d/projects.yaml#L6904-L6908 | 14:53 |
| @fungicide:matrix.org | since projects can choose to run jobs defined in other projects, there's no strict need to put the job definitions in a trusted config project, but here's an example of branch-specific job definitions in one untrusted project which will be run by other projects when added to their pipelines: https://opendev.org/openstack/openstack-zuul-jobs/src/branch/master/zuul.d/jobs.yaml#L63-L106 | 14:55 |
| @fungicide:matrix.org | we usually try to do as little as possible in trusted config projects, since as i mentioned earlier, you can't speculatively execute proposed configuration changes for them, so it's harder to be sure what you're proposing works until you merge it | 14:56 |
| @fungicide:matrix.org | actually i should have included the definition prior to those two as well. all three of them define the same job name but are set to run for different branches | 14:59 |
| @fungicide:matrix.org | the first does it with a branch regex, while the others use lists of discrete branch names | 15:00 |
| @fungicide:matrix.org | so if a project adds that `openstack-tox` job to one of its pipelines, which of the three definitions gets used for that will depend on which branch the job is being triggered for | 15:01 |
| @bingberg:matrix.org | I still don't understand how we can model the connection using the branch specifiers on jobs only. If I had a configuration like this in a trusted config project: | 15:10 |
| ``` | ||
| - project: | ||
| name: dep1 | ||
| templates: | ||
| - con1-test-suite | ||
| - project: | ||
| name: dep2 | ||
| templates: | ||
| - con1-test-suite | ||
| ``` | ||
| How would I modify that so that the `con1-test-suite` is only run on changes to dep1 for `master` and `release/alpha`, not on changes to `release/beta`. While still running the test suite on changes to `con1` for any branch abd `dep2` for `master` and `release/beta`. | ||
| Wouldn't adding branch specifiers to the job kill its ability to run for other triggers? | ||
| @bingberg:matrix.org | * I still don't understand how we can model the connection using the branch specifiers on jobs only. If I had a configuration like this in a trusted config project: | 15:10 |
| ``` | ||
| - project: | ||
| name: dep1 | ||
| templates: | ||
| - con1-test-suite | ||
| - project: | ||
| name: dep2 | ||
| templates: | ||
| - con1-test-suite | ||
| ``` | ||
| How would I modify that so that the `con1-test-suite` is only run on changes to dep1 for `master` and `release/alpha`, not on changes to `release/beta`. While still running the test suite on changes to `con1` for any branch and `dep2` for `master` and `release/beta`. | ||
| Wouldn't adding branch specifiers to the job kill its ability to run for other triggers? | ||
| @bingberg:matrix.org | we still want the job to run on all branches, just not triggered by `dep1` | 15:11 |
| @clarkb:matrix.org | When you add the job to dep1 you add it as a variant using a branch override int he pipeline definition | 15:13 |
| @bingberg:matrix.org | So flattening the template and extending the jobs into something like: | 15:21 |
| ```` | ||
| - project: | ||
| name: dep1 | ||
| pipeline1: | ||
| jobs: | ||
| - job1-of-pipeline1 | ||
| branches: | ||
| master | ||
| release/alpha | ||
| - job2-of-pipeline1 | ||
| branches: | ||
| master | ||
| release/alpha | ||
| ... | ||
| pipeline2: | ||
| ... | ||
| - project: | ||
| name: dep2 | ||
| pipeline1: | ||
| jobs: | ||
| - job1-of-pipeline1 | ||
| branches: | ||
| master | ||
| release/alpha | ||
| release/beta | ||
| ... | ||
| pipeline2: | ||
| ... | ||
| ```` | ||
| Would work? | ||
| @jim:acmegating.com | yep (modulo yaml syntax errors, corrected here: https://etherpad.opendev.org/p/JPQhbtykMuA0UA1JJBcY ) - if the list gets long and repetitive, you can use yaml anchors to reduce duplication. | 15:23 |
| @bingberg:matrix.org | well it requires moving ownership of the template into the config project as well | 15:23 |
| @bingberg:matrix.org | but we could probably build a script that would perform that reconfiguration | 15:24 |
| @bingberg:matrix.org | but putting a pin in that solution, what is the hinderance of allowing branch specifications in project configurations? | 15:28 |
| i.e. why not allow this syntax: | ||
| ``` | ||
| - project: | ||
| name: dep1 | ||
| branches: | ||
| - master | ||
| - release/beta | ||
| templates: | ||
| - con1-test-suite | ||
| - project: | ||
| name: dep2 | ||
| branches: | ||
| - master | ||
| - release/alpha | ||
| - release/beta | ||
| templates: | ||
| - con1-test-suite | ||
| ``` | ||
| @jim:acmegating.com | bingberg: i didn't see it mentioned, but you're aware of the standard method of dealing with this which is to locate the project configuration on the untrusted repo itself so that each branch of the repo defines what jobs run on that branch via implied branch matching, right (this makes branch selection intuitive and automatic)? i assume that just doesn't work for your case for some reason? | 15:33 |
| @bingberg:matrix.org | > <@bingberg:matrix.org> So we have a situation where we have a dependency projects `dep1`, `dep2` etc and several consumer projects `con1`, `con2` etc. We wish to gate changes in the dependency projects such that the consumer projects will be compatible with any changes in the dependency projects | 15:36 |
| > | ||
| > In regular zuul configuration this could be modeled by the `dep1` project having a .zuul.yaml file which triggers the integration tests specified by a template e.g. | ||
| > | ||
| > ``` | ||
| > // .zuul.yaml in dep1 | ||
| > project: | ||
| > templates: | ||
| > - con1-integration-tests | ||
| > // .zuul.yaml in dep2 | ||
| > project: | ||
| > templates: | ||
| > - con1-integration-tests | ||
| > - con2-integration-tests | ||
| > ``` | ||
| > | ||
| > The consumer projects would then have the definition of their respective integration tests and release branches with different dependencies than master would work properly. | ||
| > | ||
| > Due to several reasons we would like the dependency configuration to be wholly located inside of the consumer projects, i.e. we would instead like to write something like: | ||
| > | ||
| > ``` | ||
| > // .zuul.yaml in con1 | ||
| > project: | ||
| > name: dep1 | ||
| > templates: | ||
| > - con1-integration-tests | ||
| > project: | ||
| > name: dep2 | ||
| > templates: | ||
| > - con1-integration-tests | ||
| > // .zuul.yaml in con2 | ||
| > project: | ||
| > name: dep2 | ||
| > templates: | ||
| > - con2-integration-tests | ||
| > ``` | ||
| > This is not allowed since untrusted projects can't modify other project configurations. Changing the consumer projects to a config-project (or writing it to a separate config project) is also not an option since config projects only examines the master branch and therefore release branches would not be running with their dependencies. | ||
| > | ||
| > Does anyone have an idea how we could achieve this without exposing the dependents in the dependency projects? | ||
| corvus: I assume you mean the first mentioned configuration up here? | ||
| @jim:acmegating.com | i mean just stick `project: {templates: con1-test-suite}` in .zuul.yaml on the `release/beta` branch of `dep` | 15:37 |
| @bingberg:matrix.org | yes exactly | 15:38 |
| @bingberg:matrix.org | that would be neat but due to reasons we can't do that | 15:38 |
| @bingberg:matrix.org | ostensibly we could have shadow repositories which add the .zuul.yaml files on top of the real repositories but I don't think that would actually be an easier solution | 15:39 |
| @fungicide:matrix.org | i wanted to run a nodepool unit test locally on my workstation to check something, but all i get is zookeeper connection timeouts when i try. what/where should i be looking to troubleshoot that? | 16:37 |
| @fungicide:matrix.org | nevermind! now i see we call that out explicitly in TESTING.rst | 16:39 |
| @fungicide:matrix.org | i need to have a running zk service somewhere in order to be able to do the unit tests | 16:39 |
| @fungicide:matrix.org | is there a reason we don't include it in bindep.txt? | 16:40 |
| @clarkb:matrix.org | because you have to configure it with ssl and simply putting it in bindep isn't enough | 16:41 |
| @clarkb:matrix.org | there should be a docker-compose file like zuul though iirc | 16:41 |
| @jim:acmegating.com | fungi: tools/test-setup-docker.sh is what i use | 16:41 |
| @fungicide:matrix.org | mmm, the TESTING.rst says nothing about configuring zookeeperd, just says to install and start it | 16:42 |
| @fungicide:matrix.org | thanks corvus! | 16:42 |
| @fungicide:matrix.org | looks like there are only a few commands in that script which need privileged access to connect to dockerd. i hacked in a handful of sudo additions for myself for now, but would a cli parameter or envvar or something be welcome to allow running the bulk of that script as a normal user? | 16:52 |
| @fungicide:matrix.org | mainly to avoid creating the ca files as root inside the git worktree | 16:54 |
| @jim:acmegating.com | don't see why not | 16:54 |
| @fungicide:matrix.org | thanks, i'll push up something along those lines, and some improvements to TESTING.rst as well | 16:55 |
| @fungicide:matrix.org | woohoo! i was able to run a nodepool unit test and reproduce the failure i wanted | 16:56 |
| @clarkb:matrix.org | fungi: you should probably update the zuul script too | 17:03 |
| @fungicide:matrix.org | if it's similar, then yeah i'll port the same change to it once the first one is to everyone's liking | 17:03 |
| @fungicide:matrix.org | for the failures on https://review.opendev.org/852264 is my assessment correct that the mapping of diskimages to labels happens at the provider pool layer, so we need to enumerate the labels in the provider's pools to see if they specify the image? | 17:14 |
| @jim:acmegating.com | fungi: what you wrote on the change is absolutely correct -- diskimages and labels do not need to have the same name (nor should they -- labels bind images+flavors, so a given image can be associated with many flavors). | 17:17 |
| @fungicide:matrix.org | so is the approach there simply incomplete, or going the wrong direction entirely? for the record, the typos it's intended to catch are along the lines of what this corrected: https://review.opendev.org/852127 | 17:19 |
| @jim:acmegating.com | it looks like maybe the issue was that a pool label referenced a diskimage that wasn't attached to the provider? i'm not sure, i haven't looked up the line numbers in the traceback | 17:19 |
| @jim:acmegating.com | oh, that's a provider diskimage referencing a missing top-level diskimage | 17:20 |
| @jim:acmegating.com | nothing about the fix needs to include the word "label" | 17:21 |
| @fungicide:matrix.org | yeah, agreed. looking more closely at the typo fix, the diskimage doesn't exist | 17:22 |
| @fungicide:matrix.org | rather the entry in the provider's diskimages list doesn't refer to a diskimage actuallt defined in the main diskimages list | 17:22 |
| @fungicide:matrix.org | * rather the entry in the provider's diskimages list doesn't refer to a diskimage actually defined in the main diskimages list | 17:23 |
| @jim:acmegating.com | yep | 17:23 |
| @jim:acmegating.com | if you have some time to look at https://review.opendev.org/q/hashtag:freeze-job that'd be great :) it's a handful of changes, but it's a fun stack. :) | 17:25 |
| @fungicide:matrix.org | sure, i'll take a look once i finish climbing out of the current hole | 17:25 |
| -@gerrit:opendev.org- Jeremy Stanley https://matrix.to/#/@fungicide:matrix.org proposed on behalf of Dr. Jens Harbott: [zuul/nodepool] 852264: Validation check for missing openstack diskimages https://review.opendev.org/c/zuul/nodepool/+/852264 | 17:47 | |
| -@gerrit:opendev.org- Jeremy Stanley https://matrix.to/#/@fungicide:matrix.org proposed: [zuul/nodepool] 852285: Update unit test container setup and instructions https://review.opendev.org/c/zuul/nodepool/+/852285 | 17:52 | |
| -@gerrit:opendev.org- Jeremy Stanley https://matrix.to/#/@fungicide:matrix.org proposed: [zuul/nodepool] 852285: Update unit test container setup and instructions https://review.opendev.org/c/zuul/nodepool/+/852285 | 18:28 | |
| @fungicide:matrix.org | working on porting that update to zuul/zuul as well, i see its TESTING.rst recommends using the tox-docker extension to set up zk in `-docker` testenvs. does anyone know if that's still usable now that zk needs a ca and we're also requiring a db service to run tests? unfortunately i don't have enough disk space to test it out myself | 19:20 |
| @fungicide:matrix.org | if someone can confirm it's now basically useless, i'm happy to also work on cleaning it up | 19:21 |
| @fungicide:matrix.org | otherwise i'll keep the bits related to it in the doc | 19:21 |
| @clarkb:matrix.org | fungi: I think the expectation is you use the same tools script you used for nodepool | 19:26 |
| @fungicide:matrix.org | yeah, that's the impression i'm getting too, mainly just trying to figure out if it's safe to clean up the tox-docker stuff or whether someone's still somehow using that | 19:29 |
| @clarkb:matrix.org | I don't think I've ever used it as one data point | 19:34 |
| @fungicide:matrix.org | i'll rip it out as part of the proposed change, and we can try to get consensus in review | 19:35 |
| -@gerrit:opendev.org- Jeremy Stanley https://matrix.to/#/@fungicide:matrix.org proposed: [zuul/zuul] 852289: Update unit test container setup and instructions https://review.opendev.org/c/zuul/zuul/+/852289 | 21:03 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Dr. Jens Harbott: [zuul/nodepool] 852264: Validation check for missing openstack diskimages https://review.opendev.org/c/zuul/nodepool/+/852264 | 21:04 | |
| @vlotorev:matrix.org | Hi, zuul 6.2.0 was release yesterday, but there is no 6.2.0 chapter in relnotes https://zuul-ci.org/docs/zuul/latest/releasenotes.html. | 21:08 |
| @vlotorev:matrix.org | * Hi, zuul 6.2.0 was released yesterday, but there is no 6.2.0 chapter in relnotes https://zuul-ci.org/docs/zuul/latest/releasenotes.html. | 21:08 |
| @fungicide:matrix.org | vlotorev: at the moment, the master docs only get new builds when new changes merge to the branch, so the latest tag won't show up there until at least one more change merges, which is why the release announcements point to release-specific builds of releasenotes instead: https://lists.zuul-ci.org/pipermail/zuul-announce/2022-August/000130.html | 21:11 |
| @fungicide:matrix.org | but that reminds me, i should finish working on https://review.opendev.org/839614 to address that | 21:11 |
| @fungicide:matrix.org | does this failure in test_node_assignment_at_tenant_quota_ram look familiar to anyone? https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_40f/852285/2/gate/nodepool-tox-py38/40fd88f/testr_results.html | 21:13 |
| @fungicide:matrix.org | i'm having trouble figuring out what node request it gave up waiting for in that test | 21:14 |
| @jim:acmegating.com | fungi: it may be a flaky test; xref with the other python version and see if it failed there. | 21:14 |
| @fungicide:matrix.org | it only failed on that version, and only after passing in check, and was for an approved docs change, so yes it seems to be nondeterministic, just not sure how common | 21:15 |
| @fungicide:matrix.org | a similar docs change to zuul had two tests fail under py38 (tests.unit.test_configloader.TestTenantFromScript and tests.unit.test_scheduler.TestScheduler) while everything worked under py310: https://17d78cb20b61f601d4e0-9dc687b1ca3b864db38ca15564d88e40.ssl.cf5.rackcdn.com/852289/1/check/zuul-tox-py38/c1799e8/testr_results.html | 22:09 |
| -@gerrit:opendev.org- Zuul merged on behalf of Jeremy Stanley https://matrix.to/#/@fungicide:matrix.org: [zuul/nodepool] 852285: Update unit test container setup and instructions https://review.opendev.org/c/zuul/nodepool/+/852285 | 22:23 | |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: | 23:47 | |
| - [zuul/nodepool] 848337: Include user/driver data in node detail list https://review.opendev.org/c/zuul/nodepool/+/848337 | ||
| - [zuul/nodepool] 848344: Include backing node request id in metastatic log https://review.opendev.org/c/zuul/nodepool/+/848344 | ||
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!