Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Feast Operator RBAC example with Kubernetes Authentication … #5077

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

redhatHameed
Copy link
Contributor

What this PR does / why we need it:

Adding Feast Operator RBAC example with Kubernetes Authentication

Which issue(s) this PR fixes:

Misc

@redhatHameed redhatHameed requested a review from a team as a code owner February 20, 2025 20:46
@redhatHameed
Copy link
Contributor Author

@tchughesiv @franciscojavierarceo @dmartinol when you get a chance, could you please provide review/feedback on this PR? Thanks!

@redhatHameed redhatHameed force-pushed the operator-rbac-example branch 2 times, most recently from 2bb0cd0 to 5f02b7b Compare February 25, 2025 18:37
@accorvin
Copy link

@redhatHameed one thing that's not obvious to me is how the test.py script makes use of the service account that is being used to connect to the feature store. I'm assuming it's automatically using some environment variable somewhere or something like that, but I'm not sure. Can we document that so it's more explicit?

I expect users will want to be able to connect to a feature store instance from a script that isn't running inside of a kubernetes pod, and I'm assuming doing so would require them to specify the token somewhere, so we should make it clear how they can do so.

@redhatHameed
Copy link
Contributor Author

@redhatHameed one thing that's not obvious to me is how the test.py script makes use of the service account that is being used to connect to the feature store. I'm assuming it's automatically using some environment variable somewhere or something like that, but I'm not sure. Can we document that so it's more explicit?

@accorvin Right, it's getting directly from the pod location. /var/run/secrets/kubernetes.io/serviceaccount/token
https://github.com/feast-dev/feast/blob/master/sdk/python/feast/permissions/client/kubernetes_auth_client_manager.py#L15
I will add this note.

I expect users will want to be able to connect to a feature store instance from a script that isn't running inside of a kubernetes pod, and I'm assuming doing so would require them to specify the token somewhere, so we should make it clear how they can do so.

That's also possible, we can use environment variable LOCAL_K8S_TOKEN https://github.com/feast-dev/feast/blob/master/sdk/python/feast/permissions/client/kubernetes_auth_client_manager.py#L50
let me change the example using locally instead from the pod and add missing document.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Install the Feast Operator section has quay image path, should it be dockerhub for now ? Also, quay.io/feastdev-ci or quay.io/feastdev ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntkathole Thanks right, this will be updated with quay once this PR merged

@redhatHameed redhatHameed marked this pull request as draft February 27, 2025 16:01
@redhatHameed redhatHameed force-pushed the operator-rbac-example branch 4 times, most recently from dfdc884 to 4a06332 Compare February 27, 2025 21:34
@redhatHameed redhatHameed marked this pull request as ready for review February 27, 2025 21:35
@redhatHameed
Copy link
Contributor Author

@accorvin can you take another look of this PR, I have updated the notebook to use local client instead of the pod. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants