Use dynamic ports for BackgroundHTTPServer

[comps]

This is a follow-up to 3277a78 from #210.

I've tested

/hardening/anaconda/stig$
/hardening/image-builder/stig$
/scanning/disa-alignment/anaconda$

(the main/only tests impacted by this) and they all seem to pass/warn just fine, so I presume it works.

I also ran some multi-profile runs of anaconda / image-builder to make sure there isn't anything persistent that would break 2 or more tests running sequentially on a single system.

[comps]

For the record (in case you get the idea during review), this:

-srv = util.BackgroundHTTPServer(virt.NETWORK_HOST, 0)
-with srv:
+with util.BackgroundHTTPServer(virt.NETWORK_HOST, 0) as srv:

doesn't work - the __enter__ and __exit__ functions are methods of srv, which (the same object) is then further used inside the with block. The as syntax is for when __enter__ itself returns something to be used in the block.

I guess we could return self from __enter__, but maybe that would be even more confusing for readers / maintainers, as there would be 2 not-so-obvious ways to get the object.

[mildas]

I guess we could return self from enter, but maybe that would be even more confusing for readers / maintainers, as there would be 2 not-so-obvious ways to get the object.

What do you mean by there would be 2 not-so-obvious ways to get the object? If we return self from __enter__ and use it only in with+as, then there would be basically "only" one way we would be okay with.

[mildas]

To be consistent with other BackgroundHTTPServer use cases, shouldn't it be like this?

Suggested change

	srv = util.BackgroundHTTPServer(NETWORK_HOST, 0)
	srv.add_dir(repo, 'repo')
	stack.enter_context(srv)
	srv = util.BackgroundHTTPServer(NETWORK_HOST, 0)
	stack.enter_context(srv)
	srv.add_dir(repo, 'repo')

[comps]

No, it's better to add the file before starting the server if possible (to fully eliminate any chance of race conditions).

In the other cases, it's not possible.

[mildas]

Why not? For example in hardening/anaconda you can add remediation-ds.xml outside of srv context and so add_file before context as well.

[comps]

Hmm, that's a good point - the anaconda use case could probably be done before starting the server.

I'll fix it and add .start() to the server, so that object initialization is more clearly decoupled from starting the server and we don't rely on the sneaky context manager behavior.

[comps]

I guess we could return self from enter, but maybe that would be even more confusing for readers / maintainers, as there would be 2 not-so-obvious ways to get the object.

What do you mean by there would be 2 not-so-obvious ways to get the object? If we return self from __enter__ and use it only in with+as, then there would be basically "only" one way we would be okay with.

The problem is the enter_context() cases where we need to

1. get the server object, for adding files to
2. add files to the object
3. start the object as a server

with some other http-server-unrelated code in between.

It's a subtle difference but we would essentially be doing the equivalent of

with ExitStack() as stack:
    f = open('file.txt')
    stack.enter_context(f)

which is not the same as

with ExitStack() as stack:
    stack.enter_context(open('file.txt'))

due to how the context manager is called.

---

Maybe I should side-step this issue and a .start() function, to remove the confusion of the cases mentioned above,

with util.BackgroundHTTPServer('127.0.0.1', 0) as srv:
    srv.add_file(...)
    srv.start()   # can be swapped with add_file()
    ...

or

srv = util.BackgroundHTTPServer('127.0.0.1', 0)
srv.add_file(...)
stack.enter_context(srv.start())   # can be swapped with add_file()

?

[comps]

I've pushed an alternate approach - have a look at the module help for lib/util/httpsrv.py.

Hopefully this makes things a bit clearer.

[comps]

Tested on -t '/hardening/image-builder/(ospp|stig)$' -t '/hardening/anaconda/(ospp|stig)$' -t '/scanning/disa-alignment/anaconda$' --arch x86_64 --rhel 9 --rhel 8 and it seems to work.

[mildas]

Yep, code looks good

Nit-pick: f8af92c is unnecessary now. Could you drop it?

[comps]

Yep, code looks good

Nit-pick: f8af92c is unnecessary now. Could you drop it?

Done.