[OBS] add Open Build Service service-badge #6993
Conversation
|
|
Never done this before, so I expected to fail some ci tests. However I did add tests ( I also don't want to give circleci rw access to all my repositories ( + the likely inbox spam that'd follow if I have them access to my account), so I'd be great if some one could tell me why it's failing so bad :D. |
Thank you for sharing this perspective. I've been using Circle for so long in one project or another that I didn't even realize that was a requirement. We have had some casual conversations about CI and investigating GitHub Actions, and although I don't anticipate we'll be making changes (at least not in the immediate future), I do think this POV and impact on contributors is something we should keep in mind @badges/shields-ops
looks like all the failures stem back to this:
You should be able to test this locally via |
it seems like that the replaceAll method of the String type was only introduced in node 15.0.0 (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll#browser_compatibility). That's why the service tester worked flawlessly on my node 16.6.2 machine. (however the main@node-16 test also seems to be failing on ci) |
Linter is failing too:
|
ah, I didn't run that. Assumed that was part of the pre-commit hook |
Do we know if this is just the case with the reference server, or is it the open build service product that enforces this? If it's the former, then the implementation could be much simpler, but if it's indeed the latter then I can see why you went the way you did. |
AFAIK every buildserivce instance enforces it. It seems to be part of OBS documentation and testing build.openeuler.org and build.tizen.org, both return <status code="anonymous_user">
<summary>Anonymous user is not allowed here - please login</summary>
</status> |
|
Got it, thanks! After thinking on this one a bit, I actually don't think we need the complexity (or "hack" as you called it 😄) given that the service is utilizing basic auth as opposed to instance specific tokens. In theory, anyone deploying a Shields server (including our main Shields.io) could setup an account with the same user/pass on multiple OBS instances, and could leverage the current auth config structure with a multi-entry value for That would warrant providing users the ability to specify the instance url via a query param, which is a very common pattern implemented in various other services. I think we'd probably still want to use a default url of the main reference instance, which we could either just bake in as the main reference instance (via default value on said query param), or if we really feel there's a need for self-hosters to be able to specify their own non-reference instance as the default then pivoting a bit on what you have here currently to make it just the default instance, but don't use it to lock down/reset the authorized origins to that single instance. Hope that makes sense, but let me know if you have any questions |
|
Yeah, I see what you mean. I like the idea of supporting multiple instances. However I'm unsure how using the same username and password holds up security wise. I'd rather have a obs: Joi.object().pattern(requiredUrl, Joi.object({
user: Joi.string(),
pass: Joi.string()
}))in the private config that allows a user/pass pair for each instance. However I'm unsure how that could work in regards to the auth-helper :) |
Fundamentally I just don't want to introduce the extra code and deployment complexity that's primarily just to rule out such a use case. I understand the thrust of your point on the security, but I don't think general security practices around reusing credentials are much of a factor in this specific context. For the Shields.io case, having our typical degree of flexibility could allow us to support more than one instance which would be useful especially if there's a couple widely used instances beyond the reference one. We treat the security and protection of the auth information used on Shields.io very seriously, and equal across the many that we have. Even in some worst case scenario, there's no substantial difference in the hypothetical blast radius of the exposure our non-active/real account info for both https://api.opensuse.org and https://build.openeuler.org, as opposed to just one. For the self-hosting badge server, I don't anticipate the need for multiple instances being very common, but I also don't view it as our job to promote/prevent account strategies on a different tool either. Folks would always have the option of creating their own "badge reader" type of account that could have authorization managed appropriately within the instances. Finally, while admittedly the accounts on OBS seem local (though haven't looked to closely), it's somewhat common for self-hosting Shields users to interact with multiple instances of a tool that works with a backing IdP (e.g. Jira Server/DC) and have the ability to utilize a single, privileged account in the server auth config and the those valid instances being allowed in the
Absolutely! It's something we've been talking about for a while and there's plenty of use cases, likely including this one, where it'd be extremely helpful. However, that's not an easy left and is unlikely to be available soon, so we'll need to work with what's possible to do. |
|
Updated. I hope I haven't missed an easier method for authhelper to get the specified authorizedOrigins |
There was a problem hiding this comment.
Thanks again for the PR! I've left a number of inline comments below in places where we'll need some changes. It may seem like a high quantity at first glance, but it's really just several smaller opportunities for simplification and some more minor updates to be more consistent with our typical build badges.
Let us know if you have any questions!
obs_userName is now called obs_user and obs_userPass is called obs_pass Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>
|
This is up and live now on https://shields.io, but the upstream API/reference server is very sluggish in its responses which means that a lot of the badges will exceed GitHub's timeout and they won't render There's not much utility in us providing badges that aren't practically usable, so we'll need to keep an eye out and potentially need to revisit. |
|
hmm, it usually isn't, I also never had issues with it during testing. However it seems like the reference instance has some issues right now, most of the times my browser times out before it loads; status.opensuse.org however tells "Operational". I'll ask on buildservice@lists.opensuse.org if this is a known issue, however I don't expect an answer during the weekend. Edit: nvmd, someone already had that issue today: |
|
should work again. The badges load far faster for me now (almost instant :)) and I can access the webui again. According to Henne Vogelsang the issue was, that:
I guess you concern regarding
is therefore resolved? |
Yes indeed! Thanks for following up. Obviously windows like this happen from time to time (including with our own service), but wanted to make sure that was an exception/issue driven behavior as opposed to the standard response times we could expect. |
this is my attempt of implementing a package status badge, like I suggested in #5862.
However, I did not follow the badge guidelines entirely (repository names are put in the "label" part of the badge, which may conflict with the "not advertise" and lowercase rule of it). I also made, what can be considered a "dirty" hack, by overwriting authHelper._authorizedOrigins in the constructor (due to the nature of authentication, there can only be a OBS single instance that can be used).