-
Notifications
You must be signed in to change notification settings - Fork 951
Description
I am hitting the same issue as this: #397
I would like to propose a solution that I can then put up some PRs to implement. The issue we are facing is that our Metadata service is behind authentication, and currently we are able to access it by specifying the auth headers using the METAFLOW_SERVICE_HEADERS environmental variable. This works until the token needs to be rotated. It also makes specifying the header a little wonky as we end up setting an environment variable within our code.
My proposal is to wrap requests.Session in an abstract class:
https://github.com/Netflix/metaflow/blob/master/metaflow/plugins/metadata_providers/service.py#L39
Then the end-user could override this implementation with their own which handles their custom authentication environment, e.g. handling private certificates, token refreshing, and more.
Q: Should consider consolidating any of the other places SERVICE_HEADERS + raw requests is used into this abstraction?
E.g.
Q: Do we have good enough test coverage to stop regressions on plugins that might try and contact the metadata service in different runtime environments? Or, should we include adding test coverage for those runtimes as part of the implementation plan below?
- https://github.com/Netflix/metaflow/blob/master/metaflow/plugins/kubernetes/kubernetes.py#L243
- https://github.com/Netflix/metaflow/blob/master/metaflow/plugins/airflow/airflow.py#L383
Overall game plan of implementation:
- Add unit tests to entry points we decide to refactor, starting with
ServiceMetadataProvider. - Consolidate
SERVICE_HEADERSusing request calls to a common class, if more than one spot is identified for the refactor. - Abstract out the request class and make it user override-able. Q: Thoughts on the best interface for specifying a user override this deep inside Metaflow?