search

open-feature / ofep

A focal point for OpenFeature research, proposals and requests for comments
https://openfeature.dev
20 stars 15 forks source link

feat: add proposal for ofo flagd service #59

Closed skyerus closed 1 year ago

skyerus commented 1 year ago

This PR

OFEP for ofo flagd service

Related Issues

https://github.com/open-feature/open-feature-operator/issues/371

Notes

Follow-up Tasks

How to test

toddbaert commented 1 year ago

As a Platform Eng, I would have some reservation deploying an operator that can control K8s Services in any namespace. In the implementation details, I would suggest to enable/disable this behavior via (Open)Feauture Flag wink In the docs, we could explain that if this asking is too much permissions, this behavior can also be done manually by applying a manifest with the svc

I think it might make sense to restrict OFO to only create services in it's own namespace (which is customizable via helm). I think that would work for most use-cases and would help keep the permissions surface area smaller. I don't see a need for OFO to have the ability to create services in any namespace. Not sure that needs to be mentioned in this OFEP though

skyerus commented 1 year ago

As a Platform Eng, I would have some reservation deploying an operator that can control K8s Services in any namespace. In the implementation details, I would suggest to enable/disable this behavior via (Open)Feauture Flag wink In the docs, we could explain that if this asking is too much permissions, this behavior can also be done manually by applying a manifest with the svc

I think it might make sense to restrict OFO to only create services in it's own namespace (which is customizable via helm). I think that would work for most use-cases and would help keep the permissions surface area smaller. I don't see a need for OFO to have the ability to create services in any namespace. Not sure that needs to be mentioned in this OFEP though

I agree with both points. I have updated the OFEP to restrict OFO's Services permissions to its own namespace.

skyerus commented 1 year ago

Further POC work has led me to discover that services cannot expose deployments across namespaces.

The simplest circumvention to this is to enforce both the Deployment and Service to be created in OFO's namespace. If this isn't extensive enough, an alternative could be to provide a list of allowed namespaces to OFO in which Service CRUD is permitted. I prefer the former due to its simplicity. If OFO's namespace ends up being too limiting it can be built upon in the future.

Thoughts @toddbaert @beeme1mr @thisthat @AlexsJones ?

AlexsJones commented 1 year ago

I am really confused, I left a comment on this and it's been merged but I can't find the comment

AlexsJones commented 1 year ago

Okay I'll add what I think I wrote the other day:

The driving force behind this is to simplify the deployment of flag providers for use by client side applications

I think this is somewhat misleading, this doesn't make deploying clientside application deployment simpler, it makes it prescriptive, it's providing a way we think is best to do it, rather than letting people do it themself.

This is a common deployment pattern permitting access by any component that routes to the created Service (e.g. Ingress/Load Balancer).

This is going to make all the service and endpoint types resident inside the OFO namespace, meaning you can route between them, it breaks tenancy isolation boundaries.

In contrast, the described FlagService pattern permits an externally routable flag provider.

Why not simply route from the container that has the client SDK inside itself?

beeme1mr commented 1 year ago

I am really confused, I left a comment on this and it's been merged but I can't find the comment

Here's a link to your comment: https://github.com/open-feature/ofep/pull/59#discussion_r1145018209

skyerus commented 1 year ago

I am really confused, I left a comment on this and it's been merged but I can't find the comment

Your comment is still visible to me: https://github.com/open-feature/ofep/pull/59#discussion_r1145018209

Okay I'll add what I think I wrote the other day:

The driving force behind this is to simplify the deployment of flag providers for use by client side applications

I think this is somewhat misleading, this doesn't make deploying clientside application deployment simpler, it makes it prescriptive, it's providing a way we think is best to do it, rather than letting people do it themself.

People are still able to do it themselves if they don't fit to this prescription but imo the Service behind Deployment is common enough to warrant this enhancement.

In contrast, the described FlagService pattern permits an externally routable flag provider.

Why not simply route from the container that has the client SDK inside itself?

Can you elaborate further on this please? Not sure I fully understand.

toddbaert commented 1 year ago

@AlexsJones I think you may be misunderstanding what we mean when we say "client flags".

Why not simply route from the container that has the client SDK inside itself?

In client side flagging, there is no container with an SDK. The SDK and the provider exist in client code (an iOS app, web browser, etc).

toddbaert commented 1 year ago

See my previous comment here for more background on the paradigm.