ADR-92 & AP-19: BFF proposal #99

andreaTP · 2023-03-14T15:57:08Z

Opening this PR as Draft to foster collaboration on the Proposal itself and for early comments.

cc. @EricWittmann @riccardo-forina @MikeEdgar @dimakis @lburgazzoli

andreaTP · 2023-03-14T16:03:10Z

Question for @tombentley and @emmanuelbernard , I got again confused regarding AP vs. ADR 😢
@tombentley booked it as AP but:

it doesn't automatically apply to specific situations and highly depends on the current system setup and capabilities
it's not a generic solution or pattern as it highly depends on the design(accidental or deliberate) of the rest of the system
I do not want to "sell BFF" as a general solution as, almost everything here, is based on tradeoffs

I ended up framing it as an ADR, let me know if this works for you.

emmanuelbernard · 2023-03-14T16:32:32Z

If you feel BFF should not be used for other services like RHOC or whatever is coming up, then it is the right choice. I had a different impression in our conversation last Wed.

andreaTP · 2023-03-14T16:35:52Z

If you feel BFF should not be used for other services like RHOC or whatever is coming up, then it is the right choice. I had a different impression in our conversation last Wed.

I haven't analyzed the current situation in RHOC to be able to answer your question 😢 , let me gather @lburgazzoli feedback to address this.
If we reach an agreement I'll bundle this ADR up with an AP.

emmanuelbernard · 2023-03-14T16:59:13Z

If we reach an agreement I'll bundle this ADR up with an AP.

Not bundle, since AP have a different intent and template, it's a partial rework.

EricWittmann · 2023-03-16T19:06:57Z

_adr/92/index.adoc

+
+The integration of the Enterprise/Hybrid option of RHOSAK in UI, SDKs and CLI has shown that there is a margin for improvement in how we expose and consume our services APIs.
+
+* the current services are offering the bare functionality leaving room for integration code to live in the services' consumer codebases


I'm not sure I understand this 100%. Can you explain a bit more what you mean and maybe we can reword a bit to be more clear?

Tentatively:

the services are exposing APIs for the offered specific functionality, but we lack an integration layer

what we are trying to say is that there is no integration logic exposed by the current services.
The services are offering APIs modeled after their specific functionality and are very vertical, but any operation that involves more than one service becomes difficult and involves writing the integration logic on the client side.

Feel free to propose a better working 🙏 !

EricWittmann · 2023-03-16T19:09:22Z

_adr/92/index.adoc

+* the current services are offering the bare functionality leaving room for integration code to live in the services' consumer codebases
+* the data aggregation logic is duplicated, scattered, and eventually diverging in various codebases (especially UI and CLI)
+* there is no "single source of truth" for data aggregated from various services
+* the current REST API layer exposes the user to the entire complexity of the underlying infrastructure


Is it the complexity of the infrastructure that's exposed or is it the complexity of the main service PLUS the supporting services (e.g. AMS)? Infrastructure makes me think we're exposing network routing and whatnot. That may be true for RHOSAK Enterprise maybe?

Renaming infrastructure -> architecture.
We do not expose (much)"infrastructure" detail, but the user really needs to know the architecture of the system to navigate it.

_adr/92/index.adoc

EricWittmann · 2023-03-16T19:17:36Z

Added some comments but looks good overall. Something that's missing perhaps is a comment about potential lack of control (not now I guess but maybe in the future) over APIs being aggregated. Another benefit of the BFF approach is when we are running a service using e.g. an Apache project, we might not be able to modify the API to make it more suitable for use by the UI/CLI (something you could argue we can mostly do today).

andreaTP · 2023-03-20T10:19:11Z

Thanks, @EricWittmann for the review!
I attempted to address your points, I believe that your last comment on "OSS projects APIs" is covered by the current wording in "Goals", feel free to suggest an improvement there though!

lburgazzoli · 2023-03-20T10:35:30Z

If you feel BFF should not be used for other services like RHOC or whatever is coming up, then it is the right choice. I had a different impression in our conversation last Wed.

I haven't analyzed the current situation in RHOC to be able to answer your question cry , let me gather @lburgazzoli feedback to address this. If we reach an agreement I'll bundle this ADR up with an AP.

