Open Stack

On Tue, Dec 17, 2019, at 10:16 AM, Saul Wold wrote:
> 
> 
> On 12/16/19 8:22 AM, Clark Boylan wrote:
> > On Mon, Dec 16, 2019, at 7:46 AM, Saul Wold wrote:
> >>
> >> Hi Clark,
> >>
> >> Sorry, I only get the archive of Infra and Ghada is not on the list, if
> >> you can please reply to us and the list that would be great.
> >>>
> >>> I think what happened here is you merged bug fixes (in this case cve bug fixes) from master into a feature branch. Then when you pushed that merge commit and merged it, the bot noticed that those bug fixes had merged to the feature branch and commented with those details on the bug. I believe this is "correct" behavior from the bot.
> >>
> >> Is there a different way to do the merge activity?
> >>
> >>>
> >>> Is the issue the existence of comments like https://bugs.launchpad.net/starlingx/+bug/1844579/comments/18 on the bugs? Or is there some other metadata that is being added that I am missing?
> >>>
> >> Yes, that comment does not belong with that bug and because the comment
> >> includes CVE-2019-XXXXX formating it adds the CVE References metadata also.
> > 
> > Can you expand on this? Why does the comment not belong with the bug? The bug was fixed on the f/centos8 branch and that is what the comment is telling you. Where is the CVE References metadata?
> > 
> The "merge commit" message contains all the commits that are part of the 
> merge commit.  I guess the hook sees the merge commit with the Closes: 
> tag and adds the complete commit message to the associated launchpad 
> bugs (in the case of multiple closes due to multiple commit messages in 
> the merge commit.
> 
> Since that larger "merge commit" message contains CVE reference they get 
> added to the Closes: tagged bugs. Look again at 
> https://bugs.launchpad.net/starlingx/+bug/1844579
> Below the description is the CVE Reference with links to the CVE 
> mentioned.  This launchpad has nothing to do with the CVEs in question. 
> I guess this is done inside launchpad, not in the opendev bugtask.
> 
> Does that make more sense?

Yes, that helps. And yes I believe launchpad is doing its own string scraping and deciding to list those CVEs. I don't believe we are triggering that explicitly.

> 
> >>
> >>> If we don't want comments like that to appear you'd need to modify your merged trees so that bug fixes don't go from master into the feature branch. Or we'd need to come up with some rule set we can apply to the bot to filter bugs out in certain circumstances.
> >>
> >> Modifying the merge trees would defeat the purpose of doing the merge I
> >> think. Does this issue not affect other projects or are we yet again
> >> doing strange operations in StarlingX ;-)!  Not sure how hare it would
> >> be to filter for feature branches.
> > 
> > Yes, you probably don't want to change the merge trees as the idea here is to bring the feature branch up to date, and probably the most important aspect of that is ensuring you've merged security fixes.
> > 
> > Use of feature branches at all may qualify as "strange". Most projects tend to develop against their target branch. You'll see large change series from nova for example rather than creating feature branches for that work. This means most projects are never in a situation to potentially hit this problem. One major historical exception to this has been the swift project. It is possible they have run into this problem but ignored it? Or not seen it as problematic?
> >
> I think we chose to use feature branches since there are multiple repos 
> in StarlingX and we need a way to coordinate work across them.

Note, Zuul's depends-on functionality is designed to address the need for coordinating work between repos without needing to drastically change workflow.

> 
> They might not have as many CVE reference also, since StarlingX has many 
> references to Linux Userspace which can contain more CVEs.
> 
> > I did double check that the change merge hook code doesn't handle feature branches as a special case already (openstack uses the feature/ prefix not f/ so thought maybe there was a difference in matchers?) but I found nothing. https://opendev.org/opendev/jeepyb/src/branch/master/jeepyb/cmd/update_bug.py#L222-L252 is the code in question and what we'd end up updating if we wanted to apply some rule set to the bot around feature branches.
> > 
> Yeah, I agree a check here might be the right place for this.

