| openstackgerrit | Merged openstack-infra/zuul-jobs master: Use shell for apt-get update https://review.openstack.org/492716 | 00:09 |
|---|---|---|
| jeblair | mordred: more paint spilled on 492614 | 00:17 |
| mordred | jeblair: good paint | 00:19 |
| jamielennox | does anyone here have any experience with ansible-semaphore? | 00:29 |
| jamielennox | i'm liking the reports generated by ARA, but i'd kind of like to replace our cron system with something that can take github triggers and show a report | 00:29 |
| jamielennox | and they _still_ haven't open sourced tower | 00:30 |
| pabelanger | sorry, no | 00:31 |
| jamielennox | what i'd really like is for all these products to ship with a -builder program for yaml -> api | 00:32 |
| jeblair | jamielennox: eventually zuul should be able to do that (we need static nodes in nodepool, and some method for node acls) | 00:33 |
| jeblair | (that's the CD part of the zuulv3 spec) | 00:33 |
| jeblair | (alternatively, you could probably hack something together today with secrets, or executor-local ssh keys) | 00:33 |
| pabelanger | pretty excited the CD part of zuulv3 spec is getting closer! | 00:34 |
| jamielennox | that'd be great for a bunch of things, i'm a little concerned about the loop of zuul deploying zuul | 00:35 |
| jamielennox | but then again i'm already talking about using ansible to deploy the thing that controls ansible so maybe it doesn't matter | 00:35 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Bindmount /etc/lsb-release into bubblewrap https://review.openstack.org/490200 | 00:36 |
| *** cinerama` has joined #zuul | 02:13 | |
| *** cinerama has quit IRC | 02:18 | |
| *** maxamillion has quit IRC | 02:18 | |
| *** openstackgerrit has quit IRC | 02:21 | |
| *** maxamillion has joined #zuul | 02:27 | |
| jlk | I may have missed it, but did http://marc.info/?l=git&m=150238802328673&w=2 make the rounds in infra? | 03:48 |
| jeblair | jlk: yeah, i believe fungi mentioned it earlier | 03:56 |
| *** amoralej|off is now known as amoralej | 06:57 | |
| *** bhavik1 has joined #zuul | 08:43 | |
| *** bhavik1 has quit IRC | 08:51 | |
| *** electrofelix has joined #zuul | 08:53 | |
| *** bhavik1 has joined #zuul | 08:58 | |
| *** bhavik1 has quit IRC | 09:39 | |
| *** jkilpatr has quit IRC | 10:52 | |
| *** jkilpatr has joined #zuul | 11:12 | |
| *** jkilpatr has quit IRC | 11:24 | |
| *** jkilpatr has joined #zuul | 11:51 | |
| *** TheJulia has quit IRC | 12:19 | |
| *** TheJulia has joined #zuul | 12:19 | |
| *** dkranz_ has joined #zuul | 12:30 | |
| *** openstackgerrit has joined #zuul | 13:26 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add zuul.project.src_dir variable https://review.openstack.org/491915 | 13:26 |
| *** amoralej is now known as amoralej|lunch | 13:35 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 13:35 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Rename tox_command_line in docs to tox_extra_args https://review.openstack.org/489758 | 13:37 |
| dmsimard | mordred: btw I replied on the ara future etherpad but tl;dr: yes, the current refactor with the API is built with the decoupling of the components (cli, callback, web app) in mind so that they can be split in different packages with each their own deps | 14:05 |
| dmsimard | mordred: and grouping playbooks is something that we can most probably do, just need to think about how to do it. Workaround I see would be to have a playbook that include other playbooks for the time being. | 14:06 |
| dmsimard | Depending on how things are going, this might or might not land in time for 1.0 | 14:06 |
| *** amoralej|lunch is now known as amoralej | 14:20 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename allow-secrets to pre-review https://review.openstack.org/493054 | 15:48 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename allow-secrets to post-review https://review.openstack.org/493059 | 15:57 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename allow-secrets to pre-review https://review.openstack.org/493054 | 15:57 |
| jeblair | pabelanger, mordred, clarkb, fungi, SpamapS, jlk, anyone else: i have proposed 3 alternative changes to rename the allow-secrets and untrusted-secrets attributes. can you take a look at them and let me know which you prefer (and why you prefer it if you feel like sharing): https://review.openstack.org/492614 https://review.openstack.org/493054 https://review.openstack.org/493059 | 16:03 |
| jeblair | (perhaps inappropriately, i've developed a preference and have gone ahead and voted on those, but seriously, this is subtle so i'm very open to change, including adding more options) | 16:04 |
| jlk | Will read shortly | 16:46 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename allow-secrets to post-review https://review.openstack.org/493059 | 16:50 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool feature/zuulv3: Add node.cloud to zookeeper https://review.openstack.org/493086 | 17:01 |
| pabelanger | jeblair: mordred: Shrews:^ Create node.cloud in zookeepr. Ready for painting | 17:02 |
| pabelanger | this is because we'd like to also have nodepool.cloud in our inventory files | 17:02 |
| pabelanger | because provider.name could be different | 17:02 |
| jeblair | lgtm | 17:03 |
| *** amoralej is now known as amoralej|off | 17:04 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: Create nodepool.cloud inventory variable https://review.openstack.org/493088 | 17:12 |
| pabelanger | and zuul change | 17:13 |
| *** persia has quit IRC | 17:18 | |
| fungi | jeblair: looking over the three options, i'm first questioning the relevance of making it post-review rather than post-merge... what jobs do we have now which we would only trust to run in the gate pipeline (post-review but pre-merge)? or are there some you have in mind we would likely add? or is it more that if we trust jobs to run in post/release pipelines then we lose nothing by also allowing them to run in | 17:21 |
| fungi | the gate pipeline? | 17:21 |
| fungi | just curious whether there are corner cases where we would only trust certain jobs to run after the full set of gate jobs has vetted them | 17:22 |
| fungi | and i'm failing to think of any jobs we have right now which run in a "trusted" context (on our persistent slaves in v2) but in the gate pipeline | 17:23 |
| fungi | at least up to now, the untrusted/trusted demarcation (so much as we have one) is at the point where changes merge after gating | 17:24 |
| fungi | certainly we wouldn't want to run jobs which upload durable artifacts in the gate since they might get rerun multiple times or even aborted mid-upload depending on whether changes ahead of them fail | 17:26 |
| fungi | similarly jobs which push content into review or translation systems | 17:26 |
| jeblair | fungi: ah good question -- i'm imagining a hypothetical job for shade which has all of mordred's cloud credentials and runs after +2 to test changes against real public clouds. maybe that's in gate, or maybe it's a new on-demand pipeline. | 17:27 |
| jeblair | fungi: (whether we actually make that job or not i don't know, but that's the use case i've been keeping in mind that expands this from merely post-merge to post-review) | 17:27 |
| fungi | got it, so as i suspected, this shift from post-merge to post-review is designed to support a new class of jobs which get access to sensitive credentials pre-merge | 17:29 |
| jeblair | fungi: but yes, considering the tarball upload job (which is the most immediate example -- we want a way for folks to upload things to tarballs.o.o, but it would be bonkers to do that in the gate), this doesn't *quite* fit the bill there. this is enough to prevent someone abusing that job to expose the credentials, but it's not enough to prevent someone from doing something dumb and adding it to gate. | 17:29 |
| fungi | and i guess the idea is we trust core reviewers for that project to not approve changes which could disclose those | 17:29 |
| jeblair | fungi: exactly | 17:30 |
| *** electrofelix has quit IRC | 17:30 | |
| fungi | e.g., shade job maintained in the shade repo which provides credentials known to shade core reviewers, and by approving reviewed changes they have a vested interest in not exposing those with a malicious or malformed change | 17:30 |
| jeblair | yep | 17:31 |
| jeblair | fungi: to address the tarballs-in-gate case, we could expand this by adding a post-merge flag as well, or we could fix it in the job itself. perhaps have an ansible assertion in the playbook that errors if it detects (via zuul variables) that it's being run pre-merge. | 17:31 |
| fungi | right, i'm starting to feel like this exposes two different (but related/overlapping) concerns | 17:31 |
| jeblair | agreed | 17:32 |
| fungi | things we want to allow to use secrets in the gate pipeline, and things we want to prevent people running in pipelines where they might get rerun (more of a foot cannon sheild) | 17:32 |
| jeblair | yep | 17:33 |
| fungi | having one control for both either exposes some risk or fails to address a desired use case | 17:33 |
| fungi | i'm on board with the possibility of having jobs just include sanity checks for the post-merge assertion | 17:34 |
| fungi | but we might want to be clear that post-review doesn't provide that guarantee automatically | 17:35 |
| jeblair | it's at least easy to start with that and then find that if it causes us to write awkward playbooks, we can add a post-merge job flag | 17:35 |
| fungi | wfm | 17:35 |
| fungi | do you think the documentation needs clarification on that point? trying to figure out where, if so | 17:36 |
| jeblair | fungi: i'm not sure where to put that; most of the related documentation is around secrets, which is the 'other' concern here. maybe we need a 'good practices for jobs' document or something. | 17:37 |
| fungi | yeah, i'll noodle on that some | 17:37 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename allow-secrets to post-review https://review.openstack.org/493059 | 17:40 |
| clarkb | jeblair: having read the allow-untrusted-secrets change docs a couple times and the pre-review docs once. I far prefer pre-review. It is a lot easier to understand what setting of the boolean flags will do | 17:43 |
| clarkb | I think it helps beause the defaults are inverted? basically its a direct assertion | 17:44 |
| jeblair | clarkb: the intent is the behavior is the same in all three changes (though pre-review probably needs some revision to actually make it work since the values are reversed from the other two) | 17:45 |
| clarkb | ya the behavior is the same but it is easier to understand a true assertion than a negative assertion (and thus need to infer what truth means) | 17:46 |
| clarkb | at least for me when I read it | 17:46 |
| jeblair | clarkb: so with pre-review, you would need to set "pre-review: false" on gate, post, release, etc. and "allow-in-pre-review-pipelines: false" on jobs like the tarball upload job | 17:47 |
| jeblair | (both of those values would typicall default to true) | 17:48 |
| clarkb | not (untrusted and secrets) -> not untrusted or not secrets? I think the intent is trusted or untrusted but no secrets right? | 17:49 |
| jeblair | clarkb: i like your reasoning, but i think i used the same reasoning to come to favor post-review ("this is a post review pipeline; this job must only run in post review pipelines") | 17:49 |
| clarkb | anyways I think ^ is what my brain is struggling to come to terms with when reading allow-untrusted-secrets | 17:49 |
| clarkb | jeblair: ya I think post review works too | 17:50 |
| jeblair | clarkb: 'untrusted-secrets' is shorthand for "secrets which are used by playbooks which are defined in untrusted projects". that is the case where (no matter what we choose here), we must now allow those playbooks to be run in check. | 17:52 |
| clarkb | because the more I think about it I'm struggling more with the terms in allow-untrusted-secrets than the defaults | 17:52 |
| jeblair | so yeah, i like the other two options (pre-/post- review) because they aren't shorthand for a crazy complicated thing. they're easier to spot if you're about to do something wrong. | 17:53 |
| clarkb | jeblair: ya but untrusted-secrets is defined by allow-untrusted-secrets (at least in the docs) | 17:53 |
| jeblair | (if you say "pipeline: name: check, post-review: true" it's pretty easy to spot the error) | 17:53 |
| clarkb | jeblair: ++ to easy to spot errors | 17:54 |
| fungi | i was already leaning toward post-review of the three, but the arugment above solidified it for me i think | 17:55 |
| fungi | i was having a hard time articulating why it seemed simpler to express in the jobs, but it struck me as the one people defining their own jobs in their individual repos would be least likely to get wrong | 17:56 |
| jeblair | (they all function the same: allow-secrets == allow-untrusted-secrets == post-review == !pre-review; and untrusted-secrets == !allow-in-pre-review-pipelines == post-review) | 17:56 |
| fungi | yeah, that's probably a good summary | 17:57 |
| clarkb | jeblair: also I think it would be more clear if you added your shorthand definition to the docs so that the two terms aren't only defined in terms of each other | 17:57 |
| jeblair | clarkb: the pre-review and post-review changes both drop the phrase 'untrusted-secrets' from the docs... do you mean update something about the docs in those changes, or in the untrusted-secrets change? | 17:58 |
| clarkb | jeblair: for pre/post review I would have something like "Setting post-review to true protects secrets used in untrusted repos from being disclosed as these jobs can only run after review" | 18:00 |
| clarkb | "Reviewers must still perform due diligence to prevent secrets from leaking" | 18:00 |
| jeblair | you write good docs :) | 18:00 |
| jeblair | clarkb: i think we can work that into whatever approach we favor | 18:05 |
| jeblair | clarkb: so we've established your preference for pre/post over 'allow' -- have you looked and pre vs post to favor one over the other? | 18:06 |
| clarkb | I think I have a slgiht preference for post now that I think about it more. As its a clear assertion that "this only runs code that is safe" | 18:07 |
| clarkb | for values of safe :) | 18:07 |
| *** jkilpatr has quit IRC | 19:49 | |
| *** jkilpatr has joined #zuul | 20:25 | |
| jlk | "This is code we can blame a human for should it do bad things" | 20:40 |
| mordred | pabelanger: https://review.openstack.org/#/c/491915/ (I'm only partially here - looking at your patches right now) | 20:46 |
| mordred | jeblair: I like post-review - and have thusly voted | 20:50 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add zuul.project.src_dir variable https://review.openstack.org/491915 | 20:50 |
| mordred | jeblair: https://review.openstack.org/#/c/491915/ still got gated/merged by v2 | 21:07 |
| jeblair | mordred: https://review.openstack.org/493097 | 21:08 |
| jeblair | that should take care of it i think | 21:09 |
| mordred | jeblair: ah | 21:10 |
| jeblair | pabelanger: you want to push that through ^ ? | 21:11 |
| pabelanger | sure | 21:12 |
| jeblair | okay, i'm going to call it for post-review and push it through and abandon the others | 21:13 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Rename allow-secrets to post-review https://review.openstack.org/493059 | 21:17 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-sphinx master: Add jobvar and rolevar directives https://review.openstack.org/493248 | 22:05 |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Add node.cloud to zookeeper https://review.openstack.org/493086 | 22:10 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Use new sphinx roles in docs https://review.openstack.org/493250 | 22:10 |
| jeblair | pabelanger: ^ those two changes should get us a little closer to nice docs | 22:11 |
| pabelanger | cool, will look at docs output shortly | 22:13 |
| jeblair | pabelanger: unfortunately, i think we'll need another zuul-sphinx release before we can see the second one (though that should get easier when we land mordred's tox munging change) | 22:15 |
| pabelanger | ya | 22:16 |
| jeblair | pabelanger: i updated the examples in zuul-sphinx to mimic what happens in zuul-jobs though, so with some imagination, you can get the idea | 22:17 |
| pabelanger | yup, looking at that review now | 22:17 |
| pabelanger | jeblair: +2 493248 but with comment | 22:21 |
| jeblair | pabelanger: well, i think the issue is that sphinx and ansible both have a thing called a role. | 22:35 |
| jeblair | i can't really change either name :) | 22:35 |
| pabelanger | jeblair: ya, I think that is what is confusing me. I didn't know sphinx had roles | 22:38 |
| jeblair | pabelanger: yeah, a bit of text that's marked up is called a role. so: :foo:`bar` is the text "bar" marked up with the role "foo". the "foo" role may apply some style, or other kind of transformation. in our case, it decorates it with a cross reference. | 22:40 |
| jeblair | pabelanger: so :zuul:job:`foo` will cross-reference the zuul job 'foo'. and likewise :zuul:role:`foo`. | 22:41 |
| pabelanger | okay, that helps | 22:42 |
| jeblair | pabelanger: the only time we need to use the phrase "job role" (or "role role") is in explaning what those are to someone writing zuul-sphinx docs. so yeah, we'll have to stress that point when we get around to writing the docs for zuul-sphinx itself. | 22:42 |
| pabelanger | Ya, I think that helps too | 22:43 |
| jeblair | it's definitely worth an explicit mention when we do that | 22:43 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!