feat: add haproxy_spoe_auth interface library

[Thanhphan1147]

Add an interface to allow connecting haproxy with SPOE ( stream process offloading engine ) agents for OIDC authentication.

SPOE ( Stream Process Offloading Engine ) allows haproxy to be extended with "middlewares". SPOA are agents that talk to haproxy using the Stream Process Offloading Protocol ( SPOP ). In short, haproxy sends some information about the request to the agent, and the agent replies with a set of flags containing information, most of the time they indicate which actions need to be taken.

Our goal is to support the Canonical Identity Platform authentication flow and use haproxy as an authentication proxy. In this context, when haproxy is integrated with the spoe-auth agent, every request reaching the haproxy frontend will be passed through the authentication proxy and requires a successful authentication with the IDP to be able to be routed.

[dimaqq]

Design doc: https://discourse.charmhub.io/t/specification-isd257-haproxy-auth-proxy-using-spoe-auth-relation/19264

[james-garner-canonical]

Hi @Thanhphan1147, thanks for your PR, it's very nice to see. Some additional context for the PR description would be helpful. We dug a little bit and found your spec, and the haproxy-spoe-auth-operator PR and the (closed) library PR. Since this is a public interface (e.g. used by a listed charm, intended to support other providers/requirers), the library should indeed live in this repo.

There are a couple of documentation related things that would need to be addressed before merging, and we'll also take some time to review the interface design.

Library documentation:

• The package docstring should provide some usage instructions, perhaps with an example of how to set this up from the charm side.
• The docs build is failing because of the Pydantic classes redundantly documenting their fields in the class docstring and the Pydantic fields -- I'd suggest dropping the copy in the class docstrings.

Interface documentation (perhaps best addressed alongside the interface design feedback):

• The interface itself should be documented under interfaces/haproxy_spoe_auth/interface/v0/README.yaml in human-readable form (like this).
• It may be worth moving the databag model classes to interface/v0/schema.py, but let me get back to you on that one.
• The current provider and requirer should be documented under interface/v0/interface.yaml like this (example from tracing):

providers:
  - name: tempo-coordinator-k8s
    url: https://github.com/canonical/tempo-operators

requirers:
  - name: pyroscope-coordinator-k8s
    url: https://github.com/canonical/pyroscope-operators

Design review to follow.

[Thanhphan1147]

Hi @james-garner-canonical, thanks for reviewing the PR! I've updated the PR to include the library and the interface documentation. Let me know if they make sense. Thanks!

[Thanhphan1147]

I've also update the PR description to include a bit more context.

[dimaqq]

Question: what happens when the provider is scaled out?

Are all ip addresses equivalent? Should haproxy use a random ip in the set? Can the provider unit signal that while it exists, it's not ready to receive spop callbacks?

[Thanhphan1147]

It's possible for haproxy to handle multiple units of the spoe-agent. In short, haproxy will use the addresses for 2 backends:

1. The spop backend ( mode tcp, used to send/receive SPOE messages )
   If there are 2 units with IP at 10.0.0.1 and 10.0.0.2 then the section will look something like this:

backend spoe_agents
    mode tcp
    balance source
    hash-type consistent
    option spop-check
    
    server unit-1 10.0.0.1:12345 check
    server unit-1 10.0.0.2:12345 check    

2. The OIDC callback backend ( mode http, used to receive the authorization code and issue redirect to set the browser cookie )

As we'll add health-check for each of the server in the backend, if one of the provider's unit is not responding to health-check requests, then the unit will be marked as down and request won't be forwarded to it anymore. If no backend is available I believe haproxy will give you a 502/503 directly

[Thanhphan1147]

One thing to note also that haproxy makes no assumption about the implementation details of the provider, so it's entirely possible that the agent is not stateless and doesn't work well when scaled out, but in those cases the agent charm should be conscious about that and not allow more than one unit.

And haproxy will also try its best to ensure stickiness with

    balance source
    hash-type consistent

but ultimately ensuring consistency when scaled out should be the responsibility of the agent charm's workload I think

[Thanhphan1147]

@dimaqq Thanks for the review! I've replied to the comments that you made. Please let me know if there are something that are still not clear in my replies!

[Thanhphan1147]

Hi, @james-garner-canonical / @dimaqq, thanks for the reviews on the PR so far! I just had a discussion with my team and it seems like we'd have to pause work on this PR while we rethink some of the aspects of the design of the overall flow.

To not waste your time reviewing this PR which will change again please allow me to move this PR back to draft for now and I'll mark it ready for review once it's ready.

Thanks!

[james-garner-canonical]

No problem, @Thanhphan1147, I'll wait till the PR is marked ready for review before commenting further (or you can ping me).

[dimaqq]

A couple of high-level comments:

1. the integration is very specific to haproxy, but then again the library is marked as such, so I guess that's OK?
2. please document more extensively. it's better to repeat yourself than rely on the next engineer to go digging in haproxy docs to understand the a) problem being solved and b) the value of some particular field

[dimaqq]

Suggested change

# "ops",

[dimaqq]

Since ops with a version specifier is required above.

[dimaqq]

Is there a reason to expose the exact package version?

If there isn't a good reason, how about versioning the package in a simple way:

• pyproject.toml version field
• remove this file
• remove the test
• remove the dummy charm used to test the version

WDYT?

[james-garner-canonical]

I'm all for removing the version tests once a library has real tests instead, but I think it would make sense for charm libraries to keep following the template and expose and define their versions like this for consistency. Personally I think __version__ can be handy for debugging.

BTW note that @Thanhphan1147 commented earlier:

Hi, @james-garner-canonical / @dimaqq, thanks for the reviews on the PR so far! I just had a discussion with my team and it seems like we'd have to pause work on this PR while we rethink some of the aspects of the design of the overall flow.
To not waste your time reviewing this PR which will change again please allow me to move this PR back to draft for now and I'll mark it ready for review once it's ready.

[Thanhphan1147]

Hi again, after some discussions we've decided that this specific interface should not be in charmlibs, mainly because the interface has been designed to be a temporary solution until we have a more solid approach to authentication and we don't expect ( and wouldn't recommend ) the interface to be used by other charms. I'm therefore closing this PR.

Thank you for taking the time to review the PR regardless!