Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing embeddable-build-status plugin #3013

Closed
daniel-beck opened this issue Jun 22, 2022 · 31 comments
Closed

Consider removing embeddable-build-status plugin #3013

daniel-beck opened this issue Jun 22, 2022 · 31 comments

Comments

@daniel-beck
Copy link

Service(s)

ci.jenkins.io

Summary

https://plugins.jenkins.io/embeddable-build-status/ is not actively maintained. The Jenkins security team had to address multiple vulnerabilities in https://www.jenkins.io/security/advisory/2022-06-22/ to protect Jenkins project infrastructure. While the maintainer (thomas-dee) was happy to let us release the fixes we wrote, i.e. was responsive via email, he also told us he has no capacity to assist in any way. This is unsustainable. Consider removing the plugin from ci.jenkins.io.

Reproduction steps

No response

@daniel-beck daniel-beck added the triage Incoming issues that need review label Jun 22, 2022
@dduportal dduportal added this to the infra-team-sync-2022-06-28 milestone Jun 22, 2022
@NotMyFault
Copy link
Member

+1, but then I'd vouch for removing it from all CI instances, including infra, release and weekly.
I'm not able to tell if it's available on cert and trusted too, ftr.

@daniel-beck
Copy link
Author

all CI instances, including infra, release and weekly

It's unlikely to be a big problem inside the VPN, but yes, that never made sense to me. My guess: a leftover from extending jenkins-infra/jenkins-infra config to more than just ci.j.io.

@dduportal
Copy link
Contributor

PR to remove it from the weekly and LTS images used on our Kubernetes-hosted controllers (weekly, infra and release):

For trusted and cert, a manual check is required

For ci.jenkins.io, we'll need to let developer knows on the mailing list. Reason is some plugins developers might have a badge on their README.md|.adoc and that would be a nice courtesy for them

@MarkEWaite
Copy link

Thanks for considering the impact on plugins @dduportal . I find 169 plugin repositories that refer to embeddable build status from ci.jenkins.io. The only harm from the removal is that the images will display as broken on the pages where they are referenced, but it would certainly be a nice gesture to plugin maintainers if we submitted pull requests that remove the embeddable build status references from those plugin pages.

@NotMyFault
Copy link
Member

but it would certainly be a nice gesture to plugin maintainers if we submitted pull requests that remove the embeddable build status references from those plugin pages.

If https://shields.io/ supports the use case of displaying a build status or code coverage for ci.jenkins, URLs could be updated to use that.
I know that @jetersen shared their workflow to update the maven cd action across a lot of repositories recently, unfortunately I don't have it at hand atm :(

@jetersen
Copy link

Check my gist profile 😃

@dduportal dduportal removed the triage Incoming issues that need review label Jun 28, 2022
@timja
Copy link
Member

timja commented Jun 28, 2022

I came across this earlier could be useful:
https://github.com/lindell/multi-gitter

@jetersen
Copy link

multi gitter seems easier than whatever I was hacking away at.
Although I used code search to find the repos I needed to mess with.
There seems to be no filter options other than clone all repos in the org.

That is quite a few in jenkinsci org 😅

@lemeurherve
Copy link
Member

multi-gitter is perfect for the job, thanks for the pointer @timja!

There seems to be no filter options other than clone all repos in the org.

You can choose which repository you want to include or exclude.

I've ran some tests, it doesn't take too long to run multi-gitter on all jenkinsci repositories, and there is a concurrency parameter we can use if needed.

I've prepared the bulk PRs, ready to go, here is the pull request I've generated as an example, WDYT?

@jetersen
Copy link

jetersen commented Jul 9, 2022

@lemeurherve couldn't you use shields.io? https://shields.io/category/build

Also your commit shows up unsigned 😉

@lemeurherve
Copy link
Member

couldn't you use shields.io? shields.io/category/build

Damn, I should have checked it more thoroughly when @NotMyFault mentioned it, I'll look into it, thanks for the additional suggestion @jetersen

Also your commit shows up unsigned 😉

Yup, noticed that too, preparing a multi-gitter fix 🙂

@jetersen
Copy link

jetersen commented Jul 9, 2022

Seems like jenkins.io is not populating the json api anymore.
So shields.io won't work.

https://ci.jenkins.io/job/Plugins/job/scm-filter-branch-pr-plugin/job/master/api/json

Looking at the code for shields.io this the URL https://ci.jenkins.io/job/Plugins/job/scm-filter-branch-pr-plugin/job/master you would provide and it would append /api/json and look at the status field which is fairly innocent:

https://github.com/badges/shields/blob/c73072deed8eb6a5b9f51a06377f5f396ef21842/services/jenkins/jenkins-build.service.js#L72-L78

@lemeurherve
Copy link
Member

Seems like jenkins.io is not populating the json api anymore.

Do you/someone know where I can find more about this?

Their example instance is still providing the json: https://wso2.org/jenkins/view/All%20Builds/job/archetypes/api/json

@lemeurherve
Copy link
Member