For RHOC things are exacerbates by the fact that we must interact with multiple services, i.e. Kafka & Service Registry so we would benefit from a BFF that knows how to interact with such services (i.e. to set-up/check permissions, gather additional information, etc).

andreaTP · 2023-03-20T10:54:37Z

Added the AP proposal, marking as Ready For Review now

riccardo-forina · 2023-03-20T11:03:10Z

Could we also try to solve the subscription problem as well? We use the polling pattern now to get the data we need when it changes, but it's not the cleanest solution as it unnecessarily requires a lot of micromanagement and hammers the backend.

_adr/92/index.adoc

gashcrumb · 2023-03-20T13:43:53Z

I'd suggest just a BFF isn't enough. I think offloading client-specific concerns such as handling update subscriptions and caching is great and we should adopt a BFF for those reasons to support each client-side experience.

But I'd also suggest we probably need a separate business integration service that handles the orchestration of the service APIs and it needs to be adaptable to changing business concerns, kind of the two things we currently try and spread between our UI and API a bit.

andreaTP · 2023-03-20T14:51:49Z

we probably need a separate business integration service that handles the orchestration of the service APIs and it needs to be adaptable to changing business concerns

@gashcrumb this proposal is exclusively targeting the BFF as an architectural component to respond to the current concerns in development.
Feel free to go ahead and issue a follow-up ADR/AP to further modularize and separate the concerns of the BFF.
More specifically I do not intend to increase the scope of this proposal for the following reasons:

developing and (especially)running a component is a significant team effort, doubling from the start makes it more challenging to get something done
we have already been questioned about public API support and APIs proliferation, those are concerns that we should be seriously re-considered when proposing an additional component
early optimization, I would like to see the BFF in place and re-evaluate the situation other than separating concerns in the first place
risk management, going live with one new service instead of two improves the situation by limiting the unknowns and having more predictable timelines

That said I'm supportive of reconsidering your proposal after the first version of the BFF is in place 👍

andreaTP · 2023-03-21T14:42:04Z

@tombentley I moved this PR from Draft to Ready for Review do you mind adding the @bf2fc6cc711aee1a0c2a/service-leads as the list of reviewers?
thanks in advance 🙏

_adr/92/index.adoc

Co-authored-by: Aurélien Pupier <apupier@redhat.com>

_adr/92/index.adoc

EricWittmann

LGTM

andreaTP · 2023-03-30T11:40:00Z

We have gone through discussions and agreements on all of the open points.
I now need a review from all of the @bf2fc6cc711aee1a0c2a/service-leads 🙏

_adr/92/index.adoc

_ap/19/index.adoc

andreaTP · 2023-04-03T15:18:27Z

A polite ping to @danielezonca @maleck13 @tombentley @lburgazzoli for the review! Thanks in advance 🙏

_adr/92/index.adoc

maleck13

A concern I have here is called out as a challenge, it does not work for private data planes, which could restrict the potential customer / user base and whether that is something that is an acceptable tradeoff.

andreaTP · 2023-04-04T10:55:10Z

it does not work for private data planes

@maleck13 thanks a lot for the review! I do believe that we have full freedom to move this component to the data plane when/if necessary.
We marked it as an open point as it depends on other architectural changes and proposals that are being discussed those days.
Does this and Riccardo's answer your question?

maleck13 · 2023-04-04T10:59:14Z

Yes the answers helped thanks

danielezonca

A minor comment but LGTM 👍

_adr/92/index.adoc

danielezonca · 2023-04-04T14:33:00Z

_adr/92/index.adoc

+[NOTE]
+*Assumption* as a first iteration we are assuming that the CLI needs can be covered by a strict subset of the functionalities needed for the UI.
+
+We propose to integrate *one* server side component commonly known as https://samnewman.io/patterns/architectural/bff/[BFF] (Backend For Frontend).


Just to make sure I understand the proposal: this proposal covers the use case where BFF is part of a "single service" in terms of ownership/SLO (like KAS) so this is an additional endpoint is going to be part of the API delivered by that specific service.
So if a feature can be reused (i.e. some common pattern to create SA and grant access to topic) or involves multiple services (with different CP) the endpoint is going to be "assigned" to one service (or duplicated?) that is going to own this new component.

If "cross service BFF" is not covered, I would probably add this to the "Non-goals" section.
Wdyt?

so this is an additional endpoint is going to be part of the API delivered by that specific service.