Based on the above description of the problem what we'd need to do here is remove CVE references from merge commit comments on Launchpad? The tricky bit is knowing when that is appropriate or not and "this is a merge commit" might be the answer. One way to do that would be to report only the merge commit message to the bug. You'd potentially lose launchpad synchronization if you wanted updates in the child commits though.

> 
> > Something else to keep in mind, there has been some discussion of replacing these existing bots with Zuul jobs similar to how github replication is done. That could possibly give different repos far more flexibility through Zuul configuration specific to that repo. This may be another approach worth taking if we find we end up doing something StarlingX specific.
> > 
> Something to consider down the road.
> 
> Sau!
> 
> >>
> >> Thanks
> >> Sau!
> >>
> >>
> >>
> >>
> >>
> >> On 12/13/19 8:48 AM, Saul Wold wrote:
> >>>
> >>> Hello Infra team:
> >>>
> >>> Apparently something got messed up with Launchpad and updating a number
> >>> of starlingx repos with a feature branch.
> >>>
> >>> I was following the methodology of updating a feature branch with
> >>> changes from master via merges and I guess when I pushed that to gerrit
> >>> and it merged, it caused some Launchpad ugliness. See email below.
> >>>
> >>> Thoughts?
> >>>
> >>> Thanks
> >>> Sau!
> >>>
> >>>
> >>>
> >>> -------- Forwarded Message --------
> >>> Subject:     CVE References in LPs are messed up after centos feature
> >>> branch rebase
> >>> Date:     Fri, 13 Dec 2019 00:30:26 +0000
> >>> From:     Khalil, Ghada <Ghada.Khalil at windriver.com>
> >>> To:     Saul Wold <sgw at linux.intel.com>
> >>>
> >>>
> >>>
> >>> Hi Saul,
> >>>
> >>> The CVE References in about 15 LPs are now messed up after the rebase of
> >>> the f-centos8 feature branch. The rebase updated a large # of launchpads
> >>> and somehow automatically added CVE references (from a subset of bugs)
> >>> to all of them. Any idea what is going on here?
> >>>
> >>> Here are some examples:
> >>>
> >>> https://bugs.launchpad.net/starlingx/+bug/1844579
> >>>
> >>> Originally had no CVE References. Now it has 3 references.
> >>>
> >>> https://bugs.launchpad.net/starlingx/+bug/1849200
> >>>
> >>> Originally only had CVE-2018-15686 as a CVE Reference. Now it has all
> >>> the recently fixed CVEs linked to this bug.
> >>>
> >>> Snapshot from the full activity log:
> >>>
> >>> Here is the query that shows that all the bugs that were picked up in
> >>> the rebase now have CVE links:
> >>>
> >>> https://bugs.launchpad.net/starlingx/+bugs?field.searchtext=&orderby=-importance&field.status%3Alist=NEW&field.status%3Alist=OPINION&field.status%3Alist=INVALID&field.status%3Alist=WONTFIX&field.status%3Alist=EXPIRED&field.status%3Alist=CONFIRMED&field.status%3Alist=TRIAGED&field.status%3Alist=INPROGRESS&field.status%3Alist=FIXCOMMITTED&field.status%3Alist=FIXRELEASED&field.status%3Alist=INCOMPLETE_WITH_RESPONSE&field.status%3Alist=INCOMPLETE_WITHOUT_RESPONSE&assignee_option=any&field.assignee=&field.bug_reporter=&field.bug_commenter=&field.subscriber=&field.structural_subscriber=&field.tag=in-f-centos8&field.tags_combinator=ANY&field.has_cve.used=&field.has_cve=on&field.omit_dupes.used=&field.affects_me.used=&field.has_patch.used=&field.has_branches.used=&field.has_branches=on&field.has_no_branches.used=&field.has_no_branches=on&field.has_blueprints.used=&field.has_blueprints=on&field.has_no_blueprints.used=&field.has_no_blueprints=on&search=Search
> >>>
> >>>
> >>> *Ghada Khalil*, Manager, Titanium Cloud, *Wind River*
> >>> direct 613.270.2273  skype ghada.khalil.ottawa
> >>>
> >>> 350 Terry Fox Drive, Suite 200, Kanata, ON K2K 2W5
> >>>
>