We can add a cacheSeconds parameter to the URL (from https://shields.io/endpoint/):

cacheSeconds: (Default: 300, min 300.) Set the HTTP cache lifetime in seconds, which should be respected by the Shields' CDN and downstream users. Values below 300 will be ignored. This lets you tune performance and traffic vs. responsiveness. The value you specify can be overridden by the user via the query string, but only to a longer value

@lemeurherve
Copy link
Member

lemeurherve commented Jul 10, 2022

@calebcartwright @chris48s is there a list of the IPs used by shield.io available somewhere so we could whitelist them to replace jenkinsci badges by shield.io ones?
I looked around but didn't found anything.
Found this page, will also ask on your discord server.

Also saw badges/shields#4962 and badges/shields#5346
I'm wondering if a similar token or a user/pwd pair could be added for ci.jenkins.io
WDYT @dduportal ?

@lemeurherve
Copy link
Member

Meanwhile, I managed to get PR with signed commit: jenkinsci/jenkins-infra-test-plugin#38

@dduportal
Copy link
Contributor

@calebcartwright @chris48s is there a list of the IPs used by shield.io available somewhere so we could whitelist them to replace jenkinsci badges by shield.io ones? I looked around but didn't found anything. Found this page, will also ask on your discord server.

Also saw badges/shields#4962 and badges/shields#5346 I'm wondering if a similar token or a user/pwd pair could be added for ci.jenkins.io WDYT @dduportal ?

I'm not sure to understand: where would be store and use this user/password?

@lemeurherve
Copy link
Member

lemeurherve commented Jul 10, 2022

IIUC, we could let shield.io requests pass with a token or a basic auth stored on our side as secret.
On shield.io side, it would be stored server side as secrets.

@dduportal
Copy link
Contributor

IIUC, we could let shield.io requests pass with a token or a basic auth stored on our side as secret. On shield.io side, it would be stored as secrets server side.

Can you dig a bit and lists here the procedure? The link you provided is a YAML file with a list of parameters referencing upper cases strings that I assume to be environment variables.
But where are such token set up ? Is is a server managed by someone else ? A server to host on our own ?

It seems a great feature but it really not clear what it does

@calebcartwright
Copy link

@calebcartwright @chris48s is there a list of the IPs used by shield.io available somewhere so we could whitelist them to replace jenkinsci badges by shield.io ones?

As far as I know, we still do not view the the IP allowlist based approach for services to grant Shields.io an increased rate limit as being a viable strategy. We used to go this route with vendors/upstream services when our environment was deployed on lower level IaaS type services where we had more direct control over networking configuration, but our move to more PaaS/SaaS runtime environments largely obfuscates the networking specifics from us and is subject to change at any time beyond our control and without any warning nor notification.

As such, in cases where Shields.io would need to send more traffic to an upstream platform/service than their rate limits would permit, the path we've often taken is to collaborate with the upstream vendor/maintainers/etc. to obtain something Shields.io can then send in the request headers (most often auth related) whicht the upstream platform can then utilize to grant a higher rate limit to Shields.io specifically.

I confess I'm not fully up to date on the specifics of this thread and the various moving parts, but I believe/hope the above covers the Shields related questions. Let me know if not though, and/or if there's any other questions we can answer!

@lemeurherve
Copy link
Member

Thanks for your response @calebcartwright! I'm not sure proposing a patch in your server codebase to take care of ci.jenkins.io requests would really interest you as there aren't that many uses.

I've suggested to self-host a shield.io instance so we could deal ourselves with the auth.

Now the question is: do we remove the existing embedabble badges or do we replace them with badges pointing to a self-hosted shield.io instance?

@dduportal
Copy link
Contributor

Now the question is: do we remove the existing embedabble badges or do we replace them with badges pointing to a self-hosted shield.io instance?

I vote we remove the plugin + badges (with the batch PR). But we mention the new issue #3044 to explain we are working on something else soon.

Reason of this "2 steps proposal": the origin of this issue is the security team asking to remove the plugin from ci.jenkins.io.

@daniel-beck
Copy link
Author

Reason of this "2 steps proposal": the origin of this issue is the security team asking to remove the plugin from ci.jenkins.io.

FTR we fixed what was wrong with the plugin in https://www.jenkins.io/security/advisory/2022-06-22/ with the maintainer's approval, but it's still not a well-maintained plugin, and fairly exposed. It is not urgent for us, but would be useful longer term.

@jglick
Copy link

jglick commented Jul 11, 2022

jenkinsci/archetypes#228 turned out to be prescient…

timja pushed a commit to jenkins-infra/repository-permissions-updater that referenced this issue Jul 14, 2022
…2648)

https://docs.google.com/document/d/11Nr8QpqYgBiZjORplL_3Zkwys2qK1vEvK-NYyYa4rzg/edit?disco=AAAAYXxgRHI
provides more context for the adoption request.  We believe it may be
cheaper and easier to adopt this plugin than to remove the plugin from
ci.jenkins.io and update the many repositories that refer to it.

Help desk jenkins-infra/helpdesk#3013 suggests
that we should consider removing the plugin because it is not actively
maintained.

Help desk jenkins-infra/helpdesk#3044 proposes
steps to replace the use of this plugin with something hosted elsewhere
on Jenkins infrastructure.

One of the existing maintainers needs to approve the adoption request.
Existing maintainers are:

* @thomas-dee
* @mgedmin
* @christiangalsterer
* @jglick
@lemeurherve
Copy link
Member

As @MarkEWaite and @darinpope adopted the plugin, it doesn't have to be removed from ci.jenkins.io.

@lemeurherve
Copy link
Member

Thanks for these links @jglick

@jetersen
Copy link

jetersen commented Jul 15, 2022

@lemeurherve perhaps you could write a gist or github repo for how you did the multi-gitter stuff :)

I may want to rerun jenkins-infra/github-reusable-workflow to discover plugins not yet moved. I have found some manually after the fact that was not found through code search

@lemeurherve
Copy link
Member

@jetersen
Copy link

@lemeurherve neat! I'll try and see what I can catch with multi-gitter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants