Open Stack

Clark:

Happy New Year!

Circling back on this after the holidays, where do we stand, I am about 
to do another merge for the feature branch, since for the work we are 
doing across repos is best via feature branch vs Depends-On as talked 
about below.

We talked about only reporting an abbreviated commit message when doing 
a merge commit, and I asked if a storyboard or launchpad was needed, did 
I lose a reply?

Remember not all of us are on the Infra mailing list.

Thanks
   Sau!

On 12/18/19 10:19 AM, Saul Wold wrote:
> 
> 
> On 12/17/19 10:36 AM, Clark Boylan wrote:
>> 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.
>>
> Yes, I realized that after looking at the code you shared with me 
> earlier.  This is part of why we might want to consider a simpler merge 
> message to avoid this scraping problem.
> 
>>>
>>>>>
>>>>>> 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.
>>
> We are aware of that, but it's more about the StarlingX workflow and 
> enabling of changes like moving to a new base OS needs to be done 
> outside of master.  So we still need to use the feature branch to enable 
> and test new functionality outside of master.
>>>
>>> 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.
>>
> Yes, As I mentioned above having an abbreviated commit message posted 
> via the hooks is likely the best approach so we can at least track the 
> merge happend for those bugs.
> 
> Do you need some kind of storyboard or launchpad for this kind of change?
> 
> Thanks
>     Sau!
> 
>>>
>>>> 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
>>>>>>
>>>