this is not correct, here we are proposing BFF as a separate service with a new spec that is going to aggregate multiple different services.
We are somehow covering only what you call "cross service BFF", as having one BFF associated with one service almost defeats the purpose of BFF (at least in this context).
Also, we are here proposing a single BFF service for the entire platform.

If this is not clear, please, point out the confusing sections and I will try to re-phrase 👍

Thanks for the clarification, fine for me to keep current phrasing 👍

here proposing a single BFF service for the entire platform.

Define entire platform.

Entire platform means RHOSAK RHODS RHOC as I understand it

I don't think we have enough context to include RHODS in the scope of this decision, I would "limit" the scope this ADR to RHOSAK+RHOC+RHOSR and postpone to a revision RHODS but also RHOAM and other services

From what I foresee, we won't have a choice if we want a good UX. So let's go with this bold intent and cast exclusions as we go.

I see this as a very bold intent given that RHODS for example has the on-prem as a very important setup and also private cloud data plane configurations are not fully explored. :)

Btw I was mainly referring to the ADR that is "Kafka ecosystem" specific while the AP is probably generic enough and I agree that we aim to have a coherent UX across the whole portfolio so the problem statement and approach for the solution can be valid for every service

Co-authored-by: Daniele Zonca <dzonca@redhat.com>

andreaTP · 2023-04-05T10:45:42Z

@tombentley can you please, review or delegate the review of this PR? Thanks in advance 🙏

This change has two objectives: 1. Make the AP more concrete and clearer to teams that will read it in the future and the constraints it puts on them. In particular, limits and choices are detailed 2. Clarify that GraphQL was not chosen but keep the door open to a future use.

emmanuelbernard · 2023-04-06T08:37:23Z

Hey @andreaTP @riccardo-forina and all,

In order to help move forward, I've sent a PR to your PR (not sure how well it will work).
andreaTP#1

I've closed some conversations in the ADR that were pending here.
I've also adjusted the AP to show what I was trying to convey and the choices that I think are important.
In particular, I want it to be closer to actionable than theoretical.

emmanuelbernard · 2023-04-06T08:38:26Z

I tried to add commits to Andrea's own branch but without success, hence the PR

tombentley · 2023-04-06T08:48:57Z

@andreaTP this was top of my todo list today, but do you want me to review this now, or once you've merged Emmanuel's PR into the branch for this PR?

Co-authored-by: Daniele Zonca <dzonca@redhat.com>

Bff proposal additional improvements

andreaTP · 2023-04-06T11:24:44Z

thanks @emmanuelbernard for the PR, merged 👍
@tombentley ready for review now

tombentley

Thanks for your efforts on this @andreaTP and @riccardo-forina

_ap/19/index.adoc

Co-authored-by: Tom Bentley <tombentley@users.noreply.github.com>

andreaTP · 2023-04-06T15:40:58Z

Ready to merge AFAIU 🙏 thanks everyone for the input!

andreaTP · 2023-04-17T14:11:44Z

🎉 🎉 🎉

WIP BFF proposal

c392a0d

andreaTP changed the title ~~WIP BFF proposal~~ [WIP] BFF proposal Mar 14, 2023

EricWittmann reviewed Mar 16, 2023

View reviewed changes

_adr/92/index.adoc Show resolved Hide resolved

EricWittmann reviewed Mar 16, 2023

View reviewed changes

_adr/92/index.adoc Outdated Show resolved Hide resolved

eric's review

6ecc529

update

79c2ba0

andreaTP marked this pull request as ready for review March 20, 2023 10:54

andreaTP changed the title ~~[WIP] BFF proposal~~ BFF proposal Mar 20, 2023

MikeEdgar reviewed Mar 20, 2023

View reviewed changes

_adr/92/index.adoc Show resolved Hide resolved

review

f07e020

tombentley changed the title ~~BFF proposal~~ ADR-92 & AP-19: BFF proposal Mar 21, 2023

apupier reviewed Mar 28, 2023

View reviewed changes

_adr/92/index.adoc Outdated Show resolved Hide resolved

chirino suggested changes Mar 29, 2023

View reviewed changes

_adr/92/index.adoc Outdated Show resolved Hide resolved

_adr/92/index.adoc Outdated Show resolved Hide resolved

_adr/92/index.adoc Outdated Show resolved Hide resolved

_adr/92/index.adoc Show resolved Hide resolved

andreaTP and others added 2 commits March 29, 2023 13:46

Update _adr/92/index.adoc

e2e1e1c

Co-authored-by: Aurélien Pupier <apupier@redhat.com>

agreed GraphQL analysis

877e4c8

machi1990 reviewed Mar 29, 2023

View reviewed changes

_adr/92/index.adoc Show resolved Hide resolved

EricWittmann approved these changes Mar 29, 2023

View reviewed changes

emmanuelbernard requested changes Apr 3, 2023

View reviewed changes

_adr/92/index.adoc Outdated Show resolved Hide resolved

_ap/19/index.adoc Show resolved Hide resolved

note on the propagating the access token

efe4231

lburgazzoli approved these changes Apr 3, 2023

View reviewed changes

maleck13 reviewed Apr 4, 2023

View reviewed changes

_adr/92/index.adoc Show resolved Hide resolved

maleck13 reviewed Apr 4, 2023

View reviewed changes

_adr/92/index.adoc Show resolved Hide resolved

maleck13 approved these changes Apr 4, 2023

View reviewed changes

danielezonca approved these changes Apr 5, 2023

View reviewed changes

Update _adr/92/index.adoc

9f4893b

Co-authored-by: Daniele Zonca <dzonca@redhat.com>

emmanuelbernard added 4 commits April 6, 2023 09:49

Clarify priority for access token propagation

464b322

Clarify potential impact of private data planes on this choice

418dd37

Minor wording improvements in the AP

8a27d75

emmanuelbernard and others added 2 commits April 6, 2023 12:28

Update _adr/92/index.adoc

8b57e24

Co-authored-by: Daniele Zonca <dzonca@redhat.com>

Merge pull request #1 from emmanuelbernard/bff-proposal

34f5845

Bff proposal additional improvements

tombentley approved these changes Apr 6, 2023

View reviewed changes

_ap/19/index.adoc Outdated Show resolved Hide resolved

_ap/19/index.adoc Outdated Show resolved Hide resolved

andreaTP and others added 2 commits April 6, 2023 16:39

Update _ap/19/index.adoc

44c7671

Co-authored-by: Tom Bentley <tombentley@users.noreply.github.com>

Update _ap/19/index.adoc

7d701b2

Co-authored-by: Tom Bentley <tombentley@users.noreply.github.com>

tombentley merged commit 7d7be37 into bf2fc6cc711aee1a0c2a:main Apr 17, 2023


		The integration of the Enterprise/Hybrid option of RHOSAK in UI, SDKs and CLI has shown that there is a margin for improvement in how we expose and consume our services APIs.

		* the current services are offering the bare functionality leaving room for integration code to live in the services' consumer codebases

ADR-92 & AP-19: BFF proposal #99

ADR-92 & AP-19: BFF proposal #99

Conversation

andreaTP commented Mar 14, 2023

andreaTP commented Mar 14, 2023

emmanuelbernard commented Mar 14, 2023

andreaTP commented Mar 14, 2023

emmanuelbernard commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EricWittmann commented Mar 16, 2023

andreaTP commented Mar 20, 2023

lburgazzoli commented Mar 20, 2023

andreaTP commented Mar 20, 2023

riccardo-forina commented Mar 20, 2023

gashcrumb commented Mar 20, 2023 • edited Loading

andreaTP commented Mar 20, 2023

andreaTP commented Mar 21, 2023 • edited Loading

EricWittmann left a comment

Choose a reason for hiding this comment

andreaTP commented Mar 30, 2023

andreaTP commented Apr 3, 2023

maleck13 left a comment

Choose a reason for hiding this comment

andreaTP commented Apr 4, 2023

maleck13 commented Apr 4, 2023

danielezonca left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaTP commented Apr 5, 2023

emmanuelbernard commented Apr 6, 2023

emmanuelbernard commented Apr 6, 2023

tombentley commented Apr 6, 2023

andreaTP commented Apr 6, 2023

tombentley left a comment

Choose a reason for hiding this comment

andreaTP commented Apr 6, 2023

andreaTP commented Apr 17, 2023

gashcrumb commented Mar 20, 2023 •

edited

Loading

andreaTP commented Mar 21, 2023 •

edited

Loading