#400 Various enhancements to make life easier for Bodhi development environment
Merged by ngompa. Opened by adamwill.
adamwill/ipsilon dev-fixes  into  master

Download 400.patch

This PR rolls up several changes to make it easier to use an Ipsilon server using testauth running on localhost (in a podman pod) as the auth backend for a container-based Bodhi development environment.

The HTTP config file changes are a bit arguable, I guess, so I split them into two commits so we can take the first without the second if wanted. I could get by with neither of them, it'd just mean doing a bit more work in the container file, but I do think overall they make sense.

The default attribute mapping change makes a Bodhi client work without the ipa extension, which without this commit is the only easy way to set the default attribute mapping (otherwise we'd have to hack it straight into the database, which is kind of a pain). We need to map a few attributes that Bodhi expects to see (like 'groups' and 'nickname').

The special-username-groups mechanism isn't strictly required but it makes it much easier to test the effects of different group memberships in the Bodhi dev env without having to e.g. change the --testauth-groups arg and redeploy. You can use the "user is always a member of the group with the same name" mechanism to some extent, but if you want a user to be a member of two specific groups, or you want to make a user not a member of a group that's in the default groups, that's not flexible enough. Initially I implemented this as a deploy-time config setting to specify "special users", but I like this more - it's way more flexible and actually shorter/easier code.

I'll try and add tests for some of these soon, but I wanted to send out the PR so folks looking at the Bodhi dev env PR - https://github.com/fedora-infra/bodhi/pull/5565 - know what the changes I made to Ipsilon actually are.

These changes look good, overall. Looking forward to tests for this change set! :smile:

oh, yeah, forgot I said I was going to try and add tests...

Ugh.

Well, I looked at it for a couple of hours, and frankly I don't know where to start. This project's test suite is, well, weird. It doesn't use any standard test framework, it invented its own. It doesn't really seem to have any conception of unit testing, it's all functional testing. It's just very difficult to fit testing of the first four commits into this framework at all - they all do stuff which it just isn't really set up to test. The test process does involve writing out HTTP config files, for instance, but this all happens in setup steps, I cannot see any sane way to get a check that the config file contains expected content to happen in a test phase rather than the test setup phase. (AFAICS we are not explicitly testing anything else about how we write HTTP server config files - we're just implicitly testing them by relying on the servers working as configured for the other tests, but of course it's not at all clear whether that actually properly exercises all possible paths for the HTTP server config generation stuff).

The fifth commit might fit, but...there's no existing test which actually checks that testauth works and does what it's supposed to. It's implicitly tested by all the other tests using testauth, but I don't really know where - if anywhere - it would be appropriate to crowbar in a test of this (or exactly how, given that the login process seems quite obfuscated in the tests).

Sorry. If anyone has any advice or ideas on how to add tests for this, I will give it another shot. But I don't want to waste any more time trying to puzzle it out for myself.

@ngompa @puiterwijk is anyone able to review this? The Bodhi PR that uses these changes is now merged, so I'd kinda like to put them in the Fedora packages so that Bodhi feature doesn't need to use a side build of ipsilon I have to remember to keep up-to-date.

I'm sorry I couldn't add tests, but it just really isn't obvious how to do so. I am happy to try again if someone has pointers.

If I don't hear back on this in a few weeks I'm gonna go ahead and backport the changes to Fedora packages.

These changes seem reasonable. :thumbsup:

note I can't merge myself, I'm waiting for a maintainer to do so.

Pull-Request has been merged by ngompa

Metadata